nbi_impl: set speed/duplex before setting link up

Link speed/duplex changes do not trigger any notifications, only link
state changes do. Therefore we need to make sure that speed/duplex is
set correctly before setting the link state to up for ports.

When setting the link state first, it leaves a window where a process
reacting to the link state event may read out old speed/duplex before we
had a change to update them, and the default speed/duplex is 10mbit/half
for TAP.

This can for example lead to breakage in LACP, where it uses the link
speed to calculate the actor key. If it uses the wrong/old speed, it
calculates a different key than the other side, and LACP will then fail
to synchronize, breaking the link.

So make sure to set the speed and duplex first on link up. On link down,
we do not need to set any speed or duplex values, since their values are
undefined.

Fixes: b5fe46c ("tap_manager: set port speed on tap interfaces")
Signed-off-by: Jonas Gorski <[email protected]>

src/netlink/nbi_impl.cc

@@ -62,8 +62,9 @@ void nbi_impl::port_notification(
     case PORT_EVENT_MODIFY:
       switch (get_port_type(ntfy.port_id)) {
       case nbi::port_type_physical:
+        if (ntfy.status)
+          port_man->set_port_speed(ntfy.name, ntfy.speed, ntfy.duplex);
         port_man->change_port_status(ntfy.name, ntfy.status);
-        port_man->set_port_speed(ntfy.name, ntfy.speed, ntfy.duplex);
         break;
       default:
         LOG(ERROR) << __FUNCTION__ << ": unknown port";
@@ -73,8 +74,9 @@ void nbi_impl::port_notification(
     case PORT_EVENT_ADD:
       switch (get_port_type(ntfy.port_id)) {
       case nbi::port_type_physical:
+        if (ntfy.status)
+          port_man->set_port_speed(ntfy.name, ntfy.speed, ntfy.duplex);
         port_man->change_port_status(ntfy.name, ntfy.status);
-        port_man->set_port_speed(ntfy.name, ntfy.speed, ntfy.duplex);
         break;
       case nbi::port_type_vxlan:
         // XXX TODO notify this?