[patchv2] teamd: lacp: update port state according to partner's sync bit
diff mbox series

Message ID 20190307024743.7836-1-liuhangbin@gmail.com
State New
Headers show
Series
  • [patchv2] teamd: lacp: update port state according to partner's sync bit
Related show

Commit Message

Hangbin Liu March 7, 2019, 2:47 a.m. UTC
According to 6.4.15 of IEEE 802.1AX-2014, Figure 6-22, the state that the
port is selected moves MUX state from DETACHED to ATTACHED.

But ATTACHED sate does not mean that the port can send and receive user frames.
COLLECTING_DISTRIBUTION state is the sate that the port can send and receive
user frames. To move MUX state from ATTACHED to COLLECTING_DISTRIBUTION, the
partner state should be sync as well as the port selected.

In function lacp_port_actor_update(), only INFO_STATE_SYNCHRONIZATION should
be set to the actor.state when the port is selected. INFO_STATE_COLLECTING and
INFO_STATE_DISTRIBUTING should be set to false with ATTACHED mode and set to
true when INFO_STATE_SYNCHRONIZATION of partner.state is set.

In function lacp_port_should_be_{enabled, disabled}(), we also need to check
the INFO_STATE_SYNCHRONIZATION bit of partner.state.

Reported-by: Cisco Systems
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 teamd/teamd_runner_lacp.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Jiri Pirko March 8, 2019, 8:59 a.m. UTC | #1
Thu, Mar 07, 2019 at 03:47:43AM CET, liuhangbin@gmail.com wrote:
>According to 6.4.15 of IEEE 802.1AX-2014, Figure 6-22, the state that the
>port is selected moves MUX state from DETACHED to ATTACHED.
>
>But ATTACHED sate does not mean that the port can send and receive user frames.

s/sate/state/


>COLLECTING_DISTRIBUTION state is the sate that the port can send and receive

s/sate/state/


>user frames. To move MUX state from ATTACHED to COLLECTING_DISTRIBUTION, the

With text description lines should not be longer than 72 cols.


>partner state should be sync as well as the port selected.
>
>In function lacp_port_actor_update(), only INFO_STATE_SYNCHRONIZATION should
>be set to the actor.state when the port is selected. INFO_STATE_COLLECTING and
>INFO_STATE_DISTRIBUTING should be set to false with ATTACHED mode and set to
>true when INFO_STATE_SYNCHRONIZATION of partner.state is set.
>
>In function lacp_port_should_be_{enabled, disabled}(), we also need to check
>the INFO_STATE_SYNCHRONIZATION bit of partner.state.
>
>Reported-by: Cisco Systems

Should be person with email.


>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>---
> teamd/teamd_runner_lacp.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
>diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
>index 7b8f0a7..4de30f3 100644
>--- a/teamd/teamd_runner_lacp.c
>+++ b/teamd/teamd_runner_lacp.c
>@@ -333,7 +333,8 @@ static int lacp_port_should_be_enabled(struct lacp_port *lacp_port)
> 	struct lacp *lacp = lacp_port->lacp;
> 
> 	if (lacp_port_selected(lacp_port) &&
>-	    lacp_port->agg_lead == lacp->selected_agg_lead)
>+	    lacp_port->agg_lead == lacp->selected_agg_lead &&
>+	    lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION)
> 		return true;
> 	return false;
> }
>@@ -343,7 +344,8 @@ static int lacp_port_should_be_disabled(struct lacp_port *lacp_port)
> 	struct lacp *lacp = lacp_port->lacp;
> 
> 	if (!lacp_port_selected(lacp_port) ||
>-	    lacp_port->agg_lead != lacp->selected_agg_lead)
>+	    lacp_port->agg_lead != lacp->selected_agg_lead ||
>+	    !(lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION))
> 		return true;
> 	return false;
> }
>@@ -914,9 +916,13 @@ static void lacp_port_actor_update(struct lacp_port *lacp_port)
> 	if (lacp_port->lacp->cfg.fast_rate)
> 		state |= INFO_STATE_LACP_TIMEOUT;
> 	if (lacp_port_selected(lacp_port) &&
>-	    lacp_port_agg_selected(lacp_port))
>-		state |= INFO_STATE_SYNCHRONIZATION |
>-			 INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING;
>+	    lacp_port_agg_selected(lacp_port)) {
>+		state |= INFO_STATE_SYNCHRONIZATION;
>+		state &= ~(INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING);
>+		if (lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION)
>+			state |= INFO_STATE_COLLECTING |
>+				 INFO_STATE_DISTRIBUTING;
>+	}
> 	if (lacp_port->state == PORT_STATE_EXPIRED)
> 		state |= INFO_STATE_EXPIRED;
> 	if (lacp_port->state == PORT_STATE_DEFAULTED)
>-- 
>2.19.2
>
Hangbin Liu March 8, 2019, 11:19 a.m. UTC | #2
On Fri, Mar 08, 2019 at 09:59:28AM +0100, Jiri Pirko wrote:
> Thu, Mar 07, 2019 at 03:47:43AM CET, liuhangbin@gmail.com wrote:
> >According to 6.4.15 of IEEE 802.1AX-2014, Figure 6-22, the state that the
> >port is selected moves MUX state from DETACHED to ATTACHED.
> >
> >But ATTACHED sate does not mean that the port can send and receive user frames.
> 
> s/sate/state/
> 
> 
> >COLLECTING_DISTRIBUTION state is the sate that the port can send and receive
> 
> s/sate/state/
> 
> 
> >user frames. To move MUX state from ATTACHED to COLLECTING_DISTRIBUTION, the
> 
> With text description lines should not be longer than 72 cols.
> 
> 
> >partner state should be sync as well as the port selected.
> >
> >In function lacp_port_actor_update(), only INFO_STATE_SYNCHRONIZATION should
> >be set to the actor.state when the port is selected. INFO_STATE_COLLECTING and
> >INFO_STATE_DISTRIBUTING should be set to false with ATTACHED mode and set to
> >true when INFO_STATE_SYNCHRONIZATION of partner.state is set.
> >
> >In function lacp_port_should_be_{enabled, disabled}(), we also need to check
> >the INFO_STATE_SYNCHRONIZATION bit of partner.state.
> >
> >Reported-by: Cisco Systems
> 
> Should be person with email.

OK, I will remove the Reported-by as I don't know the person's email..

Patch
diff mbox series

diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
index 7b8f0a7..4de30f3 100644
--- a/teamd/teamd_runner_lacp.c
+++ b/teamd/teamd_runner_lacp.c
@@ -333,7 +333,8 @@  static int lacp_port_should_be_enabled(struct lacp_port *lacp_port)
 	struct lacp *lacp = lacp_port->lacp;
 
 	if (lacp_port_selected(lacp_port) &&
-	    lacp_port->agg_lead == lacp->selected_agg_lead)
+	    lacp_port->agg_lead == lacp->selected_agg_lead &&
+	    lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION)
 		return true;
 	return false;
 }
@@ -343,7 +344,8 @@  static int lacp_port_should_be_disabled(struct lacp_port *lacp_port)
 	struct lacp *lacp = lacp_port->lacp;
 
 	if (!lacp_port_selected(lacp_port) ||
-	    lacp_port->agg_lead != lacp->selected_agg_lead)
+	    lacp_port->agg_lead != lacp->selected_agg_lead ||
+	    !(lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION))
 		return true;
 	return false;
 }
@@ -914,9 +916,13 @@  static void lacp_port_actor_update(struct lacp_port *lacp_port)
 	if (lacp_port->lacp->cfg.fast_rate)
 		state |= INFO_STATE_LACP_TIMEOUT;
 	if (lacp_port_selected(lacp_port) &&
-	    lacp_port_agg_selected(lacp_port))
-		state |= INFO_STATE_SYNCHRONIZATION |
-			 INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING;
+	    lacp_port_agg_selected(lacp_port)) {
+		state |= INFO_STATE_SYNCHRONIZATION;
+		state &= ~(INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING);
+		if (lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION)
+			state |= INFO_STATE_COLLECTING |
+				 INFO_STATE_DISTRIBUTING;
+	}
 	if (lacp_port->state == PORT_STATE_EXPIRED)
 		state |= INFO_STATE_EXPIRED;
 	if (lacp_port->state == PORT_STATE_DEFAULTED)