On 28 Dec 2025, at 13:48, 543981924--- via dev wrote:
> From: yaolingfei <[email protected]> Hi, Thanks for the patch. Would it be possible to include your full name in the Signed-off-by so we know what to put in the AUTHORS.rst file? > > The "member_may_enable__" function should be called after the "attached" > variable is assigned a value of "true"; otherwise, "enable" will be > permanently set to "false", which may result in failure to select the correct > lead member. > 96686dc6e0e7 (LACP: Check active partner sys id) If you feel like this is the issue you're fixing, you should use the following format (with the correct sha): Fixes: 6f50056c4fe7 ("LACP: Check active partner sys id") > Signed-off-by: yaolingfei <[email protected]> Can you use your full name here, see above. > --- > lib/lacp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/lacp.c b/lib/lacp.c > index 3252f17eb..cacedff32 100644 > --- a/lib/lacp.c > +++ b/lib/lacp.c > @@ -723,7 +723,6 @@ lacp_update_attached(struct lacp *lacp) > OVS_REQUIRES(mutex) > } > > member_get_priority(member, &pri); > - bool enable = member_may_enable__(member); > > /* Check if partner MAC address is the same as on the working > * interface. Activate member only if the MAC is the same, or > @@ -733,6 +732,8 @@ lacp_update_attached(struct lacp *lacp) > OVS_REQUIRES(mutex) > lead_current->partner.sys_id))) { > member->attached = true; > } > + > + bool enable = member_may_enable__(member); Maybe move the member_get_priority() function as well, so it’s all kept together as it was. So something like: if (member->status == LACP_DEFAULTED) { if (lacp->fallback_ab) { member->attached = true; } continue; } /* Check if partner MAC address is the same as on the working * interface. Activate member only if the MAC is the same, or * there is no working interface. */ if (!lead_current || (lead_current && eth_addr_equals(member->partner.sys_id, lead_current->partner.sys_id))) { member->attached = true; } member_get_priority(member, &pri); bool enable = member_may_enable__(member); if (member->attached && (!lead || enable > lead_enable || (enable == lead_enable && memcmp(&pri, &lead_pri, sizeof pri) < 0))) { lead = member; lead_enable = enable; lead_pri = pri; } } > if (member->attached && > (!lead > || enable > lead_enable Would it be possible to add a unit test for this? In addition, I CC'd Mike, who has more LACP experience, any additional feedback? Cheers, Eelco _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
