There is a bug that when you modify lacp_rate via sysfs,
802.3ad won't use the new value of lacp_rate to transmit packets.
That is because port->actor_oper_port_state isn't changed.
And I want to use AD_STATE_LACP_TIMEOUT,
so I move related macros from bond_3ad.c into bond_3ad.h.
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bond_3ad.c | 48 --------------------------------------
drivers/net/bonding/bond_3ad.h | 48 ++++++++++++++++++++++++++++++++++++++
drivers/net/bonding/bond_sysfs.c | 15 +++++++++++-
3 files changed, 62 insertions(+), 49 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c7537abc..9083258 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -34,54 +34,6 @@
#include "bonding.h"
#include "bond_3ad.h"
-// General definitions
-#define AD_SHORT_TIMEOUT 1
-#define AD_LONG_TIMEOUT 0
-#define AD_STANDBY 0x2
-#define AD_MAX_TX_IN_SECOND 3
-#define AD_COLLECTOR_MAX_DELAY 0
-
-// Timer definitions(43.4.4 in the 802.3ad standard)
-#define AD_FAST_PERIODIC_TIME 1
-#define AD_SLOW_PERIODIC_TIME 30
-#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
-#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
-#define AD_CHURN_DETECTION_TIME 60
-#define AD_AGGREGATE_WAIT_TIME 2
-
-// Port state definitions(43.4.2.2 in the 802.3ad standard)
-#define AD_STATE_LACP_ACTIVITY 0x1
-#define AD_STATE_LACP_TIMEOUT 0x2
-#define AD_STATE_AGGREGATION 0x4
-#define AD_STATE_SYNCHRONIZATION 0x8
-#define AD_STATE_COLLECTING 0x10
-#define AD_STATE_DISTRIBUTING 0x20
-#define AD_STATE_DEFAULTED 0x40
-#define AD_STATE_EXPIRED 0x80
-
-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
-#define AD_PORT_BEGIN 0x1
-#define AD_PORT_LACP_ENABLED 0x2
-#define AD_PORT_ACTOR_CHURN 0x4
-#define AD_PORT_PARTNER_CHURN 0x8
-#define AD_PORT_READY 0x10
-#define AD_PORT_READY_N 0x20
-#define AD_PORT_MATCHED 0x40
-#define AD_PORT_STANDBY 0x80
-#define AD_PORT_SELECTED 0x100
-#define AD_PORT_MOVED 0x200
-
-// Port Key definitions
-// key is determined according to the link speed, duplex and
-// user key(which is yet not supported)
-// ------------------------------------------------------------
-// Port key : | User key | Speed |Duplex|
-// ------------------------------------------------------------
-// 16 6 1 0
-#define AD_DUPLEX_KEY_BITS 0x1
-#define AD_SPEED_KEY_BITS 0x3E
-#define AD_USER_KEY_BITS 0xFFC0
-
//dalloun
#define AD_LINK_SPEED_BITMASK_1MBPS 0x1
#define AD_LINK_SPEED_BITMASK_10MBPS 0x2
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 0ee3f16..6ce1f6b 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -37,6 +37,54 @@
#define AD_LACP_SLOW 0
#define AD_LACP_FAST 1
+#define AD_SHORT_TIMEOUT 1
+#define AD_LONG_TIMEOUT 0
+#define AD_STANDBY 0x2
+#define AD_MAX_TX_IN_SECOND 3
+#define AD_COLLECTOR_MAX_DELAY 0
+
+// Timer definitions(43.4.4 in the 802.3ad standard)
+#define AD_FAST_PERIODIC_TIME 1
+#define AD_SLOW_PERIODIC_TIME 30
+#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
+#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
+#define AD_CHURN_DETECTION_TIME 60
+#define AD_AGGREGATE_WAIT_TIME 2
+
+// Port state definitions(43.4.2.2 in the 802.3ad standard)
+#define AD_STATE_LACP_ACTIVITY 0x1
+#define AD_STATE_LACP_TIMEOUT 0x2
+#define AD_STATE_AGGREGATION 0x4
+#define AD_STATE_SYNCHRONIZATION 0x8
+#define AD_STATE_COLLECTING 0x10
+#define AD_STATE_DISTRIBUTING 0x20
+#define AD_STATE_DEFAULTED 0x40
+#define AD_STATE_EXPIRED 0x80
+
+// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
+#define AD_PORT_BEGIN 0x1
+#define AD_PORT_LACP_ENABLED 0x2
+#define AD_PORT_ACTOR_CHURN 0x4
+#define AD_PORT_PARTNER_CHURN 0x8
+#define AD_PORT_READY 0x10
+#define AD_PORT_READY_N 0x20
+#define AD_PORT_MATCHED 0x40
+#define AD_PORT_STANDBY 0x80
+#define AD_PORT_SELECTED 0x100
+#define AD_PORT_MOVED 0x200
+
+// Port Key definitions
+// key is determined according to the link speed, duplex and
+// user key(which is yet not supported)
+// ------------------------------------------------------------
+// Port key : | User key | Speed |Duplex|
+// ------------------------------------------------------------
+// 16 6 1 0
+#define AD_DUPLEX_KEY_BITS 0x1
+#define AD_SPEED_KEY_BITS 0x3E
+#define AD_USER_KEY_BITS 0xFFC0
+
+
typedef struct mac_addr {
u8 mac_addr_value[ETH_ALEN];
} __packed mac_addr_t;
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 88fcb25..2cad514 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -783,8 +783,13 @@ static ssize_t bonding_store_lacp(struct device *d,
struct device_attribute *attr,
const char *buf, size_t count)
{
- int new_value, ret = count;
+ int new_value, i, ret = count;
struct bonding *bond = to_bond(d);
+ struct slave *slave;
+ struct port *port = NULL;
+
+ if (!rtnl_trylock())
+ return restart_syscall();
if (bond->dev->flags & IFF_UP) {
pr_err("%s: Unable to update LACP rate because interface is up.\n",
@@ -804,6 +809,13 @@ static ssize_t bonding_store_lacp(struct device *d,
if ((new_value == 1) || (new_value == 0)) {
bond->params.lacp_fast = new_value;
+ bond_for_each_slave(bond, slave, i) {
+ port = &(slave->ad_info.port);
+ if (new_value)
+ port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
+ else
+ port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
+ }
pr_info("%s: Setting LACP rate to %s (%d).\n",
bond->dev->name, bond_lacp_tbl[new_value].modename,
new_value);
@@ -813,6 +825,7 @@ static ssize_t bonding_store_lacp(struct device *d,
ret = -EINVAL;
}
out:
+ rtnl_unlock();
return ret;
}
static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
--
1.7.4.4
Fri, Jun 03, 2011 at 04:35:33PM CEST, [email protected] wrote:
>There is a bug that when you modify lacp_rate via sysfs,
>802.3ad won't use the new value of lacp_rate to transmit packets.
>That is because port->actor_oper_port_state isn't changed.
>
>And I want to use AD_STATE_LACP_TIMEOUT,
>so I move related macros from bond_3ad.c into bond_3ad.h.
>
>Signed-off-by: Weiping Pan <[email protected]>
>---
> drivers/net/bonding/bond_3ad.c | 48 --------------------------------------
> drivers/net/bonding/bond_3ad.h | 48 ++++++++++++++++++++++++++++++++++++++
> drivers/net/bonding/bond_sysfs.c | 15 +++++++++++-
> 3 files changed, 62 insertions(+), 49 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c7537abc..9083258 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -34,54 +34,6 @@
> #include "bonding.h"
> #include "bond_3ad.h"
>
>-// General definitions
>-#define AD_SHORT_TIMEOUT 1
>-#define AD_LONG_TIMEOUT 0
>-#define AD_STANDBY 0x2
>-#define AD_MAX_TX_IN_SECOND 3
>-#define AD_COLLECTOR_MAX_DELAY 0
>-
>-// Timer definitions(43.4.4 in the 802.3ad standard)
>-#define AD_FAST_PERIODIC_TIME 1
>-#define AD_SLOW_PERIODIC_TIME 30
>-#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>-#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>-#define AD_CHURN_DETECTION_TIME 60
>-#define AD_AGGREGATE_WAIT_TIME 2
>-
>-// Port state definitions(43.4.2.2 in the 802.3ad standard)
>-#define AD_STATE_LACP_ACTIVITY 0x1
>-#define AD_STATE_LACP_TIMEOUT 0x2
>-#define AD_STATE_AGGREGATION 0x4
>-#define AD_STATE_SYNCHRONIZATION 0x8
>-#define AD_STATE_COLLECTING 0x10
>-#define AD_STATE_DISTRIBUTING 0x20
>-#define AD_STATE_DEFAULTED 0x40
>-#define AD_STATE_EXPIRED 0x80
>-
>-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>-#define AD_PORT_BEGIN 0x1
>-#define AD_PORT_LACP_ENABLED 0x2
>-#define AD_PORT_ACTOR_CHURN 0x4
>-#define AD_PORT_PARTNER_CHURN 0x8
>-#define AD_PORT_READY 0x10
>-#define AD_PORT_READY_N 0x20
>-#define AD_PORT_MATCHED 0x40
>-#define AD_PORT_STANDBY 0x80
>-#define AD_PORT_SELECTED 0x100
>-#define AD_PORT_MOVED 0x200
>-
>-// Port Key definitions
>-// key is determined according to the link speed, duplex and
>-// user key(which is yet not supported)
>-// ------------------------------------------------------------
>-// Port key : | User key | Speed |Duplex|
>-// ------------------------------------------------------------
>-// 16 6 1 0
>-#define AD_DUPLEX_KEY_BITS 0x1
>-#define AD_SPEED_KEY_BITS 0x3E
>-#define AD_USER_KEY_BITS 0xFFC0
>-
> //dalloun
> #define AD_LINK_SPEED_BITMASK_1MBPS 0x1
> #define AD_LINK_SPEED_BITMASK_10MBPS 0x2
>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>index 0ee3f16..6ce1f6b 100644
>--- a/drivers/net/bonding/bond_3ad.h
>+++ b/drivers/net/bonding/bond_3ad.h
>@@ -37,6 +37,54 @@
> #define AD_LACP_SLOW 0
> #define AD_LACP_FAST 1
>
>+#define AD_SHORT_TIMEOUT 1
>+#define AD_LONG_TIMEOUT 0
>+#define AD_STANDBY 0x2
>+#define AD_MAX_TX_IN_SECOND 3
>+#define AD_COLLECTOR_MAX_DELAY 0
>+
>+// Timer definitions(43.4.4 in the 802.3ad standard)
>+#define AD_FAST_PERIODIC_TIME 1
>+#define AD_SLOW_PERIODIC_TIME 30
>+#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>+#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>+#define AD_CHURN_DETECTION_TIME 60
>+#define AD_AGGREGATE_WAIT_TIME 2
>+
>+// Port state definitions(43.4.2.2 in the 802.3ad standard)
>+#define AD_STATE_LACP_ACTIVITY 0x1
>+#define AD_STATE_LACP_TIMEOUT 0x2
>+#define AD_STATE_AGGREGATION 0x4
>+#define AD_STATE_SYNCHRONIZATION 0x8
>+#define AD_STATE_COLLECTING 0x10
>+#define AD_STATE_DISTRIBUTING 0x20
>+#define AD_STATE_DEFAULTED 0x40
>+#define AD_STATE_EXPIRED 0x80
>+
>+// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>+#define AD_PORT_BEGIN 0x1
>+#define AD_PORT_LACP_ENABLED 0x2
>+#define AD_PORT_ACTOR_CHURN 0x4
>+#define AD_PORT_PARTNER_CHURN 0x8
>+#define AD_PORT_READY 0x10
>+#define AD_PORT_READY_N 0x20
>+#define AD_PORT_MATCHED 0x40
>+#define AD_PORT_STANDBY 0x80
>+#define AD_PORT_SELECTED 0x100
>+#define AD_PORT_MOVED 0x200
>+
>+// Port Key definitions
>+// key is determined according to the link speed, duplex and
>+// user key(which is yet not supported)
>+// ------------------------------------------------------------
>+// Port key : | User key | Speed |Duplex|
>+// ------------------------------------------------------------
>+// 16 6 1 0
>+#define AD_DUPLEX_KEY_BITS 0x1
>+#define AD_SPEED_KEY_BITS 0x3E
>+#define AD_USER_KEY_BITS 0xFFC0
>+
>+
> typedef struct mac_addr {
> u8 mac_addr_value[ETH_ALEN];
> } __packed mac_addr_t;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 88fcb25..2cad514 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -783,8 +783,13 @@ static ssize_t bonding_store_lacp(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int new_value, i, ret = count;
> struct bonding *bond = to_bond(d);
>+ struct slave *slave;
>+ struct port *port = NULL;
>+
>+ if (!rtnl_trylock())
>+ return restart_syscall();
>
> if (bond->dev->flags & IFF_UP) {
> pr_err("%s: Unable to update LACP rate because interface is up.\n",
>@@ -804,6 +809,13 @@ static ssize_t bonding_store_lacp(struct device *d,
>
> if ((new_value == 1) || (new_value == 0)) {
> bond->params.lacp_fast = new_value;
>+ bond_for_each_slave(bond, slave, i) {
you should hold read_lock(&bond->lock) when you iterate over slaves
>+ port = &(slave->ad_info.port);
>+ if (new_value)
>+ port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
>+ else
>+ port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
>+ }
> pr_info("%s: Setting LACP rate to %s (%d).\n",
> bond->dev->name, bond_lacp_tbl[new_value].modename,
> new_value);
>@@ -813,6 +825,7 @@ static ssize_t bonding_store_lacp(struct device *d,
> ret = -EINVAL;
> }
> out:
>+ rtnl_unlock();
> return ret;
> }
> static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
>--
>1.7.4.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
Weiping Pan <[email protected]> wrote:
>There is a bug that when you modify lacp_rate via sysfs,
>802.3ad won't use the new value of lacp_rate to transmit packets.
>That is because port->actor_oper_port_state isn't changed.
>
>And I want to use AD_STATE_LACP_TIMEOUT,
>so I move related macros from bond_3ad.c into bond_3ad.h.
>
>Signed-off-by: Weiping Pan <[email protected]>
>---
> drivers/net/bonding/bond_3ad.c | 48 --------------------------------------
> drivers/net/bonding/bond_3ad.h | 48 ++++++++++++++++++++++++++++++++++++++
> drivers/net/bonding/bond_sysfs.c | 15 +++++++++++-
> 3 files changed, 62 insertions(+), 49 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c7537abc..9083258 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -34,54 +34,6 @@
> #include "bonding.h"
> #include "bond_3ad.h"
>
>-// General definitions
>-#define AD_SHORT_TIMEOUT 1
>-#define AD_LONG_TIMEOUT 0
>-#define AD_STANDBY 0x2
>-#define AD_MAX_TX_IN_SECOND 3
>-#define AD_COLLECTOR_MAX_DELAY 0
>-
>-// Timer definitions(43.4.4 in the 802.3ad standard)
>-#define AD_FAST_PERIODIC_TIME 1
>-#define AD_SLOW_PERIODIC_TIME 30
>-#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>-#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>-#define AD_CHURN_DETECTION_TIME 60
>-#define AD_AGGREGATE_WAIT_TIME 2
>-
>-// Port state definitions(43.4.2.2 in the 802.3ad standard)
>-#define AD_STATE_LACP_ACTIVITY 0x1
>-#define AD_STATE_LACP_TIMEOUT 0x2
>-#define AD_STATE_AGGREGATION 0x4
>-#define AD_STATE_SYNCHRONIZATION 0x8
>-#define AD_STATE_COLLECTING 0x10
>-#define AD_STATE_DISTRIBUTING 0x20
>-#define AD_STATE_DEFAULTED 0x40
>-#define AD_STATE_EXPIRED 0x80
>-
>-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>-#define AD_PORT_BEGIN 0x1
>-#define AD_PORT_LACP_ENABLED 0x2
>-#define AD_PORT_ACTOR_CHURN 0x4
>-#define AD_PORT_PARTNER_CHURN 0x8
>-#define AD_PORT_READY 0x10
>-#define AD_PORT_READY_N 0x20
>-#define AD_PORT_MATCHED 0x40
>-#define AD_PORT_STANDBY 0x80
>-#define AD_PORT_SELECTED 0x100
>-#define AD_PORT_MOVED 0x200
>-
>-// Port Key definitions
>-// key is determined according to the link speed, duplex and
>-// user key(which is yet not supported)
>-// ------------------------------------------------------------
>-// Port key : | User key | Speed |Duplex|
>-// ------------------------------------------------------------
>-// 16 6 1 0
>-#define AD_DUPLEX_KEY_BITS 0x1
>-#define AD_SPEED_KEY_BITS 0x3E
>-#define AD_USER_KEY_BITS 0xFFC0
>-
> //dalloun
> #define AD_LINK_SPEED_BITMASK_1MBPS 0x1
> #define AD_LINK_SPEED_BITMASK_10MBPS 0x2
>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>index 0ee3f16..6ce1f6b 100644
>--- a/drivers/net/bonding/bond_3ad.h
>+++ b/drivers/net/bonding/bond_3ad.h
>@@ -37,6 +37,54 @@
> #define AD_LACP_SLOW 0
> #define AD_LACP_FAST 1
>
>+#define AD_SHORT_TIMEOUT 1
>+#define AD_LONG_TIMEOUT 0
>+#define AD_STANDBY 0x2
>+#define AD_MAX_TX_IN_SECOND 3
>+#define AD_COLLECTOR_MAX_DELAY 0
>+
>+// Timer definitions(43.4.4 in the 802.3ad standard)
>+#define AD_FAST_PERIODIC_TIME 1
>+#define AD_SLOW_PERIODIC_TIME 30
>+#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>+#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>+#define AD_CHURN_DETECTION_TIME 60
>+#define AD_AGGREGATE_WAIT_TIME 2
>+
>+// Port state definitions(43.4.2.2 in the 802.3ad standard)
>+#define AD_STATE_LACP_ACTIVITY 0x1
>+#define AD_STATE_LACP_TIMEOUT 0x2
>+#define AD_STATE_AGGREGATION 0x4
>+#define AD_STATE_SYNCHRONIZATION 0x8
>+#define AD_STATE_COLLECTING 0x10
>+#define AD_STATE_DISTRIBUTING 0x20
>+#define AD_STATE_DEFAULTED 0x40
>+#define AD_STATE_EXPIRED 0x80
>+
>+// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>+#define AD_PORT_BEGIN 0x1
>+#define AD_PORT_LACP_ENABLED 0x2
>+#define AD_PORT_ACTOR_CHURN 0x4
>+#define AD_PORT_PARTNER_CHURN 0x8
>+#define AD_PORT_READY 0x10
>+#define AD_PORT_READY_N 0x20
>+#define AD_PORT_MATCHED 0x40
>+#define AD_PORT_STANDBY 0x80
>+#define AD_PORT_SELECTED 0x100
>+#define AD_PORT_MOVED 0x200
>+
>+// Port Key definitions
>+// key is determined according to the link speed, duplex and
>+// user key(which is yet not supported)
>+// ------------------------------------------------------------
>+// Port key : | User key | Speed |Duplex|
>+// ------------------------------------------------------------
>+// 16 6 1 0
>+#define AD_DUPLEX_KEY_BITS 0x1
>+#define AD_SPEED_KEY_BITS 0x3E
>+#define AD_USER_KEY_BITS 0xFFC0
>+
>+
> typedef struct mac_addr {
> u8 mac_addr_value[ETH_ALEN];
> } __packed mac_addr_t;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 88fcb25..2cad514 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -783,8 +783,13 @@ static ssize_t bonding_store_lacp(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int new_value, i, ret = count;
> struct bonding *bond = to_bond(d);
>+ struct slave *slave;
>+ struct port *port = NULL;
>+
>+ if (!rtnl_trylock())
>+ return restart_syscall();
>
> if (bond->dev->flags & IFF_UP) {
> pr_err("%s: Unable to update LACP rate because interface is up.\n",
>@@ -804,6 +809,13 @@ static ssize_t bonding_store_lacp(struct device *d,
>
> if ((new_value == 1) || (new_value == 0)) {
> bond->params.lacp_fast = new_value;
>+ bond_for_each_slave(bond, slave, i) {
>+ port = &(slave->ad_info.port);
>+ if (new_value)
>+ port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
>+ else
>+ port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
>+ }
I think this would be cleaner if done via a helper function in
bond_3ad.c rather than inline.
Also, this should either have a comment about not needing
locking beyond RTNL, or just do the "correct" locking.
Since this requires the bond to be down, there is no real
contention for the port state (because the state machines and LACPDU
processing does not run), and holding RTNL is enough to mutex. If the
!IFF_UP restriction is ever removed, though, this would require holding
bond->lock for read and locking each port's state machine lock before
altering actor_oper_port_state.
-J
> pr_info("%s: Setting LACP rate to %s (%d).\n",
> bond->dev->name, bond_lacp_tbl[new_value].modename,
> new_value);
>@@ -813,6 +825,7 @@ static ssize_t bonding_store_lacp(struct device *d,
> ret = -EINVAL;
> }
> out:
>+ rtnl_unlock();
> return ret;
> }
> static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
>--
>1.7.4.4
>
---
-Jay Vosburgh, IBM Linux Technology Center, [email protected]
There is a bug that when you modify lacp_rate via sysfs,
802.3ad won't use the new value of lacp_rate to transmit packets.
That is because port->actor_oper_port_state isn't changed.
Change Notes:
v2)
1 Hold read_lock(&bond->lock) when iterate slaves, suggested by Jiri Pirko.
2 Modify actor_oper_port_state via a helper function in bond_3ad.c,
suggested by Jay Vosburgh.
3 Hold slave->state_machine_lock,
so we can modify port->actor_oper_port_state, no matter bond is up or down.
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bond_3ad.c | 27 +++++++++++++++++++++++++++
drivers/net/bonding/bond_3ad.h | 1 +
drivers/net/bonding/bond_sysfs.c | 1 +
3 files changed, 29 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c7537abc..5111e0d 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2473,3 +2473,30 @@ void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len);
read_unlock(&bond->lock);
}
+
+/*
+ * When modify lacp_rate parameter via sysfs,
+ * update actor_oper_port_state of each port.
+ *
+ * Hold slave->state_machine_lock,
+ * so we can modify port->actor_oper_port_state,
+ * no matter bond is up or down.
+ */
+void bond_3ad_update_lacp_rate(struct bonding *bond)
+{
+ int i;
+ struct slave *slave;
+ struct port *port = NULL;
+
+ read_lock(&bond->lock);
+ bond_for_each_slave(bond, slave, i) {
+ port = &(slave->ad_info.port);
+ __get_state_machine_lock(port);
+ if (bond->params.lacp_fast)
+ port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
+ else
+ port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
+ __release_state_machine_lock(port);
+ }
+ read_unlock(&bond->lock);
+}
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 0ee3f16..e466faf 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -282,5 +282,6 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
struct slave *slave);
int bond_3ad_set_carrier(struct bonding *bond);
+void bond_3ad_update_lacp_rate(struct bonding *bond);
#endif //__BOND_3AD_H__
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 88fcb25..03d1196 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -804,6 +804,7 @@ static ssize_t bonding_store_lacp(struct device *d,
if ((new_value == 1) || (new_value == 0)) {
bond->params.lacp_fast = new_value;
+ bond_3ad_update_lacp_rate(bond);
pr_info("%s: Setting LACP rate to %s (%d).\n",
bond->dev->name, bond_lacp_tbl[new_value].modename,
new_value);
--
1.7.4
On Sun, Jun 5, 2011 at 11:16 AM, Weiping Pan <[email protected]> wrote:
> There is a bug that when you modify lacp_rate via sysfs,
> 802.3ad won't use the new value of lacp_rate to transmit packets.
> That is because port->actor_oper_port_state isn't changed.
>
> Change Notes:
> v2)
> 1 Hold read_lock(&bond->lock) when iterate slaves, suggested by Jiri Pirko.
> 2 Modify actor_oper_port_state via a helper function in bond_3ad.c,
> suggested by Jay Vosburgh.
> 3 Hold slave->state_machine_lock,
> so we can modify port->actor_oper_port_state, no matter bond is up or down.
>
> Signed-off-by: Weiping Pan <[email protected]>
> ---
> drivers/net/bonding/bond_3ad.c | 27 +++++++++++++++++++++++++++
> drivers/net/bonding/bond_3ad.h | 1 +
> drivers/net/bonding/bond_sysfs.c | 1 +
> 3 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index c7537abc..5111e0d 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -2473,3 +2473,30 @@ void bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
> bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len);
> read_unlock(&bond->lock);
> }
> +
> +/*
> + * When modify lacp_rate parameter via sysfs,
> + * update actor_oper_port_state of each port.
> + *
> + * Hold slave->state_machine_lock,
> + * so we can modify port->actor_oper_port_state,
> + * no matter bond is up or down.
> + */
> +void bond_3ad_update_lacp_rate(struct bonding *bond)
> +{
> + int i;
> + struct slave *slave;
> + struct port *port = NULL;
> +
> + read_lock(&bond->lock);
> + bond_for_each_slave(bond, slave, i) {
> + port = &(slave->ad_info.port);
Please use SLAVE_AD_INFO().
Other than this, it looks good to me,
Reviewed-by: WANG Cong <[email protected]>
Thanks!