On Mon, Sep 09, 2024 at 08:59:21AM +0100, Jonathan Davies wrote:
> 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?
Hi Jonathan,
I would need to dig deeper in order to make any suggestions.
But I agree with your analysis above: let's treat this as a separate issue.
> > 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.
Thanks.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
