On 13/08/2024 11:42, Adrián Moreno wrote:
On Thu, Aug 08, 2024 at 05:46:01PM GMT, Jonathan Davies wrote:Hi ovs-dev, I'm seeing behaviour that feels like a bug, related to preservation of active bond member selection across OVS restarts. But the behaviour has been there for several years, so I wonder whether I'm missing something. The OVS DB has a bond_active_slave field. This gets read by port_configure_bond() into struct bond_setting's active_member_mac field. But this gets ignored by bond_create(), which sets bond->active_slave_mac to eth_addr_zero. Why is it being set to eth_addr_zero rather than to the value of active_member_mac from the struct bond_settings that is passed to bond_create()? It means that the bond_active_slave field is ignored. The behaviour was introduced in commit 30353934 ("ofproto/bond: Validate active-slave mac.") but I'm having a hard time understanding why it changed from using s->active_member_mac to eth_addr_zero when the initialisation logic moved to bond_create(). I'd be grateful for more context. If this is the intended behaviour, why do we bother saving the bond_active_slave field at all? The original rationale described in commit 3e5aeeb5 ("bridge: Keep bond active slave selection across OVS restart") still seems sound to me.Hi Jonathan, Took a look at the commits you mention and I think I agree with you. I don't have more context unfortunately. Looking at 30353934, I see that it's trying to solve a race condition that could make an invalid mac end up being configured. But I don't see a reason why it should be initialized to zero. Maybe Ilya knows more but AFAICT, this is a bug.
Thanks for checking, Adrián. In that case, I'll send a bugfix patch. Jonathan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
