From eac8d30fae63cd4802d9f80e8af54eb26b54eb32 Mon Sep 17 00:00:00 2001 From: Jonas Gorski Date: Wed, 18 Sep 2024 14:08:58 +0200 Subject: [PATCH] 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: b5fe46ca64d4 ("tap_manager: set port speed on tap interfaces") Signed-off-by: Jonas Gorski --- src/netlink/nbi_impl.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/netlink/nbi_impl.cc b/src/netlink/nbi_impl.cc index b550ed78..f369598f 100644 --- a/src/netlink/nbi_impl.cc +++ b/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?