Bug 201916 - [patch] mac address does not update when removing the primary iface from a lagg
Summary: [patch] mac address does not update when removing the primary iface from a lagg
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Hiroki Sato
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2015-07-27 08:41 UTC by shahark
Modified: 2019-01-21 17:58 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description shahark 2015-07-27 08:41:21 UTC
When removing the primary iface froma lagg - the mac should update to the mac address of the next iface in the lagg list.
Currently it happens but not to the one that is now the new primary.

To reproduce:

# ifconfig lagg0 create laggport ifp0 laggport ifp1
# ifconfig lagg0 -laggport ifp0
and look at the macs before and after


The problem is that the code mark the next ifp as primary(@lagg_port_lladdr) and then skip updating it because it is primary(@lagg_port_setlladdr).

suggested fix:

diff --git a/sys/net/if_lagg.c b/sys/net/if_lagg.c
index dcd005a..f9e2312 100644
--- a/sys/net/if_lagg.c
+++ b/sys/net/if_lagg.c
@@ -691,22 +691,23 @@ lagg_port_setlladdr(void *arg, int pending)
         * Traverse the queue and set the lladdr on each ifp. It is safe to do
         * unlocked as we have the only reference to it.
         */
+
        for (llq = head; llq != NULL; llq = head) {
                ifp = llq->llq_ifp;
-
                CURVNET_SET(ifp->if_vnet);
-           if (llq->llq_primary == 0) {
-                   /*
-                    * Set the link layer address on the laggport interface.
-                    * if_setlladdr() triggers gratuitous ARPs for INET.
-                    */
-                   error = if_setlladdr(ifp, llq->llq_lladdr,
-                       ETHER_ADDR_LEN);
-                   if (error)
-                           printf("%s: setlladdr failed on %s\n", __func__,
-                               ifp->if_xname);
-           } else
+
+         /*
+          * Set the link layer address on the laggport interface.
+          * if_setlladdr() triggers gratuitous ARPs for INET.
+          */
+         error = if_setlladdr(ifp, llq->llq_lladdr,
+                         ETHER_ADDR_LEN);
+         if (error)
+                 printf("%s: setlladdr failed on %s\n", __func__,
+                                 ifp->if_xname);
+         if (llq->llq_primary) {
                        EVENTHANDLER_INVOKE(iflladdr_event, ifp);
+         }
                CURVNET_RESTORE();
                head = SLIST_NEXT(llq, llq_entries);
                free(llq, M_DEVBUF);
Comment 1 Hiren Panchasara freebsd_committer freebsd_triage 2015-07-30 21:33:39 UTC
shahark@ : if possible, create phabricator review at reviews.freebsd.org
Comment 2 Jason Wolfe 2015-10-04 21:49:14 UTC
https://reviews.freebsd.org/D3301 was the Phab for this one.
Comment 3 commit-hook freebsd_committer freebsd_triage 2015-10-07 06:33:13 UTC
A commit references this bug:

Author: hrs
Date: Wed Oct  7 06:32:34 UTC 2015
New revision: 288980
URL: https://svnweb.freebsd.org/changeset/base/288980

Log:
  Fix a bug that caused reinitialization failure of MAC addresses on
  the lagg interface when removing the primary port.

  PR:			201916
  Differential Revision:	https://reviews.freebsd.org/D3301

Changes:
  head/sys/net/if_lagg.c
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2017-08-24 13:59:55 UTC
Committed back in 2015.
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:58 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.
Comment 6 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-21 17:58:57 UTC
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved.

Thanks