* [PATCH v4 net-next 12/27] bonding: rework rlb_next_rx_slave() to use bond_for_each_slave()
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Currently, we're using bond_for_each_slave_from(), which is really hard to
implement under RCU and/or neighbour list.
Remove it and use bond_for_each_slave() instead, taking care of the last
used slave.
Also, rename next_rx_slave to rx_slave and store the current (last)
rx_slave.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
New patch.
drivers/net/bonding/bond_alb.c | 41 +++++++++++++++++++++--------------------
drivers/net/bonding/bond_alb.h | 4 +---
2 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index f4929ce..c5b85ad 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -383,30 +383,31 @@ out:
static struct slave *rlb_next_rx_slave(struct bonding *bond)
{
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
- struct slave *rx_slave, *slave, *start_at;
- int i = 0;
-
- if (bond_info->next_rx_slave)
- start_at = bond_info->next_rx_slave;
- else
- start_at = bond_first_slave(bond);
-
- rx_slave = NULL;
+ struct slave *before = NULL, *rx_slave = NULL, *slave;
+ struct list_head *iter;
+ bool found = false;
- bond_for_each_slave_from(bond, slave, i, start_at) {
- if (SLAVE_IS_OK(slave)) {
- if (!rx_slave) {
- rx_slave = slave;
- } else if (slave->speed > rx_slave->speed) {
+ bond_for_each_slave(bond, slave, iter) {
+ if (!SLAVE_IS_OK(slave))
+ continue;
+ if (!found) {
+ if (!before || before->speed < slave->speed)
+ before = slave;
+ } else {
+ if (!rx_slave || rx_slave->speed < slave->speed)
rx_slave = slave;
- }
}
+ if (slave == bond_info->rx_slave)
+ found = true;
}
+ /* we didn't find anything after the current or we have something
+ * better before and up to the current slave
+ */
+ if (!rx_slave || (before && rx_slave->speed < before->speed))
+ rx_slave = before;
- if (rx_slave) {
- slave = bond_next_slave(bond, rx_slave);
- bond_info->next_rx_slave = slave;
- }
+ if (rx_slave)
+ bond_info->rx_slave = rx_slave;
return rx_slave;
}
@@ -1611,7 +1612,7 @@ void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
tlb_clear_slave(bond, slave, 0);
if (bond->alb_info.rlb_enabled) {
- bond->alb_info.next_rx_slave = NULL;
+ bond->alb_info.rx_slave = NULL;
rlb_clear_slave(bond, slave);
}
}
diff --git a/drivers/net/bonding/bond_alb.h b/drivers/net/bonding/bond_alb.h
index c5eff5d..4226044 100644
--- a/drivers/net/bonding/bond_alb.h
+++ b/drivers/net/bonding/bond_alb.h
@@ -154,9 +154,7 @@ struct alb_bond_info {
u8 rx_ntt; /* flag - need to transmit
* to all rx clients
*/
- struct slave *next_rx_slave;/* next slave to be assigned
- * to a new rx client for
- */
+ struct slave *rx_slave;/* last slave to xmit from */
u8 primary_is_promisc; /* boolean */
u32 rlb_promisc_timeout_counter;/* counts primary
* promiscuity time
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 13/27] bonding: rework bond_find_best_slave() to use bond_for_each_slave()
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
bond_find_best_slave() does not have to be balanced - i.e. return the slave
that is *after* some other slave, but rather return the best slave that
suits, except of bond->primary_slave - in which case we just return it if
it's suitable.
After that we just look through all the slaves and return either first up
slave or the slave whose link came back earliest.
We also don't care about curr_active_slave lock cause we use it in
bond_should_change_active() only and there we take it right away - i.e. it
won't go away.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
New patch.
drivers/net/bonding/bond_main.c | 43 ++++++++++++-----------------------------
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 85e99ae..6abbfac 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -785,43 +785,24 @@ static bool bond_should_change_active(struct bonding *bond)
/**
* find_best_interface - select the best available slave to be the active one
* @bond: our bonding struct
- *
- * Warning: Caller must hold curr_slave_lock for writing.
*/
static struct slave *bond_find_best_slave(struct bonding *bond)
{
- struct slave *new_active, *old_active;
- struct slave *bestslave = NULL;
+ struct slave *slave, *bestslave = NULL;
+ struct list_head *iter;
int mintime = bond->params.updelay;
- int i;
- new_active = bond->curr_active_slave;
-
- if (!new_active) { /* there were no active slaves left */
- new_active = bond_first_slave(bond);
- if (!new_active)
- return NULL; /* still no slave, return NULL */
- }
+ if (bond->primary_slave && bond->primary_slave->link == BOND_LINK_UP &&
+ bond_should_change_active(bond))
+ return bond->primary_slave;
- if ((bond->primary_slave) &&
- bond->primary_slave->link == BOND_LINK_UP &&
- bond_should_change_active(bond)) {
- new_active = bond->primary_slave;
- }
-
- /* remember where to stop iterating over the slaves */
- old_active = new_active;
-
- bond_for_each_slave_from(bond, new_active, i, old_active) {
- if (new_active->link == BOND_LINK_UP) {
- return new_active;
- } else if (new_active->link == BOND_LINK_BACK &&
- IS_UP(new_active->dev)) {
- /* link up, but waiting for stabilization */
- if (new_active->delay < mintime) {
- mintime = new_active->delay;
- bestslave = new_active;
- }
+ bond_for_each_slave(bond, slave, iter) {
+ if (slave->link == BOND_LINK_UP)
+ return slave;
+ if (slave->link == BOND_LINK_BACK && IS_UP(slave->dev) &&
+ slave->delay < mintime) {
+ mintime = slave->delay;
+ bestslave = slave;
}
}
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 11/27] bonding: rework bond_3ad_xmit_xor() to use bond_for_each_slave() only
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Currently, there are two loops - first we find the first slave in an
aggregator after the xmit_hash_policy() returned number, and after that we
loop from that slave, over bonding head, and till that slave to find any
suitable slave to send the packet through.
Replace it by just one bond_for_each_slave() loop, which first loops
through the requested number of slaves, saving the first suitable one, and
after that we've hit the requested number of slaves to skip - search for
any up slave to send the packet through. If we don't find such kind of
slave - then just send the packet through the first suitable slave found.
Logic remains unchainged, and we skip two loops. Also, refactor it a bit
for readability.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
New patch.
drivers/net/bonding/bond_3ad.c | 46 ++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 24 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 3847aee..c861ee7 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2417,15 +2417,15 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
{
- struct slave *slave, *start_at;
struct bonding *bond = netdev_priv(dev);
+ struct slave *slave, *first_ok_slave;
+ struct aggregator *agg;
+ struct ad_info ad_info;
struct list_head *iter;
- int slave_agg_no;
int slaves_in_agg;
- int agg_id;
- int i;
- struct ad_info ad_info;
+ int slave_agg_no;
int res = 1;
+ int agg_id;
read_lock(&bond->lock);
if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
@@ -2438,20 +2438,28 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
agg_id = ad_info.aggregator_id;
if (slaves_in_agg == 0) {
- /*the aggregator is empty*/
pr_debug("%s: Error: active aggregator is empty\n", dev->name);
goto out;
}
slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
+ first_ok_slave = NULL;
bond_for_each_slave(bond, slave, iter) {
- struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
+ agg = SLAVE_AD_INFO(slave).port.aggregator;
+ if (!agg || agg->aggregator_identifier != agg_id)
+ continue;
- if (agg && (agg->aggregator_identifier == agg_id)) {
+ if (slave_agg_no >= 0) {
+ if (!first_ok_slave && SLAVE_IS_OK(slave))
+ first_ok_slave = slave;
slave_agg_no--;
- if (slave_agg_no < 0)
- break;
+ continue;
+ }
+
+ if (SLAVE_IS_OK(slave)) {
+ res = bond_dev_queue_xmit(bond, skb, slave->dev);
+ goto out;
}
}
@@ -2461,20 +2469,10 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
goto out;
}
- start_at = slave;
-
- bond_for_each_slave_from(bond, slave, i, start_at) {
- int slave_agg_id = 0;
- struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
-
- if (agg)
- slave_agg_id = agg->aggregator_identifier;
-
- if (SLAVE_IS_OK(slave) && agg && (slave_agg_id == agg_id)) {
- res = bond_dev_queue_xmit(bond, skb, slave->dev);
- break;
- }
- }
+ /* we couldn't find any suitable slave after the agg_no, so use the
+ * first suitable found, if found. */
+ if (first_ok_slave)
+ res = bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
out:
read_unlock(&bond->lock);
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 10/27] bonding: use bond_for_each_slave() in bond_uninit()
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
We're safe agains removal there, cause we use neighbours primitives.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
Move the patch rigth after we start using neighbour lists for
bond_for_each_slave().
drivers/net/bonding/bond_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9064e24..85e99ae 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4090,12 +4090,13 @@ static void bond_setup(struct net_device *bond_dev)
static void bond_uninit(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct slave *slave, *tmp_slave;
+ struct list_head *iter;
+ struct slave *slave;
bond_netpoll_cleanup(bond_dev);
/* Release the bonded slaves */
- list_for_each_entry_safe(slave, tmp_slave, &bond->slave_list, list)
+ bond_for_each_slave(bond, slave, iter)
__bond_release_one(bond_dev, slave->dev, true);
pr_info("%s: released all slaves\n", bond_dev->name);
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 09/27] bonding: make bond_for_each_slave() use lower neighbour's private
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek,
Dimitris Michailidis
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
It needs a list_head *iter, so add it wherever needed. Use both non-rcu and
rcu variants.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Dimitris Michailidis <dm@chelsio.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
Move the patch right after we're ready to use it - for easier review.
Follow renaming strategy.
Add cxgb4 driver - it also began to use bond_for_each_slave().
drivers/net/bonding/bond_3ad.c | 6 +-
drivers/net/bonding/bond_alb.c | 18 ++++--
drivers/net/bonding/bond_main.c | 86 ++++++++++++++++---------
drivers/net/bonding/bond_procfs.c | 5 +-
drivers/net/bonding/bond_sysfs.c | 23 ++++---
drivers/net/bonding/bonding.h | 12 ++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 +-
7 files changed, 98 insertions(+), 55 deletions(-)
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 0d8f427..3847aee 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2419,6 +2419,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
{
struct slave *slave, *start_at;
struct bonding *bond = netdev_priv(dev);
+ struct list_head *iter;
int slave_agg_no;
int slaves_in_agg;
int agg_id;
@@ -2444,7 +2445,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
if (agg && (agg->aggregator_identifier == agg_id)) {
@@ -2515,11 +2516,12 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
void bond_3ad_update_lacp_rate(struct bonding *bond)
{
struct port *port = NULL;
+ struct list_head *iter;
struct slave *slave;
int lacp_fast;
lacp_fast = bond->params.lacp_fast;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
port = &(SLAVE_AD_INFO(slave).port);
__get_state_machine_lock(port);
if (lacp_fast)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 6437657..f4929ce 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -223,13 +223,14 @@ static long long compute_gap(struct slave *slave)
static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
{
struct slave *slave, *least_loaded;
+ struct list_head *iter;
long long max_gap;
least_loaded = NULL;
max_gap = LLONG_MIN;
/* Find the slave with the largest gap */
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (SLAVE_IS_OK(slave)) {
long long gap = compute_gap(slave);
@@ -1172,8 +1173,9 @@ static void alb_change_hw_addr_on_detach(struct bonding *bond, struct slave *sla
*/
static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slave *slave)
{
- struct slave *tmp_slave1, *free_mac_slave = NULL;
struct slave *has_bond_addr = bond->curr_active_slave;
+ struct slave *tmp_slave1, *free_mac_slave = NULL;
+ struct list_head *iter;
if (list_empty(&bond->slave_list)) {
/* this is the first slave */
@@ -1196,7 +1198,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
/* The slave's address is equal to the address of the bond.
* Search for a spare address in the bond for this slave.
*/
- bond_for_each_slave(bond, tmp_slave1) {
+ bond_for_each_slave(bond, tmp_slave1, iter) {
if (!bond_slave_has_mac(bond, tmp_slave1->perm_hwaddr)) {
/* no slave has tmp_slave1's perm addr
* as its curr addr
@@ -1247,6 +1249,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
static int alb_set_mac_address(struct bonding *bond, void *addr)
{
struct slave *slave, *rollback_slave;
+ struct list_head *iter;
struct sockaddr sa;
char tmp_addr[ETH_ALEN];
int res;
@@ -1254,7 +1257,7 @@ static int alb_set_mac_address(struct bonding *bond, void *addr)
if (bond->alb_info.rlb_enabled)
return 0;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
/* save net_device's current hw address */
memcpy(tmp_addr, slave->dev->dev_addr, ETH_ALEN);
@@ -1274,7 +1277,7 @@ unwind:
sa.sa_family = bond->dev->type;
/* unwind from head to the slave that failed */
- bond_for_each_slave(bond, rollback_slave) {
+ bond_for_each_slave(bond, rollback_slave, iter) {
if (rollback_slave == slave)
break;
memcpy(tmp_addr, rollback_slave->dev->dev_addr, ETH_ALEN);
@@ -1460,6 +1463,7 @@ void bond_alb_monitor(struct work_struct *work)
struct bonding *bond = container_of(work, struct bonding,
alb_work.work);
struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
+ struct list_head *iter;
struct slave *slave;
read_lock(&bond->lock);
@@ -1482,7 +1486,7 @@ void bond_alb_monitor(struct work_struct *work)
*/
read_lock(&bond->curr_slave_lock);
- bond_for_each_slave(bond, slave)
+ bond_for_each_slave(bond, slave, iter)
alb_send_learning_packets(slave, slave->dev->dev_addr);
read_unlock(&bond->curr_slave_lock);
@@ -1495,7 +1499,7 @@ void bond_alb_monitor(struct work_struct *work)
read_lock(&bond->curr_slave_lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
tlb_clear_slave(bond, slave, 1);
if (slave == bond->curr_active_slave) {
SLAVE_TLB_INFO(slave).load =
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d94b6c1..9064e24 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -333,9 +333,10 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave, *rollback_slave;
+ struct list_head *iter;
int res;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
res = vlan_vid_add(slave->dev, proto, vid);
if (res)
goto unwind;
@@ -345,7 +346,7 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
unwind:
/* unwind to the slave that failed */
- bond_for_each_slave(bond, rollback_slave) {
+ bond_for_each_slave(bond, rollback_slave, iter) {
if (rollback_slave == slave)
break;
@@ -364,9 +365,10 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
__be16 proto, u16 vid)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct list_head *iter;
struct slave *slave;
- bond_for_each_slave(bond, slave)
+ bond_for_each_slave(bond, slave, iter)
vlan_vid_del(slave->dev, proto, vid);
if (bond_is_lb(bond))
@@ -386,6 +388,7 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
*/
static int bond_set_carrier(struct bonding *bond)
{
+ struct list_head *iter;
struct slave *slave;
if (list_empty(&bond->slave_list))
@@ -394,7 +397,7 @@ static int bond_set_carrier(struct bonding *bond)
if (bond->params.mode == BOND_MODE_8023AD)
return bond_3ad_set_carrier(bond);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (slave->link == BOND_LINK_UP) {
if (!netif_carrier_ok(bond->dev)) {
netif_carrier_on(bond->dev);
@@ -526,7 +529,9 @@ static int bond_check_dev_link(struct bonding *bond,
*/
static int bond_set_promiscuity(struct bonding *bond, int inc)
{
+ struct list_head *iter;
int err = 0;
+
if (USES_PRIMARY(bond->params.mode)) {
/* write lock already acquired */
if (bond->curr_active_slave) {
@@ -536,7 +541,7 @@ static int bond_set_promiscuity(struct bonding *bond, int inc)
} else {
struct slave *slave;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
err = dev_set_promiscuity(slave->dev, inc);
if (err)
return err;
@@ -550,7 +555,9 @@ static int bond_set_promiscuity(struct bonding *bond, int inc)
*/
static int bond_set_allmulti(struct bonding *bond, int inc)
{
+ struct list_head *iter;
int err = 0;
+
if (USES_PRIMARY(bond->params.mode)) {
/* write lock already acquired */
if (bond->curr_active_slave) {
@@ -560,7 +567,7 @@ static int bond_set_allmulti(struct bonding *bond, int inc)
} else {
struct slave *slave;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
err = dev_set_allmulti(slave->dev, inc);
if (err)
return err;
@@ -1050,9 +1057,10 @@ static void bond_poll_controller(struct net_device *bond_dev)
static void bond_netpoll_cleanup(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct list_head *iter;
struct slave *slave;
- bond_for_each_slave(bond, slave)
+ bond_for_each_slave(bond, slave, iter)
if (IS_UP(slave->dev))
slave_disable_netpoll(slave);
}
@@ -1060,10 +1068,11 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni, gfp_t gfp)
{
struct bonding *bond = netdev_priv(dev);
+ struct list_head *iter;
struct slave *slave;
int err = 0;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
err = slave_enable_netpoll(slave);
if (err) {
bond_netpoll_cleanup(dev);
@@ -1091,6 +1100,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
netdev_features_t features)
{
struct bonding *bond = netdev_priv(dev);
+ struct list_head *iter;
netdev_features_t mask;
struct slave *slave;
@@ -1104,7 +1114,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
features &= ~NETIF_F_ONE_FOR_ALL;
features |= NETIF_F_ALL_FOR_ALL;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
features = netdev_increment_features(features,
slave->dev->features,
mask);
@@ -1122,16 +1132,17 @@ static void bond_compute_features(struct bonding *bond)
{
unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE;
netdev_features_t vlan_features = BOND_VLAN_FEATURES;
+ struct net_device *bond_dev = bond->dev;
+ struct list_head *iter;
+ struct slave *slave;
unsigned short max_hard_header_len = ETH_HLEN;
unsigned int gso_max_size = GSO_MAX_SIZE;
- struct net_device *bond_dev = bond->dev;
u16 gso_max_segs = GSO_MAX_SEGS;
- struct slave *slave;
if (list_empty(&bond->slave_list))
goto done;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
vlan_features = netdev_increment_features(vlan_features,
slave->dev->vlan_features, BOND_VLAN_FEATURES);
@@ -1993,11 +2004,12 @@ static int bond_info_query(struct net_device *bond_dev, struct ifbond *info)
static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *info)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct list_head *iter;
int i = 0, res = -ENODEV;
struct slave *slave;
read_lock(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (i++ == (int)info->slave_id) {
res = 0;
strcpy(info->slave_name, slave->dev->name);
@@ -2018,12 +2030,13 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
static int bond_miimon_inspect(struct bonding *bond)
{
int link_state, commit = 0;
+ struct list_head *iter;
struct slave *slave;
bool ignore_updelay;
ignore_updelay = !bond->curr_active_slave ? true : false;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
slave->new_link = BOND_LINK_NOCHANGE;
link_state = bond_check_dev_link(bond, slave->dev, 0);
@@ -2117,9 +2130,10 @@ static int bond_miimon_inspect(struct bonding *bond)
static void bond_miimon_commit(struct bonding *bond)
{
+ struct list_head *iter;
struct slave *slave;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
switch (slave->new_link) {
case BOND_LINK_NOCHANGE:
continue;
@@ -2513,6 +2527,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
struct bonding *bond = container_of(work, struct bonding,
arp_work.work);
struct slave *slave, *oldcurrent;
+ struct list_head *iter;
int do_failover = 0;
read_lock(&bond->lock);
@@ -2529,7 +2544,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
* TODO: what about up/down delay in arp mode? it wasn't here before
* so it can wait
*/
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
unsigned long trans_start = dev_trans_start(slave->dev);
if (slave->link != BOND_LINK_UP) {
@@ -2620,10 +2635,11 @@ re_arm:
static int bond_ab_arp_inspect(struct bonding *bond)
{
unsigned long trans_start, last_rx;
+ struct list_head *iter;
struct slave *slave;
int commit = 0;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
slave->new_link = BOND_LINK_NOCHANGE;
last_rx = slave_last_rx(bond, slave);
@@ -2690,9 +2706,10 @@ static int bond_ab_arp_inspect(struct bonding *bond)
static void bond_ab_arp_commit(struct bonding *bond)
{
unsigned long trans_start;
+ struct list_head *iter;
struct slave *slave;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
switch (slave->new_link) {
case BOND_LINK_NOCHANGE:
continue;
@@ -3156,13 +3173,14 @@ static void bond_work_cancel_all(struct bonding *bond)
static int bond_open(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct list_head *iter;
struct slave *slave;
/* reset slave->backup and slave->inactive */
read_lock(&bond->lock);
if (!list_empty(&bond->slave_list)) {
read_lock(&bond->curr_slave_lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
&& (slave != bond->curr_active_slave)) {
bond_set_slave_inactive_flags(slave);
@@ -3222,12 +3240,13 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
{
struct bonding *bond = netdev_priv(bond_dev);
struct rtnl_link_stats64 temp;
+ struct list_head *iter;
struct slave *slave;
memset(stats, 0, sizeof(*stats));
read_lock_bh(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
const struct rtnl_link_stats64 *sstats =
dev_get_stats(slave->dev, &temp);
@@ -3394,6 +3413,7 @@ static void bond_change_rx_flags(struct net_device *bond_dev, int change)
static void bond_set_rx_mode(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct list_head *iter;
struct slave *slave;
ASSERT_RTNL();
@@ -3405,7 +3425,7 @@ static void bond_set_rx_mode(struct net_device *bond_dev)
dev_mc_sync(slave->dev, bond_dev);
}
} else {
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
dev_uc_sync_multiple(slave->dev, bond_dev);
dev_mc_sync_multiple(slave->dev, bond_dev);
}
@@ -3473,6 +3493,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave, *rollback_slave;
+ struct list_head *iter;
int res = 0;
pr_debug("bond=%p, name=%s, new_mtu=%d\n", bond,
@@ -3493,7 +3514,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
* call to the base driver.
*/
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
pr_debug("s %p s->p %p c_m %p\n",
slave,
bond_prev_slave(bond, slave),
@@ -3521,7 +3542,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
unwind:
/* unwind from head to the slave that failed */
- bond_for_each_slave(bond, rollback_slave) {
+ bond_for_each_slave(bond, rollback_slave, iter) {
int tmp_res;
if (rollback_slave == slave)
@@ -3549,6 +3570,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave, *rollback_slave;
struct sockaddr *sa = addr, tmp_sa;
+ struct list_head *iter;
int res = 0;
if (bond->params.mode == BOND_MODE_ALB)
@@ -3582,7 +3604,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
* call to the base driver.
*/
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
const struct net_device_ops *slave_ops = slave->dev->netdev_ops;
pr_debug("slave %p %s\n", slave, slave->dev->name);
@@ -3614,7 +3636,7 @@ unwind:
tmp_sa.sa_family = bond_dev->type;
/* unwind from head to the slave that failed */
- bond_for_each_slave(bond, rollback_slave) {
+ bond_for_each_slave(bond, rollback_slave, iter) {
int tmp_res;
if (rollback_slave == slave)
@@ -3642,11 +3664,12 @@ unwind:
*/
void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id)
{
+ struct list_head *iter;
struct slave *slave;
int i = slave_id;
/* Here we start from the slave with slave_id */
- bond_for_each_slave_rcu(bond, slave) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (--i < 0) {
if (slave_can_tx(slave)) {
bond_dev_queue_xmit(bond, skb, slave->dev);
@@ -3657,7 +3680,7 @@ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id)
/* Here we start from the first slave up to slave_id */
i = slave_id;
- bond_for_each_slave_rcu(bond, slave) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (--i < 0)
break;
if (slave_can_tx(slave)) {
@@ -3734,8 +3757,9 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
struct slave *slave = NULL;
+ struct list_head *iter;
- bond_for_each_slave_rcu(bond, slave) {
+ bond_for_each_slave_rcu(bond, slave, iter) {
if (bond_is_last_slave(bond, slave))
break;
if (IS_UP(slave->dev) && slave->link == BOND_LINK_UP) {
@@ -3784,13 +3808,14 @@ static inline int bond_slave_override(struct bonding *bond,
{
struct slave *slave = NULL;
struct slave *check_slave;
+ struct list_head *iter;
int res = 1;
if (!skb->queue_mapping)
return 1;
/* Find out if any slaves have the same mapping as this skb. */
- bond_for_each_slave_rcu(bond, check_slave) {
+ bond_for_each_slave_rcu(bond, check_slave, iter) {
if (check_slave->queue_id == skb->queue_mapping) {
slave = check_slave;
break;
@@ -3922,6 +3947,7 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
{
struct bonding *bond = netdev_priv(bond_dev);
unsigned long speed = 0;
+ struct list_head *iter;
struct slave *slave;
ecmd->duplex = DUPLEX_UNKNOWN;
@@ -3933,7 +3959,7 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
* this is an accurate maximum.
*/
read_lock(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (SLAVE_IS_OK(slave)) {
if (slave->speed != SPEED_UNKNOWN)
speed += slave->speed;
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 20a6ee2..7af5646 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -10,8 +10,9 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
__acquires(&bond->lock)
{
struct bonding *bond = seq->private;
- loff_t off = 0;
+ struct list_head *iter;
struct slave *slave;
+ loff_t off = 0;
/* make sure the bond won't be taken away */
rcu_read_lock();
@@ -20,7 +21,7 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
if (*pos == 0)
return SEQ_START_TOKEN;
- bond_for_each_slave(bond, slave)
+ bond_for_each_slave(bond, slave, iter)
if (++off == *pos)
return slave;
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index c29b836..e747415 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -210,11 +210,12 @@ static ssize_t bonding_show_slaves(struct device *d,
struct device_attribute *attr, char *buf)
{
struct bonding *bond = to_bond(d);
+ struct list_head *iter;
struct slave *slave;
int res = 0;
read_lock(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (res > (PAGE_SIZE - IFNAMSIZ)) {
/* not enough space for another interface name */
if ((PAGE_SIZE - res) > 10)
@@ -656,6 +657,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
const char *buf, size_t count)
{
struct bonding *bond = to_bond(d);
+ struct list_head *iter;
struct slave *slave;
__be32 newtarget, *targets;
unsigned long *targets_rx;
@@ -688,7 +690,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
&newtarget);
/* not to race with bond_arp_rcv */
write_lock_bh(&bond->lock);
- bond_for_each_slave(bond, slave)
+ bond_for_each_slave(bond, slave, iter)
slave->target_last_arp_rx[ind] = jiffies;
targets[ind] = newtarget;
write_unlock_bh(&bond->lock);
@@ -714,7 +716,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
&newtarget);
write_lock_bh(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
targets_rx = slave->target_last_arp_rx;
j = ind;
for (; (j < BOND_MAX_ARP_TARGETS-1) && targets[j+1]; j++)
@@ -1111,6 +1113,7 @@ static ssize_t bonding_store_primary(struct device *d,
const char *buf, size_t count)
{
struct bonding *bond = to_bond(d);
+ struct list_head *iter;
char ifname[IFNAMSIZ];
struct slave *slave;
@@ -1138,7 +1141,7 @@ static ssize_t bonding_store_primary(struct device *d,
goto out;
}
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
pr_info("%s: Setting %s as primary slave.\n",
bond->dev->name, slave->dev->name);
@@ -1286,6 +1289,7 @@ static ssize_t bonding_store_active_slave(struct device *d,
{
struct slave *slave, *old_active, *new_active;
struct bonding *bond = to_bond(d);
+ struct list_head *iter;
char ifname[IFNAMSIZ];
if (!rtnl_trylock())
@@ -1313,7 +1317,7 @@ static ssize_t bonding_store_active_slave(struct device *d,
goto out;
}
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
old_active = bond->curr_active_slave;
new_active = slave;
@@ -1493,6 +1497,7 @@ static ssize_t bonding_show_queue_id(struct device *d,
char *buf)
{
struct bonding *bond = to_bond(d);
+ struct list_head *iter;
struct slave *slave;
int res = 0;
@@ -1500,7 +1505,7 @@ static ssize_t bonding_show_queue_id(struct device *d,
return restart_syscall();
read_lock(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (res > (PAGE_SIZE - IFNAMSIZ - 6)) {
/* not enough space for another interface_name:queue_id pair */
if ((PAGE_SIZE - res) > 10)
@@ -1529,6 +1534,7 @@ static ssize_t bonding_store_queue_id(struct device *d,
{
struct slave *slave, *update_slave;
struct bonding *bond = to_bond(d);
+ struct list_head *iter;
u16 qid;
int ret = count;
char *delim;
@@ -1565,7 +1571,7 @@ static ssize_t bonding_store_queue_id(struct device *d,
/* Search for thes slave and check for duplicate qids */
update_slave = NULL;
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (sdev == slave->dev)
/*
* We don't need to check the matching
@@ -1619,6 +1625,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
{
struct bonding *bond = to_bond(d);
int new_value, ret = count;
+ struct list_head *iter;
struct slave *slave;
if (sscanf(buf, "%d", &new_value) != 1) {
@@ -1641,7 +1648,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
}
read_lock(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (!bond_is_active_slave(slave)) {
if (new_value)
slave->inactive = 0;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 54cc718..c4f9d16 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -110,15 +110,16 @@
* bond_for_each_slave - iterate over all slaves
* @bond: the bond holding this list
* @pos: current slave
+ * @iter: list_head * iterator
*
* Caller must hold bond->lock
*/
-#define bond_for_each_slave(bond, pos) \
- list_for_each_entry(pos, &(bond)->slave_list, list)
+#define bond_for_each_slave(bond, pos, iter) \
+ netdev_for_each_lower_private((bond)->dev, pos, iter)
/* Caller must have rcu_read_lock */
-#define bond_for_each_slave_rcu(bond, pos) \
- list_for_each_entry_rcu(pos, &(bond)->slave_list, list)
+#define bond_for_each_slave_rcu(bond, pos, iter) \
+ netdev_for_each_lower_private_rcu((bond)->dev, pos, iter)
#ifdef CONFIG_NET_POLL_CONTROLLER
extern atomic_t netpoll_block_tx;
@@ -477,9 +478,10 @@ static inline void bond_destroy_proc_dir(struct bond_net *bn)
static inline struct slave *bond_slave_has_mac(struct bonding *bond,
const u8 *mac)
{
+ struct list_head *iter;
struct slave *tmp;
- bond_for_each_slave(bond, tmp)
+ bond_for_each_slave(bond, tmp, iter)
if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
return tmp;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index c73cabd..85d0cda 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -3983,6 +3983,7 @@ static int cxgb4_inet6addr_handler(struct notifier_block *this,
struct net_device *event_dev;
int ret = NOTIFY_DONE;
struct bonding *bond = netdev_priv(ifa->idev->dev);
+ struct list_head *iter;
struct slave *slave;
struct pci_dev *first_pdev = NULL;
@@ -3995,7 +3996,7 @@ static int cxgb4_inet6addr_handler(struct notifier_block *this,
* in all of them only once.
*/
read_lock(&bond->lock);
- bond_for_each_slave(bond, slave) {
+ bond_for_each_slave(bond, slave, iter) {
if (!first_pdev) {
ret = clip_add(slave->dev, ifa, event);
/* If clip_add is success then only initialize
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 08/27] bonding: remove bond_for_each_slave_continue_reverse()
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
We only use it in rollback scenarios and can easily use the standart
bond_for_each_dev() instead.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
Remove the trailing semicolon after if:
+ if (rollback_slave == slave);
+ break;
It was painful...
v2 -> v3:
Add the forgotten _continue to the subject.
v1 -> v2:
No changes.
RFC -> v1:
Move the patch right before we change bond_for_each_slave() conversion,
so that we don't have to convert bond_for_each_slave_reverse() too.
v3 -> v4:
No change.
v2 -> v3:
Add the forgotten _continue to the subject.
v1 -> v2:
No changes.
RFC -> v1:
Move the patch right before we change bond_for_each_slave() conversion,
so that we don't have to convert bond_for_each_slave_reverse() too.
drivers/net/bonding/bond_alb.c | 14 ++++++++------
drivers/net/bonding/bond_main.c | 34 ++++++++++++++++++++++------------
drivers/net/bonding/bonding.h | 10 ----------
3 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 8524e33..6437657 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1246,9 +1246,9 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
*/
static int alb_set_mac_address(struct bonding *bond, void *addr)
{
- char tmp_addr[ETH_ALEN];
- struct slave *slave;
+ struct slave *slave, *rollback_slave;
struct sockaddr sa;
+ char tmp_addr[ETH_ALEN];
int res;
if (bond->alb_info.rlb_enabled)
@@ -1274,10 +1274,12 @@ unwind:
sa.sa_family = bond->dev->type;
/* unwind from head to the slave that failed */
- bond_for_each_slave_continue_reverse(bond, slave) {
- memcpy(tmp_addr, slave->dev->dev_addr, ETH_ALEN);
- dev_set_mac_address(slave->dev, &sa);
- memcpy(slave->dev->dev_addr, tmp_addr, ETH_ALEN);
+ bond_for_each_slave(bond, rollback_slave) {
+ if (rollback_slave == slave)
+ break;
+ memcpy(tmp_addr, rollback_slave->dev->dev_addr, ETH_ALEN);
+ dev_set_mac_address(rollback_slave->dev, &sa);
+ memcpy(rollback_slave->dev->dev_addr, tmp_addr, ETH_ALEN);
}
return res;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8e41416..d94b6c1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -332,7 +332,7 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
__be16 proto, u16 vid)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct slave *slave;
+ struct slave *slave, *rollback_slave;
int res;
bond_for_each_slave(bond, slave) {
@@ -344,9 +344,13 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
return 0;
unwind:
- /* unwind from the slave that failed */
- bond_for_each_slave_continue_reverse(bond, slave)
- vlan_vid_del(slave->dev, proto, vid);
+ /* unwind to the slave that failed */
+ bond_for_each_slave(bond, rollback_slave) {
+ if (rollback_slave == slave)
+ break;
+
+ vlan_vid_del(rollback_slave->dev, proto, vid);
+ }
return res;
}
@@ -3468,7 +3472,7 @@ static int bond_neigh_setup(struct net_device *dev,
static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
{
struct bonding *bond = netdev_priv(bond_dev);
- struct slave *slave;
+ struct slave *slave, *rollback_slave;
int res = 0;
pr_debug("bond=%p, name=%s, new_mtu=%d\n", bond,
@@ -3517,13 +3521,16 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
unwind:
/* unwind from head to the slave that failed */
- bond_for_each_slave_continue_reverse(bond, slave) {
+ bond_for_each_slave(bond, rollback_slave) {
int tmp_res;
- tmp_res = dev_set_mtu(slave->dev, bond_dev->mtu);
+ if (rollback_slave == slave)
+ break;
+
+ tmp_res = dev_set_mtu(rollback_slave->dev, bond_dev->mtu);
if (tmp_res) {
pr_debug("unwind err %d dev %s\n",
- tmp_res, slave->dev->name);
+ tmp_res, rollback_slave->dev->name);
}
}
@@ -3540,8 +3547,8 @@ unwind:
static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
{
struct bonding *bond = netdev_priv(bond_dev);
+ struct slave *slave, *rollback_slave;
struct sockaddr *sa = addr, tmp_sa;
- struct slave *slave;
int res = 0;
if (bond->params.mode == BOND_MODE_ALB)
@@ -3607,13 +3614,16 @@ unwind:
tmp_sa.sa_family = bond_dev->type;
/* unwind from head to the slave that failed */
- bond_for_each_slave_continue_reverse(bond, slave) {
+ bond_for_each_slave(bond, rollback_slave) {
int tmp_res;
- tmp_res = dev_set_mac_address(slave->dev, &tmp_sa);
+ if (rollback_slave == slave)
+ break;
+
+ tmp_res = dev_set_mac_address(rollback_slave->dev, &tmp_sa);
if (tmp_res) {
pr_debug("unwind err %d dev %s\n",
- tmp_res, slave->dev->name);
+ tmp_res, rollback_slave->dev->name);
}
}
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 597a4ec..54cc718 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -120,16 +120,6 @@
#define bond_for_each_slave_rcu(bond, pos) \
list_for_each_entry_rcu(pos, &(bond)->slave_list, list)
-/**
- * bond_for_each_slave_reverse - iterate in reverse from a given position
- * @bond: the bond holding this list
- * @pos: slave to continue from
- *
- * Caller must hold bond->lock
- */
-#define bond_for_each_slave_continue_reverse(bond, pos) \
- list_for_each_entry_continue_reverse(pos, &(bond)->slave_list, list)
-
#ifdef CONFIG_NET_POLL_CONTROLLER
extern atomic_t netpoll_block_tx;
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 07/27] net: add for_each iterators through neighbour lower link's private
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Add a possibility to iterate through netdev_adjacent's private, currently
only for lower neighbours.
Add both RCU and RTNL/other locking variants of iterators, and make the
non-rcu variant to be safe from removal.
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
Make netdev_for_each_lower_private() be safe for the entry removal, by
storing the ->next pointer in iter.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
the naming of functions - to make the use the pattern
netdev_(all_)(lower|upper)_* . Also reword the commit message - I had to
read it three times to understand :-/, and move it closer to the beginning
of the patchset.
include/linux/netdevice.h | 17 ++++++++++++++
net/core/dev.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 75d5bea..168974e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2833,6 +2833,23 @@ extern struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *d
updev; \
updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)))
+extern void *netdev_lower_get_next_private(struct net_device *dev,
+ struct list_head **iter);
+extern void *netdev_lower_get_next_private_rcu(struct net_device *dev,
+ struct list_head **iter);
+
+#define netdev_for_each_lower_private(dev, priv, iter) \
+ for (iter = (dev)->adj_list.lower.next, \
+ priv = netdev_lower_get_next_private(dev, &(iter)); \
+ priv; \
+ priv = netdev_lower_get_next_private(dev, &(iter)))
+
+#define netdev_for_each_lower_private_rcu(dev, priv, iter) \
+ for (iter = &(dev)->adj_list.lower, \
+ priv = netdev_lower_get_next_private_rcu(dev, &(iter)); \
+ priv; \
+ priv = netdev_lower_get_next_private_rcu(dev, &(iter)))
+
extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
extern int netdev_upper_dev_link(struct net_device *dev,
diff --git a/net/core/dev.c b/net/core/dev.c
index c69ab74..0aa844a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4466,7 +4466,8 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
}
EXPORT_SYMBOL(netdev_master_upper_dev_get);
-/* netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
+/**
+ * netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
* @dev: device
* @iter: list_head ** of the current position
*
@@ -4492,6 +4493,63 @@ struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
/**
+ * netdev_lower_get_next_private - Get the next ->private from the
+ * lower neighbour list
+ * @dev: device
+ * @iter: list_head ** of the current position
+ *
+ * Gets the next netdev_adjacent->private from the dev's lower neighbour
+ * list, starting from iter position. The caller must hold either hold the
+ * RTNL lock or its own locking that guarantees that the neighbour lower
+ * list will remain unchainged.
+ */
+void *netdev_lower_get_next_private(struct net_device *dev,
+ struct list_head **iter)
+{
+ struct netdev_adjacent *lower;
+
+ lower = list_entry(*iter, struct netdev_adjacent, list);
+
+ if (&lower->list == &dev->adj_list.lower)
+ return NULL;
+
+ if (iter)
+ *iter = lower->list.next;
+
+ return lower->private;
+}
+EXPORT_SYMBOL(netdev_lower_get_next_private);
+
+/**
+ * netdev_lower_get_next_private_rcu - Get the next ->private from the
+ * lower neighbour list, RCU
+ * variant
+ * @dev: device
+ * @iter: list_head ** of the current position
+ *
+ * Gets the next netdev_adjacent->private from the dev's lower neighbour
+ * list, starting from iter position. The caller must hold RCU read lock.
+ */
+void *netdev_lower_get_next_private_rcu(struct net_device *dev,
+ struct list_head **iter)
+{
+ struct netdev_adjacent *lower;
+
+ WARN_ON_ONCE(!rcu_read_lock_held());
+
+ lower = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
+
+ if (&lower->list == &dev->adj_list.lower)
+ return NULL;
+
+ if (iter)
+ *iter = &lower->list;
+
+ return lower->private;
+}
+EXPORT_SYMBOL(netdev_lower_get_next_private_rcu);
+
+/**
* netdev_master_upper_dev_get_rcu - Get master upper device
* @dev: device
*
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 06/27] bonding: modify bond_get_slave_by_dev() to use neighbours
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev; +Cc: jiri, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
It should be used under rtnl/bonding lock, so use the non-RCU version.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
Move the patch right after we've populated ->private with bonding.
drivers/net/bonding/bonding.h | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 03cf3fd..597a4ec 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -276,13 +276,8 @@ struct bonding {
static inline struct slave *bond_get_slave_by_dev(struct bonding *bond,
struct net_device *slave_dev)
{
- struct slave *slave = NULL;
-
- bond_for_each_slave(bond, slave)
- if (slave->dev == slave_dev)
- return slave;
-
- return NULL;
+ return (struct slave *)netdev_lower_dev_get_private(bond->dev,
+ slave_dev);
}
static inline struct bonding *bond_get_bond_by_slave(struct slave *slave)
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 04/27] net: add netdev_adjacent->private and allow to use it
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Currently, even though we can access any linked device, we can't attach
anything to it, which is vital to properly manage them.
To fix this, add a new void *private to netdev_adjacent and functions
setting/getting it (per link), so that we can save, per example, bonding's
slave structures there, per slave device.
netdev_master_upper_dev_link_private(dev, upper_dev, private) links dev to
upper dev and populates the neighbour link only with private.
netdev_lower_dev_get_private{,_rcu}() returns the private, if found.
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
No change.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
the naming of functions - to make the use the pattern
netdev_(all_)(lower|upper)_* .
include/linux/netdevice.h | 7 +++++
net/core/dev.c | 68 +++++++++++++++++++++++++++++++++++++++--------
2 files changed, 64 insertions(+), 11 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 514045c..75d5bea 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2839,8 +2839,15 @@ extern int netdev_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev);
extern int netdev_master_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev);
+extern int netdev_master_upper_dev_link_private(struct net_device *dev,
+ struct net_device *upper_dev,
+ void *private);
extern void netdev_upper_dev_unlink(struct net_device *dev,
struct net_device *upper_dev);
+extern void *netdev_lower_dev_get_private_rcu(struct net_device *dev,
+ struct net_device *lower_dev);
+extern void *netdev_lower_dev_get_private(struct net_device *dev,
+ struct net_device *lower_dev);
extern int skb_checksum_help(struct sk_buff *skb);
extern struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
netdev_features_t features, bool tx_path);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9290f09..c69ab74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4376,6 +4376,9 @@ struct netdev_adjacent {
/* counter for the number of times this device was added to us */
u16 ref_nr;
+ /* private field for the users */
+ void *private;
+
struct list_head list;
struct rcu_head rcu;
};
@@ -4510,7 +4513,7 @@ EXPORT_SYMBOL(netdev_master_upper_dev_get_rcu);
static int __netdev_adjacent_dev_insert(struct net_device *dev,
struct net_device *adj_dev,
struct list_head *dev_list,
- bool master)
+ void *private, bool master)
{
struct netdev_adjacent *adj;
@@ -4528,6 +4531,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
adj->dev = adj_dev;
adj->master = master;
adj->ref_nr = 1;
+ adj->private = private;
dev_hold(adj_dev);
pr_debug("dev_hold for %s, because of link added from %s to %s\n",
@@ -4574,15 +4578,17 @@ int __netdev_adjacent_dev_link_lists(struct net_device *dev,
struct net_device *upper_dev,
struct list_head *up_list,
struct list_head *down_list,
- bool master)
+ void *private, bool master)
{
int ret;
- ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, master);
+ ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, private,
+ master);
if (ret)
return ret;
- ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, false);
+ ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, private,
+ false);
if (ret) {
__netdev_adjacent_dev_remove(dev, upper_dev, up_list);
return ret;
@@ -4597,7 +4603,7 @@ int __netdev_adjacent_dev_link(struct net_device *dev,
return __netdev_adjacent_dev_link_lists(dev, upper_dev,
&dev->all_adj_list.upper,
&upper_dev->all_adj_list.lower,
- false);
+ NULL, false);
}
void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
@@ -4619,7 +4625,7 @@ void __netdev_adjacent_dev_unlink(struct net_device *dev,
int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
struct net_device *upper_dev,
- bool master)
+ void *private, bool master)
{
int ret = __netdev_adjacent_dev_link(dev, upper_dev);
@@ -4629,7 +4635,7 @@ int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
&dev->adj_list.upper,
&upper_dev->adj_list.lower,
- master);
+ private, master);
if (ret) {
__netdev_adjacent_dev_unlink(dev, upper_dev);
return ret;
@@ -4648,7 +4654,8 @@ void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
}
static int __netdev_upper_dev_link(struct net_device *dev,
- struct net_device *upper_dev, bool master)
+ struct net_device *upper_dev, bool master,
+ void *private)
{
struct netdev_adjacent *i, *j, *to_i, *to_j;
int ret = 0;
@@ -4668,7 +4675,8 @@ static int __netdev_upper_dev_link(struct net_device *dev,
if (master && netdev_master_upper_dev_get(dev))
return -EBUSY;
- ret = __netdev_adjacent_dev_link_neighbour(dev, upper_dev, master);
+ ret = __netdev_adjacent_dev_link_neighbour(dev, upper_dev, private,
+ master);
if (ret)
return ret;
@@ -4759,7 +4767,7 @@ rollback_mesh:
int netdev_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev)
{
- return __netdev_upper_dev_link(dev, upper_dev, false);
+ return __netdev_upper_dev_link(dev, upper_dev, false, NULL);
}
EXPORT_SYMBOL(netdev_upper_dev_link);
@@ -4777,10 +4785,18 @@ EXPORT_SYMBOL(netdev_upper_dev_link);
int netdev_master_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev)
{
- return __netdev_upper_dev_link(dev, upper_dev, true);
+ return __netdev_upper_dev_link(dev, upper_dev, true, NULL);
}
EXPORT_SYMBOL(netdev_master_upper_dev_link);
+int netdev_master_upper_dev_link_private(struct net_device *dev,
+ struct net_device *upper_dev,
+ void *private)
+{
+ return __netdev_upper_dev_link(dev, upper_dev, true, private);
+}
+EXPORT_SYMBOL(netdev_master_upper_dev_link_private);
+
/**
* netdev_upper_dev_unlink - Removes a link to upper device
* @dev: device
@@ -4818,6 +4834,36 @@ void netdev_upper_dev_unlink(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_upper_dev_unlink);
+void *netdev_lower_dev_get_private_rcu(struct net_device *dev,
+ struct net_device *lower_dev)
+{
+ struct netdev_adjacent *lower;
+
+ if (!lower_dev)
+ return NULL;
+ lower = __netdev_find_adj_rcu(dev, lower_dev, &dev->adj_list.lower);
+ if (!lower)
+ return NULL;
+
+ return lower->private;
+}
+EXPORT_SYMBOL(netdev_lower_dev_get_private_rcu);
+
+void *netdev_lower_dev_get_private(struct net_device *dev,
+ struct net_device *lower_dev)
+{
+ struct netdev_adjacent *lower;
+
+ if (!lower_dev)
+ return NULL;
+ lower = __netdev_find_adj(dev, lower_dev, &dev->adj_list.lower);
+ if (!lower)
+ return NULL;
+
+ return lower->private;
+}
+EXPORT_SYMBOL(netdev_lower_dev_get_private);
+
static void dev_change_rx_flags(struct net_device *dev, int flags)
{
const struct net_device_ops *ops = dev->netdev_ops;
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 00/27] bonding: use neighbours instead of own lists
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Jay Vosburgh, Andy Gospodarek, Dimitris Michailidis,
David S. Miller, Patrick McHardy, Eric Dumazet, Alexander Duyck,
Veaceslav Falico
This patchset introduces all the needed infrastructure, on top of current
adjacent lists, to be able to remove bond's slave_list/slave->list. The
overhead in memory/CPU is minimal, and after the patchset bonding can rely
on its slave-related functions, given the proper locking. I've done some
netperf benchmarks on a vm, and the delta was about 0.1gbps for 35gbps as a
whole, so no speed fluctuations.
It also automatically creates lower/upper and master symlinks in dev's
sysfs directory.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: Dimitris Michailidis <dm@chelsio.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Patrick McHardy <kaber@trash.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
v3 -> v4:
Per the feedback received, fix the comments to be kernel-doc style.
As Ben Hutchings suggested, make the 'get next private after a
specific private' function bonding-specific - it has virtually no uses
outside bonding (and even in bonding I already have some patches of
removing it).
Also, remove the stupid 'bool upper' logic from everywhere in the logic -
as Ben suggested, it's easier to pass the lists directly - and not only
easier, it saved several hundred lines of code modified.
Finally, make netdev_for_each_lower_private() safe for removal of the
current member - it can be easily achieved and doesn't really need two
versions, as in lists.
v2 -> v3:
Fix the commit subject for patch 08 - forgot the _continue in
bond_for_each_slave_continue_reverse(), suggested by Cong Wang.
This version is already ready for the inclusion, hopefully.
Moved the changelog in cover letter to the bottom and cleaned it
up a bit.
v1 -> v2:
Fix a bug when we could remove try to remove a neighbour twice - it was
possible if we've had this neighbour already and added another devices
which had this neighbour in its lists. Fix it by following the rule - there
can be only one neighbour link, so add also _remove_neighbour() functions
to the generic _remove()s to handle the neighbour only removing, and fix
its refcounting while adding (all in patch 01).
Also, fix a vlan bug when it called unregister netdevice before unlinking -
it could possibly lead to a subte race condition - cause it was
unregistered but still in global-accessible list.
Rearrange sysfs/vlan patches so that first we fix vlan and only after we
add sysfs stuff.
RFC -> v1:
I've added proper, consistent naming for all variables/functions, uninlined
some helpers to get better backtraces, just in case (overhead is minimal,
no hot paths), rearranged patches for better review, dropped bondings
->prev and bond_for_each_slave_continue() functionality - to be able to
RCUify it easier, and renamed slave_* sysfs links to lower_* sysfs links to
maintain upper/lower naming. I've also dropped, thanks to bonding cleanup,
some heavy and ugly/intrusive patches.
---
drivers/net/bonding/bond_3ad.c | 54 ++--
drivers/net/bonding/bond_alb.c | 81 ++---
drivers/net/bonding/bond_alb.h | 4 +-
drivers/net/bonding/bond_main.c | 296 +++++++++--------
drivers/net/bonding/bond_procfs.c | 5 +-
drivers/net/bonding/bond_sysfs.c | 62 +---
drivers/net/bonding/bonding.h | 101 +++---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 +-
include/linux/netdevice.h | 55 +++-
net/8021q/vlan.c | 18 +-
net/core/dev.c | 403 +++++++++++++++++-------
11 files changed, 638 insertions(+), 444 deletions(-)
^ permalink raw reply
* [PATCH v4 net-next 03/27] net: add RCU variant to search for netdev_adjacent link
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck, Cong Wang
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Currently we have only the RTNL flavour, however we can traverse it while
holding only RCU, so add the RCU search. Add an RCU variant that uses
list_head * as an argument, so that it can be universally used afterwards.
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4:
Use the list argument, instead of bool upper.
v2 -> v3:
No change.
v1 -> v2:
No changes.
RFC -> v1:
rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
the naming of functions - to make the use the pattern
netdev_(all_)(lower|upper)_* . Uninline functions to get better stack
traces - inline doesn't give much speed improvement, but this way the stack
traces are meaningful. Remove the unused (through the whole patchset)
functions - they can easily be added afterwards.
net/core/dev.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 9a395e0..9290f09 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4380,6 +4380,19 @@ struct netdev_adjacent {
struct rcu_head rcu;
};
+static struct netdev_adjacent *__netdev_find_adj_rcu(struct net_device *dev,
+ struct net_device *adj_dev,
+ struct list_head *adj_list)
+{
+ struct netdev_adjacent *adj;
+
+ list_for_each_entry_rcu(adj, adj_list, list) {
+ if (adj->dev == adj_dev)
+ return adj;
+ }
+ return NULL;
+}
+
static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
struct net_device *adj_dev,
struct list_head *adj_list)
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 02/27] net: add adj_list to save only neighbours
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck, Cong Wang
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Currently, we distinguish neighbours (first-level linked devices) from
non-neighbours by the neighbour bool in the netdev_adjacent. This could be
quite time-consuming in case we would like to traverse *only* through
neighbours - cause we'd have to traverse through all devices and check for
this flag, and in a (quite common) scenario where we have lots of vlans on
top of bridge, which is on top of a bond - the bonding would have to go
through all those vlans to get its upper neighbour linked devices.
This situation is really unpleasant, cause there are already a lot of cases
when a device with slaves needs to go through them in hot path.
To fix this, introduce a new upper/lower device lists structure -
adj_list, which contains only the neighbours. It works always in
pair with the all_adj_list structure (renamed from upper/lower_dev_list),
i.e. both of them contain the same links, only that all_adj_list contains
also non-neighbour device links. It's really a small change visible,
currently, only for __netdev_adjacent_dev_insert/remove(), and doesn't
change the main linked logic at all.
Also, add some comments a fix a name collision in
netdev_for_each_upper_dev_rcu() and rework the naming by the following
rules:
netdev_(all_)(upper|lower)_*
If "all_" is present, then we work with the whole list of upper/lower
devices, otherwise - only with direct neighbours. Uninline functions - to
get better stack traces.
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
v3 -> v4
Remove the 'bool upper' and, instead, transmit pointers to the lists
directly.
v2 -> v3:
No change.
v1 -> v2:
Fix a bug when we've tried to add a non-neighbour dev1 to dev2, while dev2
already had dev1 as a neighbour - we shouldn't care about neighbour's
ref_nr at all cause we can only add one - so just add/remove it via
different functions. And it became more readable.
RFC -> v1:
rename neighbour_dev_list/all_dev_list to adj_list/all_adj_list and rework
the naming of functions - to make the use the pattern
netdev_(all_)(lower|upper)_* . Uninline functions to get better stack
traces - inline doesn't give much speed improvement, but this way the stack
traces are meaningful.
drivers/net/bonding/bond_alb.c | 2 +-
drivers/net/bonding/bond_main.c | 10 +-
include/linux/netdevice.h | 28 ++++--
net/core/dev.c | 203 ++++++++++++++++++++--------------------
4 files changed, 129 insertions(+), 114 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index f428ef57..8524e33 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1019,7 +1019,7 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
/* loop through vlans and send one packet for each */
rcu_read_lock();
- netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+ netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
if (upper->priv_flags & IFF_802_1Q_VLAN)
alb_send_lp_vid(slave, mac_addr,
vlan_dev_vlan_id(upper));
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 55bbb8b..91c4ab8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2267,7 +2267,7 @@ static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
return true;
rcu_read_lock();
- netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+ netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
if (ip == bond_confirm_addr(upper, 0, ip)) {
ret = true;
break;
@@ -2342,10 +2342,12 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
*
* TODO: QinQ?
*/
- netdev_for_each_upper_dev_rcu(bond->dev, vlan_upper, vlan_iter) {
+ netdev_for_each_all_upper_dev_rcu(bond->dev, vlan_upper,
+ vlan_iter) {
if (!is_vlan_dev(vlan_upper))
continue;
- netdev_for_each_upper_dev_rcu(vlan_upper, upper, iter) {
+ netdev_for_each_all_upper_dev_rcu(vlan_upper, upper,
+ iter) {
if (upper == rt->dst.dev) {
vlan_id = vlan_dev_vlan_id(vlan_upper);
rcu_read_unlock();
@@ -2358,7 +2360,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
* our upper vlans, then just search for any dev that
* matches, and in case it's a vlan - save the id
*/
- netdev_for_each_upper_dev_rcu(bond->dev, upper, iter) {
+ netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) {
if (upper == rt->dst.dev) {
/* if it's a vlan - get its VID */
if (is_vlan_dev(upper))
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3de49ac..514045c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1143,8 +1143,18 @@ struct net_device {
struct list_head dev_list;
struct list_head napi_list;
struct list_head unreg_list;
- struct list_head upper_dev_list; /* List of upper devices */
- struct list_head lower_dev_list;
+
+ /* directly linked devices, like slaves for bonding */
+ struct {
+ struct list_head upper;
+ struct list_head lower;
+ } adj_list;
+
+ /* all linked devices, *including* neighbours */
+ struct {
+ struct list_head upper;
+ struct list_head lower;
+ } all_adj_list;
/* currently active device features */
@@ -2813,15 +2823,15 @@ extern int bpf_jit_enable;
extern bool netdev_has_upper_dev(struct net_device *dev,
struct net_device *upper_dev);
extern bool netdev_has_any_upper_dev(struct net_device *dev);
-extern struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
- struct list_head **iter);
+extern struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
+ struct list_head **iter);
/* iterate through upper list, must be called under RCU read lock */
-#define netdev_for_each_upper_dev_rcu(dev, upper, iter) \
- for (iter = &(dev)->upper_dev_list, \
- upper = netdev_upper_get_next_dev_rcu(dev, &(iter)); \
- upper; \
- upper = netdev_upper_get_next_dev_rcu(dev, &(iter)))
+#define netdev_for_each_all_upper_dev_rcu(dev, updev, iter) \
+ for (iter = &(dev)->all_adj_list.upper, \
+ updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)); \
+ updev; \
+ updev = netdev_all_upper_get_next_dev_rcu(dev, &(iter)))
extern struct net_device *netdev_master_upper_dev_get(struct net_device *dev);
extern struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 9be7937..9a395e0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4373,9 +4373,6 @@ struct netdev_adjacent {
/* upper master flag, there can only be one master device per list */
bool master;
- /* indicates that this dev is our first-level lower/upper device */
- bool neighbour;
-
/* counter for the number of times this device was added to us */
u16 ref_nr;
@@ -4385,29 +4382,17 @@ struct netdev_adjacent {
static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
struct net_device *adj_dev,
- struct list_head *dev_list)
+ struct list_head *adj_list)
{
struct netdev_adjacent *adj;
- list_for_each_entry(adj, dev_list, list) {
+ list_for_each_entry(adj, adj_list, list) {
if (adj->dev == adj_dev)
return adj;
}
return NULL;
}
-static inline struct netdev_adjacent *__netdev_find_upper(struct net_device *dev,
- struct net_device *udev)
-{
- return __netdev_find_adj(dev, udev, &dev->upper_dev_list);
-}
-
-static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
- struct net_device *ldev)
-{
- return __netdev_find_adj(dev, ldev, &dev->lower_dev_list);
-}
-
/**
* netdev_has_upper_dev - Check if device is linked to an upper device
* @dev: device
@@ -4422,7 +4407,7 @@ bool netdev_has_upper_dev(struct net_device *dev,
{
ASSERT_RTNL();
- return __netdev_find_upper(dev, upper_dev);
+ return __netdev_find_adj(dev, upper_dev, &dev->all_adj_list.upper);
}
EXPORT_SYMBOL(netdev_has_upper_dev);
@@ -4437,7 +4422,7 @@ bool netdev_has_any_upper_dev(struct net_device *dev)
{
ASSERT_RTNL();
- return !list_empty(&dev->upper_dev_list);
+ return !list_empty(&dev->all_adj_list.upper);
}
EXPORT_SYMBOL(netdev_has_any_upper_dev);
@@ -4454,10 +4439,10 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
ASSERT_RTNL();
- if (list_empty(&dev->upper_dev_list))
+ if (list_empty(&dev->adj_list.upper))
return NULL;
- upper = list_first_entry(&dev->upper_dev_list,
+ upper = list_first_entry(&dev->adj_list.upper,
struct netdev_adjacent, list);
if (likely(upper->master))
return upper->dev;
@@ -4465,15 +4450,15 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
}
EXPORT_SYMBOL(netdev_master_upper_dev_get);
-/* netdev_upper_get_next_dev_rcu - Get the next dev from upper list
+/* netdev_all_upper_get_next_dev_rcu - Get the next dev from upper list
* @dev: device
* @iter: list_head ** of the current position
*
* Gets the next device from the dev's upper list, starting from iter
* position. The caller must hold RCU read lock.
*/
-struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
- struct list_head **iter)
+struct net_device *netdev_all_upper_get_next_dev_rcu(struct net_device *dev,
+ struct list_head **iter)
{
struct netdev_adjacent *upper;
@@ -4481,14 +4466,14 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
upper = list_entry_rcu((*iter)->next, struct netdev_adjacent, list);
- if (&upper->list == &dev->upper_dev_list)
+ if (&upper->list == &dev->all_adj_list.upper)
return NULL;
*iter = &upper->list;
return upper->dev;
}
-EXPORT_SYMBOL(netdev_upper_get_next_dev_rcu);
+EXPORT_SYMBOL(netdev_all_upper_get_next_dev_rcu);
/**
* netdev_master_upper_dev_get_rcu - Get master upper device
@@ -4501,7 +4486,7 @@ struct net_device *netdev_master_upper_dev_get_rcu(struct net_device *dev)
{
struct netdev_adjacent *upper;
- upper = list_first_or_null_rcu(&dev->upper_dev_list,
+ upper = list_first_or_null_rcu(&dev->adj_list.upper,
struct netdev_adjacent, list);
if (upper && likely(upper->master))
return upper->dev;
@@ -4512,14 +4497,13 @@ EXPORT_SYMBOL(netdev_master_upper_dev_get_rcu);
static int __netdev_adjacent_dev_insert(struct net_device *dev,
struct net_device *adj_dev,
struct list_head *dev_list,
- bool neighbour, bool master)
+ bool master)
{
struct netdev_adjacent *adj;
adj = __netdev_find_adj(dev, adj_dev, dev_list);
if (adj) {
- BUG_ON(neighbour);
adj->ref_nr++;
return 0;
}
@@ -4530,13 +4514,11 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
adj->dev = adj_dev;
adj->master = master;
- adj->neighbour = neighbour;
adj->ref_nr = 1;
-
dev_hold(adj_dev);
- pr_debug("dev_hold for %s, because of %s link added from %s to %s\n",
- adj_dev->name, dev_list == &dev->upper_dev_list ?
- "upper" : "lower", dev->name, adj_dev->name);
+
+ pr_debug("dev_hold for %s, because of link added from %s to %s\n",
+ adj_dev->name, dev->name, adj_dev->name);
/* Ensure that master link is always the first item in list. */
if (master)
@@ -4547,22 +4529,6 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
return 0;
}
-static inline int __netdev_upper_dev_insert(struct net_device *dev,
- struct net_device *udev,
- bool master, bool neighbour)
-{
- return __netdev_adjacent_dev_insert(dev, udev, &dev->upper_dev_list,
- neighbour, master);
-}
-
-static inline int __netdev_lower_dev_insert(struct net_device *dev,
- struct net_device *ldev,
- bool neighbour)
-{
- return __netdev_adjacent_dev_insert(dev, ldev, &dev->lower_dev_list,
- neighbour, false);
-}
-
void __netdev_adjacent_dev_remove(struct net_device *dev,
struct net_device *adj_dev,
struct list_head *dev_list)
@@ -4571,73 +4537,102 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
adj = __netdev_find_adj(dev, adj_dev, dev_list);
- if (!adj)
+ if (!adj) {
+ pr_err("tried to remove device %s from %s\n",
+ dev->name, adj_dev->name);
BUG();
+ }
if (adj->ref_nr > 1) {
+ pr_debug("%s to %s ref_nr-- = %d\n", dev->name, adj_dev->name,
+ adj->ref_nr-1);
adj->ref_nr--;
return;
}
list_del_rcu(&adj->list);
- pr_debug("dev_put for %s, because of %s link removed from %s to %s\n",
- adj_dev->name, dev_list == &dev->upper_dev_list ?
- "upper" : "lower", dev->name, adj_dev->name);
+ pr_debug("dev_put for %s, because link removed from %s to %s\n",
+ adj_dev->name, dev->name, adj_dev->name);
dev_put(adj_dev);
kfree_rcu(adj, rcu);
}
-static inline void __netdev_upper_dev_remove(struct net_device *dev,
- struct net_device *udev)
-{
- return __netdev_adjacent_dev_remove(dev, udev, &dev->upper_dev_list);
-}
-
-static inline void __netdev_lower_dev_remove(struct net_device *dev,
- struct net_device *ldev)
-{
- return __netdev_adjacent_dev_remove(dev, ldev, &dev->lower_dev_list);
-}
-
-int __netdev_adjacent_dev_insert_link(struct net_device *dev,
- struct net_device *upper_dev,
- bool master, bool neighbour)
+int __netdev_adjacent_dev_link_lists(struct net_device *dev,
+ struct net_device *upper_dev,
+ struct list_head *up_list,
+ struct list_head *down_list,
+ bool master)
{
int ret;
- ret = __netdev_upper_dev_insert(dev, upper_dev, master, neighbour);
+ ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, master);
if (ret)
return ret;
- ret = __netdev_lower_dev_insert(upper_dev, dev, neighbour);
+ ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, false);
if (ret) {
- __netdev_upper_dev_remove(dev, upper_dev);
+ __netdev_adjacent_dev_remove(dev, upper_dev, up_list);
return ret;
}
return 0;
}
-static inline int __netdev_adjacent_dev_link(struct net_device *dev,
- struct net_device *udev)
+int __netdev_adjacent_dev_link(struct net_device *dev,
+ struct net_device *upper_dev)
{
- return __netdev_adjacent_dev_insert_link(dev, udev, false, false);
+ return __netdev_adjacent_dev_link_lists(dev, upper_dev,
+ &dev->all_adj_list.upper,
+ &upper_dev->all_adj_list.lower,
+ false);
}
-static inline int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
- struct net_device *udev,
- bool master)
+void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
+ struct net_device *upper_dev,
+ struct list_head *up_list,
+ struct list_head *down_list)
{
- return __netdev_adjacent_dev_insert_link(dev, udev, master, true);
+ __netdev_adjacent_dev_remove(dev, upper_dev, up_list);
+ __netdev_adjacent_dev_remove(upper_dev, dev, down_list);
}
void __netdev_adjacent_dev_unlink(struct net_device *dev,
struct net_device *upper_dev)
{
- __netdev_upper_dev_remove(dev, upper_dev);
- __netdev_lower_dev_remove(upper_dev, dev);
+ __netdev_adjacent_dev_unlink_lists(dev, upper_dev,
+ &dev->all_adj_list.upper,
+ &upper_dev->all_adj_list.lower);
+}
+
+int __netdev_adjacent_dev_link_neighbour(struct net_device *dev,
+ struct net_device *upper_dev,
+ bool master)
+{
+ int ret = __netdev_adjacent_dev_link(dev, upper_dev);
+
+ if (ret)
+ return ret;
+
+ ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
+ &dev->adj_list.upper,
+ &upper_dev->adj_list.lower,
+ master);
+ if (ret) {
+ __netdev_adjacent_dev_unlink(dev, upper_dev);
+ return ret;
+ }
+
+ return 0;
}
+void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
+ struct net_device *upper_dev)
+{
+ __netdev_adjacent_dev_unlink(dev, upper_dev);
+ __netdev_adjacent_dev_unlink_lists(dev, upper_dev,
+ &dev->adj_list.upper,
+ &upper_dev->adj_list.lower);
+}
static int __netdev_upper_dev_link(struct net_device *dev,
struct net_device *upper_dev, bool master)
@@ -4651,10 +4646,10 @@ static int __netdev_upper_dev_link(struct net_device *dev,
return -EBUSY;
/* To prevent loops, check if dev is not upper device to upper_dev. */
- if (__netdev_find_upper(upper_dev, dev))
+ if (__netdev_find_adj(upper_dev, dev, &upper_dev->all_adj_list.upper))
return -EBUSY;
- if (__netdev_find_upper(dev, upper_dev))
+ if (__netdev_find_adj(dev, upper_dev, &dev->all_adj_list.upper))
return -EEXIST;
if (master && netdev_master_upper_dev_get(dev))
@@ -4665,12 +4660,14 @@ static int __netdev_upper_dev_link(struct net_device *dev,
return ret;
/* Now that we linked these devs, make all the upper_dev's
- * upper_dev_list visible to every dev's lower_dev_list and vice
+ * all_adj_list.upper visible to every dev's all_adj_list.lower an
* versa, and don't forget the devices itself. All of these
* links are non-neighbours.
*/
- list_for_each_entry(i, &dev->lower_dev_list, list) {
- list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
+ list_for_each_entry(i, &dev->all_adj_list.lower, list) {
+ list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
+ pr_debug("Interlinking %s with %s, non-neighbour\n",
+ i->dev->name, j->dev->name);
ret = __netdev_adjacent_dev_link(i->dev, j->dev);
if (ret)
goto rollback_mesh;
@@ -4678,14 +4675,18 @@ static int __netdev_upper_dev_link(struct net_device *dev,
}
/* add dev to every upper_dev's upper device */
- list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
+ list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
+ pr_debug("linking %s's upper device %s with %s\n",
+ upper_dev->name, i->dev->name, dev->name);
ret = __netdev_adjacent_dev_link(dev, i->dev);
if (ret)
goto rollback_upper_mesh;
}
/* add upper_dev to every dev's lower device */
- list_for_each_entry(i, &dev->lower_dev_list, list) {
+ list_for_each_entry(i, &dev->all_adj_list.lower, list) {
+ pr_debug("linking %s's lower device %s with %s\n", dev->name,
+ i->dev->name, upper_dev->name);
ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
if (ret)
goto rollback_lower_mesh;
@@ -4696,7 +4697,7 @@ static int __netdev_upper_dev_link(struct net_device *dev,
rollback_lower_mesh:
to_i = i;
- list_for_each_entry(i, &dev->lower_dev_list, list) {
+ list_for_each_entry(i, &dev->all_adj_list.lower, list) {
if (i == to_i)
break;
__netdev_adjacent_dev_unlink(i->dev, upper_dev);
@@ -4706,7 +4707,7 @@ rollback_lower_mesh:
rollback_upper_mesh:
to_i = i;
- list_for_each_entry(i, &upper_dev->upper_dev_list, list) {
+ list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
if (i == to_i)
break;
__netdev_adjacent_dev_unlink(dev, i->dev);
@@ -4717,8 +4718,8 @@ rollback_upper_mesh:
rollback_mesh:
to_i = i;
to_j = j;
- list_for_each_entry(i, &dev->lower_dev_list, list) {
- list_for_each_entry(j, &upper_dev->upper_dev_list, list) {
+ list_for_each_entry(i, &dev->all_adj_list.lower, list) {
+ list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
if (i == to_i && j == to_j)
break;
__netdev_adjacent_dev_unlink(i->dev, j->dev);
@@ -4727,7 +4728,7 @@ rollback_mesh:
break;
}
- __netdev_adjacent_dev_unlink(dev, upper_dev);
+ __netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
return ret;
}
@@ -4781,23 +4782,23 @@ void netdev_upper_dev_unlink(struct net_device *dev,
struct netdev_adjacent *i, *j;
ASSERT_RTNL();
- __netdev_adjacent_dev_unlink(dev, upper_dev);
+ __netdev_adjacent_dev_unlink_neighbour(dev, upper_dev);
/* Here is the tricky part. We must remove all dev's lower
* devices from all upper_dev's upper devices and vice
* versa, to maintain the graph relationship.
*/
- list_for_each_entry(i, &dev->lower_dev_list, list)
- list_for_each_entry(j, &upper_dev->upper_dev_list, list)
+ list_for_each_entry(i, &dev->all_adj_list.lower, list)
+ list_for_each_entry(j, &upper_dev->all_adj_list.upper, list)
__netdev_adjacent_dev_unlink(i->dev, j->dev);
/* remove also the devices itself from lower/upper device
* list
*/
- list_for_each_entry(i, &dev->lower_dev_list, list)
+ list_for_each_entry(i, &dev->all_adj_list.lower, list)
__netdev_adjacent_dev_unlink(i->dev, upper_dev);
- list_for_each_entry(i, &upper_dev->upper_dev_list, list)
+ list_for_each_entry(i, &upper_dev->all_adj_list.upper, list)
__netdev_adjacent_dev_unlink(dev, i->dev);
call_netdevice_notifiers(NETDEV_CHANGEUPPER, dev);
@@ -6059,8 +6060,10 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
INIT_LIST_HEAD(&dev->napi_list);
INIT_LIST_HEAD(&dev->unreg_list);
INIT_LIST_HEAD(&dev->link_watch_list);
- INIT_LIST_HEAD(&dev->upper_dev_list);
- INIT_LIST_HEAD(&dev->lower_dev_list);
+ INIT_LIST_HEAD(&dev->adj_list.upper);
+ INIT_LIST_HEAD(&dev->adj_list.lower);
+ INIT_LIST_HEAD(&dev->all_adj_list.upper);
+ INIT_LIST_HEAD(&dev->all_adj_list.lower);
dev->priv_flags = IFF_XMIT_DST_RELEASE;
setup(dev);
--
1.8.4
^ permalink raw reply related
* [PATCH v4 net-next 01/27] net: use lists as arguments instead of bool upper
From: Veaceslav Falico @ 2013-09-24 11:46 UTC (permalink / raw)
To: netdev
Cc: jiri, Veaceslav Falico, David S. Miller, Eric Dumazet,
Alexander Duyck, Cong Wang
In-Reply-To: <1380023227-9576-1-git-send-email-vfalico@redhat.com>
Currently we make use of bool upper when we want to specify if we want to
work with upper/lower list. It's, however, harder to read, debug and
occupies a lot more code.
Fix this by just passing the correct upper/lower_dev_list list_head pointer
instead of bool upper, and work internally with it.
CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@resnulli.us>
CC: Alexander Duyck <alexander.h.duyck@intel.com>
CC: Cong Wang <amwang@redhat.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Notes:
RFC -> v4:
New patch.
net/core/dev.c | 54 ++++++++++++++++++++++--------------------------------
1 file changed, 22 insertions(+), 32 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 5c713f2..9be7937 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4385,12 +4385,9 @@ struct netdev_adjacent {
static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
struct net_device *adj_dev,
- bool upper)
+ struct list_head *dev_list)
{
struct netdev_adjacent *adj;
- struct list_head *dev_list;
-
- dev_list = upper ? &dev->upper_dev_list : &dev->lower_dev_list;
list_for_each_entry(adj, dev_list, list) {
if (adj->dev == adj_dev)
@@ -4402,13 +4399,13 @@ static struct netdev_adjacent *__netdev_find_adj(struct net_device *dev,
static inline struct netdev_adjacent *__netdev_find_upper(struct net_device *dev,
struct net_device *udev)
{
- return __netdev_find_adj(dev, udev, true);
+ return __netdev_find_adj(dev, udev, &dev->upper_dev_list);
}
static inline struct netdev_adjacent *__netdev_find_lower(struct net_device *dev,
struct net_device *ldev)
{
- return __netdev_find_adj(dev, ldev, false);
+ return __netdev_find_adj(dev, ldev, &dev->lower_dev_list);
}
/**
@@ -4514,12 +4511,12 @@ EXPORT_SYMBOL(netdev_master_upper_dev_get_rcu);
static int __netdev_adjacent_dev_insert(struct net_device *dev,
struct net_device *adj_dev,
- bool neighbour, bool master,
- bool upper)
+ struct list_head *dev_list,
+ bool neighbour, bool master)
{
struct netdev_adjacent *adj;
- adj = __netdev_find_adj(dev, adj_dev, upper);
+ adj = __netdev_find_adj(dev, adj_dev, dev_list);
if (adj) {
BUG_ON(neighbour);
@@ -4538,19 +4535,14 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
dev_hold(adj_dev);
pr_debug("dev_hold for %s, because of %s link added from %s to %s\n",
- adj_dev->name, upper ? "upper" : "lower", dev->name,
- adj_dev->name);
+ adj_dev->name, dev_list == &dev->upper_dev_list ?
+ "upper" : "lower", dev->name, adj_dev->name);
- if (!upper) {
- list_add_tail_rcu(&adj->list, &dev->lower_dev_list);
- return 0;
- }
-
- /* Ensure that master upper link is always the first item in list. */
+ /* Ensure that master link is always the first item in list. */
if (master)
- list_add_rcu(&adj->list, &dev->upper_dev_list);
+ list_add_rcu(&adj->list, dev_list);
else
- list_add_tail_rcu(&adj->list, &dev->upper_dev_list);
+ list_add_tail_rcu(&adj->list, dev_list);
return 0;
}
@@ -4559,27 +4551,25 @@ static inline int __netdev_upper_dev_insert(struct net_device *dev,
struct net_device *udev,
bool master, bool neighbour)
{
- return __netdev_adjacent_dev_insert(dev, udev, neighbour, master,
- true);
+ return __netdev_adjacent_dev_insert(dev, udev, &dev->upper_dev_list,
+ neighbour, master);
}
static inline int __netdev_lower_dev_insert(struct net_device *dev,
struct net_device *ldev,
bool neighbour)
{
- return __netdev_adjacent_dev_insert(dev, ldev, neighbour, false,
- false);
+ return __netdev_adjacent_dev_insert(dev, ldev, &dev->lower_dev_list,
+ neighbour, false);
}
void __netdev_adjacent_dev_remove(struct net_device *dev,
- struct net_device *adj_dev, bool upper)
+ struct net_device *adj_dev,
+ struct list_head *dev_list)
{
struct netdev_adjacent *adj;
- if (upper)
- adj = __netdev_find_upper(dev, adj_dev);
- else
- adj = __netdev_find_lower(dev, adj_dev);
+ adj = __netdev_find_adj(dev, adj_dev, dev_list);
if (!adj)
BUG();
@@ -4591,8 +4581,8 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
list_del_rcu(&adj->list);
pr_debug("dev_put for %s, because of %s link removed from %s to %s\n",
- adj_dev->name, upper ? "upper" : "lower", dev->name,
- adj_dev->name);
+ adj_dev->name, dev_list == &dev->upper_dev_list ?
+ "upper" : "lower", dev->name, adj_dev->name);
dev_put(adj_dev);
kfree_rcu(adj, rcu);
}
@@ -4600,13 +4590,13 @@ void __netdev_adjacent_dev_remove(struct net_device *dev,
static inline void __netdev_upper_dev_remove(struct net_device *dev,
struct net_device *udev)
{
- return __netdev_adjacent_dev_remove(dev, udev, true);
+ return __netdev_adjacent_dev_remove(dev, udev, &dev->upper_dev_list);
}
static inline void __netdev_lower_dev_remove(struct net_device *dev,
struct net_device *ldev)
{
- return __netdev_adjacent_dev_remove(dev, ldev, false);
+ return __netdev_adjacent_dev_remove(dev, ldev, &dev->lower_dev_list);
}
int __netdev_adjacent_dev_insert_link(struct net_device *dev,
--
1.8.4
^ permalink raw reply related
* Re: [PATCH net-next] xfrm: Simplify SA looking up when using wildcard source address
From: Steffen Klassert @ 2013-09-24 11:45 UTC (permalink / raw)
To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1379927917-17365-1-git-send-email-fan.du@windriver.com>
On Mon, Sep 23, 2013 at 05:18:37PM +0800, Fan Du wrote:
> I'm not quite sure I get this "wildcard source address" right,
> IMHO if a host needs to protect every traffic for a given remote host,
> then the source address is wildcard address, i.e. all ZEROs.
> (Please correct me if I'm bloodly wrong。。。)
The above does not belong to a commit message, really.
If you are not sure and you want comments on your patch,
mark your patch as RFC. You should be sure that your patch
is correct when you submit, at least in the moment you
send it. I know that this can change a second after,
but in that moment you should be sure.
>
> Here is the argument if above statement stands true:
> __xfrm4/6_state_addr_check is a four steps check, all we need to do
> is checking whether the destination address match. Passing saddr from
> flow is worst option, as the checking needs to reach the fourth step.
>
> So, simply this process by only checking destination address only when
> using wildcard source address for looking up SAs.
>
> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---
If you have further comments on your patch that should not be
included in the commit message, you can add them here.
> include/net/xfrm.h | 31 +++++++++++++++++++++++++++++++
> net/xfrm/xfrm_state.c | 2 +-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index e253bf0..fdb9343 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1282,6 +1282,37 @@ xfrm_state_addr_check(const struct xfrm_state *x,
> }
>
> static __inline__ int
> +__xfrm4_state_daddr_check(const struct xfrm_state *x,
> + const xfrm_address_t *daddr)
> +{
> + return ((daddr->a4 == x->id.daddr.a4) ? 1 : 0);
> +}
> +
> +static __inline__ int
> +__xfrm6_state_daddr_check(const struct xfrm_state *x,
> + const xfrm_address_t *daddr)
> +{
> + if (ipv6_addr_equal((struct in6_addr *)daddr, (struct in6_addr *)&x->id.daddr))
> + return 1;
> + else
> + return 0;
> +}
> +
> +static __inline__ int
> +xfrm_state_daddr_check(const struct xfrm_state *x,
> + const xfrm_address_t *daddr,
> + unsigned short family)
> +{
> + switch (family) {
> + case AF_INET:
> + return __xfrm4_state_daddr_check(x, daddr);
> + case AF_INET6:
> + return __xfrm6_state_daddr_check(x, daddr);
> + }
> + return 0;
> +}
You used whitespaces where you should use tabs in the whole patch.
Please do the formating right to avoid cleanup patches.
^ permalink raw reply
* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: Toshiaki Makita @ 2013-09-24 11:45 UTC (permalink / raw)
To: vyasevic
Cc: Toshiaki Makita, David Miller, netdev, Fernando Luis Vazquez Cao,
Patrick McHardy
In-Reply-To: <524052FC.5010301@redhat.com>
On Mon, 2013-09-23 at 10:41 -0400, Vlad Yasevich wrote:
> On 09/17/2013 04:12 AM, Toshiaki Makita wrote:
> > On Mon, 2013-09-16 at 13:49 -0400, Vlad Yasevich wrote:
> >> On 09/13/2013 08:06 AM, Toshiaki Makita wrote:
> >>> On Thu, 2013-09-12 at 16:00 -0400, David Miller wrote:
> >>>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >>>> Date: Tue, 10 Sep 2013 19:27:54 +0900
> >>>>
> >>>>> There seem to be some undesirable behaviors related with PVID.
> >>>>> 1. It has no effect assigning PVID to a port. PVID cannot be applied
> >>>>> to any frame regardless of whether we set it or not.
> >>>>> 2. FDB entries learned via frames applied PVID are registered with
> >>>>> VID 0 rather than VID value of PVID.
> >>>>> 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> >>>>> This leads interoperational problems such as sending frames with VID
> >>>>> 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> >>>>> 0 as they belong to VLAN 0, which is expected to be handled as they have
> >>>>> no VID according to IEEE 802.1Q.
> >>>>>
> >>>>> Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> >>>>> is fixed, because we cannot activate PVID due to it.
> >>>>
> >>>> Please work out the issues in patch #2 with Vlad and resubmit this
> >>>> series.
> >>>>
> >>>> Thank you.
> >>>
> >>> I'm hovering between whether we should fix the issue by changing vlan 0
> >>> interface behavior in 8021q module or enabling a bridge port to sending
> >>> priority-tagged frames, or another better way.
> >>>
> >>> If you could comment it, I'd appreciate it :)
> >>>
> >>>
> >>> BTW, I think what is discussed in patch #2 is another problem about
> >>> handling priority-tags, and it exists without this patch set applied.
> >>> It looks like that we should prepare another patch set than this to fix
> >>> that problem.
> >>>
> >>> Should I include patches that fix the priority-tags problem in this
> >>> patch set and resubmit them all together?
> >>>
> >>
> >> I am thinking that we might need to do it in bridge and it looks like
> >> the simplest way to do it is to have default priority regeneration table
> >> (table 6-5 from 802.1Q doc).
> >>
> >> That way I think we would conform to the spec.
> >>
> >> -vlad
> >
> > Unfortunately I don't think the default priority regeneration table
> > resolves the problem because IEEE 802.1Q says that a VLAN-aware bridge
> > can transmit untagged or VLAN-tagged frames only (the end of section 7.5
> > and 8.1.7).
> >
> > No mechanism to send priority-tagged frames is found as far as I can see
> > the standard. I think the regenerated priority is used for outgoing PCP
> > field only if egress policy is not untagged (i.e. transmitting as
> > VLAN-tagged), and unused if untagged (Section 6.9.2 3rd/4th Paragraph).
> >
> > If we want to transmit priority-tagged frames from a bridge port, I
> > think we need to implement a new (optional) feature that is above the
> > standard, as I stated previously.
> >
> > How do you feel about adding a per-port policy that enables a bridge to
> > send priority-tagged frames instead of untagged frames when egress
> > policy for the port is untagged?
> > With this change, we can transmit frames for a given vlan as either all
> > untagged, all priority-tagged or all VLAN-tagged.
>
> That would work. What I am thinking is that we do it by special casing
> the vid 0 egress policy specification. Let it be untagged by default
> and if it is tagged, then we preserve the priority field and forward
> it on.
>
> This keeps the API stable and doesn't require user/admin from knowing
> exactly what happens. Default operation conforms to the spec and allows
> simple change to make it backward-compatible.
>
> What do you think. I've done a simple prototype of this an it seems to
> work with the VMs I am testing with.
Are you saying that
- by default, set the 0th bit of untagged_bitmap; and
- if we unset the 0th bit and set the "vid"th bit, we transmit frames
classified as belonging to VLAN "vid" as priority-tagged?
If so, though it's attractive to keep current API, I'm worried about if
it could be a bit confusing and not intuitive for kernel/iproute2
developers that VID 0 has a special meaning only in the egress policy.
Wouldn't it be better to adding a new member to struct net_port_vlans
instead of using VID 0 of untagged_bitmap?
Or are you saying that we use a new flag in struct net_port_vlans but
use the BRIDGE_VLAN_INFO_UNTAGGED bit with VID 0 in netlink to set the
flag?
Even in that case, I'm afraid that it might be confusing for developers
for the same reason. We are going to prohibit to specify VID with 0 (and
4095) in adding/deleting a FDB entry or a vlan filtering entry, but it
would allow us to use VID 0 only when a vlan filtering entry is
configured.
I am thinking a new nlattr is a straightforward approach to configure
it.
Thanks,
Toshiaki Makita
>
> -vlad
>
> >
> > Thanks,
> >
> > Toshiaki Makita
> >
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Toshiaki Makita
> >>>
> >>>>
> >>>> --
> >>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>> the body of a message to majordomo@vger.kernel.org
> >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>
> >>>
> >>>
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
^ permalink raw reply
* Re: [net 5/6] i40e: better return values
From: Joe Perches @ 2013-09-24 11:34 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann
In-Reply-To: <1380015910-25927-6-git-send-email-jeffrey.t.kirsher@intel.com>
On Tue, 2013-09-24 at 02:45 -0700, Jeff Kirsher wrote:
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
[]
> @@ -3339,9 +3345,7 @@ static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
> /* Traffic class index starts from zero so
> * increment to return the actual count
> */
> - num_tc++;
> -
> - return num_tc;
> + return num_tc++;
Ick. post_increment problem.
return ++num_tc;
There's nothing wrong with the original code
unless this is a bugfix which should be documented
better than "better return values".
^ permalink raw reply
* Re: [PATCH 12/19] wireless: Change variable type to bool
From: Kalle Valo @ 2013-09-24 10:54 UTC (permalink / raw)
To: Peter Senna Tschudin
Cc: Larry.Finger, chaoming_li, linville, linux-wireless, netdev,
linux-kernel, kernel-janitors
In-Reply-To: <1379802471-30252-12-git-send-email-peter.senna@gmail.com>
Peter Senna Tschudin <peter.senna@gmail.com> writes:
> The variable continual is only assigned the values true and false.
> Change its type to bool.
>
> The simplified semantic patch that find this problem is as
> follows (http://coccinelle.lip6.fr/):
>
> @exists@
> type T;
> identifier b;
> @@
> - T
> + bool
> b = ...;
> ... when any
> b = \(true\|false\)
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
> drivers/net/wireless/rtlwifi/efuse.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Please prefix the patch title with "rtlwifi:". We use "wireless:" for
changes to net/wireless.
--
Kalle Valo
^ permalink raw reply
* Re: linux-next: manual merge of the ipsec-next tree with the net-next tree
From: Steffen Klassert @ 2013-09-24 10:25 UTC (permalink / raw)
To: Stephen Rothwell
Cc: linux-next, linux-kernel, Fan Du, Joe Perches, David Miller,
netdev
In-Reply-To: <20130924121629.f818475ddf308b0494cfce2a@canb.auug.org.au>
Hi Stephen.
On Tue, Sep 24, 2013 at 12:16:29PM +1000, Stephen Rothwell wrote:
> Hi Steffen,
>
> Today's linux-next merge of the ipsec-next tree got a conflict in
> include/net/xfrm.h between commit d511337a1eda ("xfrm.h: Remove extern
> from function prototypes") from the net-next tree and commit aba826958830
> ("{ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode
> callback") from the ipsec-next tree.
>
Thanks for the information, I'll do a rebase of the ipsec-next
tree tomorrow.
^ permalink raw reply
* [net 5/6] i40e: better return values
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
As mentioned by Joe Perches, clean up return values in some functions
making sure to have consistent return types, not mixing types.
A couple of Joe's comments suggested returning void, but since
the functions in question are ndo defined, the return values are fixed.
So make a comment in the header that notes this is a function called by
net_device_ops.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 37 ++++++++++++++++-------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 60c7152..4cbedbd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1787,6 +1787,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
* i40e_vsi_kill_vlan - Remove vsi membership for given vlan
* @vsi: the vsi being configured
* @vid: vlan id to be removed (0 = untagged only , -1 = any)
+ *
+ * Return: 0 on success or negative otherwise
**/
int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
{
@@ -1860,37 +1862,39 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
* i40e_vlan_rx_add_vid - Add a vlan id filter to HW offload
* @netdev: network interface to be adjusted
* @vid: vlan id to be added
+ *
+ * net_device_ops implementation for adding vlan ids
**/
static int i40e_vlan_rx_add_vid(struct net_device *netdev,
__always_unused __be16 proto, u16 vid)
{
struct i40e_netdev_priv *np = netdev_priv(netdev);
struct i40e_vsi *vsi = np->vsi;
- int ret;
+ int ret = 0;
if (vid > 4095)
- return 0;
+ return -EINVAL;
+
+ netdev_info(netdev, "adding %pM vid=%d\n", netdev->dev_addr, vid);
- netdev_info(vsi->netdev, "adding %pM vid=%d\n",
- netdev->dev_addr, vid);
/* If the network stack called us with vid = 0, we should
* indicate to i40e_vsi_add_vlan() that we want to receive
* any traffic (i.e. with any vlan tag, or untagged)
*/
ret = i40e_vsi_add_vlan(vsi, vid ? vid : I40E_VLAN_ANY);
- if (!ret) {
- if (vid < VLAN_N_VID)
- set_bit(vid, vsi->active_vlans);
- }
+ if (!ret && (vid < VLAN_N_VID))
+ set_bit(vid, vsi->active_vlans);
- return 0;
+ return ret;
}
/**
* i40e_vlan_rx_kill_vid - Remove a vlan id filter from HW offload
* @netdev: network interface to be adjusted
* @vid: vlan id to be removed
+ *
+ * net_device_ops implementation for adding vlan ids
**/
static int i40e_vlan_rx_kill_vid(struct net_device *netdev,
__always_unused __be16 proto, u16 vid)
@@ -1898,15 +1902,16 @@ static int i40e_vlan_rx_kill_vid(struct net_device *netdev,
struct i40e_netdev_priv *np = netdev_priv(netdev);
struct i40e_vsi *vsi = np->vsi;
- netdev_info(vsi->netdev, "removing %pM vid=%d\n",
- netdev->dev_addr, vid);
+ netdev_info(netdev, "removing %pM vid=%d\n", netdev->dev_addr, vid);
+
/* return code is ignored as there is nothing a user
* can do about failure to remove and a log message was
- * already printed from another function
+ * already printed from the other function
*/
i40e_vsi_kill_vlan(vsi, vid);
clear_bit(vid, vsi->active_vlans);
+
return 0;
}
@@ -3324,7 +3329,8 @@ static void i40e_pf_unquiesce_all_vsi(struct i40e_pf *pf)
**/
static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
{
- int num_tc = 0, i;
+ u8 num_tc = 0;
+ int i;
/* Scan the ETS Config Priority Table to find
* traffic class enabled for a given priority
@@ -3339,9 +3345,7 @@ static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
/* Traffic class index starts from zero so
* increment to return the actual count
*/
- num_tc++;
-
- return num_tc;
+ return num_tc++;
}
/**
@@ -3491,6 +3495,7 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
/* 3 bits out of 4 for each TC */
vsi->bw_ets_max_quanta[i] = (u8)((tc_bw_max >> (i*4)) & 0x7);
}
+
return 0;
}
--
1.8.3.1
^ permalink raw reply related
* [net 6/6] i40e: clean up coccicheck reported errors
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
coccicheck shows:
drivers/net/ethernet/intel/i40e/i40e_adminq.c:704:2-8: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_adminq.c:763:1-7: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_adminq.c:810:2-8: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_common.c:510:2-8: Replace memcpy
with struct assignment
Fix each of them with a *a = *b;
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 7 +++----
drivers/net/ethernet/intel/i40e/i40e_common.c | 2 +-
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index 0c524fa..cfef7fc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -701,8 +701,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
details = I40E_ADMINQ_DETAILS(hw->aq.asq, hw->aq.asq.next_to_use);
if (cmd_details) {
- memcpy(details, cmd_details,
- sizeof(struct i40e_asq_cmd_details));
+ *details = *cmd_details;
/* If the cmd_details are defined copy the cookie. The
* cpu_to_le32 is not needed here because the data is ignored
@@ -760,7 +759,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
desc_on_ring = I40E_ADMINQ_DESC(hw->aq.asq, hw->aq.asq.next_to_use);
/* if the desc is available copy the temp desc to the right place */
- memcpy(desc_on_ring, desc, sizeof(struct i40e_aq_desc));
+ *desc_on_ring = *desc;
/* if buff is not NULL assume indirect command */
if (buff != NULL) {
@@ -807,7 +806,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
/* if ready, copy the desc back to temp */
if (i40e_asq_done(hw)) {
- memcpy(desc, desc_on_ring, sizeof(struct i40e_aq_desc));
+ *desc = *desc_on_ring;
if (buff != NULL)
memcpy(buff, dma_buff->va, buff_size);
retval = le16_to_cpu(desc->retval);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index c21df7b..1e4ea13 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -507,7 +507,7 @@ i40e_status i40e_aq_get_link_info(struct i40e_hw *hw,
/* save link status information */
if (link)
- memcpy(link, hw_link_info, sizeof(struct i40e_link_status));
+ *link = *hw_link_info;
/* flag cleared so helper functions don't call AQ again */
hw->phy.get_link_info = false;
--
1.8.3.1
^ permalink raw reply related
* [net 4/6] i40e: convert ret to aq_ret
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
When calling admin queue functions the driver should use aq_ret
variable to help make clear that the return value is not a regular
return variable.
This allows for clean up of the return types that were previously
converted to int.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 107 ++++++++++++++--------------
1 file changed, 52 insertions(+), 55 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 865bc6b..60c7152 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1388,7 +1388,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
bool add_happened = false;
int filter_list_len = 0;
u32 changed_flags = 0;
- i40e_status ret = 0;
+ i40e_status aq_ret = 0;
struct i40e_pf *pf;
int num_add = 0;
int num_del = 0;
@@ -1449,28 +1449,28 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
/* flush a full buffer */
if (num_del == filter_list_len) {
- ret = i40e_aq_remove_macvlan(&pf->hw,
+ aq_ret = i40e_aq_remove_macvlan(&pf->hw,
vsi->seid, del_list, num_del,
NULL);
num_del = 0;
memset(del_list, 0, sizeof(*del_list));
- if (ret)
+ if (aq_ret)
dev_info(&pf->pdev->dev,
"ignoring delete macvlan error, err %d, aq_err %d while flushing a full buffer\n",
- ret,
+ aq_ret,
pf->hw.aq.asq_last_status);
}
}
if (num_del) {
- ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid,
+ aq_ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid,
del_list, num_del, NULL);
num_del = 0;
- if (ret)
+ if (aq_ret)
dev_info(&pf->pdev->dev,
"ignoring delete macvlan error, err %d, aq_err %d\n",
- ret, pf->hw.aq.asq_last_status);
+ aq_ret, pf->hw.aq.asq_last_status);
}
kfree(del_list);
@@ -1515,32 +1515,30 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
/* flush a full buffer */
if (num_add == filter_list_len) {
- ret = i40e_aq_add_macvlan(&pf->hw,
- vsi->seid,
- add_list,
- num_add,
- NULL);
+ aq_ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
+ add_list, num_add,
+ NULL);
num_add = 0;
- if (ret)
+ if (aq_ret)
break;
memset(add_list, 0, sizeof(*add_list));
}
}
if (num_add) {
- ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
- add_list, num_add, NULL);
+ aq_ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
+ add_list, num_add, NULL);
num_add = 0;
}
kfree(add_list);
add_list = NULL;
- if (add_happened && (!ret)) {
+ if (add_happened && (!aq_ret)) {
/* do nothing */;
- } else if (add_happened && (ret)) {
+ } else if (add_happened && (aq_ret)) {
dev_info(&pf->pdev->dev,
"add filter failed, err %d, aq_err %d\n",
- ret, pf->hw.aq.asq_last_status);
+ aq_ret, pf->hw.aq.asq_last_status);
if ((pf->hw.aq.asq_last_status == I40E_AQ_RC_ENOSPC) &&
!test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
&vsi->state)) {
@@ -1556,28 +1554,27 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
if (changed_flags & IFF_ALLMULTI) {
bool cur_multipromisc;
cur_multipromisc = !!(vsi->current_netdev_flags & IFF_ALLMULTI);
- ret = i40e_aq_set_vsi_multicast_promiscuous(&vsi->back->hw,
- vsi->seid,
- cur_multipromisc,
- NULL);
- if (ret)
+ aq_ret = i40e_aq_set_vsi_multicast_promiscuous(&vsi->back->hw,
+ vsi->seid,
+ cur_multipromisc,
+ NULL);
+ if (aq_ret)
dev_info(&pf->pdev->dev,
"set multi promisc failed, err %d, aq_err %d\n",
- ret, pf->hw.aq.asq_last_status);
+ aq_ret, pf->hw.aq.asq_last_status);
}
if ((changed_flags & IFF_PROMISC) || promisc_forced_on) {
bool cur_promisc;
cur_promisc = (!!(vsi->current_netdev_flags & IFF_PROMISC) ||
test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
&vsi->state));
- ret = i40e_aq_set_vsi_unicast_promiscuous(&vsi->back->hw,
- vsi->seid,
- cur_promisc,
- NULL);
- if (ret)
+ aq_ret = i40e_aq_set_vsi_unicast_promiscuous(&vsi->back->hw,
+ vsi->seid,
+ cur_promisc, NULL);
+ if (aq_ret)
dev_info(&pf->pdev->dev,
"set uni promisc failed, err %d, aq_err %d\n",
- ret, pf->hw.aq.asq_last_status);
+ aq_ret, pf->hw.aq.asq_last_status);
}
clear_bit(__I40E_CONFIG_BUSY, &vsi->state);
@@ -1936,10 +1933,10 @@ static void i40e_restore_vlan(struct i40e_vsi *vsi)
* @vsi: the vsi being adjusted
* @vid: the vlan id to set as a PVID
**/
-i40e_status i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
+int i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
{
struct i40e_vsi_context ctxt;
- i40e_status ret;
+ i40e_status aq_ret;
vsi->info.valid_sections = cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
vsi->info.pvid = cpu_to_le16(vid);
@@ -1948,14 +1945,15 @@ i40e_status i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
ctxt.seid = vsi->seid;
memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
- ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
- if (ret) {
+ aq_ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
+ if (aq_ret) {
dev_info(&vsi->back->pdev->dev,
"%s: update vsi failed, aq_err=%d\n",
__func__, vsi->back->hw.aq.asq_last_status);
+ return -ENOENT;
}
- return ret;
+ return 0;
}
/**
@@ -3451,28 +3449,27 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
struct i40e_aqc_query_vsi_bw_config_resp bw_config = {0};
struct i40e_pf *pf = vsi->back;
struct i40e_hw *hw = &pf->hw;
+ i40e_status aq_ret;
u32 tc_bw_max;
- int ret;
int i;
/* Get the VSI level BW configuration */
- ret = i40e_aq_query_vsi_bw_config(hw, vsi->seid, &bw_config, NULL);
- if (ret) {
+ aq_ret = i40e_aq_query_vsi_bw_config(hw, vsi->seid, &bw_config, NULL);
+ if (aq_ret) {
dev_info(&pf->pdev->dev,
"couldn't get pf vsi bw config, err %d, aq_err %d\n",
- ret, pf->hw.aq.asq_last_status);
- return ret;
+ aq_ret, pf->hw.aq.asq_last_status);
+ return -EINVAL;
}
/* Get the VSI level BW configuration per TC */
- ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid,
- &bw_ets_config,
- NULL);
- if (ret) {
+ aq_ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid, &bw_ets_config,
+ NULL);
+ if (aq_ret) {
dev_info(&pf->pdev->dev,
"couldn't get pf vsi ets bw config, err %d, aq_err %d\n",
- ret, pf->hw.aq.asq_last_status);
- return ret;
+ aq_ret, pf->hw.aq.asq_last_status);
+ return -EINVAL;
}
if (bw_config.tc_valid_bits != bw_ets_config.tc_valid_bits) {
@@ -3494,7 +3491,7 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
/* 3 bits out of 4 for each TC */
vsi->bw_ets_max_quanta[i] = (u8)((tc_bw_max >> (i*4)) & 0x7);
}
- return ret;
+ return 0;
}
/**
@@ -3505,30 +3502,30 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
*
* Returns 0 on success, negative value on failure
**/
-static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi,
- u8 enabled_tc,
+static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi, u8 enabled_tc,
u8 *bw_share)
{
struct i40e_aqc_configure_vsi_tc_bw_data bw_data;
- int i, ret = 0;
+ i40e_status aq_ret;
+ int i;
bw_data.tc_valid_bits = enabled_tc;
for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
bw_data.tc_bw_credits[i] = bw_share[i];
- ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid,
- &bw_data, NULL);
- if (ret) {
+ aq_ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid, &bw_data,
+ NULL);
+ if (aq_ret) {
dev_info(&vsi->back->pdev->dev,
"%s: AQ command Config VSI BW allocation per TC failed = %d\n",
__func__, vsi->back->hw.aq.asq_last_status);
- return ret;
+ return -EINVAL;
}
for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
vsi->info.qs_handle[i] = bw_data.qs_handles[i];
- return ret;
+ return 0;
}
/**
--
1.8.3.1
^ permalink raw reply related
* [net 2/6] i40e: use common failure flow
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
As mentioned by Joe Perches, we should be using
foo = alloc(...)
if (!foo)
return -ENOMEM;
return 0;
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 601d482..67f8fd5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -101,10 +101,10 @@ int i40e_allocate_dma_mem_d(struct i40e_hw *hw, struct i40e_dma_mem *mem,
mem->size = ALIGN(size, alignment);
mem->va = dma_zalloc_coherent(&pf->pdev->dev, mem->size,
&mem->pa, GFP_KERNEL);
- if (mem->va)
- return 0;
+ if (!mem->va)
+ return -ENOMEM;
- return -ENOMEM;
+ return 0;
}
/**
@@ -136,10 +136,10 @@ int i40e_allocate_virt_mem_d(struct i40e_hw *hw, struct i40e_virt_mem *mem,
mem->size = size;
mem->va = kzalloc(size, GFP_KERNEL);
- if (mem->va)
- return 0;
+ if (!mem->va)
+ return -ENOMEM;
- return -ENOMEM;
+ return 0;
}
/**
--
1.8.3.1
^ permalink raw reply related
* [net 3/6] i40e: small clean ups from review
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jesse Brandeburg <jesse.brandeburg@intel.com>
As mentioned by Joe Perches clean up a loop flow.
Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 67f8fd5..865bc6b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -174,8 +174,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
u16 needed, u16 id)
{
int ret = -ENOMEM;
- int i = 0;
- int j = 0;
+ int i, j;
if (!pile || needed == 0 || id >= I40E_PILE_VALID_BIT) {
dev_info(&pf->pdev->dev,
@@ -186,7 +185,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
/* start the linear search with an imperfect hint */
i = pile->search_hint;
- while (i < pile->num_entries && ret < 0) {
+ while (i < pile->num_entries) {
/* skip already allocated entries */
if (pile->list[i] & I40E_PILE_VALID_BIT) {
i++;
@@ -205,6 +204,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
pile->list[i+j] = id | I40E_PILE_VALID_BIT;
ret = i;
pile->search_hint = i + j;
+ break;
} else {
/* not enough, so skip over it and continue looking */
i += j;
--
1.8.3.1
^ permalink raw reply related
* [net 0/6][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to igb and i40e.
Todd provides a fix for 82580 devices in igb, where the ethtool
loopback test was missing 82580 copper devices.
Jesse provides five fixes/cleanups to i40e based on feedback from
Joe Perches and the community.
The following are changes since commit 9fe34f5d920b183ec063550e0f4ec854aa373316:
mrp: add periodictimer to allow retries when packets get lost
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master
Todd Fujinaka (1):
igb: Fix ethtool loopback test for 82580 copper
Jesse Brandeburg (5):
i40e: use common failure flow
i40e: small clean ups from review
i40e: convert ret to aq_ret
i40e: better return values
i40e: clean up coccicheck reported errors
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 7 +-
drivers/net/ethernet/intel/i40e/i40e_common.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_main.c | 162 +++++++++++++-------------
drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 +
4 files changed, 89 insertions(+), 85 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [net 1/6] igb: Fix ethtool loopback test for 82580 copper
From: Jeff Kirsher @ 2013-09-24 9:45 UTC (permalink / raw)
To: davem; +Cc: Todd Fujinaka, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Todd Fujinaka <todd.fujinaka@intel.com>
Add back 82580 loopback tests to ethtool.
Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 48cbc83..86d5142 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1607,6 +1607,9 @@ static int igb_integrated_phy_loopback(struct igb_adapter *adapter)
igb_write_phy_reg(hw, I347AT4_PAGE_SELECT, 0);
igb_write_phy_reg(hw, PHY_CONTROL, 0x4140);
}
+ } else if (hw->phy.type == e1000_phy_82580) {
+ /* enable MII loopback */
+ igb_write_phy_reg(hw, I82580_PHY_LBK_CTRL, 0x8041);
}
/* add small delay to avoid loopback test failure */
--
1.8.3.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox