There is bug that when you modify lacp_rate via sysfs,
802.3ad won't use the new value of lacp_rate to transmit packets.
This is because port->actor_oper_port_state isn't changed.
As for ad_select, it can work,
but both struct bond_params and ad_bond_info have lacp_fast and ad_select,
they are duplicate and need extra synchronization.
802.3ad can get them from bond_params directly every time.
And ad_timer and arp_mon_pt aren't used any more, just delete them.
changelog:
v2:
add bond_3ad_update_lacp_rate() as a helper function,
and hold bond->lock when iterates slave list.
v3:
delete duplicate lacp_fast and agg_select_mode from struct ad_bond_info.
v4:
delete unused ad_timer and arp_mon_pt.
Weiping Pan (5):
bonding: make 802.3ad use latest lacp_rate
bonding:delete lacp_fast from ad_bond_info
bonding:delete agg_select_mode from ad_bond_info
bonding: delete unused ad_timer
bonding: delete unused arp_mon_pt
drivers/net/bonding/bond_3ad.c | 41 ++++++++++++++++++++++++++++++++-----
drivers/net/bonding/bond_3ad.h | 8 +-----
drivers/net/bonding/bond_main.c | 3 +-
drivers/net/bonding/bond_sysfs.c | 1 +
drivers/net/bonding/bonding.h | 1 -
5 files changed, 39 insertions(+), 15 deletions(-)
--
1.7.4.4
There is bug that when you modify lacp_rate via sysfs,
802.3ad won't use the new value of lacp_rate to transmit packets.
This is because port->actor_oper_port_state isn't changed.
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bond_3ad.c | 31 +++++++++++++++++++++++++++++++
drivers/net/bonding/bond_3ad.h | 1 +
drivers/net/bonding/bond_sysfs.c | 1 +
3 files changed, 33 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index c7537abc..4512bc4 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2473,3 +2473,34 @@ 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;
+ int lacp_fast;
+
+ read_lock(&bond->lock);
+ lacp_fast = bond->params.lacp_fast;
+
+ bond_for_each_slave(bond, slave, i) {
+ port = &(SLAVE_AD_INFO(slave).port);
+ __get_state_machine_lock(port);
+ if (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.4
These is also a bug, that if you modify lacp_rate via sysfs,
and add new slaves in bonding, new slaves won't use the latest lacp_rate,
since ad_bond_info->lacp_fast is initialized only once,
in bond_3ad_initialize().
Since both struct bond_params and ad_bond_info have lacp_fast,
they are duplicate and need extra synchronization.
bond_3ad_bind_slave() can use bond_params->lacp_fast to initialize port.
So we can just remove lacp_fast from struct ad_bond_info.
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bond_3ad.c | 7 +++----
drivers/net/bonding/bond_3ad.h | 5 +----
drivers/net/bonding/bond_main.c | 3 +--
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 4512bc4..013a801 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1868,11 +1868,10 @@ static u16 aggregator_identifier;
* bond_3ad_initialize - initialize a bond's 802.3ad parameters and structures
* @bond: bonding struct to work on
* @tick_resolution: tick duration (millisecond resolution)
- * @lacp_fast: boolean. whether fast periodic should be used
*
* Can be called only after the mac address of the bond is set.
*/
-void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast)
+void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
{
// check that the bond is not initialized yet
if (MAC_ADDRESS_COMPARE(&(BOND_AD_INFO(bond).system.sys_mac_addr),
@@ -1880,7 +1879,6 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fas
aggregator_identifier = 0;
- BOND_AD_INFO(bond).lacp_fast = lacp_fast;
BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
@@ -1903,6 +1901,7 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fas
int bond_3ad_bind_slave(struct slave *slave)
{
struct bonding *bond = bond_get_bond_by_slave(slave);
+ int lacp_fast = bond->params.lacp_fast;
struct port *port;
struct aggregator *aggregator;
@@ -1918,7 +1917,7 @@ int bond_3ad_bind_slave(struct slave *slave)
// port initialization
port = &(SLAVE_AD_INFO(slave).port);
- ad_initialize_port(port, BOND_AD_INFO(bond).lacp_fast);
+ ad_initialize_port(port, lacp_fast);
port->slave = slave;
port->actor_port_number = SLAVE_AD_INFO(slave).id;
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index e466faf..9782785 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -254,9 +254,6 @@ struct ad_bond_info {
struct ad_system system; /* 802.3ad system structure */
u32 agg_select_timer; // Timer to select aggregator after all adapter's hand shakes
u32 agg_select_mode; // Mode of selection of active aggregator(bandwidth/count)
- int lacp_fast; /* whether fast periodic tx should be
- * requested
- */
struct timer_list ad_timer;
};
@@ -269,7 +266,7 @@ struct ad_slave_info {
};
// ================= AD Exported functions to the main bonding code ==================
-void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast);
+void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution);
int bond_3ad_bind_slave(struct slave *slave);
void bond_3ad_unbind_slave(struct slave *slave);
void bond_3ad_state_machine_handler(struct work_struct *);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 716c852..bb1af9c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1843,8 +1843,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
/* Initialize AD with the number of times that the AD timer is called in 1 second
* can be called only after the mac address of the bond is set
*/
- bond_3ad_initialize(bond, 1000/AD_TIMER_INTERVAL,
- bond->params.lacp_fast);
+ bond_3ad_initialize(bond, 1000/AD_TIMER_INTERVAL);
} else {
SLAVE_AD_INFO(new_slave).id =
SLAVE_AD_INFO(new_slave->prev).id + 1;
--
1.7.4.4
bond_params->ad_select and ad_bond_info->agg_select_mode have the same
meaning, they are duplicate and need extra synchronization.
__get_agg_selection_mode() get ad_select from bond_params directly.
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bond_3ad.c | 3 +--
drivers/net/bonding/bond_3ad.h | 1 -
2 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 013a801..6122725 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -262,7 +262,7 @@ static inline u32 __get_agg_selection_mode(struct port *port)
if (bond == NULL)
return BOND_AD_STABLE;
- return BOND_AD_INFO(bond).agg_select_mode;
+ return bond->params.ad_select;
}
/**
@@ -1859,7 +1859,6 @@ static void ad_marker_response_received(struct bond_marker *marker,
void bond_3ad_initiate_agg_selection(struct bonding *bond, int timeout)
{
BOND_AD_INFO(bond).agg_select_timer = timeout;
- BOND_AD_INFO(bond).agg_select_mode = bond->params.ad_select;
}
static u16 aggregator_identifier;
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 9782785..1682e69 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -253,7 +253,6 @@ struct ad_system {
struct ad_bond_info {
struct ad_system system; /* 802.3ad system structure */
u32 agg_select_timer; // Timer to select aggregator after all adapter's hand shakes
- u32 agg_select_mode; // Mode of selection of active aggregator(bandwidth/count)
struct timer_list ad_timer;
};
--
1.7.4.4
Now we use agg_select_timer and ad_work.
Reviewed-by: WANG Cong <[email protected]>
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bond_3ad.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 1682e69..235b2cc 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -253,7 +253,6 @@ struct ad_system {
struct ad_bond_info {
struct ad_system system; /* 802.3ad system structure */
u32 agg_select_timer; // Timer to select aggregator after all adapter's hand shakes
- struct timer_list ad_timer;
};
struct ad_slave_info {
--
1.7.4.4
Now all received packets are handled by bond_handle_frame,
and arp_mon_pt isn't used any more.
Reviewed-by: WANG Cong <[email protected]>
Signed-off-by: Weiping Pan <[email protected]>
---
drivers/net/bonding/bonding.h | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ea1d005..382903f 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -240,7 +240,6 @@ struct bonding {
struct bond_params params;
struct list_head vlan_list;
struct vlan_group *vlgrp;
- struct packet_type arp_mon_pt;
struct workqueue_struct *wq;
struct delayed_work mii_work;
struct delayed_work arp_work;
--
1.7.4.4
My head will spin if I see yet another submission of these
patches :-(
On 06/09/2011 03:21 PM, David Miller wrote:
> My head will spin if I see yet another submission of these
> patches :-(
sorry for the inconvenience
Weiping Pan
Weiping Pan <[email protected]> wrote:
>There is bug that when you modify lacp_rate via sysfs,
>802.3ad won't use the new value of lacp_rate to transmit packets.
>This is because port->actor_oper_port_state isn't changed.
>
>As for ad_select, it can work,
>but both struct bond_params and ad_bond_info have lacp_fast and ad_select,
>they are duplicate and need extra synchronization.
>802.3ad can get them from bond_params directly every time.
>
>And ad_timer and arp_mon_pt aren't used any more, just delete them.
>
>changelog:
>v2:
>add bond_3ad_update_lacp_rate() as a helper function,
>and hold bond->lock when iterates slave list.
>
>v3:
>delete duplicate lacp_fast and agg_select_mode from struct ad_bond_info.
>
>v4:
>delete unused ad_timer and arp_mon_pt.
All patches in the series look reasonable.
Signed-off-by: Jay Vosburgh <[email protected]>
-J
>Weiping Pan (5):
> bonding: make 802.3ad use latest lacp_rate
> bonding:delete lacp_fast from ad_bond_info
> bonding:delete agg_select_mode from ad_bond_info
> bonding: delete unused ad_timer
> bonding: delete unused arp_mon_pt
>
> drivers/net/bonding/bond_3ad.c | 41 ++++++++++++++++++++++++++++++++-----
> drivers/net/bonding/bond_3ad.h | 8 +-----
> drivers/net/bonding/bond_main.c | 3 +-
> drivers/net/bonding/bond_sysfs.c | 1 +
> drivers/net/bonding/bonding.h | 1 -
> 5 files changed, 39 insertions(+), 15 deletions(-)
>
>--
>1.7.4.4
>
From: Jay Vosburgh <[email protected]>
Date: Thu, 09 Jun 2011 14:58:11 -0700
> Weiping Pan <[email protected]> wrote:
>
>>There is bug that when you modify lacp_rate via sysfs,
>>802.3ad won't use the new value of lacp_rate to transmit packets.
>>This is because port->actor_oper_port_state isn't changed.
>>
>>As for ad_select, it can work,
>>but both struct bond_params and ad_bond_info have lacp_fast and ad_select,
>>they are duplicate and need extra synchronization.
>>802.3ad can get them from bond_params directly every time.
>>
>>And ad_timer and arp_mon_pt aren't used any more, just delete them.
>>
>>changelog:
>>v2:
>>add bond_3ad_update_lacp_rate() as a helper function,
>>and hold bond->lock when iterates slave list.
>>
>>v3:
>>delete duplicate lacp_fast and agg_select_mode from struct ad_bond_info.
>>
>>v4:
>>delete unused ad_timer and arp_mon_pt.
>
> All patches in the series look reasonable.
>
> Signed-off-by: Jay Vosburgh <[email protected]>
All applied, thanks everyone.