* [PATCH net v4 03/12] bonding: fix unexpected IFF_BONDING bit unset
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
The IFF_BONDING means bonding master or bonding slave device.
->ndo_add_slave() sets IFF_BONDING flag and ->ndo_del_slave() unsets
IFF_BONDING flag.
bond0<--bond1
Both bond0 and bond1 are bonding device and these should keep having
IFF_BONDING flag until they are removed.
But bond1 would lose IFF_BONDING at ->ndo_del_slave() because that routine
do not check whether the slave device is the bonding type or not.
This patch adds the interface type check routine before removing
IFF_BONDING flag.
Test commands:
ip link add bond0 type bond
ip link add bond1 type bond
ip link set bond1 master bond0
ip link set bond1 nomaster
ip link del bond1 type bond
ip link add bond1 type bond
Splat looks like:
[ 38.843933] proc_dir_entry 'bonding/bond1' already registered
[ 38.844741] WARNING: CPU: 1 PID: 631 at fs/proc/generic.c:361 proc_register+0x2a9/0x3e0
[ 38.845741] Modules linked in: bonding ip_tables x_tables
[ 38.846432] CPU: 1 PID: 631 Comm: ip Not tainted 5.3.0+ #3
[ 38.847234] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 38.848489] RIP: 0010:proc_register+0x2a9/0x3e0
[ 38.849164] Code: 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 39 01 00 00 48 8b 04 24 48 89 ea 48 c7 c7 e0 2b 34 b3 48 8b b0 e
0 00 00 00 e8 c7 b6 89 ff <0f> 0b 48 c7 c7 40 3d c5 b3 e8 99 7a 38 01 48 8b 4c 24 10 48 b8 00
[ 38.851317] RSP: 0018:ffff888061527078 EFLAGS: 00010282
[ 38.851902] RAX: dffffc0000000008 RBX: ffff888064dc8cb0 RCX: ffffffffb1d252a2
[ 38.852684] RDX: 0000000000000001 RSI: 0000000000000008 RDI: ffff88806cbf6b8c
[ 38.853464] RBP: ffff888064dc8f33 R08: ffffed100d980019 R09: ffffed100d980019
[ 38.854242] R10: 0000000000000001 R11: ffffed100d980018 R12: ffff888064dc8e48
[ 38.855929] R13: ffff888064dc8f32 R14: dffffc0000000000 R15: ffffed100c9b91e6
[ 38.856695] FS: 00007fc9fcc230c0(0000) GS:ffff88806ca00000(0000) knlGS:0000000000000000
[ 38.857541] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 38.858150] CR2: 000055948b91c118 CR3: 0000000057110006 CR4: 00000000000606e0
[ 38.858957] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 38.859785] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 38.860700] Call Trace:
[ 38.861004] proc_create_seq_private+0xb3/0xf0
[ 38.861460] bond_create_proc_entry+0x1b3/0x3f0 [bonding]
[ 38.862113] bond_netdev_event+0x433/0x970 [bonding]
[ 38.862762] ? __module_text_address+0x13/0x140
[ 38.867678] notifier_call_chain+0x90/0x160
[ 38.868257] register_netdevice+0x9b3/0xd80
[ 38.868791] ? alloc_netdev_mqs+0x854/0xc10
[ 38.869335] ? netdev_change_features+0xa0/0xa0
[ 38.869852] ? rtnl_create_link+0x2ed/0xad0
[ 38.870423] bond_newlink+0x2a/0x60 [bonding]
[ 38.870935] __rtnl_newlink+0xb9f/0x11b0
[ ... ]
Fixes: 0b680e753724 ("[PATCH] bonding: Add priv_flag to avoid event mishandling")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v2 -> v4 :
- This patch is not changed
v1 -> v2 :
- Do not add a new priv_flag.
drivers/net/bonding/bond_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 931d9d935686..0db12fcfc953 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1816,7 +1816,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
slave_disable_netpoll(new_slave);
err_close:
- slave_dev->priv_flags &= ~IFF_BONDING;
+ if (!netif_is_bond_master(slave_dev))
+ slave_dev->priv_flags &= ~IFF_BONDING;
dev_close(slave_dev);
err_restore_mac:
@@ -2017,7 +2018,8 @@ static int __bond_release_one(struct net_device *bond_dev,
else
dev_set_mtu(slave_dev, slave->original_mtu);
- slave_dev->priv_flags &= ~IFF_BONDING;
+ if (!netif_is_bond_master(slave_dev))
+ slave_dev->priv_flags &= ~IFF_BONDING;
bond_free_slave(slave);
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 04/12] bonding: use dynamic lockdep key instead of subclass
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
All bonding device has same lockdep key and subclass is initialized with
nest_level.
But actual nest_level value can be changed when a lower device is attached.
And at this moment, the subclass should be updated but it seems to be
unsafe.
So this patch makes bonding use dynamic lockdep key instead of the
subclass.
Test commands:
ip link add bond0 type bond
for i in {1..5}
do
let A=$i-1
ip link add bond$i type bond
ip link set bond$i master bond$A
done
ip link set bond5 master bond0
Splat looks like:
[ 29.858108] WARNING: possible recursive locking detected
[ 29.858630] 5.3.0+ #3 Not tainted
[ 29.858946] --------------------------------------------
[ 29.859501] ip/629 is trying to acquire lock:
[ 29.860591] ffff88806801cf00 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[ 29.861677]
[ 29.861677] but task is already holding lock:
[ 29.862307] ffff88806801ada0 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[ 29.863406]
[ 29.863406] other info that might help us debug this:
[ 29.864092] Possible unsafe locking scenario:
[ 29.864092]
[ 29.864715] CPU0
[ 29.864968] ----
[ 29.865225] lock(&(&bond->stats_lock)->rlock#2/2);
[ 29.865731] lock(&(&bond->stats_lock)->rlock#2/2);
[ 29.866235]
[ 29.866235] *** DEADLOCK ***
[ 29.866235]
[ 29.866829] May be due to missing lock nesting notation
[ 29.866829]
[ 29.867632] 3 locks held by ip/629:
[ 29.868077] #0: ffffffffb4ec7a30 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[ 29.869141] #1: ffff88806801ada0 (&(&bond->stats_lock)->rlock#2/2){+.+.}, at: bond_get_stats+0xb8/0x500 [bonding]
[ 29.870504] #2: ffffffffb4b22780 (rcu_read_lock){....}, at: bond_get_stats+0x9f/0x500 [bonding]
[ 29.875917]
[ 29.875917] stack backtrace:
[ 29.876533] CPU: 0 PID: 629 Comm: ip Not tainted 5.3.0+ #3
[ 29.877254] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 29.878344] Call Trace:
[ 29.878697] dump_stack+0x7c/0xbb
[ 29.879167] __lock_acquire+0x26a9/0x3df0
[ 29.879660] ? register_lock_class+0x14d0/0x14d0
[ 29.880067] lock_acquire+0x164/0x3b0
[ 29.880402] ? bond_get_stats+0xb8/0x500 [bonding]
[ 29.880826] _raw_spin_lock_nested+0x2e/0x60
[ 29.881206] ? bond_get_stats+0xb8/0x500 [bonding]
[ 29.881725] bond_get_stats+0xb8/0x500 [bonding]
[ ... ]
Fixes: d3fff6c443fe ("net: add netdev_lockdep_set_classes() helper")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v1 -> v4 :
- This patch is not changed
drivers/net/bonding/bond_main.c | 61 ++++++++++++++++++++++++++++++---
include/net/bonding.h | 3 ++
2 files changed, 59 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0db12fcfc953..7f574e74ed78 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1857,6 +1857,32 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
return res;
}
+static void bond_dev_set_lockdep_one(struct net_device *dev,
+ struct netdev_queue *txq,
+ void *_unused)
+{
+ struct bonding *bond = netdev_priv(dev);
+
+ lockdep_set_class(&txq->_xmit_lock, &bond->xmit_lock_key);
+}
+
+static void bond_update_lock_key(struct net_device *dev)
+{
+ struct bonding *bond = netdev_priv(dev);
+
+ lockdep_unregister_key(&bond->stats_lock_key);
+ lockdep_unregister_key(&bond->addr_lock_key);
+ lockdep_unregister_key(&bond->xmit_lock_key);
+
+ lockdep_register_key(&bond->stats_lock_key);
+ lockdep_register_key(&bond->addr_lock_key);
+ lockdep_register_key(&bond->xmit_lock_key);
+
+ lockdep_set_class(&bond->stats_lock, &bond->stats_lock_key);
+ lockdep_set_class(&dev->addr_list_lock, &bond->addr_lock_key);
+ netdev_for_each_tx_queue(dev, bond_dev_set_lockdep_one, NULL);
+}
+
/* Try to release the slave device <slave> from the bond device <master>
* It is legal to access curr_active_slave without a lock because all the function
* is RTNL-locked. If "all" is true it means that the function is being called
@@ -2022,6 +2048,8 @@ static int __bond_release_one(struct net_device *bond_dev,
slave_dev->priv_flags &= ~IFF_BONDING;
bond_free_slave(slave);
+ if (netif_is_bond_master(slave_dev))
+ bond_update_lock_key(slave_dev);
return 0;
}
@@ -3459,7 +3487,7 @@ static void bond_get_stats(struct net_device *bond_dev,
struct list_head *iter;
struct slave *slave;
- spin_lock_nested(&bond->stats_lock, bond_get_nest_level(bond_dev));
+ spin_lock(&bond->stats_lock);
memcpy(stats, &bond->bond_stats, sizeof(*stats));
rcu_read_lock();
@@ -4297,8 +4325,6 @@ void bond_setup(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
- spin_lock_init(&bond->mode_lock);
- spin_lock_init(&bond->stats_lock);
bond->params = bonding_defaults;
/* Initialize pointers */
@@ -4367,6 +4393,9 @@ static void bond_uninit(struct net_device *bond_dev)
list_del(&bond->bond_list);
+ lockdep_unregister_key(&bond->stats_lock_key);
+ lockdep_unregister_key(&bond->addr_lock_key);
+ lockdep_unregister_key(&bond->xmit_lock_key);
bond_debug_unregister(bond);
}
@@ -4758,6 +4787,29 @@ static int bond_check_params(struct bond_params *params)
return 0;
}
+static struct lock_class_key qdisc_tx_busylock_key;
+static struct lock_class_key qdisc_running_key;
+
+static void bond_dev_set_lockdep_class(struct net_device *dev)
+{
+ struct bonding *bond = netdev_priv(dev);
+
+ dev->qdisc_tx_busylock = &qdisc_tx_busylock_key;
+ dev->qdisc_running_key = &qdisc_running_key;
+
+ spin_lock_init(&bond->mode_lock);
+
+ spin_lock_init(&bond->stats_lock);
+ lockdep_register_key(&bond->stats_lock_key);
+ lockdep_set_class(&bond->stats_lock, &bond->stats_lock_key);
+
+ lockdep_register_key(&bond->addr_lock_key);
+ lockdep_set_class(&dev->addr_list_lock, &bond->addr_lock_key);
+
+ lockdep_register_key(&bond->xmit_lock_key);
+ netdev_for_each_tx_queue(dev, bond_dev_set_lockdep_one, NULL);
+}
+
/* Called from registration process */
static int bond_init(struct net_device *bond_dev)
{
@@ -4771,8 +4823,7 @@ static int bond_init(struct net_device *bond_dev)
return -ENOMEM;
bond->nest_level = SINGLE_DEPTH_NESTING;
- netdev_lockdep_set_classes(bond_dev);
-
+ bond_dev_set_lockdep_class(bond_dev);
list_add_tail(&bond->bond_list, &bn->dev_list);
bond_prepare_sysfs_group(bond);
diff --git a/include/net/bonding.h b/include/net/bonding.h
index f7fe45689142..c39ac7061e41 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -239,6 +239,9 @@ struct bonding {
struct dentry *debug_dir;
#endif /* CONFIG_DEBUG_FS */
struct rtnl_link_stats64 bond_stats;
+ struct lock_class_key stats_lock_key;
+ struct lock_class_key xmit_lock_key;
+ struct lock_class_key addr_lock_key;
};
#define bond_slave_get_rcu(dev) \
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 05/12] team: use dynamic lockdep key instead of static key
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
In the current code, all team devices have same static lockdep key
and team devices could be nested so that it makes unnecessary
lockdep warning.
Test commands:
ip link add team0 type team
for i in {1..7}
do
let A=$i-1
ip link add team$i type team
ip link set team$i master team$A
done
ip link del team0
Splat looks like:
[ 32.862645] WARNING: possible recursive locking detected
[ 32.863304] 5.3.0+ #3 Not tainted
[ 32.863700] --------------------------------------------
[ 32.864358] ip/647 is trying to acquire lock:
[ 32.864968] ffff8880666a6ad8 (&dev_addr_list_lock_key/1){+...}, at: dev_uc_sync_multiple+0xfa/0x1a0
[ 32.866047]
[ 32.866047] but task is already holding lock:
[ 32.866744] ffff888067402558 (&dev_addr_list_lock_key/1){+...}, at: dev_uc_unsync+0x10c/0x1b0
[ 32.867774]
[ 32.867774] other info that might help us debug this:
[ 32.868513] Possible unsafe locking scenario:
[ 32.868513]
[ 32.869180] CPU0
[ 32.872973] ----
[ 32.876717] lock(&dev_addr_list_lock_key/1);
[ 32.877130] lock(&dev_addr_list_lock_key/1);
[ 32.877621]
[ 32.877621] *** DEADLOCK ***
[ 32.877621]
[ 32.878284] May be due to missing lock nesting notation
[ 32.878284]
[ 32.878999] 5 locks held by ip/647:
[ 32.879382] #0: ffffffff8fec7a30 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[ 32.880110] #1: ffff888068d5e300 (&team->lock){+.+.}, at: team_uninit+0x3a/0x1a0 [team]
[ 32.880889] #2: ffff888068d5d978 (&dev_addr_list_lock_key){+...}, at: dev_uc_unsync+0x98/0x1b0
[ 32.881660] #3: ffff888067402558 (&dev_addr_list_lock_key/1){+...}, at: dev_uc_unsync+0x10c/0x1b0
[ 32.882451] #4: ffffffff8fb22780 (rcu_read_lock){....}, at: team_set_rx_mode+0x5/0x1d0 [team]
[ 32.883209]
[ 32.883209] stack backtrace:
[ 32.883605] CPU: 0 PID: 647 Comm: ip Not tainted 5.3.0+ #3
[ 32.884144] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 32.884926] Call Trace:
[ 32.885151] dump_stack+0x7c/0xbb
[ 32.885460] __lock_acquire+0x26a9/0x3df0
[ 32.885964] ? register_lock_class+0x14d0/0x14d0
[ 32.886522] ? register_lock_class+0x14d0/0x14d0
[ 32.887114] lock_acquire+0x164/0x3b0
[ 32.887578] ? dev_uc_sync_multiple+0xfa/0x1a0
[ 32.888130] _raw_spin_lock_nested+0x2e/0x60
[ 32.888725] ? dev_uc_sync_multiple+0xfa/0x1a0
[ 32.889264] dev_uc_sync_multiple+0xfa/0x1a0
[ 32.889779] team_set_rx_mode+0xa9/0x1d0 [team]
[ 32.892841] dev_uc_unsync+0x151/0x1b0
[ ... ]
Fixes: 3d249d4ca7d0 ("net: introduce ethernet teaming device")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v1 -> v4 :
- This patch is not changed
drivers/net/team/team.c | 61 ++++++++++++++++++++++++++++++++++++++---
include/linux/if_team.h | 5 ++++
2 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index e8089def5a46..bfcd6ed57493 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -1607,6 +1607,34 @@ static const struct team_option team_options[] = {
},
};
+static void team_dev_set_lockdep_one(struct net_device *dev,
+ struct netdev_queue *txq,
+ void *_unused)
+{
+ struct team *team = netdev_priv(dev);
+
+ lockdep_set_class(&txq->_xmit_lock, &team->xmit_lock_key);
+}
+
+static struct lock_class_key qdisc_tx_busylock_key;
+static struct lock_class_key qdisc_running_key;
+
+static void team_dev_set_lockdep_class(struct net_device *dev)
+{
+ struct team *team = netdev_priv(dev);
+
+ dev->qdisc_tx_busylock = &qdisc_tx_busylock_key;
+ dev->qdisc_running_key = &qdisc_running_key;
+
+ lockdep_register_key(&team->team_lock_key);
+ __mutex_init(&team->lock, "team->team_lock_key", &team->team_lock_key);
+
+ lockdep_register_key(&team->addr_lock_key);
+ lockdep_set_class(&dev->addr_list_lock, &team->addr_lock_key);
+
+ lockdep_register_key(&team->xmit_lock_key);
+ netdev_for_each_tx_queue(dev, team_dev_set_lockdep_one, NULL);
+}
static int team_init(struct net_device *dev)
{
@@ -1615,7 +1643,6 @@ static int team_init(struct net_device *dev)
int err;
team->dev = dev;
- mutex_init(&team->lock);
team_set_no_mode(team);
team->pcpu_stats = netdev_alloc_pcpu_stats(struct team_pcpu_stats);
@@ -1642,7 +1669,7 @@ static int team_init(struct net_device *dev)
goto err_options_register;
netif_carrier_off(dev);
- netdev_lockdep_set_classes(dev);
+ team_dev_set_lockdep_class(dev);
return 0;
@@ -1673,6 +1700,11 @@ static void team_uninit(struct net_device *dev)
team_queue_override_fini(team);
mutex_unlock(&team->lock);
netdev_change_features(dev);
+
+ lockdep_unregister_key(&team->team_lock_key);
+ lockdep_unregister_key(&team->addr_lock_key);
+ lockdep_unregister_key(&team->xmit_lock_key);
+
}
static void team_destructor(struct net_device *dev)
@@ -1967,6 +1999,23 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev,
return err;
}
+static void team_update_lock_key(struct net_device *dev)
+{
+ struct team *team = netdev_priv(dev);
+
+ lockdep_unregister_key(&team->team_lock_key);
+ lockdep_unregister_key(&team->addr_lock_key);
+ lockdep_unregister_key(&team->xmit_lock_key);
+
+ lockdep_register_key(&team->team_lock_key);
+ lockdep_register_key(&team->addr_lock_key);
+ lockdep_register_key(&team->xmit_lock_key);
+
+ lockdep_set_class(&team->lock, &team->team_lock_key);
+ lockdep_set_class(&dev->addr_list_lock, &team->addr_lock_key);
+ netdev_for_each_tx_queue(dev, team_dev_set_lockdep_one, NULL);
+}
+
static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
{
struct team *team = netdev_priv(dev);
@@ -1976,8 +2025,12 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev)
err = team_port_del(team, port_dev);
mutex_unlock(&team->lock);
- if (!err)
- netdev_change_features(dev);
+ if (err)
+ return err;
+
+ if (netif_is_team_master(port_dev))
+ team_update_lock_key(port_dev);
+ netdev_change_features(dev);
return err;
}
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 06faa066496f..9c97bb19ed34 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -223,6 +223,11 @@ struct team {
atomic_t count_pending;
struct delayed_work dw;
} mcast_rejoin;
+
+ struct lock_class_key team_lock_key;
+ struct lock_class_key xmit_lock_key;
+ struct lock_class_key addr_lock_key;
+
long mode_priv[TEAM_MODE_PRIV_LONGS];
};
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 06/12] macsec: use dynamic lockdep key instead of subclass
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
All macsec device has same lockdep key and subclass is initialized with
nest_level.
But actual nest_level value can be changed when a lower device is attached.
And at this moment, the subclass should be updated but it seems to be
unsafe.
So this patch makes macsec use dynamic lockdep key instead of the subclass.
Test commands:
ip link add bond0 type bond
ip link add dummy0 type dummy
ip link add macsec0 link bond0 type macsec
ip link add macsec1 link dummy0 type macsec
ip link set bond0 mtu 1000
ip link set macsec1 master bond0
ip link set bond0 up
ip link set macsec0 up
ip link set dummy0 up
ip link set macsec1 up
Splat looks like:
[ 29.758606] WARNING: possible recursive locking detected
[ 29.759626] 5.3.0+ #3 Not tainted
[ 29.760670] --------------------------------------------
[ 29.761385] ip/639 is trying to acquire lock:
[ 29.761938] ffff888067680298 (&macsec_netdev_addr_lock_key/1){+...}, at: dev_uc_sync_multiple+0xfa/0x1a0
[ 29.763073]
[ 29.763073] but task is already holding lock:
[ 29.763840] ffff888060148298 (&macsec_netdev_addr_lock_key/1){+...}, at: dev_set_rx_mode+0x19/0x30
[ 29.764931]
[ 29.764931] other info that might help us debug this:
[ 29.765721] Possible unsafe locking scenario:
[ 29.765721]
[ 29.766615] CPU0
[ 29.766914] ----
[ 29.767256] lock(&macsec_netdev_addr_lock_key/1);
[ 29.767847] lock(&macsec_netdev_addr_lock_key/1);
[ 29.768441]
[ 29.768441] *** DEADLOCK ***
[ 29.768441]
[ 29.769158] May be due to missing lock nesting notation
[ 29.769158]
[ 29.770083] 4 locks held by ip/639:
[ 29.770908] #0: ffffffff93ec7a30 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[ 29.771970] #1: ffff888060148298 (&macsec_netdev_addr_lock_key/1){+...}, at: dev_set_rx_mode+0x19/0x30
[ 29.773216] #2: ffff888063e58298 (&dev_addr_list_lock_key/3){+...}, at: dev_mc_sync+0xfa/0x1a0
[ 29.774324] #3: ffffffff93b22780 (rcu_read_lock){....}, at: bond_set_rx_mode+0x5/0x3c0 [bonding]
[ 29.775459]
[ 29.775459] stack backtrace:
[ 29.775986] CPU: 0 PID: 639 Comm: ip Not tainted 5.3.0+ #3
[ 29.776719] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 29.777707] Call Trace:
[ 29.778012] dump_stack+0x7c/0xbb
[ 29.778434] __lock_acquire+0x26a9/0x3df0
[ 29.778920] ? register_lock_class+0x14d0/0x14d0
[ 29.779537] lock_acquire+0x164/0x3b0
[ 29.779981] ? dev_uc_sync_multiple+0xfa/0x1a0
[ 29.780523] ? rcu_read_lock_held+0x90/0xa0
[ 29.781028] _raw_spin_lock_nested+0x2e/0x60
[ 29.781550] ? dev_uc_sync_multiple+0xfa/0x1a0
[ 29.782311] dev_uc_sync_multiple+0xfa/0x1a0
[ 29.782832] bond_set_rx_mode+0x269/0x3c0 [bonding]
[ ... ]
Fixes: e20038724552 ("macsec: fix lockdep splats when nesting devices")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v1 -> v4 :
- This patch is not changed
drivers/net/macsec.c | 37 ++++++++++++++++++++++++++++++++-----
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index cb7637364b40..c4a41b90c846 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -267,6 +267,8 @@ struct macsec_dev {
struct pcpu_secy_stats __percpu *stats;
struct list_head secys;
struct gro_cells gro_cells;
+ struct lock_class_key xmit_lock_key;
+ struct lock_class_key addr_lock_key;
unsigned int nest_level;
};
@@ -2750,7 +2752,32 @@ static netdev_tx_t macsec_start_xmit(struct sk_buff *skb,
#define MACSEC_FEATURES \
(NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST)
-static struct lock_class_key macsec_netdev_addr_lock_key;
+
+static void macsec_dev_set_lockdep_one(struct net_device *dev,
+ struct netdev_queue *txq,
+ void *_unused)
+{
+ struct macsec_dev *macsec = macsec_priv(dev);
+
+ lockdep_set_class(&txq->_xmit_lock, &macsec->xmit_lock_key);
+}
+
+static struct lock_class_key qdisc_tx_busylock_key;
+static struct lock_class_key qdisc_running_key;
+
+static void macsec_dev_set_lockdep_class(struct net_device *dev)
+{
+ struct macsec_dev *macsec = macsec_priv(dev);
+
+ dev->qdisc_tx_busylock = &qdisc_tx_busylock_key;
+ dev->qdisc_running_key = &qdisc_running_key;
+
+ lockdep_register_key(&macsec->addr_lock_key);
+ lockdep_set_class(&dev->addr_list_lock, &macsec->addr_lock_key);
+
+ lockdep_register_key(&macsec->xmit_lock_key);
+ netdev_for_each_tx_queue(dev, macsec_dev_set_lockdep_one, NULL);
+}
static int macsec_dev_init(struct net_device *dev)
{
@@ -2781,6 +2808,7 @@ static int macsec_dev_init(struct net_device *dev)
if (is_zero_ether_addr(dev->broadcast))
memcpy(dev->broadcast, real_dev->broadcast, dev->addr_len);
+ macsec_dev_set_lockdep_class(dev);
return 0;
}
@@ -2790,6 +2818,9 @@ static void macsec_dev_uninit(struct net_device *dev)
gro_cells_destroy(&macsec->gro_cells);
free_percpu(dev->tstats);
+
+ lockdep_unregister_key(&macsec->addr_lock_key);
+ lockdep_unregister_key(&macsec->xmit_lock_key);
}
static netdev_features_t macsec_fix_features(struct net_device *dev,
@@ -3264,10 +3295,6 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
dev_hold(real_dev);
macsec->nest_level = dev_get_nest_level(real_dev) + 1;
- netdev_lockdep_set_classes(dev);
- lockdep_set_class_and_subclass(&dev->addr_list_lock,
- &macsec_netdev_addr_lock_key,
- macsec_get_nest_level(dev));
err = netdev_upper_dev_link(real_dev, dev, extack);
if (err < 0)
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 07/12] macvlan: use dynamic lockdep key instead of subclass
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
All macvlan device has same lockdep key and subclass is initialized with
nest_level.
But actual nest_level value can be changed when a lower device is attached.
And at this moment, the subclass should be updated but it seems to be
unsafe.
So this patch makes macvlan use dynamic lockdep key instead of the
subclass.
Test commands:
ip link add bond0 type bond
ip link add dummy0 type dummy
ip link add macvlan0 link bond0 type macvlan mode bridge
ip link add macvlan1 link dummy0 type macvlan mode bridge
ip link set bond0 mtu 1000
ip link set macvlan1 master bond0
ip link set bond0 up
ip link set macvlan0 up
ip link set dummy0 up
ip link set macvlan1 up
Splat looks like:
[ 30.281866] WARNING: possible recursive locking detected
[ 30.282374] 5.3.0+ #3 Not tainted
[ 30.282673] --------------------------------------------
[ 30.283138] ip/643 is trying to acquire lock:
[ 30.283522] ffff88806750c818 (&macvlan_netdev_addr_lock_key/1){+...}, at: dev_uc_sync_multiple+0xfa/0x1a0
[ 30.284363]
[ 30.284363] but task is already holding lock:
[ 30.284878] ffff88806853ead8 (&macvlan_netdev_addr_lock_key/1){+...}, at: dev_set_rx_mode+0x19/0x30
[ 30.285680]
[ 30.285680] other info that might help us debug this:
[ 30.286274] Possible unsafe locking scenario:
[ 30.286274]
[ 30.286903] CPU0
[ 30.287192] ----
[ 30.287475] lock(&macvlan_netdev_addr_lock_key/1);
[ 30.288121] lock(&macvlan_netdev_addr_lock_key/1);
[ 30.288818]
[ 30.288818] *** DEADLOCK ***
[ 30.288818]
[ 30.294651] May be due to missing lock nesting notation
[ 30.294651]
[ 30.295660] 4 locks held by ip/643:
[ 30.296076] #0: ffffffff93ec7a30 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x466/0x8a0
[ 30.297030] #1: ffff88806853ead8 (&macvlan_netdev_addr_lock_key/1){+...}, at: dev_set_rx_mode+0x19/0x30
[ 30.298749] #2: ffff888063b8a3f8 (&dev_addr_list_lock_key/3){+...}, at: dev_uc_sync+0xfa/0x1a0
[ 30.299727] #3: ffffffff93b22780 (rcu_read_lock){....}, at: bond_set_rx_mode+0x5/0x3c0 [bonding]
[ 30.302803]
[ 30.302803] stack backtrace:
[ 30.303254] CPU: 1 PID: 643 Comm: ip Not tainted 5.3.0+ #3
[ 30.303907] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 30.310458] Call Trace:
[ 30.310694] dump_stack+0x7c/0xbb
[ 30.311016] __lock_acquire+0x26a9/0x3df0
[ 30.311390] ? register_lock_class+0x14d0/0x14d0
[ 30.311815] lock_acquire+0x164/0x3b0
[ 30.312237] ? dev_uc_sync_multiple+0xfa/0x1a0
[ 30.312776] ? rcu_read_lock_held+0x90/0xa0
[ 30.313293] _raw_spin_lock_nested+0x2e/0x60
[ 30.313819] ? dev_uc_sync_multiple+0xfa/0x1a0
[ 30.314429] dev_uc_sync_multiple+0xfa/0x1a0
[ 30.314950] bond_set_rx_mode+0x269/0x3c0 [bonding]
[ 30.315541] ? bond_init+0x6f0/0x6f0 [bonding]
[ 30.316075] dev_uc_sync+0x15a/0x1a0
[ ... ]
Fixes: c674ac30c549 ("macvlan: Fix lockdep warnings with stacked macvlan devices")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v1 -> v4 :
- This patch is not changed
drivers/net/macvlan.c | 35 +++++++++++++++++++++++++++--------
include/linux/if_macvlan.h | 2 ++
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 940192c057b6..dae368a2e8d1 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -852,8 +852,6 @@ static int macvlan_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
* "super class" of normal network devices; split their locks off into a
* separate class since they always nest.
*/
-static struct lock_class_key macvlan_netdev_addr_lock_key;
-
#define ALWAYS_ON_OFFLOADS \
(NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE | \
NETIF_F_GSO_ROBUST | NETIF_F_GSO_ENCAP_ALL)
@@ -874,12 +872,30 @@ static int macvlan_get_nest_level(struct net_device *dev)
return ((struct macvlan_dev *)netdev_priv(dev))->nest_level;
}
-static void macvlan_set_lockdep_class(struct net_device *dev)
+static void macvlan_dev_set_lockdep_one(struct net_device *dev,
+ struct netdev_queue *txq,
+ void *_unused)
+{
+ struct macvlan_dev *macvlan = netdev_priv(dev);
+
+ lockdep_set_class(&txq->_xmit_lock, &macvlan->xmit_lock_key);
+}
+
+static struct lock_class_key qdisc_tx_busylock_key;
+static struct lock_class_key qdisc_running_key;
+
+static void macvlan_dev_set_lockdep_class(struct net_device *dev)
{
- netdev_lockdep_set_classes(dev);
- lockdep_set_class_and_subclass(&dev->addr_list_lock,
- &macvlan_netdev_addr_lock_key,
- macvlan_get_nest_level(dev));
+ struct macvlan_dev *macvlan = netdev_priv(dev);
+
+ dev->qdisc_tx_busylock = &qdisc_tx_busylock_key;
+ dev->qdisc_running_key = &qdisc_running_key;
+
+ lockdep_register_key(&macvlan->addr_lock_key);
+ lockdep_set_class(&dev->addr_list_lock, &macvlan->addr_lock_key);
+
+ lockdep_register_key(&macvlan->xmit_lock_key);
+ netdev_for_each_tx_queue(dev, macvlan_dev_set_lockdep_one, NULL);
}
static int macvlan_init(struct net_device *dev)
@@ -900,7 +916,7 @@ static int macvlan_init(struct net_device *dev)
dev->gso_max_segs = lowerdev->gso_max_segs;
dev->hard_header_len = lowerdev->hard_header_len;
- macvlan_set_lockdep_class(dev);
+ macvlan_dev_set_lockdep_class(dev);
vlan->pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
if (!vlan->pcpu_stats)
@@ -922,6 +938,9 @@ static void macvlan_uninit(struct net_device *dev)
port->count -= 1;
if (!port->count)
macvlan_port_destroy(port->dev);
+
+ lockdep_unregister_key(&vlan->addr_lock_key);
+ lockdep_unregister_key(&vlan->xmit_lock_key);
}
static void macvlan_dev_get_stats64(struct net_device *dev,
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index 2e55e4cdbd8a..ea5b41823287 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -31,6 +31,8 @@ struct macvlan_dev {
u16 flags;
int nest_level;
unsigned int macaddr_count;
+ struct lock_class_key xmit_lock_key;
+ struct lock_class_key addr_lock_key;
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *netpoll;
#endif
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 08/12] macsec: fix refcnt leak in module exit routine
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
When a macsec interface is created, it increases a refcnt to a lower
device(real device). when macsec interface is deleted, the refcnt is
decreased in macsec_free_netdev(), which is ->priv_destructor() of
macsec interface.
The problem scenario is this.
When nested macsec interfaces are exiting, the exit routine of the
macsec module makes refcnt leaks.
Test commands:
ip link add dummy0 type dummy
ip link add macsec0 link dummy0 type macsec
ip link add macsec1 link macsec0 type macsec
modprobe -rv macsec
[ 208.629433] unregister_netdevice: waiting for macsec0 to become free. Usage count = 1
Steps of exit routine of macsec module are below.
1. Calls ->dellink() in __rtnl_link_unregister().
2. Checks refcnt and wait refcnt to be 0 if refcnt is not 0 in
netdev_run_todo().
3. Calls ->priv_destruvtor() in netdev_run_todo().
Step2 checks refcnt, but step3 decreases refcnt.
So, step2 waits forever.
This patch makes the macsec module do not hold a refcnt of the lower
device because it already holds a refcnt of the lower device with
netdev_upper_dev_link().
Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v1 -> v4 :
- This patch is not changed
drivers/net/macsec.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index c4a41b90c846..28972da4a0b3 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3032,12 +3032,10 @@ static const struct nla_policy macsec_rtnl_policy[IFLA_MACSEC_MAX + 1] = {
static void macsec_free_netdev(struct net_device *dev)
{
struct macsec_dev *macsec = macsec_priv(dev);
- struct net_device *real_dev = macsec->real_dev;
free_percpu(macsec->stats);
free_percpu(macsec->secy.tx_sc.stats);
- dev_put(real_dev);
}
static void macsec_setup(struct net_device *dev)
@@ -3292,8 +3290,6 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
if (err < 0)
return err;
- dev_hold(real_dev);
-
macsec->nest_level = dev_get_nest_level(real_dev) + 1;
err = netdev_upper_dev_link(real_dev, dev, extack);
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 09/12] net: core: add ignore flag to netdev_adjacent structure
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
In order to link an adjacent node, netdev_upper_dev_link() is used
and in order to unlink an adjacent node, netdev_upper_dev_unlink() is used.
unlink operation does not fail, but link operation can fail.
In order to exchange adjacent nodes, we should unlink an old adjacent
node first. then, link a new adjacent node.
If link operation is failed, we should link an old adjacent node again.
But this link operation can fail too.
It eventually breaks the adjacent link relationship.
This patch adds an ignore flag into the netdev_adjacent structure.
If this flag is set, netdev_upper_dev_link() ignores an old adjacent
node for a moment.
This patch also adds new functions for other modules.
netdev_adjacent_change_prepare()
netdev_adjacent_change_commit()
netdev_adjacent_change_abort()
netdev_adjacent_change_prepare() inserts new device into adjacent list
but new device is not allowed to use immediately.
If netdev_adjacent_change_prepare() fails, it internally rollbacks
adjacent list so that we don't need any other action.
netdev_adjacent_change_commit() deletes old device in the adjacent list
and allows new device to use.
netdev_adjacent_change_abort() rollbacks adjacent list.
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v3 -> v4 :
- Add missing static keyword in the dev.c
- Expose netdev_adjacent_change_{prepare/commit/abort} instead of
netdev_adjacent_dev_{enable/disable}
v2 -> v3 :
- Modify nesting infra code to use iterator instead of recursive
v1 -> v2 :
- This patch is not changed
include/linux/netdevice.h | 10 ++
net/core/dev.c | 234 ++++++++++++++++++++++++++++++++++----
2 files changed, 222 insertions(+), 22 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 613007aa5986..d1f99d4f41bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4333,6 +4333,16 @@ int netdev_master_upper_dev_link(struct net_device *dev,
struct netlink_ext_ack *extack);
void netdev_upper_dev_unlink(struct net_device *dev,
struct net_device *upper_dev);
+int netdev_adjacent_change_prepare(struct net_device *old_dev,
+ struct net_device *new_dev,
+ struct net_device *dev,
+ struct netlink_ext_ack *extack);
+void netdev_adjacent_change_commit(struct net_device *old_dev,
+ struct net_device *new_dev,
+ struct net_device *dev);
+void netdev_adjacent_change_abort(struct net_device *old_dev,
+ struct net_device *new_dev,
+ struct net_device *dev);
void netdev_adjacent_rename_links(struct net_device *dev, char *oldname);
void *netdev_lower_dev_get_private(struct net_device *dev,
struct net_device *lower_dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 13cb646fb98f..0b60bcd5033e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6490,6 +6490,9 @@ struct netdev_adjacent {
/* upper master flag, there can only be one master device per list */
bool master;
+ /* lookup ignore flag */
+ bool ignore;
+
/* counter for the number of times this device was added to us */
u16 ref_nr;
@@ -6512,7 +6515,7 @@ static struct netdev_adjacent *__netdev_find_adj(struct net_device *adj_dev,
return NULL;
}
-static int __netdev_has_upper_dev(struct net_device *upper_dev, void *data)
+static int ____netdev_has_upper_dev(struct net_device *upper_dev, void *data)
{
struct net_device *dev = data;
@@ -6533,7 +6536,7 @@ bool netdev_has_upper_dev(struct net_device *dev,
{
ASSERT_RTNL();
- return netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev,
+ return netdev_walk_all_upper_dev_rcu(dev, ____netdev_has_upper_dev,
upper_dev);
}
EXPORT_SYMBOL(netdev_has_upper_dev);
@@ -6551,7 +6554,7 @@ EXPORT_SYMBOL(netdev_has_upper_dev);
bool netdev_has_upper_dev_all_rcu(struct net_device *dev,
struct net_device *upper_dev)
{
- return !!netdev_walk_all_upper_dev_rcu(dev, __netdev_has_upper_dev,
+ return !!netdev_walk_all_upper_dev_rcu(dev, ____netdev_has_upper_dev,
upper_dev);
}
EXPORT_SYMBOL(netdev_has_upper_dev_all_rcu);
@@ -6595,6 +6598,22 @@ struct net_device *netdev_master_upper_dev_get(struct net_device *dev)
}
EXPORT_SYMBOL(netdev_master_upper_dev_get);
+static struct net_device *__netdev_master_upper_dev_get(struct net_device *dev)
+{
+ struct netdev_adjacent *upper;
+
+ ASSERT_RTNL();
+
+ if (list_empty(&dev->adj_list.upper))
+ return NULL;
+
+ upper = list_first_entry(&dev->adj_list.upper,
+ struct netdev_adjacent, list);
+ if (likely(upper->master) && !upper->ignore)
+ return upper->dev;
+ return NULL;
+}
+
/**
* netdev_has_any_lower_dev - Check if device is linked to some device
* @dev: device
@@ -6645,8 +6664,9 @@ struct net_device *netdev_upper_get_next_dev_rcu(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_upper_get_next_dev_rcu);
-static struct net_device *netdev_next_upper_dev(struct net_device *dev,
- struct list_head **iter)
+static struct net_device *__netdev_next_upper_dev(struct net_device *dev,
+ struct list_head **iter,
+ bool *ignore)
{
struct netdev_adjacent *upper;
@@ -6656,6 +6676,7 @@ static struct net_device *netdev_next_upper_dev(struct net_device *dev,
return NULL;
*iter = &upper->list;
+ *ignore = upper->ignore;
return upper->dev;
}
@@ -6677,14 +6698,15 @@ static struct net_device *netdev_next_upper_dev_rcu(struct net_device *dev,
return upper->dev;
}
-int netdev_walk_all_upper_dev(struct net_device *dev,
- int (*fn)(struct net_device *dev,
- void *data),
- void *data)
+static int __netdev_walk_all_upper_dev(struct net_device *dev,
+ int (*fn)(struct net_device *dev,
+ void *data),
+ void *data)
{
struct net_device *udev, *next, *now, *dev_stack[MAX_NEST_DEV + 1];
struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1];
int ret, cur = 0;
+ bool ignore;
now = dev;
iter = &dev->adj_list.upper;
@@ -6698,9 +6720,11 @@ int netdev_walk_all_upper_dev(struct net_device *dev,
next = NULL;
while (1) {
- udev = netdev_next_upper_dev(now, &iter);
+ udev = __netdev_next_upper_dev(now, &iter, &ignore);
if (!udev)
break;
+ if (ignore)
+ continue;
if (!next) {
next = udev;
@@ -6777,6 +6801,15 @@ int netdev_walk_all_upper_dev_rcu(struct net_device *dev,
}
EXPORT_SYMBOL_GPL(netdev_walk_all_upper_dev_rcu);
+static bool __netdev_has_upper_dev(struct net_device *dev,
+ struct net_device *upper_dev)
+{
+ ASSERT_RTNL();
+
+ return __netdev_walk_all_upper_dev(dev, ____netdev_has_upper_dev,
+ upper_dev);
+}
+
/**
* netdev_lower_get_next_private - Get the next ->private from the
* lower neighbour list
@@ -6873,6 +6906,23 @@ static struct net_device *netdev_next_lower_dev(struct net_device *dev,
return lower->dev;
}
+static struct net_device *__netdev_next_lower_dev(struct net_device *dev,
+ struct list_head **iter,
+ bool *ignore)
+{
+ struct netdev_adjacent *lower;
+
+ lower = list_entry((*iter)->next, struct netdev_adjacent, list);
+
+ if (&lower->list == &dev->adj_list.lower)
+ return NULL;
+
+ *iter = &lower->list;
+ *ignore = lower->ignore;
+
+ return lower->dev;
+}
+
int netdev_walk_all_lower_dev(struct net_device *dev,
int (*fn)(struct net_device *dev,
void *data),
@@ -6923,6 +6973,58 @@ int netdev_walk_all_lower_dev(struct net_device *dev,
}
EXPORT_SYMBOL_GPL(netdev_walk_all_lower_dev);
+static int __netdev_walk_all_lower_dev(struct net_device *dev,
+ int (*fn)(struct net_device *dev,
+ void *data),
+ void *data)
+{
+ struct net_device *ldev, *next, *now, *dev_stack[MAX_NEST_DEV + 1];
+ struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1];
+ int ret, cur = 0;
+ bool ignore;
+
+ now = dev;
+ iter = &dev->adj_list.lower;
+
+ while (1) {
+ if (now != dev) {
+ ret = fn(now, data);
+ if (ret)
+ return ret;
+ }
+
+ next = NULL;
+ while (1) {
+ ldev = __netdev_next_lower_dev(now, &iter, &ignore);
+ if (!ldev)
+ break;
+ if (ignore)
+ continue;
+
+ if (!next) {
+ next = ldev;
+ niter = &ldev->adj_list.lower;
+ } else {
+ dev_stack[cur] = ldev;
+ iter_stack[cur++] = &ldev->adj_list.lower;
+ break;
+ }
+ }
+
+ if (!next) {
+ if (!cur)
+ return 0;
+ next = dev_stack[--cur];
+ niter = iter_stack[cur];
+ }
+
+ now = next;
+ iter = niter;
+ }
+
+ return 0;
+}
+
static struct net_device *netdev_next_lower_dev_rcu(struct net_device *dev,
struct list_head **iter)
{
@@ -6942,11 +7044,14 @@ static u8 __netdev_upper_depth(struct net_device *dev)
struct net_device *udev;
struct list_head *iter;
u8 max_depth = 0;
+ bool ignore;
for (iter = &dev->adj_list.upper,
- udev = netdev_next_upper_dev(dev, &iter);
+ udev = __netdev_next_upper_dev(dev, &iter, &ignore);
udev;
- udev = netdev_next_upper_dev(dev, &iter)) {
+ udev = __netdev_next_upper_dev(dev, &iter, &ignore)) {
+ if (ignore)
+ continue;
if (max_depth < udev->upper_level)
max_depth = udev->upper_level;
}
@@ -6959,11 +7064,14 @@ static u8 __netdev_lower_depth(struct net_device *dev)
struct net_device *ldev;
struct list_head *iter;
u8 max_depth = 0;
+ bool ignore;
for (iter = &dev->adj_list.lower,
- ldev = netdev_next_lower_dev(dev, &iter);
+ ldev = __netdev_next_lower_dev(dev, &iter, &ignore);
ldev;
- ldev = netdev_next_lower_dev(dev, &iter)) {
+ ldev = __netdev_next_lower_dev(dev, &iter, &ignore)) {
+ if (ignore)
+ continue;
if (max_depth < ldev->lower_level)
max_depth = ldev->lower_level;
}
@@ -7131,6 +7239,7 @@ static int __netdev_adjacent_dev_insert(struct net_device *dev,
adj->master = master;
adj->ref_nr = 1;
adj->private = private;
+ adj->ignore = false;
dev_hold(adj_dev);
pr_debug("Insert adjacency: dev %s adj_dev %s adj->ref_nr %d; dev_hold on %s\n",
@@ -7281,17 +7390,17 @@ 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_has_upper_dev(upper_dev, dev))
+ if (__netdev_has_upper_dev(upper_dev, dev))
return -EBUSY;
if ((dev->lower_level + upper_dev->upper_level) > MAX_NEST_DEV)
return -EMLINK;
if (!master) {
- if (netdev_has_upper_dev(dev, upper_dev))
+ if (__netdev_has_upper_dev(dev, upper_dev))
return -EEXIST;
} else {
- master_dev = netdev_master_upper_dev_get(dev);
+ master_dev = __netdev_master_upper_dev_get(dev);
if (master_dev)
return master_dev == upper_dev ? -EEXIST : -EBUSY;
}
@@ -7314,11 +7423,11 @@ static int __netdev_upper_dev_link(struct net_device *dev,
goto rollback;
__netdev_update_upper_level(dev, NULL);
- netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
+ __netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
__netdev_update_lower_level(upper_dev, NULL);
- netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level, NULL);
-
+ __netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level,
+ NULL);
return 0;
rollback:
@@ -7403,13 +7512,94 @@ void netdev_upper_dev_unlink(struct net_device *dev,
&changeupper_info.info);
__netdev_update_upper_level(dev, NULL);
- netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
+ __netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
__netdev_update_lower_level(upper_dev, NULL);
- netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level, NULL);
+ __netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level,
+ NULL);
}
EXPORT_SYMBOL(netdev_upper_dev_unlink);
+static void __netdev_adjacent_dev_set(struct net_device *upper_dev,
+ struct net_device *lower_dev,
+ bool val)
+{
+ struct netdev_adjacent *adj;
+
+ adj = __netdev_find_adj(lower_dev, &upper_dev->adj_list.lower);
+ if (adj)
+ adj->ignore = val;
+
+ adj = __netdev_find_adj(upper_dev, &lower_dev->adj_list.upper);
+ if (adj)
+ adj->ignore = val;
+}
+
+static void netdev_adjacent_dev_disable(struct net_device *upper_dev,
+ struct net_device *lower_dev)
+{
+ __netdev_adjacent_dev_set(upper_dev, lower_dev, true);
+}
+
+static void netdev_adjacent_dev_enable(struct net_device *upper_dev,
+ struct net_device *lower_dev)
+{
+ __netdev_adjacent_dev_set(upper_dev, lower_dev, false);
+}
+
+int netdev_adjacent_change_prepare(struct net_device *old_dev,
+ struct net_device *new_dev,
+ struct net_device *dev,
+ struct netlink_ext_ack *extack)
+{
+ int err;
+
+ if (!new_dev)
+ return 0;
+
+ if (old_dev && new_dev != old_dev)
+ netdev_adjacent_dev_disable(dev, old_dev);
+
+ err = netdev_upper_dev_link(new_dev, dev, extack);
+ if (err) {
+ if (old_dev && new_dev != old_dev)
+ netdev_adjacent_dev_enable(dev, old_dev);
+ return err;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(netdev_adjacent_change_prepare);
+
+void netdev_adjacent_change_commit(struct net_device *old_dev,
+ struct net_device *new_dev,
+ struct net_device *dev)
+{
+ if (!new_dev || !old_dev)
+ return;
+
+ if (new_dev == old_dev)
+ return;
+
+ netdev_adjacent_dev_enable(dev, old_dev);
+ netdev_upper_dev_unlink(old_dev, dev);
+}
+EXPORT_SYMBOL(netdev_adjacent_change_commit);
+
+void netdev_adjacent_change_abort(struct net_device *old_dev,
+ struct net_device *new_dev,
+ struct net_device *dev)
+{
+ if (!new_dev)
+ return;
+
+ if (old_dev && new_dev != old_dev)
+ netdev_adjacent_dev_enable(dev, old_dev);
+
+ netdev_upper_dev_unlink(new_dev, dev);
+}
+EXPORT_SYMBOL(netdev_adjacent_change_abort);
+
/**
* netdev_bonding_info_change - Dispatch event about slave change
* @dev: device
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 10/12] vxlan: add adjacent link to limit depth level
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
Current vxlan code doesn't limit the number of nested devices.
Nested devices would be handled recursively and this routine needs
huge stack memory. So, unlimited nested devices could make
stack overflow.
In order to fix this issue, this patch adds adjacent links.
The adjacent link APIs internally check the depth level.
Test commands:
ip link add dummy0 type dummy
ip link add vxlan0 type vxlan id 0 group 239.1.1.1 dev dummy0 \
dstport 4789
for i in {1..100}
do
let A=$i-1
ip link add vxlan$i type vxlan id $i group 239.1.1.1 \
dev vxlan$A dstport 4789
done
ip link del dummy0
The top upper link is vxlan100 and the lowest link is vxlan0.
When vxlan0 is deleting, the upper devices will be deleted recursively.
It needs huge stack memory so it makes stack overflow.
Splat looks like:
[ 229.628477] =============================================================================
[ 229.629785] BUG page->ptl (Not tainted): Padding overwritten. 0x0000000026abf214-0x0000000091f6abb2
[ 229.629785] -----------------------------------------------------------------------------
[ 229.629785]
[ 229.655439] ==================================================================
[ 229.629785] INFO: Slab 0x00000000ff7cfda8 objects=19 used=19 fp=0x00000000fe33776c flags=0x200000000010200
[ 229.655688] BUG: KASAN: stack-out-of-bounds in unmap_single_vma+0x25a/0x2e0
[ 229.655688] Read of size 8 at addr ffff888113076928 by task vlan-network-in/2334
[ 229.655688]
[ 229.629785] Padding 0000000026abf214: 00 80 14 0d 81 88 ff ff 68 91 81 14 81 88 ff ff ........h.......
[ 229.629785] Padding 0000000001e24790: 38 91 81 14 81 88 ff ff 68 91 81 14 81 88 ff ff 8.......h.......
[ 229.629785] Padding 00000000b39397c8: 33 30 62 a7 ff ff ff ff ff eb 60 22 10 f1 ff 1f 30b.......`"....
[ 229.629785] Padding 00000000bc98f53a: 80 60 07 13 81 88 ff ff 00 80 14 0d 81 88 ff ff .`..............
[ 229.629785] Padding 000000002aa8123d: 68 91 81 14 81 88 ff ff f7 21 17 a7 ff ff ff ff h........!......
[ 229.629785] Padding 000000001c8c2369: 08 81 14 0d 81 88 ff ff 03 02 00 00 00 00 00 00 ................
[ 229.629785] Padding 000000004e290c5d: 21 90 a2 21 10 ed ff ff 00 00 00 00 00 fc ff df !..!............
[ 229.629785] Padding 000000000e25d731: 18 60 07 13 81 88 ff ff c0 8b 13 05 81 88 ff ff .`..............
[ 229.629785] Padding 000000007adc7ab3: b3 8a b5 41 00 00 00 00 ...A....
[ 229.629785] FIX page->ptl: Restoring 0x0000000026abf214-0x0000000091f6abb2=0x5a
[ ... ]
Fixes: acaf4e70997f ("net: vxlan: when lower dev unregisters remove vxlan dev as well")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v3 -> v4 :
- Fix wrong usage netdev_upper_dev_link() in the vxlan.c
- Preserve reverse christmas tree variable ordering in the vxlan.c
v1 -> v3 :
- This patch is not changed
drivers/net/vxlan.c | 52 ++++++++++++++++++++++++++++++++++++---------
include/net/vxlan.h | 1 +
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 3d9bcc957f7d..5537998d6137 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3566,10 +3566,13 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
{
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
struct vxlan_dev *vxlan = netdev_priv(dev);
+ struct net_device *remote_dev = NULL;
struct vxlan_fdb *f = NULL;
bool unregister = false;
+ struct vxlan_rdst *dst;
int err;
+ dst = &vxlan->default_dst;
err = vxlan_dev_configure(net, dev, conf, false, extack);
if (err)
return err;
@@ -3577,14 +3580,14 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
dev->ethtool_ops = &vxlan_ethtool_ops;
/* create an fdb entry for a valid default destination */
- if (!vxlan_addr_any(&vxlan->default_dst.remote_ip)) {
+ if (!vxlan_addr_any(&dst->remote_ip)) {
err = vxlan_fdb_create(vxlan, all_zeros_mac,
- &vxlan->default_dst.remote_ip,
+ &dst->remote_ip,
NUD_REACHABLE | NUD_PERMANENT,
vxlan->cfg.dst_port,
- vxlan->default_dst.remote_vni,
- vxlan->default_dst.remote_vni,
- vxlan->default_dst.remote_ifindex,
+ dst->remote_vni,
+ dst->remote_vni,
+ dst->remote_ifindex,
NTF_SELF, &f);
if (err)
return err;
@@ -3595,26 +3598,41 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
goto errout;
unregister = true;
+ if (dst->remote_ifindex) {
+ remote_dev = __dev_get_by_index(net, dst->remote_ifindex);
+ if (!remote_dev)
+ goto errout;
+
+ err = netdev_upper_dev_link(remote_dev, dev, extack);
+ if (err)
+ goto errout;
+ }
+
err = rtnl_configure_link(dev, NULL);
if (err)
- goto errout;
+ goto unlink;
if (f) {
- vxlan_fdb_insert(vxlan, all_zeros_mac,
- vxlan->default_dst.remote_vni, f);
+ vxlan_fdb_insert(vxlan, all_zeros_mac, dst->remote_vni, f);
/* notify default fdb entry */
err = vxlan_fdb_notify(vxlan, f, first_remote_rtnl(f),
RTM_NEWNEIGH, true, extack);
if (err) {
vxlan_fdb_destroy(vxlan, f, false, false);
+ if (remote_dev)
+ netdev_upper_dev_unlink(remote_dev, dev);
goto unregister;
}
}
list_add(&vxlan->next, &vn->vxlan_list);
+ if (remote_dev)
+ dst->remote_dev = remote_dev;
return 0;
-
+unlink:
+ if (remote_dev)
+ netdev_upper_dev_unlink(remote_dev, dev);
errout:
/* unregister_netdevice() destroys the default FDB entry with deletion
* notification. But the addition notification was not sent yet, so
@@ -3932,11 +3950,12 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
struct netlink_ext_ack *extack)
{
struct vxlan_dev *vxlan = netdev_priv(dev);
- struct vxlan_rdst *dst = &vxlan->default_dst;
struct net_device *lowerdev;
struct vxlan_config conf;
+ struct vxlan_rdst *dst;
int err;
+ dst = &vxlan->default_dst;
err = vxlan_nl2conf(tb, data, dev, &conf, true, extack);
if (err)
return err;
@@ -3946,6 +3965,11 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
if (err)
return err;
+ err = netdev_adjacent_change_prepare(dst->remote_dev, lowerdev, dev,
+ extack);
+ if (err)
+ return err;
+
/* handle default dst entry */
if (!vxlan_addr_equal(&conf.remote_ip, &dst->remote_ip)) {
u32 hash_index = fdb_head_index(vxlan, all_zeros_mac, conf.vni);
@@ -3962,6 +3986,8 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
NTF_SELF, true, extack);
if (err) {
spin_unlock_bh(&vxlan->hash_lock[hash_index]);
+ netdev_adjacent_change_abort(dst->remote_dev,
+ lowerdev, dev);
return err;
}
}
@@ -3979,6 +4005,10 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
if (conf.age_interval != vxlan->cfg.age_interval)
mod_timer(&vxlan->age_timer, jiffies);
+ netdev_adjacent_change_commit(dst->remote_dev, lowerdev, dev);
+ if (lowerdev && lowerdev != dst->remote_dev)
+ dst->remote_dev = lowerdev;
+
vxlan_config_apply(dev, &conf, lowerdev, vxlan->net, true);
return 0;
}
@@ -3991,6 +4021,8 @@ static void vxlan_dellink(struct net_device *dev, struct list_head *head)
list_del(&vxlan->next);
unregister_netdevice_queue(dev, head);
+ if (vxlan->default_dst.remote_dev)
+ netdev_upper_dev_unlink(vxlan->default_dst.remote_dev, dev);
}
static size_t vxlan_get_size(const struct net_device *dev)
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 335283dbe9b3..373aadcfea21 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -197,6 +197,7 @@ struct vxlan_rdst {
u8 offloaded:1;
__be32 remote_vni;
u32 remote_ifindex;
+ struct net_device *remote_dev;
struct list_head list;
struct rcu_head rcu;
struct dst_cache dst_cache;
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 11/12] net: remove unnecessary variables and callback
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
This patch removes variables and callback these are related to the nested
device structure.
devices that can be nested have their own nest_level variable that
represents the depth of nested devices.
In the previous patch, new {lower/upper}_level variables are added and
they replace old private nest_level variable.
So, this patch removes all 'nest_level' variables.
In order to avoid lockdep warning, ->ndo_get_lock_subclass() was added
to get lockdep subclass value, which is actually lower nested depth value.
But now, they use the dynamic lockdep key to avoid lockdep warning instead
of the subclass.
So, this patch removes ->ndo_get_lock_subclass() callback.
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v1 -> v4 :
- This patch is not changed
drivers/net/bonding/bond_alb.c | 2 +-
drivers/net/bonding/bond_main.c | 14 -------------
.../net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
drivers/net/macsec.c | 9 ---------
drivers/net/macvlan.c | 7 -------
include/linux/if_macvlan.h | 1 -
include/linux/if_vlan.h | 12 -----------
include/linux/netdevice.h | 12 -----------
include/net/bonding.h | 1 -
net/8021q/vlan.c | 1 -
net/8021q/vlan_dev.c | 6 ------
net/core/dev.c | 20 -------------------
net/core/dev_addr_lists.c | 12 +++++------
net/smc/smc_core.c | 2 +-
net/smc/smc_pnet.c | 2 +-
15 files changed, 10 insertions(+), 93 deletions(-)
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 8c79bad2a9a5..4f2e6910c623 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -952,7 +952,7 @@ static int alb_upper_dev_walk(struct net_device *upper, void *_data)
struct bond_vlan_tag *tags;
if (is_vlan_dev(upper) &&
- bond->nest_level == vlan_get_encap_level(upper) - 1) {
+ bond->dev->lower_level == upper->lower_level - 1) {
if (upper->addr_assign_type == NET_ADDR_STOLEN) {
alb_send_lp_vid(slave, mac_addr,
vlan_dev_vlan_proto(upper),
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7f574e74ed78..69eb61466fbe 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1733,8 +1733,6 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
goto err_upper_unlink;
}
- bond->nest_level = dev_get_nest_level(bond_dev) + 1;
-
/* If the mode uses primary, then the following is handled by
* bond_change_active_slave().
*/
@@ -1983,9 +1981,6 @@ static int __bond_release_one(struct net_device *bond_dev,
if (!bond_has_slaves(bond)) {
bond_set_carrier(bond);
eth_hw_addr_random(bond_dev);
- bond->nest_level = SINGLE_DEPTH_NESTING;
- } else {
- bond->nest_level = dev_get_nest_level(bond_dev) + 1;
}
unblock_netpoll_tx();
@@ -3472,13 +3467,6 @@ static void bond_fold_stats(struct rtnl_link_stats64 *_res,
}
}
-static int bond_get_nest_level(struct net_device *bond_dev)
-{
- struct bonding *bond = netdev_priv(bond_dev);
-
- return bond->nest_level;
-}
-
static void bond_get_stats(struct net_device *bond_dev,
struct rtnl_link_stats64 *stats)
{
@@ -4298,7 +4286,6 @@ static const struct net_device_ops bond_netdev_ops = {
.ndo_neigh_setup = bond_neigh_setup,
.ndo_vlan_rx_add_vid = bond_vlan_rx_add_vid,
.ndo_vlan_rx_kill_vid = bond_vlan_rx_kill_vid,
- .ndo_get_lock_subclass = bond_get_nest_level,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_netpoll_setup = bond_netpoll_setup,
.ndo_netpoll_cleanup = bond_netpoll_cleanup,
@@ -4822,7 +4809,6 @@ static int bond_init(struct net_device *bond_dev)
if (!bond->wq)
return -ENOMEM;
- bond->nest_level = SINGLE_DEPTH_NESTING;
bond_dev_set_lockdep_class(bond_dev);
list_add_tail(&bond->bond_list, &bn->dev_list);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 3e78a727f3e6..c4c59d2e676e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -3160,7 +3160,7 @@ static int add_vlan_pop_action(struct mlx5e_priv *priv,
struct mlx5_esw_flow_attr *attr,
u32 *action)
{
- int nest_level = vlan_get_encap_level(attr->parse_attr->filter_dev);
+ int nest_level = attr->parse_attr->filter_dev->lower_level;
struct flow_action_entry vlan_act = {
.id = FLOW_ACTION_VLAN_POP,
};
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 28972da4a0b3..647aeead644d 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -269,7 +269,6 @@ struct macsec_dev {
struct gro_cells gro_cells;
struct lock_class_key xmit_lock_key;
struct lock_class_key addr_lock_key;
- unsigned int nest_level;
};
/**
@@ -2989,11 +2988,6 @@ static int macsec_get_iflink(const struct net_device *dev)
return macsec_priv(dev)->real_dev->ifindex;
}
-static int macsec_get_nest_level(struct net_device *dev)
-{
- return macsec_priv(dev)->nest_level;
-}
-
static const struct net_device_ops macsec_netdev_ops = {
.ndo_init = macsec_dev_init,
.ndo_uninit = macsec_dev_uninit,
@@ -3007,7 +3001,6 @@ static const struct net_device_ops macsec_netdev_ops = {
.ndo_start_xmit = macsec_start_xmit,
.ndo_get_stats64 = macsec_get_stats64,
.ndo_get_iflink = macsec_get_iflink,
- .ndo_get_lock_subclass = macsec_get_nest_level,
};
static const struct device_type macsec_type = {
@@ -3290,8 +3283,6 @@ static int macsec_newlink(struct net *net, struct net_device *dev,
if (err < 0)
return err;
- macsec->nest_level = dev_get_nest_level(real_dev) + 1;
-
err = netdev_upper_dev_link(real_dev, dev, extack);
if (err < 0)
goto unregister;
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index dae368a2e8d1..2c14bc606514 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -867,11 +867,6 @@ static int macvlan_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
#define MACVLAN_STATE_MASK \
((1<<__LINK_STATE_NOCARRIER) | (1<<__LINK_STATE_DORMANT))
-static int macvlan_get_nest_level(struct net_device *dev)
-{
- return ((struct macvlan_dev *)netdev_priv(dev))->nest_level;
-}
-
static void macvlan_dev_set_lockdep_one(struct net_device *dev,
struct netdev_queue *txq,
void *_unused)
@@ -1180,7 +1175,6 @@ static const struct net_device_ops macvlan_netdev_ops = {
.ndo_fdb_add = macvlan_fdb_add,
.ndo_fdb_del = macvlan_fdb_del,
.ndo_fdb_dump = ndo_dflt_fdb_dump,
- .ndo_get_lock_subclass = macvlan_get_nest_level,
#ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller = macvlan_dev_poll_controller,
.ndo_netpoll_setup = macvlan_dev_netpoll_setup,
@@ -1464,7 +1458,6 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
vlan->dev = dev;
vlan->port = port;
vlan->set_features = MACVLAN_FEATURES;
- vlan->nest_level = dev_get_nest_level(lowerdev) + 1;
vlan->mode = MACVLAN_MODE_VEPA;
if (data && data[IFLA_MACVLAN_MODE])
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index ea5b41823287..e9202edcf101 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -29,7 +29,6 @@ struct macvlan_dev {
netdev_features_t set_features;
enum macvlan_mode mode;
u16 flags;
- int nest_level;
unsigned int macaddr_count;
struct lock_class_key xmit_lock_key;
struct lock_class_key addr_lock_key;
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 1aed9f613e90..6f30284a58e5 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -182,8 +182,6 @@ struct vlan_dev_priv {
#ifdef CONFIG_NET_POLL_CONTROLLER
struct netpoll *netpoll;
#endif
- unsigned int nest_level;
-
struct lock_class_key xmit_lock_key;
struct lock_class_key addr_lock_key;
};
@@ -224,11 +222,6 @@ extern void vlan_vids_del_by_dev(struct net_device *dev,
extern bool vlan_uses_dev(const struct net_device *dev);
-static inline int vlan_get_encap_level(struct net_device *dev)
-{
- BUG_ON(!is_vlan_dev(dev));
- return vlan_dev_priv(dev)->nest_level;
-}
#else
static inline struct net_device *
__vlan_find_dev_deep_rcu(struct net_device *real_dev,
@@ -298,11 +291,6 @@ static inline bool vlan_uses_dev(const struct net_device *dev)
{
return false;
}
-static inline int vlan_get_encap_level(struct net_device *dev)
-{
- BUG();
- return 0;
-}
#endif
/**
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d1f99d4f41bb..4133db060593 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1421,7 +1421,6 @@ struct net_device_ops {
void (*ndo_dfwd_del_station)(struct net_device *pdev,
void *priv);
- int (*ndo_get_lock_subclass)(struct net_device *dev);
int (*ndo_set_tx_maxrate)(struct net_device *dev,
int queue_index,
u32 maxrate);
@@ -4060,16 +4059,6 @@ static inline void netif_addr_lock(struct net_device *dev)
spin_lock(&dev->addr_list_lock);
}
-static inline void netif_addr_lock_nested(struct net_device *dev)
-{
- int subclass = SINGLE_DEPTH_NESTING;
-
- if (dev->netdev_ops->ndo_get_lock_subclass)
- subclass = dev->netdev_ops->ndo_get_lock_subclass(dev);
-
- spin_lock_nested(&dev->addr_list_lock, subclass);
-}
-
static inline void netif_addr_lock_bh(struct net_device *dev)
{
spin_lock_bh(&dev->addr_list_lock);
@@ -4354,7 +4343,6 @@ void netdev_lower_state_changed(struct net_device *lower_dev,
extern u8 netdev_rss_key[NETDEV_RSS_KEY_LEN] __read_mostly;
void netdev_rss_key_fill(void *buffer, size_t len);
-int dev_get_nest_level(struct net_device *dev);
int skb_checksum_help(struct sk_buff *skb);
int skb_crc32c_csum_help(struct sk_buff *skb);
int skb_csum_hwoffload_help(struct sk_buff *skb,
diff --git a/include/net/bonding.h b/include/net/bonding.h
index c39ac7061e41..74f41dd73866 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -203,7 +203,6 @@ struct bonding {
struct slave __rcu *primary_slave;
struct bond_up_slave __rcu *slave_arr; /* Array of usable slaves */
bool force_primary;
- u32 nest_level;
s32 slave_cnt; /* never change this value outside the attach/detach wrappers */
int (*recv_probe)(const struct sk_buff *, struct bonding *,
struct slave *);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 54728d2eda18..d4bcfd8f95bf 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -172,7 +172,6 @@ int register_vlan_dev(struct net_device *dev, struct netlink_ext_ack *extack)
if (err < 0)
goto out_uninit_mvrp;
- vlan->nest_level = dev_get_nest_level(real_dev) + 1;
err = register_netdevice(dev);
if (err < 0)
goto out_uninit_mvrp;
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 12bc80650087..e8707827540c 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -514,11 +514,6 @@ static void vlan_dev_set_lockdep_class(struct net_device *dev)
netdev_for_each_tx_queue(dev, vlan_dev_set_lockdep_one, NULL);
}
-static int vlan_dev_get_lock_subclass(struct net_device *dev)
-{
- return vlan_dev_priv(dev)->nest_level;
-}
-
static const struct header_ops vlan_header_ops = {
.create = vlan_dev_hard_header,
.parse = eth_header_parse,
@@ -814,7 +809,6 @@ static const struct net_device_ops vlan_netdev_ops = {
.ndo_netpoll_cleanup = vlan_dev_netpoll_cleanup,
#endif
.ndo_fix_features = vlan_dev_fix_features,
- .ndo_get_lock_subclass = vlan_dev_get_lock_subclass,
.ndo_get_iflink = vlan_dev_get_iflink,
};
diff --git a/net/core/dev.c b/net/core/dev.c
index 0b60bcd5033e..3fbd42eb75d1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7712,26 +7712,6 @@ void *netdev_lower_dev_get_private(struct net_device *dev,
}
EXPORT_SYMBOL(netdev_lower_dev_get_private);
-
-int dev_get_nest_level(struct net_device *dev)
-{
- struct net_device *lower = NULL;
- struct list_head *iter;
- int max_nest = -1;
- int nest;
-
- ASSERT_RTNL();
-
- netdev_for_each_lower_dev(dev, lower, iter) {
- nest = dev_get_nest_level(lower);
- if (max_nest < nest)
- max_nest = nest;
- }
-
- return max_nest + 1;
-}
-EXPORT_SYMBOL(dev_get_nest_level);
-
/**
* netdev_lower_change - Dispatch event about lower device state change
* @lower_dev: device
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 6393ba930097..2f949b5a1eb9 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -637,7 +637,7 @@ int dev_uc_sync(struct net_device *to, struct net_device *from)
if (to->addr_len != from->addr_len)
return -EINVAL;
- netif_addr_lock_nested(to);
+ netif_addr_lock(to);
err = __hw_addr_sync(&to->uc, &from->uc, to->addr_len);
if (!err)
__dev_set_rx_mode(to);
@@ -667,7 +667,7 @@ int dev_uc_sync_multiple(struct net_device *to, struct net_device *from)
if (to->addr_len != from->addr_len)
return -EINVAL;
- netif_addr_lock_nested(to);
+ netif_addr_lock(to);
err = __hw_addr_sync_multiple(&to->uc, &from->uc, to->addr_len);
if (!err)
__dev_set_rx_mode(to);
@@ -691,7 +691,7 @@ void dev_uc_unsync(struct net_device *to, struct net_device *from)
return;
netif_addr_lock_bh(from);
- netif_addr_lock_nested(to);
+ netif_addr_lock(to);
__hw_addr_unsync(&to->uc, &from->uc, to->addr_len);
__dev_set_rx_mode(to);
netif_addr_unlock(to);
@@ -858,7 +858,7 @@ int dev_mc_sync(struct net_device *to, struct net_device *from)
if (to->addr_len != from->addr_len)
return -EINVAL;
- netif_addr_lock_nested(to);
+ netif_addr_lock(to);
err = __hw_addr_sync(&to->mc, &from->mc, to->addr_len);
if (!err)
__dev_set_rx_mode(to);
@@ -888,7 +888,7 @@ int dev_mc_sync_multiple(struct net_device *to, struct net_device *from)
if (to->addr_len != from->addr_len)
return -EINVAL;
- netif_addr_lock_nested(to);
+ netif_addr_lock(to);
err = __hw_addr_sync_multiple(&to->mc, &from->mc, to->addr_len);
if (!err)
__dev_set_rx_mode(to);
@@ -912,7 +912,7 @@ void dev_mc_unsync(struct net_device *to, struct net_device *from)
return;
netif_addr_lock_bh(from);
- netif_addr_lock_nested(to);
+ netif_addr_lock(to);
__hw_addr_unsync(&to->mc, &from->mc, to->addr_len);
__dev_set_rx_mode(to);
netif_addr_unlock(to);
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 4ca50ddf8d16..a2e91b8d04b3 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -558,7 +558,7 @@ int smc_vlan_by_tcpsk(struct socket *clcsock, struct smc_init_info *ini)
}
rtnl_lock();
- nest_lvl = dev_get_nest_level(ndev);
+ nest_lvl = ndev->lower_level;
for (i = 0; i < nest_lvl; i++) {
struct list_head *lower = &ndev->adj_list.lower;
diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index bab2da8cf17a..2920b006f65c 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -718,7 +718,7 @@ static struct net_device *pnet_find_base_ndev(struct net_device *ndev)
int i, nest_lvl;
rtnl_lock();
- nest_lvl = dev_get_nest_level(ndev);
+ nest_lvl = ndev->lower_level;
for (i = 0; i < nest_lvl; i++) {
struct list_head *lower = &ndev->adj_list.lower;
--
2.17.1
^ permalink raw reply related
* [PATCH net v4 12/12] virt_wifi: fix refcnt leak in module exit routine
From: Taehee Yoo @ 2019-09-28 16:48 UTC (permalink / raw)
To: davem, netdev, linux-wireless, jakub.kicinski, johannes,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
Cc: ap420073
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
virt_wifi_newlink() calls netdev_upper_dev_link() and it internally
holds reference count of lower interface.
Current code does not release a reference count of the lower interface
when the lower interface is being deleted.
So, reference count leaks occur.
Test commands:
ip link add dummy0 type dummy
ip link add vw1 link dummy0 type virt_wifi
Splat looks like:
[ 182.001918][ T1333] WARNING: CPU: 0 PID: 1333 at net/core/dev.c:8638 rollback_registered_many+0x75d/0xda0
[ 182.002724][ T1333] Modules linked in: virt_wifi cfg80211 dummy veth openvswitch nsh nf_conncount nf_nat nf_conntrack6
[ 182.002724][ T1333] CPU: 0 PID: 1333 Comm: ip Not tainted 5.3.0+ #370
[ 182.002724][ T1333] RIP: 0010:rollback_registered_many+0x75d/0xda0
[ 182.002724][ T1333] Code: 0c 00 00 48 89 de 4c 89 ff e8 6f 5a 04 00 48 89 df e8 c7 26 fd ff 84 c0 0f 84 a5 fd ff ff 40
[ 182.002724][ T1333] RSP: 0018:ffff88810900f348 EFLAGS: 00010286
[ 182.002724][ T1333] RAX: 0000000000000024 RBX: ffff88811361d700 RCX: 0000000000000000
[ 182.002724][ T1333] RDX: 0000000000000024 RSI: 0000000000000008 RDI: ffffed1021201e5f
[ 182.002724][ T1333] RBP: ffff88810900f4e0 R08: ffffed1022c3ff71 R09: ffffed1022c3ff71
[ 182.002724][ T1333] R10: 0000000000000001 R11: ffffed1022c3ff70 R12: dffffc0000000000
[ 182.002724][ T1333] R13: ffff88810900f460 R14: ffff88810900f420 R15: ffff8881075f1940
[ 182.002724][ T1333] FS: 00007f77c42240c0(0000) GS:ffff888116000000(0000) knlGS:0000000000000000
[ 182.002724][ T1333] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 182.002724][ T1333] CR2: 00007f77c3706240 CR3: 000000011139e000 CR4: 00000000001006f0
[ 182.002724][ T1333] Call Trace:
[ 182.002724][ T1333] ? generic_xdp_install+0x310/0x310
[ 182.002724][ T1333] ? check_chain_key+0x236/0x5d0
[ 182.002724][ T1333] ? __nla_validate_parse+0x98/0x1ad0
[ 182.002724][ T1333] unregister_netdevice_many.part.123+0x13/0x1b0
[ 182.002724][ T1333] rtnl_delete_link+0xbc/0x100
[ 182.002724][ T1333] ? rtnl_af_register+0xc0/0xc0
[ 182.002724][ T1333] rtnl_dellink+0x2e7/0x870
[ ... ]
[ 192.874736][ T1333] unregister_netdevice: waiting for dummy0 to become free. Usage count = 1
This patch adds notifier routine to delete upper interface before deleting
lower interface.
Fixes: c7cdba31ed8b ("mac80211-next: rtnetlink wifi simulation device")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
v4:
- Add this new patch to fix refcnt leaks in the virt_wifi module
drivers/net/wireless/virt_wifi.c | 51 ++++++++++++++++++++++++++++++--
1 file changed, 49 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/virt_wifi.c b/drivers/net/wireless/virt_wifi.c
index be92e1220284..aadbacb01c8d 100644
--- a/drivers/net/wireless/virt_wifi.c
+++ b/drivers/net/wireless/virt_wifi.c
@@ -590,6 +590,42 @@ static struct rtnl_link_ops virt_wifi_link_ops = {
.priv_size = sizeof(struct virt_wifi_netdev_priv),
};
+static inline bool netif_is_virt_wifi_dev(const struct net_device *dev)
+{
+ return rcu_access_pointer(dev->rx_handler) == virt_wifi_rx_handler;
+}
+
+static int virt_wifi_event(struct notifier_block *this, unsigned long event,
+ void *ptr)
+{
+ struct net_device *lower_dev = netdev_notifier_info_to_dev(ptr);
+ struct virt_wifi_netdev_priv *priv;
+ struct net_device *upper_dev;
+ LIST_HEAD(list_kill);
+
+ if (!netif_is_virt_wifi_dev(lower_dev))
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case NETDEV_UNREGISTER:
+ priv = rtnl_dereference(lower_dev->rx_handler_data);
+ if (!priv)
+ return NOTIFY_DONE;
+
+ upper_dev = priv->upperdev;
+
+ upper_dev->rtnl_link_ops->dellink(upper_dev, &list_kill);
+ unregister_netdevice_many(&list_kill);
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block virt_wifi_notifier = {
+ .notifier_call = virt_wifi_event,
+};
+
/* Acquires and releases the rtnl lock. */
static int __init virt_wifi_init_module(void)
{
@@ -598,14 +634,24 @@ static int __init virt_wifi_init_module(void)
/* Guaranteed to be locallly-administered and not multicast. */
eth_random_addr(fake_router_bssid);
+ err = register_netdevice_notifier(&virt_wifi_notifier);
+ if (err)
+ return err;
+
common_wiphy = virt_wifi_make_wiphy();
if (!common_wiphy)
- return -ENOMEM;
+ goto notifier;
err = rtnl_link_register(&virt_wifi_link_ops);
if (err)
- virt_wifi_destroy_wiphy(common_wiphy);
+ goto destroy_wiphy;
+ return 0;
+
+destroy_wiphy:
+ virt_wifi_destroy_wiphy(common_wiphy);
+notifier:
+ unregister_netdevice_notifier(&virt_wifi_notifier);
return err;
}
@@ -615,6 +661,7 @@ static void __exit virt_wifi_cleanup_module(void)
/* Will delete any devices that depend on the wiphy. */
rtnl_link_unregister(&virt_wifi_link_ops);
virt_wifi_destroy_wiphy(common_wiphy);
+ unregister_netdevice_notifier(&virt_wifi_notifier);
}
module_init(virt_wifi_init_module);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf] libbpf: count present CPUs, not theoretically possible
From: Alan Maguire @ 2019-09-28 17:46 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alan Maguire, Andrii Nakryiko, bpf, Networking,
Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzYTQbVVUiQOHEcjL8mZ=iJBSGHFRNxoayRpaMw5ys+iDw@mail.gmail.com>
On Sat, 28 Sep 2019, Andrii Nakryiko wrote:
> On Sat, Sep 28, 2019 at 4:20 AM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > On Fri, 27 Sep 2019, Andrii Nakryiko wrote:
> >
> > > This patch switches libbpf_num_possible_cpus() from using possible CPU
> > > set to present CPU set. This fixes issues with incorrect auto-sizing of
> > > PERF_EVENT_ARRAY map on HOTPLUG-enabled systems.
> > >
> > > On HOTPLUG enabled systems, /sys/devices/system/cpu/possible is going to
> > > be a set of any representable (i.e., potentially possible) CPU, which is
> > > normally way higher than real amount of CPUs (e.g., 0-127 on VM I've
> > > tested on, while there were just two CPU cores actually present).
> > > /sys/devices/system/cpu/present, on the other hand, will only contain
> > > CPUs that are physically present in the system (even if not online yet),
> > > which is what we really want, especially when creating per-CPU maps or
> > > perf events.
> > >
> > > On systems with HOTPLUG disabled, present and possible are identical, so
> > > there is no change of behavior there.
> > >
> >
> > Just curious - is there a reason for not adding a new libbpf_num_present_cpus()
> > function to cover this case, and switching to using that in various places?
>
> The reason is that libbpf_num_possible_cpus() is useless on HOTPLUG
> systems and never worked as intended. If you rely on this function to
> create perf_buffer and/or PERF_EVENT_ARRAY, it will simply fail due to
> specifying more CPUs than are present. I didn't want to keep adding
> new APIs for no good reason, while also leaving useless ones, so I
> fixed the existing API to behave as expected. It's unfortunate that
> name doesn't match sysfs file we are reading it from, of course, but
> having people to choose between libbpf_num_possible_cpus() vs
> libbpf_num_present_cpus() seems like even bigger problem, as
> differences are non-obvious.
>
> The good thing, it won't break all the non-HOTPLUG systems for sure,
> which seems to be the only cases that are used right now (otherwise
> someone would already complain about broken
> libbpf_num_possible_cpus()).
>
Understood, thanks for the explanation.
> >
> > Looking at the places libbpf_num_possible_cpus() is called in libbpf
> >
> > - __perf_buffer__new(): this could just change to use the number of
> > present CPUs, since perf_buffer__new_raw() with a cpu_cnt in struct
> > perf_buffer_raw_ops
> >
> > - bpf_object__create_maps(), which is called via bpf_oject__load_xattr().
> > In this case it seems like switching to num present makes sense, though
> > it might make sense to add a field to struct bpf_object_load_attr * to
> > allow users to explicitly set another max value.
>
> I believe more knobs is not always better for API. Plus, adding any
> field to those xxx_xattr structs is an ABI breakage and requires
> adding new APIs, so I don't think this is good enough reason to add
> new flag. See discussion in another thread about this whole API design
> w/ current attributes and ABI consequences of adding anything new to
> them.
>
> >
> > This would give the desired default behaviour, while still giving users
> > a way of specifying the possible number. What do you think? Thanks!
>
> BTW, if user wants to override the size of maps, they can do it easily
> either in map definition or programmatically after bpf_object__open,
> but before bpf_object__load, so there is no need for flags, it's all
> easily achievable with existing API.
>
Ah, I missed that. Thanks for clarifying!
Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> >
> > Alan
> >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > > ---
> > > tools/lib/bpf/libbpf.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index e0276520171b..45351c074e45 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -5899,7 +5899,7 @@ void bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear)
> > >
> > > int libbpf_num_possible_cpus(void)
> > > {
> > > - static const char *fcpu = "/sys/devices/system/cpu/possible";
> > > + static const char *fcpu = "/sys/devices/system/cpu/present";
> > > int len = 0, n = 0, il = 0, ir = 0;
> > > unsigned int start = 0, end = 0;
> > > int tmp_cpus = 0;
> > > --
> > > 2.17.1
> > >
> > >
>
^ permalink raw reply
* linux-next: Fixes tag needs some work in the net tree
From: Stephen Rothwell @ 2019-09-28 18:51 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List,
Kevin(Yudong) Yang
[-- Attachment #1: Type: text/plain, Size: 432 bytes --]
Hi all,
In commit
6b3656a60f20 ("tcp_bbr: fix quantization code to not raise cwnd if not probing bandwidth")
Fixes tag
Fixes: 3c346b233c68 ("tcp_bbr: fix bw probing to raise in-flight data for very small BDPs")
has these problem(s):
- Target SHA1 does not exist
Did you mean
Fixes: 383d470936c0 ("tcp_bbr: fix bw probing to raise in-flight data for very small BDPs")
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH net v4 12/12] virt_wifi: fix refcnt leak in module exit routine
From: Johannes Berg @ 2019-09-28 18:57 UTC (permalink / raw)
To: Taehee Yoo, davem, netdev, linux-wireless, jakub.kicinski,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
In-Reply-To: <20190928164843.31800-13-ap420073@gmail.com>
On Sat, 2019-09-28 at 16:48 +0000, Taehee Yoo wrote:
> virt_wifi_newlink() calls netdev_upper_dev_link() and it internally
> holds reference count of lower interface.
[...]
> This patch adds notifier routine to delete upper interface before deleting
> lower interface.
Good catch, thanks!
For now I'll assume this will go in through net together with the whole
series (once ready), shout if you want something else.
johannes
^ permalink raw reply
* Re: [PATCH net v4 07/12] macvlan: use dynamic lockdep key instead of subclass
From: Johannes Berg @ 2019-09-28 19:14 UTC (permalink / raw)
To: Taehee Yoo, davem, netdev, linux-wireless, jakub.kicinski,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
In-Reply-To: <20190928164843.31800-8-ap420073@gmail.com>
Hi,
I hadn't seen the previous patchsets of this, and looking briefly in the
archives didn't really seem to say anything about this.
However, I'm wondering now: patches 2-7 of this patchset look basically
all identical in a way:
* you set the addr_list_lock's class to a newly registered key inside
the netdev (or rather the private struct, but doesn't make a big
difference)
* you set each TX queue's _xmit_lock's class similarly
* you set the qdisc_tx_busylock/qdisc_running_key
The first two of these look pretty much completely identical.
Would it perhaps make sense to just do that for *every* netdev? Many of
those netdevs won't ever nest so it wouldn't really be useful, but I'm
not convinced it would put that much more strain on lockdep - if
anything, people are probably creating more VLANs than regular PF/VF
netdevs anyway?
I didn't see any discussion on this, but perhaps I missed it? The cost
would be a bigger netdev struct (when lockdep is enabled), but we
already have that for all the VLANs etc. it's just in the private data,
so it's not a _huge_ difference really I'd think, and this is quite a
bit of code for each device type now.
Alternatively, maybe there could just be some common helper code:
struct nested_netdev_lockdep {
struct lock_class_key xmit_lock_key;
struct lock_class_key addr_lock_key;
};
void netdev_init_nested_lockdep(struct net_device *dev,
struct netsted_netdev_lockdep *l)
{
/* ... */
}
so you just have to embed a "struct nested_netdev_lockdep" in your
private data structure and call the common code.
Or maybe make that
void netdev_init_nested_lockdep(
struct net_device *dev,
struct
netsted_netdev_lockdep *l,
struct lock_class_key
*qdisc_tx_busylock_key,
struct lock_class_key *qdisc_running_key)
so you can't really get that part wrong either?
> @@ -922,6 +938,9 @@ static void macvlan_uninit(struct net_device *dev)
> port->count -= 1;
> if (!port->count)
> macvlan_port_destroy(port->dev);
> +
> + lockdep_unregister_key(&vlan->addr_lock_key);
> + lockdep_unregister_key(&vlan->xmit_lock_key);
> }
OK, so I guess you need an equivalent "deinit" function too -
netdev_deinit_nested_lockdep() or so.
What's not really clear to me is why the qdisc locks can actually stay
the same at all levels? Can they just never nest? But then why are they
different per device type?
Thanks,
johannes
^ permalink raw reply
* Re: [PATCH net v4 00/12] net: fix nested device bugs
From: Johannes Berg @ 2019-09-28 19:20 UTC (permalink / raw)
To: Taehee Yoo, davem, netdev, linux-wireless, jakub.kicinski,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
In-Reply-To: <20190928164843.31800-1-ap420073@gmail.com>
> VLAN, BONDING, TEAM, MACSEC, MACVLAN, IPVLAN, VIRT_WIFI and VXLAN.
> But I couldn't test all interface types so there could be more device
> types which have similar problems.
Did you test virt_wifi? I don't see how it *doesn't* have the nesting
problem, and you didn't change it?
No, I see. You're limiting the nesting generally now in patch 1, and the
others are just lockdep fixups (I guess it's surprising virt_wifi
doesn't do this at all?).
FWIW I don't think virt_wifi really benefits at all from stacking, so we
could just do something like
--- a/drivers/net/wireless/virt_wifi.c
+++ b/drivers/net/wireless/virt_wifi.c
@@ -508,6 +508,9 @@ static int virt_wifi_newlink(struct net *src_net, struct net_device *dev,
else if (dev->mtu > priv->lowerdev->mtu)
return -EINVAL;
+ if (priv->lowerdev->ieee80211_ptr)
+ return -EINVAL;
+
err = netdev_rx_handler_register(priv->lowerdev, virt_wifi_rx_handler,
priv);
if (err) {
IMHO, but of course generally limiting the stack depth is needed anyway
and solves the problem well enough for virt_wifi.
johannes
^ permalink raw reply
* Re: [PATCH net v4 01/12] net: core: limit nested device depth
From: Johannes Berg @ 2019-09-28 19:36 UTC (permalink / raw)
To: Taehee Yoo, davem, netdev, linux-wireless, jakub.kicinski,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
In-Reply-To: <20190928164843.31800-2-ap420073@gmail.com>
Hi,
> int netdev_walk_all_upper_dev_rcu(struct net_device *dev,
> int (*fn)(struct net_device *dev,
> void *data),
> void *data)
> {
[...]
> }
>
> return 0;
> +
> }
that seems like an oversight, probably from editing the patch in
different versions?
> +static int __netdev_update_upper_level(struct net_device *dev, void *data)
> +{
> + dev->upper_level = __netdev_upper_depth(dev) + 1;
> + return 0;
> +}
> +
> +static int __netdev_update_lower_level(struct net_device *dev, void *data)
> +{
> + dev->lower_level = __netdev_lower_depth(dev) + 1;
> + return 0;
> +}
Is there any point in the return value here? You don't really use it,
afaict? I guess I might see the point if it was used for tail-call
optimisation or such?
Also, I dunno, I guess netdevs aren't as much under pressure as SKBs :-)
but do we actually gain much from storing the nesting level at all? You
have to maintain it all the time anyway when adding/removing and that's
the only place where you also check it, so perhaps it wouldn't be that
bad to just count at that time?
But then again the counting would probably be recursive again ...
> return 0;
> +
> }
> EXPORT_SYMBOL_GPL(netdev_walk_all_lower_dev_rcu);
same nit as above
> + __netdev_update_upper_level(dev, NULL);
> + netdev_walk_all_lower_dev(dev, __netdev_update_upper_level, NULL);
> +
> + __netdev_update_lower_level(upper_dev, NULL);
> + netdev_walk_all_upper_dev(upper_dev, __netdev_update_lower_level, NULL);
Actually, if I'm reading this correctly you already walk all the levels
anyway? Then couldn't you calculate the depth at this time and return
it, instead of storing it? Though, if it actually overflowed then you'd
have to walk *again* to undo that?
Hmm, actually, if you don't store the value you don't even need to walk
here I guess, or at least you would only have to do it to verify you
*can* attach, but wouldn't have to in detach?
So it looks to me like on attach (i.e. this code, quoted from
__netdev_upper_dev_link) you're already walking the entire graph to
update the level values, and could probably instead calculate the
nesting depth to validate it?
And then on netdev_upper_dev_unlink() you wouldn't even have to walk the
graph at all, since you only need that to update the values that you
stored.
But maybe I'm misinterpreting this completely?
Thanks,
johannes
^ permalink raw reply
* Re: [PATCH net v4 11/12] net: remove unnecessary variables and callback
From: Johannes Berg @ 2019-09-28 19:42 UTC (permalink / raw)
To: Taehee Yoo, davem, netdev, linux-wireless, jakub.kicinski,
j.vosburgh, vfalico, andy, jiri, sd, roopa, saeedm, manishc,
rahulv, kys, haiyangz, stephen, sashal, hare, varun, ubraun,
kgraul, jay.vosburgh, schuffelen, bjorn
In-Reply-To: <20190928164843.31800-12-ap420073@gmail.com>
On Sat, 2019-09-28 at 16:48 +0000, Taehee Yoo wrote:
> This patch removes variables and callback these are related to the nested
> device structure.
> devices that can be nested have their own nest_level variable that
> represents the depth of nested devices.
> In the previous patch, new {lower/upper}_level variables are added and
> they replace old private nest_level variable.
> So, this patch removes all 'nest_level' variables.
Ah, well, I see at least this patch also needs the nesting level tracked
in the netdev, at least the "lower_level".
johannes
^ permalink raw reply
* Re: [PATCH iproute2 net-next 1/2] bridge: fdb get support
From: Roopa Prabhu @ 2019-09-28 20:20 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, netdev, Nikolay Aleksandrov
In-Reply-To: <20190928084641.69233a5a@hermes.lan>
On Sat, Sep 28, 2019 at 8:46 AM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri, 27 Sep 2019 21:48:23 -0700
> Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>
> Overall, looks good.
>
> > + if (print_fdb(answer, (void *)stdout) < 0) {
>
> Cast to void is not necessary in C (it is in C++)
thats a copy/paste from iproute get code. will remove it and post v2, thx
^ permalink raw reply
* [PATCH iproute2 net-next v2 0/2] support for bridge fdb and neigh get
From: Roopa Prabhu @ 2019-09-28 20:22 UTC (permalink / raw)
To: dsahern; +Cc: netdev, nikolay, stephen
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This series adds iproute2 support to lookup a bridge fdb and
neigh entry.
example:
$bridge fdb get 02:02:00:00:00:03 dev test-dummy0 vlan 1002
02:02:00:00:00:03 dev test-dummy0 vlan 1002 master bridge
$ip neigh get 10.0.2.4 dev test-dummy0
10.0.2.4 dev test-dummy0 lladdr de:ad:be:ef:13:37 PERMANENT
v2 - remove cast around stdout in print_fdb as pointed out by stephen
Roopa Prabhu (2):
bridge: fdb get support
ipneigh: neigh get support
bridge/fdb.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++-
ip/ipneigh.c | 72 ++++++++++++++++++++++++++++--
man/man8/bridge.8 | 35 +++++++++++++++
man/man8/ip-neighbour.8 | 25 +++++++++++
4 files changed, 240 insertions(+), 5 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH iproute2 net-next v2 1/2] bridge: fdb get support
From: Roopa Prabhu @ 2019-09-28 20:22 UTC (permalink / raw)
To: dsahern; +Cc: netdev, nikolay, stephen
In-Reply-To: <1569702130-46433-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds support to lookup a bridge fdb entry
using recently added support in the kernel using RTM_GETNEIGH
(and AF_BRIDGE family).
example:
$bridge fdb get 02:02:00:00:00:03 dev test-dummy0 vlan 1002
02:02:00:00:00:03 dev test-dummy0 vlan 1002 master bridge
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
bridge/fdb.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
man/man8/bridge.8 | 40 +++++++++++++++++++
2 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 941ce2d..804fa36 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -40,7 +40,9 @@ static void usage(void)
" [ sticky ] [ local | static | dynamic ] [ dst IPADDR ]\n"
" [ vlan VID ] [ port PORT] [ vni VNI ] [ via DEV ]\n"
" [ src_vni VNI ]\n"
- " bridge fdb [ show [ br BRDEV ] [ brport DEV ] [ vlan VID ] [ state STATE ] ]\n");
+ " bridge fdb [ show [ br BRDEV ] [ brport DEV ] [ vlan VID ] [ state STATE ] ]\n"
+ " bridge fdb get ADDR [ br BRDEV ] { brport |dev } DEV [ vlan VID ]\n"
+ " [ vni VNI ]\n");
exit(-1);
}
@@ -518,6 +520,113 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
return 0;
}
+static int fdb_get(int argc, char **argv)
+{
+ struct {
+ struct nlmsghdr n;
+ struct ndmsg ndm;
+ char buf[1024];
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETNEIGH,
+ .ndm.ndm_family = AF_BRIDGE,
+ };
+ struct nlmsghdr *answer;
+ char *addr = NULL;
+ char *d = NULL, *br = NULL;
+ char abuf[ETH_ALEN];
+ unsigned long vni = ~0;
+ int br_ifindex = 0;
+ char *endptr;
+ short vlan = -1;
+
+ while (argc > 0) {
+ if ((strcmp(*argv, "brport") == 0) || strcmp(*argv, "dev") == 0) {
+ NEXT_ARG();
+ d = *argv;
+ } else if (strcmp(*argv, "br") == 0) {
+ NEXT_ARG();
+ br = *argv;
+ } else if (strcmp(*argv, "dev") == 0) {
+ NEXT_ARG();
+ d = *argv;
+ } else if (strcmp(*argv, "vni") == 0) {
+ NEXT_ARG();
+ vni = strtoul(*argv, &endptr, 0);
+ if ((endptr && *endptr) ||
+ (vni >> 24) || vni == ULONG_MAX)
+ invarg("invalid VNI\n", *argv);
+ } else if (strcmp(*argv, "self") == 0) {
+ req.ndm.ndm_flags |= NTF_SELF;
+ } else if (matches(*argv, "master") == 0) {
+ req.ndm.ndm_flags |= NTF_MASTER;
+ } else if (matches(*argv, "vlan") == 0) {
+ if (vlan >= 0)
+ duparg2("vlan", *argv);
+ NEXT_ARG();
+ vlan = atoi(*argv);
+ } else {
+ if (strcmp(*argv, "to") == 0)
+ NEXT_ARG();
+
+ if (matches(*argv, "help") == 0)
+ usage();
+ if (addr)
+ duparg2("to", *argv);
+ addr = *argv;
+ }
+ argc--; argv++;
+ }
+
+ if ((d == NULL && br == NULL) || addr == NULL) {
+ fprintf(stderr, "Device or master and address are required arguments.\n");
+ return -1;
+ }
+
+ if (sscanf(addr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+ abuf, abuf+1, abuf+2,
+ abuf+3, abuf+4, abuf+5) != 6) {
+ fprintf(stderr, "Invalid mac address %s\n", addr);
+ return -1;
+ }
+
+ addattr_l(&req.n, sizeof(req), NDA_LLADDR, abuf, ETH_ALEN);
+
+ if (vlan >= 0)
+ addattr16(&req.n, sizeof(req), NDA_VLAN, vlan);
+
+ if (vni != ~0)
+ addattr32(&req.n, sizeof(req), NDA_VNI, vni);
+
+ if (d) {
+ req.ndm.ndm_ifindex = ll_name_to_index(d);
+ if (!req.ndm.ndm_ifindex) {
+ fprintf(stderr, "Cannot find device \"%s\"\n", d);
+ return -1;
+ }
+ }
+
+ if (br) {
+ br_ifindex = ll_name_to_index(br);
+ if (!br_ifindex) {
+ fprintf(stderr, "Cannot find bridge device \"%s\"\n", br);
+ return -1;
+ }
+ addattr32(&req.n, sizeof(req), NDA_MASTER, br_ifindex);
+ }
+
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
+ return -2;
+
+ if (print_fdb(answer, stdout) < 0) {
+ fprintf(stderr, "An error :-)\n");
+ return -1;
+ }
+
+ return 0;
+}
+
int do_fdb(int argc, char **argv)
{
ll_init_map(&rth);
@@ -531,6 +640,8 @@ int do_fdb(int argc, char **argv)
return fdb_modify(RTM_NEWNEIGH, NLM_F_CREATE|NLM_F_REPLACE, argc-1, argv+1);
if (matches(*argv, "delete") == 0)
return fdb_modify(RTM_DELNEIGH, 0, argc-1, argv+1);
+ if (matches(*argv, "get") == 0)
+ return fdb_get(argc-1, argv+1);
if (matches(*argv, "show") == 0 ||
matches(*argv, "lst") == 0 ||
matches(*argv, "list") == 0)
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index bb4fb52..10f6cf0 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -93,6 +93,17 @@ bridge \- show / manipulate bridge addresses and devices
.IR STATE " ]"
.ti -8
+.B bridge fdb get
+.I LLADDR " [ "
+.B dev
+.IR DEV " ] [ "
+.B br
+.IR BRDEV " ] [ "
+.B vlan
+.IR VID " ] ["
+.BR self " ] [ " master " ]"
+
+.ti -8
.BR "bridge mdb" " { " add " | " del " } "
.B dev
.IR DEV
@@ -550,6 +561,35 @@ With the
option, the command becomes verbose. It prints out the last updated
and last used time for each entry.
+.SS bridge fdb get - get bridge forwarding entry.
+
+lookup a bridge forwarding table entry.
+
+.TP
+.BI "LLADDR"
+the Ethernet MAC address.
+
+.TP
+.BI dev " DEV"
+the interface to which this address is associated.
+
+.TP
+.BI brport " DEV"
+the bridge port to which this address is associated. same as dev above.
+
+.TP
+.BI br " DEV"
+the bridge to which this address is associated.
+
+.TP
+.B self
+- the address is associated with the port drivers fdb. Usually hardware.
+
+.TP
+.B master
+- the address is associated with master devices fdb. Usually software (default).
+.sp
+
.SH bridge mdb - multicast group database management
.B mdb
--
2.1.4
^ permalink raw reply related
* [PATCH iproute2 net-next v2 2/2] ipneigh: neigh get support
From: Roopa Prabhu @ 2019-09-28 20:22 UTC (permalink / raw)
To: dsahern; +Cc: netdev, nikolay, stephen
In-Reply-To: <1569702130-46433-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds support to lookup a neigh entry
using recently added support in the kernel using RTM_GETNEIGH
example:
$ip neigh get 10.0.2.4 dev test-dummy0
10.0.2.4 dev test-dummy0 lladdr de:ad:be:ef:13:37 PERMANENT
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
ip/ipneigh.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++---
man/man8/ip-neighbour.8 | 29 +++++++++++++++++
2 files changed, 109 insertions(+), 4 deletions(-)
diff --git a/ip/ipneigh.c b/ip/ipneigh.c
index a3869c8..c88c346 100644
--- a/ip/ipneigh.c
+++ b/ip/ipneigh.c
@@ -55,6 +55,7 @@ static void usage(void)
"\n"
" ip neigh { show | flush } [ proxy ] [ to PREFIX ] [ dev DEV ] [ nud STATE ]\n"
" [ vrf NAME ]\n"
+ " ip neigh get { ADDR | proxy ADDR } dev DEV\n"
"\n"
"STATE := { permanent | noarp | stale | reachable | none |\n"
" incomplete | delay | probe | failed }\n");
@@ -599,6 +600,83 @@ static int do_show_or_flush(int argc, char **argv, int flush)
return 0;
}
+static int ipneigh_get(int argc, char **argv)
+{
+ struct {
+ struct nlmsghdr n;
+ struct ndmsg ndm;
+ char buf[1024];
+ } req = {
+ .n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ndmsg)),
+ .n.nlmsg_flags = NLM_F_REQUEST,
+ .n.nlmsg_type = RTM_GETNEIGH,
+ .ndm.ndm_family = preferred_family,
+ };
+ struct nlmsghdr *answer;
+ char *d = NULL;
+ int dst_ok = 0;
+ int dev_ok = 0;
+ inet_prefix dst;
+
+ while (argc > 0) {
+ if (strcmp(*argv, "dev") == 0) {
+ NEXT_ARG();
+ d = *argv;
+ dev_ok = 1;
+ } else if (matches(*argv, "proxy") == 0) {
+ NEXT_ARG();
+ if (matches(*argv, "help") == 0)
+ usage();
+ if (dst_ok)
+ duparg("address", *argv);
+ get_addr(&dst, *argv, preferred_family);
+ dst_ok = 1;
+ dev_ok = 1;
+ req.ndm.ndm_flags |= NTF_PROXY;
+ } else {
+ if (strcmp(*argv, "to") == 0)
+ NEXT_ARG();
+
+ if (matches(*argv, "help") == 0)
+ usage();
+ if (dst_ok)
+ duparg2("to", *argv);
+ get_addr(&dst, *argv, preferred_family);
+ dst_ok = 1;
+ }
+ argc--; argv++;
+ }
+
+ if (!dev_ok || !dst_ok || dst.family == AF_UNSPEC) {
+ fprintf(stderr, "Device and address are required arguments.\n");
+ return -1;
+ }
+
+ req.ndm.ndm_family = dst.family;
+ if (addattr_l(&req.n, sizeof(req), NDA_DST, &dst.data, dst.bytelen) < 0)
+ return -1;
+
+ if (d) {
+ ll_init_map(&rth);
+ req.ndm.ndm_ifindex = ll_name_to_index(d);
+ if (!req.ndm.ndm_ifindex) {
+ fprintf(stderr, "Cannot find device \"%s\"\n", d);
+ return -1;
+ }
+ }
+
+ if (rtnl_talk(&rth, &req.n, &answer) < 0)
+ return -2;
+
+ ipneigh_reset_filter(0);
+ if (print_neigh(answer, (void *)stdout) < 0) {
+ fprintf(stderr, "An error :-)\n");
+ return -1;
+ }
+
+ return 0;
+}
+
int do_ipneigh(int argc, char **argv)
{
if (argc > 0) {
@@ -611,10 +689,8 @@ int do_ipneigh(int argc, char **argv)
return ipneigh_modify(RTM_NEWNEIGH, NLM_F_CREATE|NLM_F_REPLACE, argc-1, argv+1);
if (matches(*argv, "delete") == 0)
return ipneigh_modify(RTM_DELNEIGH, 0, argc-1, argv+1);
- if (matches(*argv, "get") == 0) {
- fprintf(stderr, "Sorry, \"neigh get\" is not implemented :-(\n");
- return -1;
- }
+ if (matches(*argv, "get") == 0)
+ return ipneigh_get(argc-1, argv+1);
if (matches(*argv, "show") == 0 ||
matches(*argv, "lst") == 0 ||
matches(*argv, "list") == 0)
diff --git a/man/man8/ip-neighbour.8 b/man/man8/ip-neighbour.8
index 4a672bb..bc77b43 100644
--- a/man/man8/ip-neighbour.8
+++ b/man/man8/ip-neighbour.8
@@ -38,6 +38,12 @@ ip-neighbour \- neighbour/arp tables management.
.IR NAME " ] "
.ti -8
+.B ip neigh get
+.IR ADDR
+.B dev
+.IR DEV
+
+.ti -8
.IR STATE " := {"
.BR permanent " | " noarp " | " stale " | " reachable " | " none " |"
.BR incomplete " | " delay " | " probe " | " failed " }"
@@ -231,6 +237,23 @@ twice,
also dumps all the deleted neighbours.
.RE
+.TP
+ip neigh get
+lookup a neighbour entry to a destination given a device
+.RS
+
+.TP
+.BI proxy
+indicates whether we should lookup a proxy neigbour entry
+
+.TP
+.BI to " ADDRESS " (default)
+the prefix selecting the neighbour to query.
+
+.TP
+.BI dev " NAME"
+get neighbour entry attached to this device.
+
.SH EXAMPLES
.PP
ip neighbour
@@ -242,6 +265,12 @@ ip neigh flush dev eth0
.RS
Removes entries in the neighbour table on device eth0.
.RE
+.PP
+ip neigh get 10.0.1.10 dev eth0
+.RS
+Performs a neighbour lookup in the kernel and returns
+a neighbour entry.
+.RE
.SH SEE ALSO
.br
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] net: dsa: rtl8366: Check VLAN ID and not ports
From: Linus Walleij @ 2019-09-28 20:26 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, Vivien Didelot, netdev
In-Reply-To: <e21e9a80-c8e0-d758-2309-1a8f03dda400@gmail.com>
On Fri, Sep 27, 2019 at 6:40 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 9/27/19 9:39 AM, Linus Walleij wrote:
> > There has been some confusion between the port number and
> > the VLAN ID in this driver. What we need to check for
> > validity is the VLAN ID, nothing else.
> >
> > The current confusion came from assigning a few default
> > VLANs for default routing and we need to rewrite that
> > properly.
> >
> > Instead of checking if the port number is a valid VLAN
> > ID, check the actual VLAN IDs passed in to the callback
> > one by one as expected.
> >
> > Fixes: d8652956cf37 ("net: dsa: realtek-smi: Add Realtek SMI driver")
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > drivers/net/dsa/rtl8366.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
> > index ca3d17e43ed8..e2c91b75e843 100644
> > --- a/drivers/net/dsa/rtl8366.c
> > +++ b/drivers/net/dsa/rtl8366.c
> > @@ -340,9 +340,11 @@ int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
> > {
> > struct realtek_smi *smi = ds->priv;
> > int ret;
> > + int i;
> >
> > - if (!smi->ops->is_vlan_valid(smi, port))
> > - return -EINVAL;
> > + for (i = vlan->vid_begin; i < vlan->vid_end; i++)
> > + if (!smi->ops->is_vlan_valid(smi, port))
> > + return -EINVAL;
>
> You are still checking the port and not the "i" (VLAN ID) argument here,
> is there something I am missing?
No you're right, just correcting old mistakes by making
new mistakes .. I'll fix, thanks!
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] net: dsa: rtl8366: Check VLAN ID and not ports
From: Florian Fainelli @ 2019-09-28 20:36 UTC (permalink / raw)
To: Linus Walleij; +Cc: Andrew Lunn, Vivien Didelot, netdev
In-Reply-To: <CACRpkdaLTf9x=yTBBcGXDUmu2fNjLhx_eWVce_LQcPCjeq9TcQ@mail.gmail.com>
On 9/28/2019 1:26 PM, Linus Walleij wrote:
> On Fri, Sep 27, 2019 at 6:40 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>> On 9/27/19 9:39 AM, Linus Walleij wrote:
>>> There has been some confusion between the port number and
>>> the VLAN ID in this driver. What we need to check for
>>> validity is the VLAN ID, nothing else.
>>>
>>> The current confusion came from assigning a few default
>>> VLANs for default routing and we need to rewrite that
>>> properly.
>>>
>>> Instead of checking if the port number is a valid VLAN
>>> ID, check the actual VLAN IDs passed in to the callback
>>> one by one as expected.
>>>
>>> Fixes: d8652956cf37 ("net: dsa: realtek-smi: Add Realtek SMI driver")
>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>> ---
>>> drivers/net/dsa/rtl8366.c | 12 ++++++++----
>>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/dsa/rtl8366.c b/drivers/net/dsa/rtl8366.c
>>> index ca3d17e43ed8..e2c91b75e843 100644
>>> --- a/drivers/net/dsa/rtl8366.c
>>> +++ b/drivers/net/dsa/rtl8366.c
>>> @@ -340,9 +340,11 @@ int rtl8366_vlan_prepare(struct dsa_switch *ds, int port,
>>> {
>>> struct realtek_smi *smi = ds->priv;
>>> int ret;
>>> + int i;
>>>
>>> - if (!smi->ops->is_vlan_valid(smi, port))
>>> - return -EINVAL;
>>> + for (i = vlan->vid_begin; i < vlan->vid_end; i++)
>>> + if (!smi->ops->is_vlan_valid(smi, port))
>>> + return -EINVAL;
>>
>> You are still checking the port and not the "i" (VLAN ID) argument here,
>> is there something I am missing?
>
> No you're right, just correcting old mistakes by making
> new mistakes .. I'll fix, thanks!
>
Do we need to duplicate the same is_vlan_valid() check in both the
vlan_prepare and vlan_add callbacks?
--
Florian
^ permalink raw reply
* Re: [PATCH] net: dsa: rtl8366: Check VLAN ID and not ports
From: Linus Walleij @ 2019-09-28 20:43 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, Vivien Didelot, netdev
In-Reply-To: <8d34070c-4d35-e378-0b9e-4cfe279a7615@gmail.com>
On Sat, Sep 28, 2019 at 10:36 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> Do we need to duplicate the same is_vlan_valid() check in both the
> vlan_prepare and vlan_add callbacks?
I'm unsure of the semantics of these calls, the check was in the
OpenWrt driver that I started from.
Is it guaranteed that .vlan_prepare() and .vlan_add() are
always called in succession? Then .vlan_prepare() should
be enough.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH net] net: dsa: sja1105: Fix sleeping while atomic in .port_hwtstamp_set
From: Vladimir Oltean @ 2019-09-28 22:07 UTC (permalink / raw)
To: richardcochran, andrew, f.fainelli, vivien.didelot, davem
Cc: netdev, Vladimir Oltean
Currently this stack trace can be seen with CONFIG_DEBUG_ATOMIC_SLEEP=y:
[ 41.568348] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:909
[ 41.576757] in_atomic(): 1, irqs_disabled(): 0, pid: 208, name: ptp4l
[ 41.583212] INFO: lockdep is turned off.
[ 41.587123] CPU: 1 PID: 208 Comm: ptp4l Not tainted 5.3.0-rc6-01445-ge950f2d4bc7f-dirty #1827
[ 41.599873] [<c0313d7c>] (unwind_backtrace) from [<c030e13c>] (show_stack+0x10/0x14)
[ 41.607584] [<c030e13c>] (show_stack) from [<c1212d50>] (dump_stack+0xd4/0x100)
[ 41.614863] [<c1212d50>] (dump_stack) from [<c037dfc8>] (___might_sleep+0x1c8/0x2b4)
[ 41.622574] [<c037dfc8>] (___might_sleep) from [<c122ea90>] (__mutex_lock+0x48/0xab8)
[ 41.630368] [<c122ea90>] (__mutex_lock) from [<c122f51c>] (mutex_lock_nested+0x1c/0x24)
[ 41.638340] [<c122f51c>] (mutex_lock_nested) from [<c0c6fe08>] (sja1105_static_config_reload+0x30/0x27c)
[ 41.647779] [<c0c6fe08>] (sja1105_static_config_reload) from [<c0c7015c>] (sja1105_hwtstamp_set+0x108/0x1cc)
[ 41.657562] [<c0c7015c>] (sja1105_hwtstamp_set) from [<c0feb650>] (dev_ifsioc+0x18c/0x330)
[ 41.665788] [<c0feb650>] (dev_ifsioc) from [<c0febbd8>] (dev_ioctl+0x320/0x6e8)
[ 41.673064] [<c0febbd8>] (dev_ioctl) from [<c0f8b1f4>] (sock_ioctl+0x334/0x5e8)
[ 41.680340] [<c0f8b1f4>] (sock_ioctl) from [<c05404a8>] (do_vfs_ioctl+0xb0/0xa10)
[ 41.687789] [<c05404a8>] (do_vfs_ioctl) from [<c0540e3c>] (ksys_ioctl+0x34/0x58)
[ 41.695151] [<c0540e3c>] (ksys_ioctl) from [<c0301000>] (ret_fast_syscall+0x0/0x28)
[ 41.702768] Exception stack(0xe8495fa8 to 0xe8495ff0)
[ 41.707796] 5fa0: beff4a8c 00000001 00000011 000089b0 beff4a8c beff4a80
[ 41.715933] 5fc0: beff4a8c 00000001 0000000c 00000036 b6fa98c8 004e19c1 00000001 00000000
[ 41.724069] 5fe0: 004dcedc beff4a6c 004c0738 b6e7af4c
[ 41.729860] BUG: scheduling while atomic: ptp4l/208/0x00000002
[ 41.735682] INFO: lockdep is turned off.
Enabling RX timestamping will logically disturb the fastpath (processing
of meta frames). Replace bool hwts_rx_en with a bit that is checked
atomically from the fastpath and temporarily unset from the sleepable
context during a change of the RX timestamping process (a destructive
operation anyways, requires switch reset).
If found unset, the fastpath will just drop any received meta frame and
not take the meta_lock at all.
Fixes: a602afd200f5 ("net: dsa: sja1105: Expose PTP timestamping ioctls to userspace")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
drivers/net/dsa/sja1105/sja1105_main.c | 20 ++++++++++++--------
include/linux/dsa/sja1105.h | 4 +++-
net/dsa/tag_sja1105.c | 12 +++++++++++-
3 files changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/net/dsa/sja1105/sja1105_main.c b/drivers/net/dsa/sja1105/sja1105_main.c
index b9def744bcb3..5a7ed64ae64e 100644
--- a/drivers/net/dsa/sja1105/sja1105_main.c
+++ b/drivers/net/dsa/sja1105/sja1105_main.c
@@ -1897,7 +1897,9 @@ static int sja1105_set_ageing_time(struct dsa_switch *ds,
return sja1105_static_config_reload(priv);
}
-/* Caller must hold priv->tagger_data.meta_lock */
+/* Must be called only with priv->tagger_data.state bit
+ * SJA1105_HWTS_RX_EN cleared
+ */
static int sja1105_change_rxtstamping(struct sja1105_private *priv,
bool on)
{
@@ -1954,16 +1956,17 @@ static int sja1105_hwtstamp_set(struct dsa_switch *ds, int port,
break;
}
- if (rx_on != priv->tagger_data.hwts_rx_en) {
- spin_lock(&priv->tagger_data.meta_lock);
+ if (rx_on != test_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state)) {
+ clear_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state);
+
rc = sja1105_change_rxtstamping(priv, rx_on);
- spin_unlock(&priv->tagger_data.meta_lock);
if (rc < 0) {
dev_err(ds->dev,
"Failed to change RX timestamping: %d\n", rc);
- return -EFAULT;
+ return rc;
}
- priv->tagger_data.hwts_rx_en = rx_on;
+ if (rx_on)
+ set_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state);
}
if (copy_to_user(ifr->ifr_data, &config, sizeof(config)))
@@ -1982,7 +1985,7 @@ static int sja1105_hwtstamp_get(struct dsa_switch *ds, int port,
config.tx_type = HWTSTAMP_TX_ON;
else
config.tx_type = HWTSTAMP_TX_OFF;
- if (priv->tagger_data.hwts_rx_en)
+ if (test_bit(SJA1105_HWTS_RX_EN, &priv->tagger_data.state))
config.rx_filter = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
else
config.rx_filter = HWTSTAMP_FILTER_NONE;
@@ -2031,7 +2034,7 @@ static bool sja1105_port_rxtstamp(struct dsa_switch *ds, int port,
struct sja1105_private *priv = ds->priv;
struct sja1105_tagger_data *data = &priv->tagger_data;
- if (!data->hwts_rx_en)
+ if (!test_bit(SJA1105_HWTS_RX_EN, &data->state))
return false;
/* We need to read the full PTP clock to reconstruct the Rx
@@ -2201,6 +2204,7 @@ static int sja1105_probe(struct spi_device *spi)
tagger_data = &priv->tagger_data;
skb_queue_head_init(&tagger_data->skb_rxtstamp_queue);
INIT_WORK(&tagger_data->rxtstamp_work, sja1105_rxtstamp_work);
+ spin_lock_init(&tagger_data->meta_lock);
/* Connections between dsa_port and sja1105_port */
for (i = 0; i < SJA1105_NUM_PORTS; i++) {
diff --git a/include/linux/dsa/sja1105.h b/include/linux/dsa/sja1105.h
index 79435cfc20eb..897e799dbcb9 100644
--- a/include/linux/dsa/sja1105.h
+++ b/include/linux/dsa/sja1105.h
@@ -31,6 +31,8 @@
#define SJA1105_META_SMAC 0x222222222222ull
#define SJA1105_META_DMAC 0x0180C200000Eull
+#define SJA1105_HWTS_RX_EN 0
+
/* Global tagger data: each struct sja1105_port has a reference to
* the structure defined in struct sja1105_private.
*/
@@ -42,7 +44,7 @@ struct sja1105_tagger_data {
* from taggers running on multiple ports on SMP systems
*/
spinlock_t meta_lock;
- bool hwts_rx_en;
+ unsigned long state;
};
struct sja1105_skb_cb {
diff --git a/net/dsa/tag_sja1105.c b/net/dsa/tag_sja1105.c
index 9c9aff3e52cf..63ef2a14c934 100644
--- a/net/dsa/tag_sja1105.c
+++ b/net/dsa/tag_sja1105.c
@@ -156,7 +156,11 @@ static struct sk_buff
/* Step 1: A timestampable frame was received.
* Buffer it until we get its meta frame.
*/
- if (is_link_local && sp->data->hwts_rx_en) {
+ if (is_link_local) {
+ if (!test_bit(SJA1105_HWTS_RX_EN, &sp->data->state))
+ /* Do normal processing. */
+ return skb;
+
spin_lock(&sp->data->meta_lock);
/* Was this a link-local frame instead of the meta
* that we were expecting?
@@ -187,6 +191,12 @@ static struct sk_buff
} else if (is_meta) {
struct sk_buff *stampable_skb;
+ /* Drop the meta frame if we're not in the right state
+ * to process it.
+ */
+ if (!test_bit(SJA1105_HWTS_RX_EN, &sp->data->state))
+ return NULL;
+
spin_lock(&sp->data->meta_lock);
stampable_skb = sp->data->stampable_skb;
--
2.17.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