Fri, Sep 11, 2015 at 03:24:07PM CEST, fbl(a)sysclose.org wrote:
On Fri, Sep 11, 2015 at 09:46:45AM +0200, Jiri Pirko wrote:
> Fri, Sep 11, 2015 at 01:14:09AM CEST, sridhar.samudrala(a)intel.com wrote:
> >Currently AGGREGATION flag is only set when there is a change in port state.
> >Adding a 2nd port should cause the AGGREGATION flag of the first port to be
> >changed to 'Yes' and removing the last but one port should cause the
> >AGGREGATION flag of the remaining port to be changed to 'No'.
> >A simple fix is set AGGREGATION flag before sending LACP frames based on
> >the number of ports in the team.
>
>
> INFO_STATE_AGGREGATION is set in lacp_port_actor_update. Why that is not
> enough?
It seems we only do that if the state of a port changes.
920 static int lacp_port_set_state(struct lacp_port *lacp_port,
921 enum lacp_port_state new_state)
922 {
923 int err;
924
*925 if (new_state == lacp_port->state)
926 return 0;
fbl
So the problematic codepath is lacp_callback_periodic->lacpdu_send where
the sate is not updated.
How about following patch:
diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
index 09899ce..1cfef36 100644
--- a/teamd/teamd_runner_lacp.c
+++ b/teamd/teamd_runner_lacp.c
@@ -888,9 +888,7 @@ static void lacp_port_actor_init(struct lacp_port *lacp_port)
lacp_port_actor_system_update(lacp_port);
}
-static int lacpdu_send(struct lacp_port *lacp_port);
-
-static int lacp_port_actor_update(struct lacp_port *lacp_port)
+static void lacp_port_actor_update(struct lacp_port *lacp_port)
{
uint8_t state = 0;
@@ -910,13 +908,10 @@ static int lacp_port_actor_update(struct lacp_port *lacp_port)
teamd_log_dbg("%s: lacp info state: 0x%02X.",
lacp_port->tdport->ifname,
state);
lacp_port->actor.state = state;
-
- if (lacp_port->periodic_on)
- return 0;
-
- return lacpdu_send(lacp_port);
}
+static int lacpdu_send(struct lacp_port *lacp_port);
+
static int lacp_port_set_state(struct lacp_port *lacp_port,
enum lacp_port_state new_state)
{
@@ -971,7 +966,10 @@ static int lacp_port_set_state(struct lacp_port *lacp_port,
if (err)
return err;
- return lacp_port_actor_update(lacp_port);
+ lacp_port_actor_update(lacp_port);
+ if (lacp_port->periodic_on)
+ return 0;
+ return lacpdu_send(lacp_port);
}
static enum lacp_port_state lacp_port_get_state(struct lacp_port *lacp_port)
@@ -1114,6 +1112,7 @@ static int lacp_callback_periodic(struct teamd_context *ctx, int
events,
{
struct lacp_port *lacp_port = priv;
+ lacp_port_actor_update(lacp_port);
return lacpdu_send(lacp_port);
}