All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v1] bonding: LACP state machine variable "port-moved" is never set
@ 2015-04-03 20:54 Mahesh Bandewar
  2015-04-06 21:01 ` David Miller
  0 siblings, 1 reply; 2+ messages in thread
From: Mahesh Bandewar @ 2015-04-03 20:54 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
	Nikolay Aleksandrov, David Miller
  Cc: Mahesh Bandewar, Maciej Zenczykowski, netdev, Eric Dumazet

The state-machine checks for the presence of this flag and attempts
to clear it but the value is never set. So why have it?

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/bonding/bond_3ad.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 451d9dbd392f..cc37183f2a8a 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1018,8 +1018,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 		port->sm_rx_state = AD_RX_INITIALIZE;
 		port->sm_vars |= AD_PORT_CHURNED;
 	/* check if port is not enabled */
-	} else if (!(port->sm_vars & AD_PORT_BEGIN)
-		 && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
+	} else if (!(port->sm_vars & AD_PORT_BEGIN) && !port->is_enabled)
 		port->sm_rx_state = AD_RX_PORT_DISABLED;
 	/* check if new lacpdu arrived */
 	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) ||
@@ -1047,11 +1046,8 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			/* if no lacpdu arrived and no timer is on */
 			switch (port->sm_rx_state) {
 			case AD_RX_PORT_DISABLED:
-				if (port->sm_vars & AD_PORT_MOVED)
-					port->sm_rx_state = AD_RX_INITIALIZE;
-				else if (port->is_enabled
-					 && (port->sm_vars
-					     & AD_PORT_LACP_ENABLED))
+				if (port->is_enabled &&
+				    (port->sm_vars & AD_PORT_LACP_ENABLED))
 					port->sm_rx_state = AD_RX_EXPIRED;
 				else if (port->is_enabled
 					 && ((port->sm_vars
@@ -1081,7 +1077,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->sm_vars &= ~AD_PORT_SELECTED;
 			__record_default(port);
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
-			port->sm_vars &= ~AD_PORT_MOVED;
 			port->sm_rx_state = AD_RX_PORT_DISABLED;
 
 			/* Fall Through */
-- 
2.2.0.rc0.207.ga3a616c

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net-next v1] bonding: LACP state machine variable "port-moved" is never set
  2015-04-03 20:54 [PATCH net-next v1] bonding: LACP state machine variable "port-moved" is never set Mahesh Bandewar
@ 2015-04-06 21:01 ` David Miller
  0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2015-04-06 21:01 UTC (permalink / raw)
  To: maheshb; +Cc: j.vosburgh, andy, vfalico, nikolay, maze, netdev, edumazet

From: Mahesh Bandewar <maheshb@google.com>
Date: Fri,  3 Apr 2015 13:54:10 -0700

> The state-machine checks for the presence of this flag and attempts
> to clear it but the value is never set. So why have it?
> 
> Signed-off-by: Mahesh Bandewar <maheshb@google.com>

This commit log message doesn't sound like you are asserting that this
adjustment should made.

It sounds more like you actually aren't really sure, and you're asking
folks on the list what the intent of this code is.

I'm not applying this until the commit log is adjusted to show that
you are absolutely certain that this change is valid.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-04-06 21:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-03 20:54 [PATCH net-next v1] bonding: LACP state machine variable "port-moved" is never set Mahesh Bandewar
2015-04-06 21:01 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.