On 30/08/2024 10:02, Simon Horman wrote:
On Thu, Aug 22, 2024 at 02:09:38PM +0000, Jonathan Davies wrote:The OVS DB has a bond_active_slave field. This gets read by port_configure_bond() into struct bond_settings' active_member_mac field. See commit 3e5aeeb5 ("bridge: Keep bond active slave selection across OVS restart") for the original rationale for preserving the active bond member.But since commit 30353934 ("ofproto/bond: Validate active-slave mac.") the bond_settings' active_member_mac field is ignored by bond_create(), which set bond->active_member_mac to eth_addr_zero. Instead, set it to the value of the bond_settings' active_member_mac so that the selection is preserved across OVS restarts. Fixes: 303539348848 ("ofproto/bond: Validate active-slave mac.") Signed-off-by: Jonathan Davies <[email protected]>Thanks Jonathan, I am wondering if this also works in the when the bond is reconfigured. Say, if the bond is created without bond_active_slave set, and then reconfigured so that it is set. Perhaps it that can't happen, or is ok for some reason. But I am concerned about the call to bond_reconfigure() in bundle_set().
I don't think that does work, ever since commit 3035393488 removed setting the active_member_mac from bond_reconfigure. Currently, there's no code path in which active_member_mac is set from the value configured in the DB. That also feels like a bug, but orthogonal to the one my patch fixes.
Both issues would be fixed by reverting commit 3035393488, but that commit seems to be addressing a legitimate issue, although I don't have a good grasp on the details to know how to accommodate fixes for all three issues... Any suggestions?
Also, I think it would be really nice to add a test for this.
That sounds like a good idea. See patch v5, coming up. Jonathan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
