* [PATCH net-next v7 04/15] net: move promiscuity handling into netdev_rx_mode_work
From: Stanislav Fomichev @ 2026-04-13 17:11 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni, Aleksandr Loktionov
In-Reply-To: <20260413171131.550126-1-sdf@fomichev.me>
Move unicast promiscuity tracking into netdev_rx_mode_work so it runs
under netdev_ops_lock instead of under the addr_lock spinlock. This
is required because __dev_set_promiscuity calls dev_change_rx_flags
and __dev_notify_flags, both of which may need to sleep.
Change ASSERT_RTNL() to netdev_ops_assert_locked() in
__dev_set_promiscuity, netif_set_allmulti and __dev_change_flags
since these are now called from the work queue under the ops lock.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
Documentation/networking/netdevices.rst | 4 ++
net/core/dev.c | 16 ++---
net/core/dev_addr_lists.c | 82 ++++++++++++++++++-------
3 files changed, 68 insertions(+), 34 deletions(-)
diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst
index e89b12d4f3a7..93e06e8d51a9 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -299,6 +299,10 @@ struct net_device synchronization rules
Notes: Async version of ndo_set_rx_mode which runs in process
context. Receives snapshots of the unicast and multicast address lists.
+ndo_change_rx_flags:
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements queue management or shaper API.
+
ndo_setup_tc:
``TC_SETUP_BLOCK`` and ``TC_SETUP_FT`` are running under NFT locks
(i.e. no ``rtnl_lock`` and no device instance lock). The rest of
diff --git a/net/core/dev.c b/net/core/dev.c
index 8597ec56fd64..8a69aed56fca 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9600,7 +9600,7 @@ int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
kuid_t uid;
kgid_t gid;
- ASSERT_RTNL();
+ netdev_ops_assert_locked(dev);
promiscuity = dev->promiscuity + inc;
if (promiscuity == 0) {
@@ -9636,16 +9636,8 @@ int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
dev_change_rx_flags(dev, IFF_PROMISC);
}
- if (notify) {
- /* The ops lock is only required to ensure consistent locking
- * for `NETDEV_CHANGE` notifiers. This function is sometimes
- * called without the lock, even for devices that are ops
- * locked, such as in `dev_uc_sync_multiple` when using
- * bonding or teaming.
- */
- netdev_ops_assert_locked(dev);
+ if (notify)
__dev_notify_flags(dev, old_flags, IFF_PROMISC, 0, NULL);
- }
return 0;
}
@@ -9667,7 +9659,7 @@ int netif_set_allmulti(struct net_device *dev, int inc, bool notify)
unsigned int old_flags = dev->flags, old_gflags = dev->gflags;
unsigned int allmulti, flags;
- ASSERT_RTNL();
+ netdev_ops_assert_locked(dev);
allmulti = dev->allmulti + inc;
if (allmulti == 0) {
@@ -9735,7 +9727,7 @@ int __dev_change_flags(struct net_device *dev, unsigned int flags,
unsigned int old_flags = dev->flags;
int ret;
- ASSERT_RTNL();
+ netdev_ops_assert_locked(dev);
/*
* Set the flags on our device.
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 88e995db15dd..49346d0cbc8a 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -1229,10 +1229,34 @@ static void netif_addr_lists_reconcile(struct net_device *dev,
&dev->rx_mode_addr_cache);
}
+/**
+ * netif_uc_promisc_update() - evaluate whether uc_promisc should be toggled.
+ * @dev: device
+ *
+ * Must be called under netif_addr_lock_bh.
+ * Return: +1 to enter promisc, -1 to leave, 0 for no change.
+ */
+static int netif_uc_promisc_update(struct net_device *dev)
+{
+ if (dev->priv_flags & IFF_UNICAST_FLT)
+ return 0;
+
+ if (!netdev_uc_empty(dev) && !dev->uc_promisc) {
+ dev->uc_promisc = true;
+ return 1;
+ }
+ if (netdev_uc_empty(dev) && dev->uc_promisc) {
+ dev->uc_promisc = false;
+ return -1;
+ }
+ return 0;
+}
+
static void netif_rx_mode_run(struct net_device *dev)
{
struct netdev_hw_addr_list uc_snap, mc_snap, uc_ref, mc_ref;
const struct net_device_ops *ops = dev->netdev_ops;
+ int promisc_inc;
int err;
might_sleep();
@@ -1246,22 +1270,39 @@ static void netif_rx_mode_run(struct net_device *dev)
if (!(dev->flags & IFF_UP) || !netif_device_present(dev))
return;
- netif_addr_lock_bh(dev);
- err = netif_addr_lists_snapshot(dev, &uc_snap, &mc_snap,
- &uc_ref, &mc_ref);
- if (err) {
- netdev_WARN(dev, "failed to sync uc/mc addresses\n");
+ if (ops->ndo_set_rx_mode_async) {
+ netif_addr_lock_bh(dev);
+ err = netif_addr_lists_snapshot(dev, &uc_snap, &mc_snap,
+ &uc_ref, &mc_ref);
+ if (err) {
+ netdev_WARN(dev, "failed to sync uc/mc addresses\n");
+ netif_addr_unlock_bh(dev);
+ return;
+ }
+
+ promisc_inc = netif_uc_promisc_update(dev);
+ netif_addr_unlock_bh(dev);
+ } else {
+ netif_addr_lock_bh(dev);
+ promisc_inc = netif_uc_promisc_update(dev);
netif_addr_unlock_bh(dev);
- return;
}
- netif_addr_unlock_bh(dev);
- ops->ndo_set_rx_mode_async(dev, &uc_snap, &mc_snap);
+ if (promisc_inc)
+ __dev_set_promiscuity(dev, promisc_inc, false);
- netif_addr_lock_bh(dev);
- netif_addr_lists_reconcile(dev, &uc_snap, &mc_snap,
- &uc_ref, &mc_ref);
- netif_addr_unlock_bh(dev);
+ if (ops->ndo_set_rx_mode_async) {
+ ops->ndo_set_rx_mode_async(dev, &uc_snap, &mc_snap);
+
+ netif_addr_lock_bh(dev);
+ netif_addr_lists_reconcile(dev, &uc_snap, &mc_snap,
+ &uc_ref, &mc_ref);
+ netif_addr_unlock_bh(dev);
+ } else if (ops->ndo_set_rx_mode) {
+ netif_addr_lock_bh(dev);
+ ops->ndo_set_rx_mode(dev);
+ netif_addr_unlock_bh(dev);
+ }
}
static void netdev_rx_mode_work(struct work_struct *work)
@@ -1312,6 +1353,7 @@ static void netif_rx_mode_queue(struct net_device *dev)
void __dev_set_rx_mode(struct net_device *dev)
{
const struct net_device_ops *ops = dev->netdev_ops;
+ int promisc_inc;
/* dev_open will call this function so the list will stay sane. */
if (!(dev->flags & IFF_UP))
@@ -1320,20 +1362,16 @@ void __dev_set_rx_mode(struct net_device *dev)
if (!netif_device_present(dev))
return;
- if (ops->ndo_set_rx_mode_async) {
+ if (ops->ndo_set_rx_mode_async || ops->ndo_change_rx_flags) {
netif_rx_mode_queue(dev);
return;
}
- if (!(dev->priv_flags & IFF_UNICAST_FLT)) {
- if (!netdev_uc_empty(dev) && !dev->uc_promisc) {
- __dev_set_promiscuity(dev, 1, false);
- dev->uc_promisc = true;
- } else if (netdev_uc_empty(dev) && dev->uc_promisc) {
- __dev_set_promiscuity(dev, -1, false);
- dev->uc_promisc = false;
- }
- }
+ /* Legacy path for non-ops-locked HW devices. */
+
+ promisc_inc = netif_uc_promisc_update(dev);
+ if (promisc_inc)
+ __dev_set_promiscuity(dev, promisc_inc, false);
if (ops->ndo_set_rx_mode)
ops->ndo_set_rx_mode(dev);
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v7 03/15] net: cache snapshot entries for ndo_set_rx_mode_async
From: Stanislav Fomichev @ 2026-04-13 17:11 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni
In-Reply-To: <20260413171131.550126-1-sdf@fomichev.me>
Add a per-device netdev_hw_addr_list cache (rx_mode_addr_cache) that
allows __hw_addr_list_snapshot() and __hw_addr_list_reconcile() to
reuse previously allocated entries instead of hitting GFP_ATOMIC on
every snapshot cycle.
snapshot pops entries from the cache when available, falling back to
__hw_addr_create(). reconcile splices both snapshot lists back into
the cache via __hw_addr_splice(). The cache is flushed in
free_netdev().
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
(cherry picked from commit ba3ab1832a511f660fdc6231245b14bf610c05bd)
---
include/linux/netdevice.h | 7 ++--
net/core/dev.c | 3 ++
net/core/dev_addr_lists.c | 66 ++++++++++++++++++++++++----------
net/core/dev_addr_lists_test.c | 60 +++++++++++++++++++++----------
4 files changed, 97 insertions(+), 39 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c2d46664f6a1..202193a44e77 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1919,6 +1919,7 @@ enum netdev_reg_state {
* does not implement ndo_set_rx_mode()
* @rx_mode_node: List entry for rx_mode work processing
* @rx_mode_tracker: Refcount tracker for rx_mode work
+ * @rx_mode_addr_cache: Recycled snapshot entries for rx_mode work
* @uc: unicast mac addresses
* @mc: multicast mac addresses
* @dev_addrs: list of device hw addresses
@@ -2312,6 +2313,7 @@ struct net_device {
bool uc_promisc;
struct list_head rx_mode_node;
netdevice_tracker rx_mode_tracker;
+ struct netdev_hw_addr_list rx_mode_addr_cache;
#ifdef CONFIG_LOCKDEP
unsigned char nested_level;
#endif
@@ -5025,10 +5027,11 @@ void __hw_addr_init(struct netdev_hw_addr_list *list);
void __hw_addr_flush(struct netdev_hw_addr_list *list);
int __hw_addr_list_snapshot(struct netdev_hw_addr_list *snap,
const struct netdev_hw_addr_list *list,
- int addr_len);
+ int addr_len, struct netdev_hw_addr_list *cache);
void __hw_addr_list_reconcile(struct netdev_hw_addr_list *real_list,
struct netdev_hw_addr_list *work,
- struct netdev_hw_addr_list *ref, int addr_len);
+ struct netdev_hw_addr_list *ref, int addr_len,
+ struct netdev_hw_addr_list *cache);
/* Functions used for device addresses handling */
void dev_addr_mod(struct net_device *dev, unsigned int offset,
diff --git a/net/core/dev.c b/net/core/dev.c
index b37061238a25..8597ec56fd64 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -12088,6 +12088,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
mutex_init(&dev->lock);
INIT_LIST_HEAD(&dev->rx_mode_node);
+ __hw_addr_init(&dev->rx_mode_addr_cache);
dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
setup(dev);
@@ -12192,6 +12193,8 @@ void free_netdev(struct net_device *dev)
kfree(rcu_dereference_protected(dev->ingress_queue, 1));
+ __hw_addr_flush(&dev->rx_mode_addr_cache);
+
/* Flush device addresses */
dev_addr_flush(dev);
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 477392127e8a..88e995db15dd 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -511,30 +511,50 @@ void __hw_addr_init(struct netdev_hw_addr_list *list)
}
EXPORT_SYMBOL(__hw_addr_init);
+static void __hw_addr_splice(struct netdev_hw_addr_list *dst,
+ struct netdev_hw_addr_list *src)
+{
+ src->tree = RB_ROOT;
+ list_splice_init(&src->list, &dst->list);
+ dst->count += src->count;
+ src->count = 0;
+}
+
/**
* __hw_addr_list_snapshot - create a snapshot copy of an address list
* @snap: destination snapshot list (needs to be __hw_addr_init-initialized)
* @list: source address list to snapshot
* @addr_len: length of addresses
+ * @cache: entry cache to reuse entries from; falls back to GFP_ATOMIC
*
- * Creates a copy of @list with individually allocated entries suitable
- * for use with __hw_addr_sync_dev() and other list manipulation helpers.
- * Each entry is allocated with GFP_ATOMIC; must be called under a spinlock.
+ * Creates a copy of @list reusing entries from @cache when available.
+ * Must be called under a spinlock.
*
* Return: 0 on success, -errno on failure.
*/
int __hw_addr_list_snapshot(struct netdev_hw_addr_list *snap,
const struct netdev_hw_addr_list *list,
- int addr_len)
+ int addr_len, struct netdev_hw_addr_list *cache)
{
struct netdev_hw_addr *ha, *entry;
list_for_each_entry(ha, &list->list, list) {
- entry = __hw_addr_create(ha->addr, addr_len, ha->type,
- false, false);
- if (!entry) {
- __hw_addr_flush(snap);
- return -ENOMEM;
+ if (cache->count) {
+ entry = list_first_entry(&cache->list,
+ struct netdev_hw_addr, list);
+ list_del(&entry->list);
+ cache->count--;
+ memcpy(entry->addr, ha->addr, addr_len);
+ entry->type = ha->type;
+ entry->global_use = false;
+ entry->synced = 0;
+ } else {
+ entry = __hw_addr_create(ha->addr, addr_len, ha->type,
+ false, false);
+ if (!entry) {
+ __hw_addr_flush(snap);
+ return -ENOMEM;
+ }
}
entry->sync_cnt = ha->sync_cnt;
entry->refcount = ha->refcount;
@@ -554,15 +574,17 @@ EXPORT_SYMBOL_IF_KUNIT(__hw_addr_list_snapshot);
* @work: the working snapshot (modified by driver via __hw_addr_sync_dev)
* @ref: the reference snapshot (untouched copy of original state)
* @addr_len: length of addresses
+ * @cache: entry cache to return snapshot entries to for reuse
*
* Walks the reference snapshot and compares each entry against the work
* snapshot to compute sync_cnt deltas. Applies those deltas to @real_list.
- * Frees both snapshots when done.
+ * Returns snapshot entries to @cache for reuse; frees both snapshots.
* Caller must hold netif_addr_lock_bh.
*/
void __hw_addr_list_reconcile(struct netdev_hw_addr_list *real_list,
struct netdev_hw_addr_list *work,
- struct netdev_hw_addr_list *ref, int addr_len)
+ struct netdev_hw_addr_list *ref, int addr_len,
+ struct netdev_hw_addr_list *cache)
{
struct netdev_hw_addr *ref_ha, *tmp, *work_ha, *real_ha;
int delta;
@@ -611,8 +633,8 @@ void __hw_addr_list_reconcile(struct netdev_hw_addr_list *real_list,
}
}
- __hw_addr_flush(work);
- __hw_addr_flush(ref);
+ __hw_addr_splice(cache, work);
+ __hw_addr_splice(cache, ref);
}
EXPORT_SYMBOL_IF_KUNIT(__hw_addr_list_reconcile);
@@ -1173,14 +1195,18 @@ static int netif_addr_lists_snapshot(struct net_device *dev,
{
int err;
- err = __hw_addr_list_snapshot(uc_snap, &dev->uc, dev->addr_len);
+ err = __hw_addr_list_snapshot(uc_snap, &dev->uc, dev->addr_len,
+ &dev->rx_mode_addr_cache);
if (!err)
- err = __hw_addr_list_snapshot(uc_ref, &dev->uc, dev->addr_len);
+ err = __hw_addr_list_snapshot(uc_ref, &dev->uc, dev->addr_len,
+ &dev->rx_mode_addr_cache);
if (!err)
err = __hw_addr_list_snapshot(mc_snap, &dev->mc,
- dev->addr_len);
+ dev->addr_len,
+ &dev->rx_mode_addr_cache);
if (!err)
- err = __hw_addr_list_snapshot(mc_ref, &dev->mc, dev->addr_len);
+ err = __hw_addr_list_snapshot(mc_ref, &dev->mc, dev->addr_len,
+ &dev->rx_mode_addr_cache);
if (err) {
__hw_addr_flush(uc_snap);
@@ -1197,8 +1223,10 @@ static void netif_addr_lists_reconcile(struct net_device *dev,
struct netdev_hw_addr_list *uc_ref,
struct netdev_hw_addr_list *mc_ref)
{
- __hw_addr_list_reconcile(&dev->uc, uc_snap, uc_ref, dev->addr_len);
- __hw_addr_list_reconcile(&dev->mc, mc_snap, mc_ref, dev->addr_len);
+ __hw_addr_list_reconcile(&dev->uc, uc_snap, uc_ref, dev->addr_len,
+ &dev->rx_mode_addr_cache);
+ __hw_addr_list_reconcile(&dev->mc, mc_snap, mc_ref, dev->addr_len,
+ &dev->rx_mode_addr_cache);
}
static void netif_rx_mode_run(struct net_device *dev)
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index fba926d5ec0d..260e71a2399f 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -251,8 +251,8 @@ static void dev_addr_test_add_excl(struct kunit *test)
*/
static void dev_addr_test_snapshot_sync(struct kunit *test)
{
+ struct netdev_hw_addr_list snap, ref, cache;
struct net_device *netdev = test->priv;
- struct netdev_hw_addr_list snap, ref;
struct dev_addr_test_priv *datp;
struct netdev_hw_addr *ha;
u8 addr[ETH_ALEN];
@@ -268,10 +268,13 @@ static void dev_addr_test_snapshot_sync(struct kunit *test)
netif_addr_lock_bh(netdev);
__hw_addr_init(&snap);
__hw_addr_init(&ref);
+ __hw_addr_init(&cache);
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN,
+ &cache));
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN,
+ &cache));
netif_addr_unlock_bh(netdev);
/* Driver syncs ADDR_A to hardware */
@@ -283,7 +286,8 @@ static void dev_addr_test_snapshot_sync(struct kunit *test)
/* Reconcile: delta=+1 applied to real entry */
netif_addr_lock_bh(netdev);
- __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN,
+ &cache);
netif_addr_unlock_bh(netdev);
/* Real entry should now reflect the sync: sync_cnt=1, refcount=2 */
@@ -301,6 +305,7 @@ static void dev_addr_test_snapshot_sync(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
+ __hw_addr_flush(&cache);
rtnl_unlock();
}
@@ -310,8 +315,8 @@ static void dev_addr_test_snapshot_sync(struct kunit *test)
*/
static void dev_addr_test_snapshot_remove_during_sync(struct kunit *test)
{
+ struct netdev_hw_addr_list snap, ref, cache;
struct net_device *netdev = test->priv;
- struct netdev_hw_addr_list snap, ref;
struct dev_addr_test_priv *datp;
struct netdev_hw_addr *ha;
u8 addr[ETH_ALEN];
@@ -327,10 +332,13 @@ static void dev_addr_test_snapshot_remove_during_sync(struct kunit *test)
netif_addr_lock_bh(netdev);
__hw_addr_init(&snap);
__hw_addr_init(&ref);
+ __hw_addr_init(&cache);
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN,
+ &cache));
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN,
+ &cache));
netif_addr_unlock_bh(netdev);
/* Driver syncs ADDR_A to hardware */
@@ -349,7 +357,8 @@ static void dev_addr_test_snapshot_remove_during_sync(struct kunit *test)
* so it gets re-inserted as stale (sync_cnt=1, refcount=1).
*/
netif_addr_lock_bh(netdev);
- __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN,
+ &cache);
netif_addr_unlock_bh(netdev);
KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
@@ -366,6 +375,7 @@ static void dev_addr_test_snapshot_remove_during_sync(struct kunit *test)
KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_unsynced);
KUNIT_EXPECT_EQ(test, 0, netdev->uc.count);
+ __hw_addr_flush(&cache);
rtnl_unlock();
}
@@ -376,8 +386,8 @@ static void dev_addr_test_snapshot_remove_during_sync(struct kunit *test)
*/
static void dev_addr_test_snapshot_readd_during_unsync(struct kunit *test)
{
+ struct netdev_hw_addr_list snap, ref, cache;
struct net_device *netdev = test->priv;
- struct netdev_hw_addr_list snap, ref;
struct dev_addr_test_priv *datp;
struct netdev_hw_addr *ha;
u8 addr[ETH_ALEN];
@@ -403,10 +413,13 @@ static void dev_addr_test_snapshot_readd_during_unsync(struct kunit *test)
netif_addr_lock_bh(netdev);
__hw_addr_init(&snap);
__hw_addr_init(&ref);
+ __hw_addr_init(&cache);
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN,
+ &cache));
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN,
+ &cache));
netif_addr_unlock_bh(netdev);
/* Driver unsyncs stale ADDR_A from hardware */
@@ -426,7 +439,8 @@ static void dev_addr_test_snapshot_readd_during_unsync(struct kunit *test)
* applied. Result: sync_cnt=0, refcount=1 (fresh).
*/
netif_addr_lock_bh(netdev);
- __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN,
+ &cache);
netif_addr_unlock_bh(netdev);
/* Entry survives as fresh: needs re-sync to HW */
@@ -443,6 +457,7 @@ static void dev_addr_test_snapshot_readd_during_unsync(struct kunit *test)
KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_synced);
KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+ __hw_addr_flush(&cache);
rtnl_unlock();
}
@@ -452,8 +467,8 @@ static void dev_addr_test_snapshot_readd_during_unsync(struct kunit *test)
*/
static void dev_addr_test_snapshot_add_and_remove(struct kunit *test)
{
+ struct netdev_hw_addr_list snap, ref, cache;
struct net_device *netdev = test->priv;
- struct netdev_hw_addr_list snap, ref;
struct dev_addr_test_priv *datp;
struct netdev_hw_addr *ha;
u8 addr[ETH_ALEN];
@@ -480,10 +495,13 @@ static void dev_addr_test_snapshot_add_and_remove(struct kunit *test)
netif_addr_lock_bh(netdev);
__hw_addr_init(&snap);
__hw_addr_init(&ref);
+ __hw_addr_init(&cache);
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN,
+ &cache));
KUNIT_EXPECT_EQ(test, 0,
- __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN,
+ &cache));
netif_addr_unlock_bh(netdev);
/* Driver syncs snapshot: ADDR_C is new -> synced; A,B already synced */
@@ -502,7 +520,8 @@ static void dev_addr_test_snapshot_add_and_remove(struct kunit *test)
* so nothing to apply to ADDR_B.
*/
netif_addr_lock_bh(netdev);
- __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN,
+ &cache);
netif_addr_unlock_bh(netdev);
/* ADDR_A: unchanged (sync_cnt=1, refcount=2)
@@ -536,13 +555,14 @@ static void dev_addr_test_snapshot_add_and_remove(struct kunit *test)
KUNIT_EXPECT_EQ(test, 1 << ADDR_B, datp->addr_unsynced);
KUNIT_EXPECT_EQ(test, 2, netdev->uc.count);
+ __hw_addr_flush(&cache);
rtnl_unlock();
}
static void dev_addr_test_snapshot_benchmark(struct kunit *test)
{
struct net_device *netdev = test->priv;
- struct netdev_hw_addr_list snap;
+ struct netdev_hw_addr_list snap, cache;
u8 addr[ETH_ALEN];
s64 duration = 0;
ktime_t start;
@@ -557,6 +577,8 @@ static void dev_addr_test_snapshot_benchmark(struct kunit *test)
KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
}
+ __hw_addr_init(&cache);
+
for (iter = 0; iter < 1000; iter++) {
netif_addr_lock_bh(netdev);
__hw_addr_init(&snap);
@@ -564,13 +586,15 @@ static void dev_addr_test_snapshot_benchmark(struct kunit *test)
start = ktime_get();
KUNIT_EXPECT_EQ(test, 0,
__hw_addr_list_snapshot(&snap, &netdev->uc,
- ETH_ALEN));
+ ETH_ALEN, &cache));
duration += ktime_to_ns(ktime_sub(ktime_get(), start));
netif_addr_unlock_bh(netdev);
__hw_addr_flush(&snap);
}
+ __hw_addr_flush(&cache);
+
kunit_info(test,
"1024 addrs x 1000 snapshots: %lld ns total, %lld ns/iter",
duration, div_s64(duration, 1000));
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v7 02/15] net: introduce ndo_set_rx_mode_async and netdev_rx_mode_work
From: Stanislav Fomichev @ 2026-04-13 17:11 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni
In-Reply-To: <20260413171131.550126-1-sdf@fomichev.me>
Add ndo_set_rx_mode_async callback that drivers can implement instead
of the legacy ndo_set_rx_mode. The legacy callback runs under the
netif_addr_lock spinlock with BHs disabled, preventing drivers from
sleeping. The async variant runs from a work queue with rtnl_lock and
netdev_lock_ops held, in fully sleepable context.
When __dev_set_rx_mode() sees ndo_set_rx_mode_async, it schedules
netdev_rx_mode_work instead of calling the driver inline. The work
function takes two snapshots of each address list (uc/mc) under
the addr_lock, then drops the lock and calls the driver with the
work copies. After the driver returns, it reconciles the snapshots
back to the real lists under the lock.
Add netif_rx_mode_sync() to opportunistically execute the pending
workqueue update inline, so that rx mode changes are committed
before returning to userspace:
- dev_change_flags (SIOCSIFFLAGS / RTM_NEWLINK)
- dev_set_promiscuity
- dev_set_allmulti
- dev_ifsioc SIOCADDMULTI / SIOCDELMULTI
- do_setlink (RTM_SETLINK)
Note that some deep hierarchies still do skip the lower updates via:
- dev_uc_sync
- dev_mc_sync
If we do end up hitting user-visible issues, we can add more calls to
netif_rx_mode_sync in specific places. But hopefully we should not,
the actual user-visible lists are still synced, it's that just HW state
that might be lagging.
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
Documentation/networking/netdevices.rst | 9 ++
include/linux/netdevice.h | 18 +++
net/core/dev.c | 43 +-----
net/core/dev.h | 3 +
net/core/dev_addr_lists.c | 194 ++++++++++++++++++++++++
net/core/dev_api.c | 3 +
net/core/dev_ioctl.c | 6 +-
net/core/rtnetlink.c | 1 +
8 files changed, 234 insertions(+), 43 deletions(-)
diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst
index 83e28b96884f..e89b12d4f3a7 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -289,6 +289,15 @@ struct net_device synchronization rules
ndo_set_rx_mode:
Synchronization: netif_addr_lock spinlock.
Context: BHs disabled
+ Notes: Deprecated in favor of ndo_set_rx_mode_async which runs
+ in process context.
+
+ndo_set_rx_mode_async:
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements queue management or shaper API.
+ Context: process (from a work queue)
+ Notes: Async version of ndo_set_rx_mode which runs in process
+ context. Receives snapshots of the unicast and multicast address lists.
ndo_setup_tc:
``TC_SETUP_BLOCK`` and ``TC_SETUP_FT`` are running under NFT locks
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4a804f552da4..c2d46664f6a1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1119,6 +1119,16 @@ struct netdev_net_notifier {
* This function is called device changes address list filtering.
* If driver handles unicast address filtering, it should set
* IFF_UNICAST_FLT in its priv_flags.
+ * Cannot sleep, called with netif_addr_lock_bh held.
+ * Deprecated in favor of ndo_set_rx_mode_async.
+ *
+ * void (*ndo_set_rx_mode_async)(struct net_device *dev,
+ * struct netdev_hw_addr_list *uc,
+ * struct netdev_hw_addr_list *mc);
+ * Async version of ndo_set_rx_mode which runs in process context
+ * with rtnl_lock and netdev_lock_ops(dev) held. The uc/mc parameters
+ * are snapshots of the address lists - iterate with
+ * netdev_hw_addr_list_for_each(ha, uc).
*
* int (*ndo_set_mac_address)(struct net_device *dev, void *addr);
* This function is called when the Media Access Control address
@@ -1439,6 +1449,10 @@ struct net_device_ops {
void (*ndo_change_rx_flags)(struct net_device *dev,
int flags);
void (*ndo_set_rx_mode)(struct net_device *dev);
+ void (*ndo_set_rx_mode_async)(
+ struct net_device *dev,
+ struct netdev_hw_addr_list *uc,
+ struct netdev_hw_addr_list *mc);
int (*ndo_set_mac_address)(struct net_device *dev,
void *addr);
int (*ndo_validate_addr)(struct net_device *dev);
@@ -1903,6 +1917,8 @@ enum netdev_reg_state {
* has been enabled due to the need to listen to
* additional unicast addresses in a device that
* does not implement ndo_set_rx_mode()
+ * @rx_mode_node: List entry for rx_mode work processing
+ * @rx_mode_tracker: Refcount tracker for rx_mode work
* @uc: unicast mac addresses
* @mc: multicast mac addresses
* @dev_addrs: list of device hw addresses
@@ -2294,6 +2310,8 @@ struct net_device {
unsigned int promiscuity;
unsigned int allmulti;
bool uc_promisc;
+ struct list_head rx_mode_node;
+ netdevice_tracker rx_mode_tracker;
#ifdef CONFIG_LOCKDEP
unsigned char nested_level;
#endif
diff --git a/net/core/dev.c b/net/core/dev.c
index e59f6025067c..b37061238a25 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9593,7 +9593,7 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
ops->ndo_change_rx_flags(dev, flags);
}
-static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
+int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
{
unsigned int old_flags = dev->flags;
unsigned int promiscuity, flags;
@@ -9697,46 +9697,6 @@ int netif_set_allmulti(struct net_device *dev, int inc, bool notify)
return 0;
}
-/*
- * Upload unicast and multicast address lists to device and
- * configure RX filtering. When the device doesn't support unicast
- * filtering it is put in promiscuous mode while unicast addresses
- * are present.
- */
-void __dev_set_rx_mode(struct net_device *dev)
-{
- const struct net_device_ops *ops = dev->netdev_ops;
-
- /* dev_open will call this function so the list will stay sane. */
- if (!(dev->flags&IFF_UP))
- return;
-
- if (!netif_device_present(dev))
- return;
-
- if (!(dev->priv_flags & IFF_UNICAST_FLT)) {
- /* Unicast addresses changes may only happen under the rtnl,
- * therefore calling __dev_set_promiscuity here is safe.
- */
- if (!netdev_uc_empty(dev) && !dev->uc_promisc) {
- __dev_set_promiscuity(dev, 1, false);
- dev->uc_promisc = true;
- } else if (netdev_uc_empty(dev) && dev->uc_promisc) {
- __dev_set_promiscuity(dev, -1, false);
- dev->uc_promisc = false;
- }
- }
-
- if (ops->ndo_set_rx_mode)
- ops->ndo_set_rx_mode(dev);
-}
-
-void dev_set_rx_mode(struct net_device *dev)
-{
- netif_addr_lock_bh(dev);
- __dev_set_rx_mode(dev);
- netif_addr_unlock_bh(dev);
-}
/**
* netif_get_flags() - get flags reported to userspace
@@ -12127,6 +12087,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
#endif
mutex_init(&dev->lock);
+ INIT_LIST_HEAD(&dev->rx_mode_node);
dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
setup(dev);
diff --git a/net/core/dev.h b/net/core/dev.h
index 585b6d7e88df..0cf24b8f5008 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -165,6 +165,9 @@ int netif_change_carrier(struct net_device *dev, bool new_carrier);
int dev_change_carrier(struct net_device *dev, bool new_carrier);
void __dev_set_rx_mode(struct net_device *dev);
+int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify);
+bool netif_rx_mode_clean(struct net_device *dev);
+void netif_rx_mode_sync(struct net_device *dev);
void __dev_notify_flags(struct net_device *dev, unsigned int old_flags,
unsigned int gchanges, u32 portid,
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index bb4851bc55ce..477392127e8a 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -11,10 +11,18 @@
#include <linux/rtnetlink.h>
#include <linux/export.h>
#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
#include <kunit/visibility.h>
#include "dev.h"
+static void netdev_rx_mode_work(struct work_struct *work);
+
+static LIST_HEAD(rx_mode_list);
+static DEFINE_SPINLOCK(rx_mode_lock);
+static DECLARE_WORK(rx_mode_work, netdev_rx_mode_work);
+
/*
* General list handling functions
*/
@@ -1156,3 +1164,189 @@ void dev_mc_init(struct net_device *dev)
__hw_addr_init(&dev->mc);
}
EXPORT_SYMBOL(dev_mc_init);
+
+static int netif_addr_lists_snapshot(struct net_device *dev,
+ struct netdev_hw_addr_list *uc_snap,
+ struct netdev_hw_addr_list *mc_snap,
+ struct netdev_hw_addr_list *uc_ref,
+ struct netdev_hw_addr_list *mc_ref)
+{
+ int err;
+
+ err = __hw_addr_list_snapshot(uc_snap, &dev->uc, dev->addr_len);
+ if (!err)
+ err = __hw_addr_list_snapshot(uc_ref, &dev->uc, dev->addr_len);
+ if (!err)
+ err = __hw_addr_list_snapshot(mc_snap, &dev->mc,
+ dev->addr_len);
+ if (!err)
+ err = __hw_addr_list_snapshot(mc_ref, &dev->mc, dev->addr_len);
+
+ if (err) {
+ __hw_addr_flush(uc_snap);
+ __hw_addr_flush(uc_ref);
+ __hw_addr_flush(mc_snap);
+ }
+
+ return err;
+}
+
+static void netif_addr_lists_reconcile(struct net_device *dev,
+ struct netdev_hw_addr_list *uc_snap,
+ struct netdev_hw_addr_list *mc_snap,
+ struct netdev_hw_addr_list *uc_ref,
+ struct netdev_hw_addr_list *mc_ref)
+{
+ __hw_addr_list_reconcile(&dev->uc, uc_snap, uc_ref, dev->addr_len);
+ __hw_addr_list_reconcile(&dev->mc, mc_snap, mc_ref, dev->addr_len);
+}
+
+static void netif_rx_mode_run(struct net_device *dev)
+{
+ struct netdev_hw_addr_list uc_snap, mc_snap, uc_ref, mc_ref;
+ const struct net_device_ops *ops = dev->netdev_ops;
+ int err;
+
+ might_sleep();
+ netdev_ops_assert_locked(dev);
+
+ __hw_addr_init(&uc_snap);
+ __hw_addr_init(&mc_snap);
+ __hw_addr_init(&uc_ref);
+ __hw_addr_init(&mc_ref);
+
+ if (!(dev->flags & IFF_UP) || !netif_device_present(dev))
+ return;
+
+ netif_addr_lock_bh(dev);
+ err = netif_addr_lists_snapshot(dev, &uc_snap, &mc_snap,
+ &uc_ref, &mc_ref);
+ if (err) {
+ netdev_WARN(dev, "failed to sync uc/mc addresses\n");
+ netif_addr_unlock_bh(dev);
+ return;
+ }
+ netif_addr_unlock_bh(dev);
+
+ ops->ndo_set_rx_mode_async(dev, &uc_snap, &mc_snap);
+
+ netif_addr_lock_bh(dev);
+ netif_addr_lists_reconcile(dev, &uc_snap, &mc_snap,
+ &uc_ref, &mc_ref);
+ netif_addr_unlock_bh(dev);
+}
+
+static void netdev_rx_mode_work(struct work_struct *work)
+{
+ struct net_device *dev;
+
+ rtnl_lock();
+
+ while (true) {
+ spin_lock_bh(&rx_mode_lock);
+ if (list_empty(&rx_mode_list)) {
+ spin_unlock_bh(&rx_mode_lock);
+ break;
+ }
+ dev = list_first_entry(&rx_mode_list, struct net_device,
+ rx_mode_node);
+ list_del_init(&dev->rx_mode_node);
+ spin_unlock_bh(&rx_mode_lock);
+
+ netdev_lock_ops(dev);
+ netif_rx_mode_run(dev);
+ netdev_unlock_ops(dev);
+ netdev_put(dev, &dev->rx_mode_tracker);
+ }
+
+ rtnl_unlock();
+}
+
+static void netif_rx_mode_queue(struct net_device *dev)
+{
+ spin_lock_bh(&rx_mode_lock);
+ if (list_empty(&dev->rx_mode_node)) {
+ list_add_tail(&dev->rx_mode_node, &rx_mode_list);
+ netdev_hold(dev, &dev->rx_mode_tracker, GFP_ATOMIC);
+ }
+ spin_unlock_bh(&rx_mode_lock);
+ schedule_work(&rx_mode_work);
+}
+
+/**
+ * __dev_set_rx_mode() - upload unicast and multicast address lists to device
+ * and configure RX filtering.
+ * @dev: device
+ *
+ * When the device doesn't support unicast filtering it is put in promiscuous
+ * mode while unicast addresses are present.
+ */
+void __dev_set_rx_mode(struct net_device *dev)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+
+ /* dev_open will call this function so the list will stay sane. */
+ if (!(dev->flags & IFF_UP))
+ return;
+
+ if (!netif_device_present(dev))
+ return;
+
+ if (ops->ndo_set_rx_mode_async) {
+ netif_rx_mode_queue(dev);
+ return;
+ }
+
+ if (!(dev->priv_flags & IFF_UNICAST_FLT)) {
+ if (!netdev_uc_empty(dev) && !dev->uc_promisc) {
+ __dev_set_promiscuity(dev, 1, false);
+ dev->uc_promisc = true;
+ } else if (netdev_uc_empty(dev) && dev->uc_promisc) {
+ __dev_set_promiscuity(dev, -1, false);
+ dev->uc_promisc = false;
+ }
+ }
+
+ if (ops->ndo_set_rx_mode)
+ ops->ndo_set_rx_mode(dev);
+}
+
+void dev_set_rx_mode(struct net_device *dev)
+{
+ netif_addr_lock_bh(dev);
+ __dev_set_rx_mode(dev);
+ netif_addr_unlock_bh(dev);
+}
+
+bool netif_rx_mode_clean(struct net_device *dev)
+{
+ bool clean = false;
+
+ spin_lock_bh(&rx_mode_lock);
+ if (!list_empty(&dev->rx_mode_node)) {
+ list_del_init(&dev->rx_mode_node);
+ clean = true;
+ }
+ spin_unlock_bh(&rx_mode_lock);
+
+ return clean;
+}
+
+/**
+ * netif_rx_mode_sync() - sync rx mode inline
+ * @dev: network device
+ *
+ * Drivers implementing ndo_set_rx_mode_async() have their rx mode callback
+ * executed from a workqueue. This allows the callback to sleep, but means
+ * the hardware update is deferred and may not be visible to userspace
+ * by the time the initiating syscall returns. netif_rx_mode_sync() steals
+ * workqueue update and executes it inline. This preserves the atomicity of
+ * operations to the userspace.
+ */
+void netif_rx_mode_sync(struct net_device *dev)
+{
+ if (netif_rx_mode_clean(dev)) {
+ netif_rx_mode_run(dev);
+ netdev_put(dev, &dev->rx_mode_tracker);
+ }
+}
diff --git a/net/core/dev_api.c b/net/core/dev_api.c
index f28852078aa6..437947dd08ed 100644
--- a/net/core/dev_api.c
+++ b/net/core/dev_api.c
@@ -66,6 +66,7 @@ int dev_change_flags(struct net_device *dev, unsigned int flags,
netdev_lock_ops(dev);
ret = netif_change_flags(dev, flags, extack);
+ netif_rx_mode_sync(dev);
netdev_unlock_ops(dev);
return ret;
@@ -285,6 +286,7 @@ int dev_set_promiscuity(struct net_device *dev, int inc)
netdev_lock_ops(dev);
ret = netif_set_promiscuity(dev, inc);
+ netif_rx_mode_sync(dev);
netdev_unlock_ops(dev);
return ret;
@@ -311,6 +313,7 @@ int dev_set_allmulti(struct net_device *dev, int inc)
netdev_lock_ops(dev);
ret = netif_set_allmulti(dev, inc, true);
+ netif_rx_mode_sync(dev);
netdev_unlock_ops(dev);
return ret;
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 7a8966544c9d..f3979b276090 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -586,24 +586,26 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, void __user *data,
return err;
case SIOCADDMULTI:
- if (!ops->ndo_set_rx_mode ||
+ if ((!ops->ndo_set_rx_mode && !ops->ndo_set_rx_mode_async) ||
ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
return -EINVAL;
if (!netif_device_present(dev))
return -ENODEV;
netdev_lock_ops(dev);
err = dev_mc_add_global(dev, ifr->ifr_hwaddr.sa_data);
+ netif_rx_mode_sync(dev);
netdev_unlock_ops(dev);
return err;
case SIOCDELMULTI:
- if (!ops->ndo_set_rx_mode ||
+ if ((!ops->ndo_set_rx_mode && !ops->ndo_set_rx_mode_async) ||
ifr->ifr_hwaddr.sa_family != AF_UNSPEC)
return -EINVAL;
if (!netif_device_present(dev))
return -ENODEV;
netdev_lock_ops(dev);
err = dev_mc_del_global(dev, ifr->ifr_hwaddr.sa_data);
+ netif_rx_mode_sync(dev);
netdev_unlock_ops(dev);
return err;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 69daba3ddaf0..b613bb6e07df 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3431,6 +3431,7 @@ static int do_setlink(const struct sk_buff *skb, struct net_device *dev,
dev->name);
}
+ netif_rx_mode_sync(dev);
netdev_unlock_ops(dev);
return err;
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v7 01/15] net: add address list snapshot and reconciliation infrastructure
From: Stanislav Fomichev @ 2026-04-13 17:11 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni, Aleksandr Loktionov
In-Reply-To: <20260413171131.550126-1-sdf@fomichev.me>
Introduce __hw_addr_list_snapshot() and __hw_addr_list_reconcile()
for use by the upcoming ndo_set_rx_mode_async callback.
The async rx_mode path needs to snapshot the device's unicast and
multicast address lists under the addr_lock, hand those snapshots
to the driver (which may sleep), and then propagate any sync_cnt
changes back to the real lists. Two identical snapshots are taken:
a work copy for the driver to pass to __hw_addr_sync_dev() and a
reference copy to compute deltas against.
__hw_addr_list_reconcile() walks the reference snapshot comparing
each entry against the work snapshot to determine what the driver
synced or unsynced. It then applies those deltas to the real list,
handling concurrent modifications:
- If the real entry was concurrently removed but the driver synced
it to hardware (delta > 0), re-insert a stale entry so the next
work run properly unsyncs it from hardware.
- If the entry still exists, apply the delta normally. An entry
whose refcount drops to zero is removed.
# dev_addr_test_snapshot_benchmark: 1024 addrs x 1000 snapshots: 89872802 ns total, 89872 ns/iter
# dev_addr_test_snapshot_benchmark.speed: slow
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
include/linux/netdevice.h | 7 +
net/core/dev.h | 1 +
net/core/dev_addr_lists.c | 109 +++++++++-
net/core/dev_addr_lists_test.c | 363 ++++++++++++++++++++++++++++++++-
4 files changed, 477 insertions(+), 3 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 47417b2d48a4..4a804f552da4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -5004,6 +5004,13 @@ void __hw_addr_unsync_dev(struct netdev_hw_addr_list *list,
int (*unsync)(struct net_device *,
const unsigned char *));
void __hw_addr_init(struct netdev_hw_addr_list *list);
+void __hw_addr_flush(struct netdev_hw_addr_list *list);
+int __hw_addr_list_snapshot(struct netdev_hw_addr_list *snap,
+ const struct netdev_hw_addr_list *list,
+ int addr_len);
+void __hw_addr_list_reconcile(struct netdev_hw_addr_list *real_list,
+ struct netdev_hw_addr_list *work,
+ struct netdev_hw_addr_list *ref, int addr_len);
/* Functions used for device addresses handling */
void dev_addr_mod(struct net_device *dev, unsigned int offset,
diff --git a/net/core/dev.h b/net/core/dev.h
index 628bdaebf0ca..585b6d7e88df 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -78,6 +78,7 @@ void linkwatch_run_queue(void);
void dev_addr_flush(struct net_device *dev);
int dev_addr_init(struct net_device *dev);
void dev_addr_check(struct net_device *dev);
+void __hw_addr_flush(struct netdev_hw_addr_list *list);
#if IS_ENABLED(CONFIG_NET_SHAPER)
void net_shaper_flush_netdev(struct net_device *dev);
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 76c91f224886..bb4851bc55ce 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -11,6 +11,7 @@
#include <linux/rtnetlink.h>
#include <linux/export.h>
#include <linux/list.h>
+#include <kunit/visibility.h>
#include "dev.h"
@@ -481,7 +482,7 @@ void __hw_addr_unsync_dev(struct netdev_hw_addr_list *list,
}
EXPORT_SYMBOL(__hw_addr_unsync_dev);
-static void __hw_addr_flush(struct netdev_hw_addr_list *list)
+void __hw_addr_flush(struct netdev_hw_addr_list *list)
{
struct netdev_hw_addr *ha, *tmp;
@@ -492,6 +493,7 @@ static void __hw_addr_flush(struct netdev_hw_addr_list *list)
}
list->count = 0;
}
+EXPORT_SYMBOL_IF_KUNIT(__hw_addr_flush);
void __hw_addr_init(struct netdev_hw_addr_list *list)
{
@@ -501,6 +503,111 @@ void __hw_addr_init(struct netdev_hw_addr_list *list)
}
EXPORT_SYMBOL(__hw_addr_init);
+/**
+ * __hw_addr_list_snapshot - create a snapshot copy of an address list
+ * @snap: destination snapshot list (needs to be __hw_addr_init-initialized)
+ * @list: source address list to snapshot
+ * @addr_len: length of addresses
+ *
+ * Creates a copy of @list with individually allocated entries suitable
+ * for use with __hw_addr_sync_dev() and other list manipulation helpers.
+ * Each entry is allocated with GFP_ATOMIC; must be called under a spinlock.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+int __hw_addr_list_snapshot(struct netdev_hw_addr_list *snap,
+ const struct netdev_hw_addr_list *list,
+ int addr_len)
+{
+ struct netdev_hw_addr *ha, *entry;
+
+ list_for_each_entry(ha, &list->list, list) {
+ entry = __hw_addr_create(ha->addr, addr_len, ha->type,
+ false, false);
+ if (!entry) {
+ __hw_addr_flush(snap);
+ return -ENOMEM;
+ }
+ entry->sync_cnt = ha->sync_cnt;
+ entry->refcount = ha->refcount;
+
+ list_add_tail(&entry->list, &snap->list);
+ __hw_addr_insert(snap, entry, addr_len);
+ snap->count++;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_IF_KUNIT(__hw_addr_list_snapshot);
+
+/**
+ * __hw_addr_list_reconcile - sync snapshot changes back and free snapshots
+ * @real_list: the real address list to update
+ * @work: the working snapshot (modified by driver via __hw_addr_sync_dev)
+ * @ref: the reference snapshot (untouched copy of original state)
+ * @addr_len: length of addresses
+ *
+ * Walks the reference snapshot and compares each entry against the work
+ * snapshot to compute sync_cnt deltas. Applies those deltas to @real_list.
+ * Frees both snapshots when done.
+ * Caller must hold netif_addr_lock_bh.
+ */
+void __hw_addr_list_reconcile(struct netdev_hw_addr_list *real_list,
+ struct netdev_hw_addr_list *work,
+ struct netdev_hw_addr_list *ref, int addr_len)
+{
+ struct netdev_hw_addr *ref_ha, *tmp, *work_ha, *real_ha;
+ int delta;
+
+ list_for_each_entry_safe(ref_ha, tmp, &ref->list, list) {
+ work_ha = __hw_addr_lookup(work, ref_ha->addr, addr_len,
+ ref_ha->type);
+ if (work_ha)
+ delta = work_ha->sync_cnt - ref_ha->sync_cnt;
+ else
+ delta = -1;
+
+ if (delta == 0)
+ continue;
+
+ real_ha = __hw_addr_lookup(real_list, ref_ha->addr, addr_len,
+ ref_ha->type);
+ if (!real_ha) {
+ /* The real entry was concurrently removed. If the
+ * driver synced this addr to hardware (delta > 0),
+ * re-insert it as a stale entry so the next work
+ * run unsyncs it from hardware.
+ */
+ if (delta > 0) {
+ rb_erase(&ref_ha->node, &ref->tree);
+ list_del(&ref_ha->list);
+ ref->count--;
+ ref_ha->sync_cnt = delta;
+ ref_ha->refcount = delta;
+ list_add_tail_rcu(&ref_ha->list,
+ &real_list->list);
+ __hw_addr_insert(real_list, ref_ha,
+ addr_len);
+ real_list->count++;
+ }
+ continue;
+ }
+
+ real_ha->sync_cnt += delta;
+ real_ha->refcount += delta;
+ if (!real_ha->refcount) {
+ rb_erase(&real_ha->node, &real_list->tree);
+ list_del_rcu(&real_ha->list);
+ kfree_rcu(real_ha, rcu_head);
+ real_list->count--;
+ }
+ }
+
+ __hw_addr_flush(work);
+ __hw_addr_flush(ref);
+}
+EXPORT_SYMBOL_IF_KUNIT(__hw_addr_list_reconcile);
+
/*
* Device addresses handling functions
*/
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 8e1dba825e94..fba926d5ec0d 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -2,22 +2,31 @@
#include <kunit/test.h>
#include <linux/etherdevice.h>
+#include <linux/math64.h>
#include <linux/netdevice.h>
#include <linux/rtnetlink.h>
static const struct net_device_ops dummy_netdev_ops = {
};
+#define ADDR_A 1
+#define ADDR_B 2
+#define ADDR_C 3
+
struct dev_addr_test_priv {
u32 addr_seen;
+ u32 addr_synced;
+ u32 addr_unsynced;
};
static int dev_addr_test_sync(struct net_device *netdev, const unsigned char *a)
{
struct dev_addr_test_priv *datp = netdev_priv(netdev);
- if (a[0] < 31 && !memchr_inv(a, a[0], ETH_ALEN))
+ if (a[0] < 31 && !memchr_inv(a, a[0], ETH_ALEN)) {
datp->addr_seen |= 1 << a[0];
+ datp->addr_synced |= 1 << a[0];
+ }
return 0;
}
@@ -26,11 +35,22 @@ static int dev_addr_test_unsync(struct net_device *netdev,
{
struct dev_addr_test_priv *datp = netdev_priv(netdev);
- if (a[0] < 31 && !memchr_inv(a, a[0], ETH_ALEN))
+ if (a[0] < 31 && !memchr_inv(a, a[0], ETH_ALEN)) {
datp->addr_seen &= ~(1 << a[0]);
+ datp->addr_unsynced |= 1 << a[0];
+ }
return 0;
}
+static void dev_addr_test_reset(struct net_device *netdev)
+{
+ struct dev_addr_test_priv *datp = netdev_priv(netdev);
+
+ datp->addr_seen = 0;
+ datp->addr_synced = 0;
+ datp->addr_unsynced = 0;
+}
+
static int dev_addr_test_init(struct kunit *test)
{
struct dev_addr_test_priv *datp;
@@ -225,6 +245,339 @@ static void dev_addr_test_add_excl(struct kunit *test)
rtnl_unlock();
}
+/* Snapshot test: basic sync with no concurrent modifications.
+ * Add one address, snapshot, driver syncs it, reconcile propagates
+ * sync_cnt delta back to real list.
+ */
+static void dev_addr_test_snapshot_sync(struct kunit *test)
+{
+ struct net_device *netdev = test->priv;
+ struct netdev_hw_addr_list snap, ref;
+ struct dev_addr_test_priv *datp;
+ struct netdev_hw_addr *ha;
+ u8 addr[ETH_ALEN];
+
+ datp = netdev_priv(netdev);
+
+ rtnl_lock();
+
+ memset(addr, ADDR_A, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+
+ /* Snapshot: ADDR_A has sync_cnt=0, refcount=1 (new) */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_init(&snap);
+ __hw_addr_init(&ref);
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ netif_addr_unlock_bh(netdev);
+
+ /* Driver syncs ADDR_A to hardware */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&snap, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+
+ /* Reconcile: delta=+1 applied to real entry */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ netif_addr_unlock_bh(netdev);
+
+ /* Real entry should now reflect the sync: sync_cnt=1, refcount=2 */
+ KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
+ ha = list_first_entry(&netdev->uc.list, struct netdev_hw_addr, list);
+ KUNIT_EXPECT_MEMEQ(test, ha->addr, addr, ETH_ALEN);
+ KUNIT_EXPECT_EQ(test, 1, ha->sync_cnt);
+ KUNIT_EXPECT_EQ(test, 2, ha->refcount);
+
+ /* Second work run: already synced, nothing to do */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&netdev->uc, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+ KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
+
+ rtnl_unlock();
+}
+
+/* Snapshot test: ADDR_A synced to hardware, then concurrently removed
+ * from the real list before reconcile runs. Reconcile re-inserts ADDR_A as
+ * a stale entry so the next work run unsyncs it from hardware.
+ */
+static void dev_addr_test_snapshot_remove_during_sync(struct kunit *test)
+{
+ struct net_device *netdev = test->priv;
+ struct netdev_hw_addr_list snap, ref;
+ struct dev_addr_test_priv *datp;
+ struct netdev_hw_addr *ha;
+ u8 addr[ETH_ALEN];
+
+ datp = netdev_priv(netdev);
+
+ rtnl_lock();
+
+ memset(addr, ADDR_A, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+
+ /* Snapshot: ADDR_A is new (sync_cnt=0, refcount=1) */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_init(&snap);
+ __hw_addr_init(&ref);
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ netif_addr_unlock_bh(netdev);
+
+ /* Driver syncs ADDR_A to hardware */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&snap, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+
+ /* Concurrent removal: user deletes ADDR_A while driver was working */
+ memset(addr, ADDR_A, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_del(netdev, addr));
+ KUNIT_EXPECT_EQ(test, 0, netdev->uc.count);
+
+ /* Reconcile: ADDR_A gone from real list but driver synced it,
+ * so it gets re-inserted as stale (sync_cnt=1, refcount=1).
+ */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ netif_addr_unlock_bh(netdev);
+
+ KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
+ ha = list_first_entry(&netdev->uc.list, struct netdev_hw_addr, list);
+ KUNIT_EXPECT_MEMEQ(test, ha->addr, addr, ETH_ALEN);
+ KUNIT_EXPECT_EQ(test, 1, ha->sync_cnt);
+ KUNIT_EXPECT_EQ(test, 1, ha->refcount);
+
+ /* Second work run: stale entry gets unsynced from HW and removed */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&netdev->uc, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_unsynced);
+ KUNIT_EXPECT_EQ(test, 0, netdev->uc.count);
+
+ rtnl_unlock();
+}
+
+/* Snapshot test: ADDR_A was stale (unsynced from hardware by driver),
+ * but concurrently re-added by the user. The re-add bumps refcount of
+ * the existing stale entry. Reconcile applies delta=-1, leaving ADDR_A
+ * as a fresh entry (sync_cnt=0, refcount=1) for the next work run.
+ */
+static void dev_addr_test_snapshot_readd_during_unsync(struct kunit *test)
+{
+ struct net_device *netdev = test->priv;
+ struct netdev_hw_addr_list snap, ref;
+ struct dev_addr_test_priv *datp;
+ struct netdev_hw_addr *ha;
+ u8 addr[ETH_ALEN];
+
+ datp = netdev_priv(netdev);
+
+ rtnl_lock();
+
+ memset(addr, ADDR_A, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+
+ /* Sync ADDR_A to hardware: sync_cnt=1, refcount=2 */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&netdev->uc, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+
+ /* User removes ADDR_A: refcount=1, sync_cnt=1 -> stale */
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_del(netdev, addr));
+
+ /* Snapshot: ADDR_A is stale (sync_cnt=1, refcount=1) */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_init(&snap);
+ __hw_addr_init(&ref);
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ netif_addr_unlock_bh(netdev);
+
+ /* Driver unsyncs stale ADDR_A from hardware */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&snap, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_unsynced);
+
+ /* Concurrent: user re-adds ADDR_A. dev_uc_add finds the existing
+ * stale entry and bumps refcount from 1 -> 2. sync_cnt stays 1.
+ */
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+ KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
+
+ /* Reconcile: ref sync_cnt=1 matches real sync_cnt=1, delta=-1
+ * applied. Result: sync_cnt=0, refcount=1 (fresh).
+ */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ netif_addr_unlock_bh(netdev);
+
+ /* Entry survives as fresh: needs re-sync to HW */
+ KUNIT_EXPECT_EQ(test, 1, netdev->uc.count);
+ ha = list_first_entry(&netdev->uc.list, struct netdev_hw_addr, list);
+ KUNIT_EXPECT_MEMEQ(test, ha->addr, addr, ETH_ALEN);
+ KUNIT_EXPECT_EQ(test, 0, ha->sync_cnt);
+ KUNIT_EXPECT_EQ(test, 1, ha->refcount);
+
+ /* Second work run: fresh entry gets synced to HW */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&netdev->uc, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_A, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+
+ rtnl_unlock();
+}
+
+/* Snapshot test: ADDR_A is new (synced by driver), and independent ADDR_B
+ * is concurrently removed from the real list. A's sync delta propagates
+ * normally; B's absence doesn't interfere.
+ */
+static void dev_addr_test_snapshot_add_and_remove(struct kunit *test)
+{
+ struct net_device *netdev = test->priv;
+ struct netdev_hw_addr_list snap, ref;
+ struct dev_addr_test_priv *datp;
+ struct netdev_hw_addr *ha;
+ u8 addr[ETH_ALEN];
+
+ datp = netdev_priv(netdev);
+
+ rtnl_lock();
+
+ /* Add ADDR_A and ADDR_B (will be synced then removed) */
+ memset(addr, ADDR_A, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+ memset(addr, ADDR_B, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+
+ /* Sync both to hardware: sync_cnt=1, refcount=2 */
+ __hw_addr_sync_dev(&netdev->uc, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+
+ /* Add ADDR_C (new, will be synced by snapshot) */
+ memset(addr, ADDR_C, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+
+ /* Snapshot: A,B synced (sync_cnt=1,refcount=2); C new (0,1) */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_init(&snap);
+ __hw_addr_init(&ref);
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&snap, &netdev->uc, ETH_ALEN));
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&ref, &netdev->uc, ETH_ALEN));
+ netif_addr_unlock_bh(netdev);
+
+ /* Driver syncs snapshot: ADDR_C is new -> synced; A,B already synced */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&snap, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_C, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_unsynced);
+
+ /* Concurrent: user removes addr B while driver was working */
+ memset(addr, ADDR_B, sizeof(addr));
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_del(netdev, addr));
+
+ /* Reconcile: ADDR_C's delta=+1 applied to real list.
+ * ADDR_B's delta=0 (unchanged in snapshot),
+ * so nothing to apply to ADDR_B.
+ */
+ netif_addr_lock_bh(netdev);
+ __hw_addr_list_reconcile(&netdev->uc, &snap, &ref, ETH_ALEN);
+ netif_addr_unlock_bh(netdev);
+
+ /* ADDR_A: unchanged (sync_cnt=1, refcount=2)
+ * ADDR_B: refcount went from 2->1 via dev_uc_del (still present, stale)
+ * ADDR_C: sync propagated (sync_cnt=1, refcount=2)
+ */
+ KUNIT_EXPECT_EQ(test, 3, netdev->uc.count);
+ netdev_hw_addr_list_for_each(ha, &netdev->uc) {
+ u8 id = ha->addr[0];
+
+ if (!memchr_inv(ha->addr, id, ETH_ALEN)) {
+ if (id == ADDR_A) {
+ KUNIT_EXPECT_EQ(test, 1, ha->sync_cnt);
+ KUNIT_EXPECT_EQ(test, 2, ha->refcount);
+ } else if (id == ADDR_B) {
+ /* B: still present but now stale */
+ KUNIT_EXPECT_EQ(test, 1, ha->sync_cnt);
+ KUNIT_EXPECT_EQ(test, 1, ha->refcount);
+ } else if (id == ADDR_C) {
+ KUNIT_EXPECT_EQ(test, 1, ha->sync_cnt);
+ KUNIT_EXPECT_EQ(test, 2, ha->refcount);
+ }
+ }
+ }
+
+ /* Second work run: ADDR_B is stale, gets unsynced and removed */
+ dev_addr_test_reset(netdev);
+ __hw_addr_sync_dev(&netdev->uc, netdev, dev_addr_test_sync,
+ dev_addr_test_unsync);
+ KUNIT_EXPECT_EQ(test, 0, datp->addr_synced);
+ KUNIT_EXPECT_EQ(test, 1 << ADDR_B, datp->addr_unsynced);
+ KUNIT_EXPECT_EQ(test, 2, netdev->uc.count);
+
+ rtnl_unlock();
+}
+
+static void dev_addr_test_snapshot_benchmark(struct kunit *test)
+{
+ struct net_device *netdev = test->priv;
+ struct netdev_hw_addr_list snap;
+ u8 addr[ETH_ALEN];
+ s64 duration = 0;
+ ktime_t start;
+ int i, iter;
+
+ rtnl_lock();
+
+ for (i = 0; i < 1024; i++) {
+ memset(addr, 0, sizeof(addr));
+ addr[0] = (i >> 8) & 0xff;
+ addr[1] = i & 0xff;
+ KUNIT_EXPECT_EQ(test, 0, dev_uc_add(netdev, addr));
+ }
+
+ for (iter = 0; iter < 1000; iter++) {
+ netif_addr_lock_bh(netdev);
+ __hw_addr_init(&snap);
+
+ start = ktime_get();
+ KUNIT_EXPECT_EQ(test, 0,
+ __hw_addr_list_snapshot(&snap, &netdev->uc,
+ ETH_ALEN));
+ duration += ktime_to_ns(ktime_sub(ktime_get(), start));
+
+ netif_addr_unlock_bh(netdev);
+ __hw_addr_flush(&snap);
+ }
+
+ kunit_info(test,
+ "1024 addrs x 1000 snapshots: %lld ns total, %lld ns/iter",
+ duration, div_s64(duration, 1000));
+
+ rtnl_unlock();
+}
+
static struct kunit_case dev_addr_test_cases[] = {
KUNIT_CASE(dev_addr_test_basic),
KUNIT_CASE(dev_addr_test_sync_one),
@@ -232,6 +585,11 @@ static struct kunit_case dev_addr_test_cases[] = {
KUNIT_CASE(dev_addr_test_del_main),
KUNIT_CASE(dev_addr_test_add_set),
KUNIT_CASE(dev_addr_test_add_excl),
+ KUNIT_CASE(dev_addr_test_snapshot_sync),
+ KUNIT_CASE(dev_addr_test_snapshot_remove_during_sync),
+ KUNIT_CASE(dev_addr_test_snapshot_readd_during_unsync),
+ KUNIT_CASE(dev_addr_test_snapshot_add_and_remove),
+ KUNIT_CASE_SLOW(dev_addr_test_snapshot_benchmark),
{}
};
@@ -243,5 +601,6 @@ static struct kunit_suite dev_addr_test_suite = {
};
kunit_test_suite(dev_addr_test_suite);
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
MODULE_DESCRIPTION("KUnit tests for struct netdev_hw_addr_list");
MODULE_LICENSE("GPL");
--
2.52.0
^ permalink raw reply related
* [PATCH net-next v7 00/15] net: sleepable ndo_set_rx_mode
From: Stanislav Fomichev @ 2026-04-13 17:11 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni
This series adds a new ndo_set_rx_mode_async callback that enables
drivers to handle address list updates in a sleepable context. The
current ndo_set_rx_mode is called under the netif_addr_lock spinlock
with BHs disabled, which prevents drivers from sleeping. This is
problematic for ops-locked drivers that need to sleep.
The approach:
1. Add snapshot/reconcile infrastructure for address lists
2. Introduce dev_rx_mode_work that takes snapshots under the lock,
drops the lock, calls the driver, then reconciles changes back
3. Move promiscuity handling into the scheduled work as well
4. Convert existing ops-locked drivers to ndo_set_rx_mode_async
5. Add a warning for ops-locked drivers still using ndo_set_rx_mode
6. Add a selftest exercising the team+bridge+macvlan topology that
triggers the addr_lock -> ops_lock ordering issue
v7:
- rebase and address netkit warning in a separate patch (Jakub)
- keep only CONFIG_NET_TEAM=y in selftest (Breno)
v6:
- relink ref_ha in __hw_addr_list_reconcile (AI Review)
- set real_ha->sync_cnt to delta, not 1 (AI Review)
- s/KUNIT_ASSERT_EQ/KUNIT_EXPECT_EQ/ (AI Review)
- drop netif_rx_mode_clean from free_netdev (AI Review)
- clarify deep hierarchy flush in commit message (AI Review)
- use dev trackers (AI Review)
- drop mc argument from bnxt_cfg_rx_mode (AI Review)
- keep uc_update, but as an argument in bnxt (AI Review)
- add BNXT_STATE_OPEN check after re-lock in bnxt (AI Review)
- add addr lock around iavf_set_rx_mode (AI Review)
v5:
- resolve 32 bit failure (Jakub)
v4:
- rebase on https://lore.kernel.org/netdev/20260319005456.82745-1-saeed@kernel.org/T/#u (Cosmin)
- reword ndo_set_rx_mode_async kdoc (Jakub)
- s/EXPORT_SYMBOL/EXPORT_SYMBOL_IF_KUNIT/ (Jakub)
- remove netif_up_and_present (Jakub)
- netif_addr_lists_snapshot + netif_addr_lists_reconcile to better
explain mix-and-match between
ndo_set_rx_mode/ndo_set_rx_mode_async/ndo_change_rx_flags (Jakub)
- s/cancel_work_sync/flush_work/ (Jakub)
- separate commit to cache snapshot entries (Jakub)
- add dev_addr_test_snapshot_benchmark (Jakub)
- dev_addr_test_snapshot_benchmark: 1024 addrs x 1000 snapshots: 89872802 ns total, 89872 ns/iter
- remove redundant bnxt_uc_list_updated (Michael)
- switch to linkwatch-like work stealing (Jakub)
v3:
- module_export(__rtnl_unlock) (nipa)
- s/netdev_uc_count/netdev_hw_addr_list_count/ in bnxt (Aleksandr)
v2:
- wifi: cfg80211: use __rtnl_unlock in nl80211_pre_doit (syzbot)
- simplify mlx5e_sync_netdev_addr for !uc (Cosmin)
- switch to snapshot in bnxt_cfg_rx_mode (Michael)
- add team to net/config (Jakub)
Stanislav Fomichev (15):
net: add address list snapshot and reconciliation infrastructure
net: introduce ndo_set_rx_mode_async and netdev_rx_mode_work
net: cache snapshot entries for ndo_set_rx_mode_async
net: move promiscuity handling into netdev_rx_mode_work
fbnic: convert to ndo_set_rx_mode_async
mlx5: convert to ndo_set_rx_mode_async
bnxt: convert to ndo_set_rx_mode_async
bnxt: use snapshot in bnxt_cfg_rx_mode
iavf: convert to ndo_set_rx_mode_async
netdevsim: convert to ndo_set_rx_mode_async
dummy: convert to ndo_set_rx_mode_async
netkit: convert to ndo_set_rx_mode_async
net: warn ops-locked drivers still using ndo_set_rx_mode
selftests: net: add team_bridge_macvlan rx_mode test
selftests: net: use ip commands instead of teamd in team rx_mode test
Documentation/networking/netdevices.rst | 13 +
drivers/net/dummy.c | 6 +-
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 58 +--
drivers/net/ethernet/intel/iavf/iavf_main.c | 16 +-
.../net/ethernet/mellanox/mlx5/core/en/fs.h | 5 +-
.../net/ethernet/mellanox/mlx5/core/en_fs.c | 32 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 13 +-
.../net/ethernet/meta/fbnic/fbnic_netdev.c | 20 +-
.../net/ethernet/meta/fbnic/fbnic_netdev.h | 4 +-
drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 4 +-
drivers/net/ethernet/meta/fbnic/fbnic_rpc.c | 2 +-
drivers/net/netdevsim/netdev.c | 8 +-
drivers/net/netkit.c | 6 +-
include/linux/netdevice.h | 28 ++
net/core/dev.c | 67 +--
net/core/dev.h | 4 +
net/core/dev_addr_lists.c | 370 ++++++++++++++++-
net/core/dev_addr_lists_test.c | 387 +++++++++++++++++-
net/core/dev_api.c | 3 +
net/core/dev_ioctl.c | 6 +-
net/core/rtnetlink.c | 1 +
.../selftests/drivers/net/bonding/lag_lib.sh | 17 +-
.../drivers/net/team/dev_addr_lists.sh | 2 -
tools/testing/selftests/net/config | 1 +
tools/testing/selftests/net/rtnetlink.sh | 44 ++
25 files changed, 977 insertions(+), 140 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCH net v3 2/5] bonding: 3ad: fix carrier when no valid slaves
From: Jay Vosburgh @ 2026-04-13 17:01 UTC (permalink / raw)
To: Louis Scalbert
Cc: netdev, andrew+netdev, edumazet, kuba, pabeni, fbl, andy,
shemminger, maheshb
In-Reply-To: <20260408152353.276204-3-louis.scalbert@6wind.com>
Louis Scalbert <louis.scalbert@6wind.com> wrote:
>Apply the "lacp_fallback" configuration from the previous commit.
>
>"lacp_fallback" mode "strict" asserts that the bonding master carrier
>only when at least 'min_links' slaves are in the collecting/distributing
>state (or collecting only if the coupled_control default behavior is
>disabled).
>
>Fixes: 655f8919d549 ("bonding: add min links parameter to 802.3ad")
>Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
>---
> drivers/net/bonding/bond_3ad.c | 26 ++++++++++++++++++++++++--
> drivers/net/bonding/bond_options.c | 1 +
> 2 files changed, 25 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index af7f74cfdc08..b79a76296966 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -745,6 +745,22 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
> }
> }
>
>+static int __agg_valid_ports(struct aggregator *agg)
>+{
>+ struct port *port;
>+ int valid = 0;
>+
>+ for (port = agg->lag_ports; port;
>+ port = port->next_port_in_aggregator) {
>+ if (port->actor_oper_port_state & LACP_STATE_COLLECTING &&
>+ (!port->slave->bond->params.coupled_control ||
>+ port->actor_oper_port_state & LACP_STATE_DISTRIBUTING))
>+ valid++;
Do we need to test coupled_control? I.e., can the test be
if ((port->actor_oper_port_state & LACP_STATE_COLLECTING) &&
(port->actor_oper_port_state & LACP_STATE_DISTRIBUTING))
To my reading, ad_mux_machine will set _COLLECTING and
_DISTRIBUTING appropriately regardless of the coupled_control selection.
>+ }
>+
>+ return valid;
>+}
>+
> static int __agg_active_ports(struct aggregator *agg)
> {
> struct port *port;
>@@ -2120,6 +2136,7 @@ static void ad_enable_collecting_distributing(struct port *port,
> port->actor_port_number,
> port->aggregator->aggregator_identifier);
> __enable_port(port);
>+ bond_3ad_set_carrier(port->slave->bond);
> /* Slave array needs update */
> *update_slave_arr = true;
> /* Should notify peers if possible */
>@@ -2141,6 +2158,7 @@ static void ad_disable_collecting_distributing(struct port *port,
> port->actor_port_number,
> port->aggregator->aggregator_identifier);
> __disable_port(port);
>+ bond_3ad_set_carrier(port->slave->bond);
> /* Slave array needs an update */
> *update_slave_arr = true;
> }
>@@ -2819,8 +2837,12 @@ int bond_3ad_set_carrier(struct bonding *bond)
> }
> active = __get_active_agg(&(SLAVE_AD_INFO(first_slave)->aggregator));
> if (active) {
>- /* are enough slaves available to consider link up? */
>- if (__agg_active_ports(active) < bond->params.min_links) {
>+ /* are enough slaves in collecting (and distributing) state to consider
>+ * link up?
>+ */
>+ if ((bond->params.lacp_fallback ? __agg_valid_ports(active)
>+ : __agg_active_ports(active)) <
>+ bond->params.min_links) {
I think the original comment is better; if the new option is
off, it doesn't require collecting / distributing state.
-J
> if (netif_carrier_ok(bond->dev)) {
> netif_carrier_off(bond->dev);
> goto out;
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index b672b8a881bb..d64a5d2f80b6 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -1706,6 +1706,7 @@ static int bond_option_lacp_fallback_set(struct bonding *bond,
> netdev_dbg(bond->dev, "Setting LACP fallback to %s (%llu)\n",
> newval->string, newval->value);
> bond->params.lacp_fallback = newval->value;
>+ bond_3ad_set_carrier(bond);
>
> return 0;
> }
>--
>2.39.2
>
---
-Jay Vosburgh, jv@jvosburgh.net
^ permalink raw reply
* Re: [PATCH net v3 3/5] bonding: 3ad: fix mux port state on oper down
From: Jay Vosburgh @ 2026-04-13 16:49 UTC (permalink / raw)
To: Louis Scalbert
Cc: netdev, andrew+netdev, edumazet, kuba, pabeni, fbl, andy,
shemminger, maheshb
In-Reply-To: <20260408152353.276204-4-louis.scalbert@6wind.com>
Louis Scalbert <louis.scalbert@6wind.com> wrote:
>When the bonding interface has carrier down due to the absence of
>valid slaves and a slave transitions from down to up, the bonding
>interface briefly goes carrier up, then down again, and finally up
>once LACP negotiates collecting and distributing on the port.
Instead of "valid," I would suggest "usable."
>The interface should not transition to carrier up until LACP
>negotiation is complete.
If the new option is off, i.e., does not require successful LACP
negotiation, it should wait some time before asserting carrier up. If
negotiation fails, however, how long should it wait?
>This happens because the actor and partner port states remain in
>collecting (and distributing) when the port goes down. When the port
>comes back up, it temporarily remains in this state until LACP
>renegotiation occurs.
>
>Previously this was mostly cosmetic, but since the bonding carrier
>state now depends on the LACP negotiation state, it causes the
>interface to flap.
"now depends" -> "may depend"
>Fix this by unsetting the SELECTED flag when a port goes down so that
>the mux state machine transitions through ATTACHED and DETACHED,
>which clears the actor collecting and distributing flags. Do not
>attempt to set the SELECTED flag if the port is still disabled.
>
>Fixes: 655f8919d549 ("bonding: add min links parameter to 802.3ad")
>Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
>---
> drivers/net/bonding/bond_3ad.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index b79a76296966..3a94fbcbf721 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -1570,6 +1570,12 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
> struct slave *slave;
> int found = 0;
>
>+ /* Disabled ports cannot be SELECTED.
>+ * Do not attempt to set the SELECTED flag if the port is still disabled.
>+ */
>+ if (!port->is_enabled)
>+ return;
>+
I think the change is fine, but the comment seems redundant to
me.
-J
> /* if the port is already Selected, do nothing */
> if (port->sm_vars & AD_PORT_SELECTED)
> return;
>@@ -2794,6 +2800,7 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
> /* link has failed */
> port->is_enabled = false;
> ad_update_actor_keys(port, true);
>+ port->sm_vars &= ~AD_PORT_SELECTED;
> }
> agg = __get_first_agg(port);
> ad_agg_selection_logic(agg, &dummy);
>--
>2.39.2
>
---
-Jay Vosburgh, jv@jvosburgh.net
^ permalink raw reply
* Re: [PATCH v2] Documentation: sysctl: document net core sysctls
From: Simon Horman @ 2026-04-13 16:47 UTC (permalink / raw)
To: Shubham Chakraborty
Cc: netdev, davem, edumazet, kuba, pabeni, kuniyu, corbet, skhan,
linux-doc, linux-kernel
In-Reply-To: <20260409174859.11854-1-chakrabortyshubham66@gmail.com>
On Thu, Apr 09, 2026 at 11:18:59PM +0530, Shubham Chakraborty wrote:
> Document missing net.core and net.unix sysctl entries in
> admin-guide/sysctl/net.rst, and correct wording for defaults
> that are derived from PAGE_SIZE, HZ, or CONFIG_MAX_SKB_FRAGS.
>
> Also clarify that the RFS and flow-limit controls are only present
> when CONFIG_RPS or CONFIG_NET_FLOW_LIMIT is enabled, and describe
> rps_sock_flow_entries the way the handler implements it: non-zero
> values are rounded up to the nearest power of two.
>
> Signed-off-by: Shubham Chakraborty <chakrabortyshubham66@gmail.com>
...
> @@ -238,6 +240,37 @@ rps_default_mask
> The default RPS CPU mask used on newly created network devices. An empty
> mask means RPS disabled by default.
>
> +rps_sock_flow_entries
> +---------------------
> +
> +The total number of entries in the RPS flow table. This is used by
Maybe s/This/The table/ to make it clearer that it is the table,
rather than the number of entries, that track CPUs.
> +RFS (Receive Flow Steering) to track which CPU is currently processing
> +a flow in userspace. Non-zero values are rounded up to the nearest
> +power of two.
> +Available only when ``CONFIG_RPS`` is enabled.
I think it would be worth noting that a value of 0 disables RPS.
> +
> +Default: 0
...
> netdev_budget_usecs
> ---------------------
>
The lines above the following hunk are:
netdev_budget_usecs
---------------------
Maximum number of microseconds in one NAPI polling cycle. Polling
> @@ -297,12 +332,16 @@ Maximum number of microseconds in one NAPI polling cycle. Polling
> will exit when either netdev_budget_usecs have elapsed during the
> poll cycle or the number of packets processed reaches netdev_budget.
>
> +Default: ``2 * USEC_PER_SEC / HZ`` (2000 when ``HZ`` is 1000)
> +
Well, that is awkward.
Looking at git history, it seems that this sysctl was added by 7acf8a1e8a28
("Replace 2 jiffies with sysctl netdev_budget_usecs to enable softirq
tuning") in 2017. And at that time the unic was us, and the default was 2000 us.
But that was changed by a fix for that commit, a4837980fd9f ("net: revert
default NAPI poll timeout to 2 jiffies"), in 2020. As a side-effect of
that commit, the default was changed to what you have documented above,
and the unit changed to jiffies.
So while what you have is correct it seems nonsensical to me for the unit
to be jiffies. Because that's not a meaningful unit for users. And because
the name of the sysctl ends in usecs.
But I'm unsure what to do about it. Since changing the unit this would
represent (another) KABI break.
* Add another knob that shadows this one (But what to call it?)
* Simply remove this one (KAPI break)
* Change the unit of this knob (KAPI break)
If the code is left as is, then I think it should be documented that the
unit is jiffies.
...
^ permalink raw reply
* Re: [PATCH net v3 1/5] bonding: 3ad: add lacp_fallback configuration knob
From: Jay Vosburgh @ 2026-04-13 16:45 UTC (permalink / raw)
To: Louis Scalbert
Cc: netdev, andrew+netdev, edumazet, kuba, pabeni, fbl, andy,
shemminger, maheshb
In-Reply-To: <20260408152353.276204-2-louis.scalbert@6wind.com>
Louis Scalbert <louis.scalbert@6wind.com> wrote:
>When an 802.3ad (LACP) bonding interface has no slaves in the
>collecting/distributing state, the bonding master still reports
>carrier as up as long as at least 'min_links' slaves have carrier.
>
>In this situation, only one slave is effectively used for TX/RX,
>while traffic received on other slaves is dropped. Upper-layer
>daemons therefore consider the interface operational, even though
>traffic may be blackholed if the lack of LACP negotiation means
>the partner is not ready to deal with traffic.
>
>Introduce a configuration knob to control this behavior. It allows
>the bonding master to assert carrier only when at least 'min_links'
>slaves are in collecting/distributing state (or collecting only
>when coupled_control is disabled).
>
>The default mode preserves the current (legacy) behavior. This
>patch only introduces the knob; its behavior is implemented in
>the subsequent commit.
>
>Fixes: 655f8919d549 ("bonding: add min links parameter to 802.3ad")
>Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
>---
> Documentation/networking/bonding.rst | 33 ++++++++++++++++++++++++++++
> drivers/net/bonding/bond_main.c | 1 +
> drivers/net/bonding/bond_netlink.c | 16 ++++++++++++++
> drivers/net/bonding/bond_options.c | 26 ++++++++++++++++++++++
> include/net/bond_options.h | 1 +
> include/net/bonding.h | 1 +
> include/uapi/linux/if_link.h | 1 +
> 7 files changed, 79 insertions(+)
>
>diff --git a/Documentation/networking/bonding.rst b/Documentation/networking/bonding.rst
>index e700bf1d095c..465d06aead27 100644
>--- a/Documentation/networking/bonding.rst
>+++ b/Documentation/networking/bonding.rst
>@@ -619,6 +619,39 @@ min_links
> aggregator cannot be active without at least one available link,
> setting this option to 0 or to 1 has the exact same effect.
>
>+lacp_fallback
>+
>+ Specifies the fallback behavior of a bonding when LACP negotiation fails on
>+ all slave links, i.e. when no slave is in the Collecting/Distributing state
>+ (or only in Collecting state when coupled_control is disabled), while at
>+ least `min_links` link still reports carrier up.
>+
>+ This option is only applicable to 802.3ad mode (mode 4).
>+
>+ Valid values are:
>+
>+ legacy or 0
>+ In this situation, the bonding master remains carrier up and
>+ randomly selects a single slave to transmit and receive traffic.
>+ Traffic received on other slaves is dropped.
>+
>+ This mode is deprecated, as it may lead to traffic blackholing
>+ when the absence of LACP negotiation means the partner is not
>+ ready to collect and distribute traffic.
>+
>+ This is the legacy default behavior.
>+
>+ strict or 1
>+ In this situation, the bonding master reports carrier down, allowing
>+ upper-layer processes to detect that the interface is not usable for
>+ collecting and distributing traffic.
>+
>+ The master transitions to carrier up only when at least
>+ `min_links` slaves reach the Collecting(/Distributing) state,
>+ allowing traffic to flow.
>+
>+ The default value is 0 (legacy).
>+
1- Please wrap text at approximately 75 columns.
2- I don't agree with the nomenclature or language of the above.
The existing behavior is not going to be deprecated or considered to be
legacy, and the option nomenclature should reflect that. I would
suggest naming the option "lacp_strict" and having it's possible
settings be on or off, with the default setting as off.
I think the behavior can be described along the lines of
off or 0
One interface of the bond is selected to be active, in
order to facilitate communication with peer devices that
do not implement LACP.
on or 1
Interfaces are only permitted to be made active if they
have an active LACP partner and have successfully reached
Collecting or Collecting_Distributing state.
The default is value is 0 (off).
-J
> mode
>
> Specifies one of the bonding policies. The default is
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index a5484d11553d..02cba0560a39 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -6440,6 +6440,7 @@ static int __init bond_check_params(struct bond_params *params)
> params->ad_user_port_key = ad_user_port_key;
> params->coupled_control = 1;
> params->broadcast_neighbor = 0;
>+ params->lacp_fallback = 0;
> if (packets_per_slave > 0) {
> params->reciprocal_packets_per_slave =
> reciprocal_value(packets_per_slave);
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 286f11c517f7..1f92ad786b51 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -130,6 +130,7 @@ static const struct nla_policy bond_policy[IFLA_BOND_MAX + 1] = {
> [IFLA_BOND_NS_IP6_TARGET] = { .type = NLA_NESTED },
> [IFLA_BOND_COUPLED_CONTROL] = { .type = NLA_U8 },
> [IFLA_BOND_BROADCAST_NEIGH] = { .type = NLA_U8 },
>+ [IFLA_BOND_LACP_FALLBACK] = { .type = NLA_U8 },
> };
>
> static const struct nla_policy bond_slave_policy[IFLA_BOND_SLAVE_MAX + 1] = {
>@@ -586,6 +587,16 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> return err;
> }
>
>+ if (data[IFLA_BOND_LACP_FALLBACK]) {
>+ int fallback_mode = nla_get_u8(data[IFLA_BOND_LACP_FALLBACK]);
>+
>+ bond_opt_initval(&newval, fallback_mode);
>+ err = __bond_opt_set(bond, BOND_OPT_LACP_FALLBACK, &newval,
>+ data[IFLA_BOND_LACP_FALLBACK], extack);
>+ if (err)
>+ return err;
>+ }
>+
> return 0;
> }
>
>@@ -658,6 +669,7 @@ static size_t bond_get_size(const struct net_device *bond_dev)
> nla_total_size(sizeof(struct in6_addr)) * BOND_MAX_NS_TARGETS +
> nla_total_size(sizeof(u8)) + /* IFLA_BOND_COUPLED_CONTROL */
> nla_total_size(sizeof(u8)) + /* IFLA_BOND_BROADCAST_NEIGH */
>+ nla_total_size(sizeof(u8)) + /* IFLA_BOND_LACP_FALLBACK */
> 0;
> }
>
>@@ -825,6 +837,10 @@ static int bond_fill_info(struct sk_buff *skb,
> bond->params.broadcast_neighbor))
> goto nla_put_failure;
>
>+ if (nla_put_u8(skb, IFLA_BOND_LACP_FALLBACK,
>+ bond->params.lacp_fallback))
>+ goto nla_put_failure;
>+
> if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> struct ad_info info;
>
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 7380cc4ee75a..b672b8a881bb 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -68,6 +68,8 @@ static int bond_option_lacp_active_set(struct bonding *bond,
> const struct bond_opt_value *newval);
> static int bond_option_lacp_rate_set(struct bonding *bond,
> const struct bond_opt_value *newval);
>+static int bond_option_lacp_fallback_set(struct bonding *bond,
>+ const struct bond_opt_value *newval);
> static int bond_option_ad_select_set(struct bonding *bond,
> const struct bond_opt_value *newval);
> static int bond_option_queue_id_set(struct bonding *bond,
>@@ -162,6 +164,12 @@ static const struct bond_opt_value bond_lacp_rate_tbl[] = {
> { NULL, -1, 0},
> };
>
>+static const struct bond_opt_value bond_lacp_fallback_tbl[] = {
>+ { "legacy", 0, BOND_VALFLAG_DEFAULT},
>+ { "strict", 1, 0},
>+ { NULL, -1, 0 }
>+};
>+
> static const struct bond_opt_value bond_ad_select_tbl[] = {
> { "stable", BOND_AD_STABLE, BOND_VALFLAG_DEFAULT},
> { "bandwidth", BOND_AD_BANDWIDTH, 0},
>@@ -363,6 +371,14 @@ static const struct bond_option bond_opts[BOND_OPT_LAST] = {
> .values = bond_lacp_rate_tbl,
> .set = bond_option_lacp_rate_set
> },
>+ [BOND_OPT_LACP_FALLBACK] = {
>+ .id = BOND_OPT_LACP_FALLBACK,
>+ .name = "lacp_fallback",
>+ .desc = "Define the LACP fallback mode when no slaves have negotiated",
>+ .unsuppmodes = BOND_MODE_ALL_EX(BIT(BOND_MODE_8023AD)),
>+ .values = bond_lacp_fallback_tbl,
>+ .set = bond_option_lacp_fallback_set
>+ },
> [BOND_OPT_MINLINKS] = {
> .id = BOND_OPT_MINLINKS,
> .name = "min_links",
>@@ -1684,6 +1700,16 @@ static int bond_option_lacp_rate_set(struct bonding *bond,
> return 0;
> }
>
>+static int bond_option_lacp_fallback_set(struct bonding *bond,
>+ const struct bond_opt_value *newval)
>+{
>+ netdev_dbg(bond->dev, "Setting LACP fallback to %s (%llu)\n",
>+ newval->string, newval->value);
>+ bond->params.lacp_fallback = newval->value;
>+
>+ return 0;
>+}
>+
> static int bond_option_ad_select_set(struct bonding *bond,
> const struct bond_opt_value *newval)
> {
>diff --git a/include/net/bond_options.h b/include/net/bond_options.h
>index e6eedf23aea1..5eb64c831f54 100644
>--- a/include/net/bond_options.h
>+++ b/include/net/bond_options.h
>@@ -79,6 +79,7 @@ enum {
> BOND_OPT_COUPLED_CONTROL,
> BOND_OPT_BROADCAST_NEIGH,
> BOND_OPT_ACTOR_PORT_PRIO,
>+ BOND_OPT_LACP_FALLBACK,
> BOND_OPT_LAST
> };
>
>diff --git a/include/net/bonding.h b/include/net/bonding.h
>index 395c6e281c5f..d8cb02643f8b 100644
>--- a/include/net/bonding.h
>+++ b/include/net/bonding.h
>@@ -132,6 +132,7 @@ struct bond_params {
> int peer_notif_delay;
> int lacp_active;
> int lacp_fast;
>+ int lacp_fallback;
> unsigned int min_links;
> int ad_select;
> char primary[IFNAMSIZ];
>diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
>index e9b5f79e1ee1..7ad3fc600c71 100644
>--- a/include/uapi/linux/if_link.h
>+++ b/include/uapi/linux/if_link.h
>@@ -1539,6 +1539,7 @@ enum {
> IFLA_BOND_NS_IP6_TARGET,
> IFLA_BOND_COUPLED_CONTROL,
> IFLA_BOND_BROADCAST_NEIGH,
>+ IFLA_BOND_LACP_FALLBACK,
> __IFLA_BOND_MAX,
> };
>
>--
>2.39.2
>
---
-Jay Vosburgh, jv@jvosburgh.net
^ permalink raw reply
* Re: [PATCH net v3 0/5] bonding: 3ad: fix carrier state with no valid slaves
From: Jay Vosburgh @ 2026-04-13 16:45 UTC (permalink / raw)
To: Louis Scalbert
Cc: netdev, andrew+netdev, edumazet, kuba, pabeni, fbl, andy,
shemminger, maheshb
In-Reply-To: <20260408152353.276204-1-louis.scalbert@6wind.com>
Louis Scalbert <louis.scalbert@6wind.com> wrote:
>Hi everyone,
>
>This series addresses a blackholing issue and a subsequent link-flapping
>issue in the 802.3ad bonding driver when dealing with inactive slaves
>and the `min_links` parameter.
>
>When an 802.3ad (LACP) bonding interface has no slaves in the
>collecting/distributing state, the bonding master still reports
>carrier as up as long as at least 'min_links' slaves have carrier.
>
>In this situation, only one slave is effectively used for TX/RX,
>while traffic received on other slaves is dropped. Upper-layer
>daemons therefore consider the interface operational, even though
>traffic may be blackholed if the lack of LACP negotiation means
>the partner is not ready to deal with traffic.
>
>The current behavior is not compliant with the LACP standard. This
>patchset introduces a working behavior that is not strictly
>standard-compliant either, but is widely adopted across the industry.
>It consists of bringing the bonding master interface down to signal to
>upper-layer processes that it is not usable.
As I've said, I believe that the current behavior is compliant
with the standard, as IEEE 802.1AX-2014 6.1.1.j, as we've discussed,
says that (to summarize) links that cannot participate in Link
Aggregation ... operate as normal, individual links. The mechanism by
which that takes place is not defined, and we, in my opinion, are not in
violation of the standard by selecting a bond member and making it
active.
>This patchset depends on the following iproute2 change:
>ip/bond: add lacp_fallback support
>
>Patch 1 introduces the lacp_fallback configuration knob, which is
>applied in the subsequent patch. The default (legacy) mode preserves
>the existing behavior, while the strict mode is intended to force the
>bonding master carrier down in this situation.
The above notwithstanding, I don't object in general to an
option that says, essentially, "require that only LACP-partnered ports
be made active."
Also, after reading through the patch set, I'm comfortable
calling the entire series a "fix," i.e., suitable for net vs net-next.
Most of the changes are genuine code fixes, and the addition of the
option is less a real feature and more of a change to better manage
connectivity in edge cases, so I'm fine with that being called a fix as
well.
-J
>Patch 2 addresses the core issue when lacp_fallback is set to strict.
>It ensures that carrier is asserted only when at least 'min_links'
>slaves are in a valid state (collecting/distributing, or collecting
>only when coupled_control is disabled).
>
>Patch 3 fixes a side effect of the first patch. Tightening the carrier
>logic exposes a state persistence bug: when a physical link goes down,
>the LACP collecting/distributing flags remain set. When the link returns,
>the interface briefly hallucinates that it is ready, bounces the carrier
>up, and then drops it again once LACP renegotiation starts. Unsetting the
>SELECTED flag when the link goes down forces the state machine through
>DETACHED, clearing the stale flags and preventing the flap.
>
>Patch 4 fixes a side effect of the second patch caused by clearing the
>SELECTED flag on disabled ports. After all ports in an aggregator go
>down, if only a subset of ports comes back up, those ports can no
>longer renegotiate LACP unless all aggregator ports come back up.
>
>Patch 5 adds a test for the bonding legacy and strict LACP fallback modes.
>
>Louis Scalbert (5):
> bonding: 3ad: add lacp_fallback configuration knob
> bonding: 3ad: fix carrier when no valid slaves
> bonding: 3ad: fix mux port state on oper down
> bonding: 3ad: fix stuck negotiation on recovery
> selftests: bonding: add test for fallback mode
>
> Documentation/networking/bonding.rst | 33 ++
> drivers/net/bonding/bond_3ad.c | 38 ++-
> drivers/net/bonding/bond_main.c | 1 +
> drivers/net/bonding/bond_netlink.c | 16 +
> drivers/net/bonding/bond_options.c | 27 ++
> include/net/bond_options.h | 1 +
> include/net/bonding.h | 1 +
> include/uapi/linux/if_link.h | 1 +
> .../selftests/drivers/net/bonding/Makefile | 1 +
> .../drivers/net/bonding/bond_lacp_fallback.sh | 299 ++++++++++++++++++
> 10 files changed, 415 insertions(+), 3 deletions(-)
> create mode 100755 tools/testing/selftests/drivers/net/bonding/bond_lacp_fallback.sh
>
>--
>2.39.2
>
---
-Jay Vosburgh, jv@jvosburgh.net
^ permalink raw reply
* [PATCH v1 net 1/1] net/sched: sch_dualpi2: fix limit/memlimit enforcement when dequeueing L-queue
From: chia-yu.chang @ 2026-04-13 16:37 UTC (permalink / raw)
To: linux-hardening, kees, gustavoars, jhs, jiri, davem, edumazet,
kuba, pabeni, linux-kernel, netdev, horms, ij, ncardwell,
koen.de_schepper, g.white, ingemar.s.johansson, mirja.kuehlewind,
cheshire, rs.ietf, Jason_Livingood, vidhi_goel
Cc: Chia-Yu Chang
From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
Fix dualpi2_change() to correctly enforce updated limit and memlimit values
after a configuration change of the dualpi2 qdisc.
Before this patch, dualpi2_change() always attempted to dequeue packets via
the root qdisc (C-queue) when reducing backlog or memory usage, and
unconditionally assumed that a valid skb will be returned. When traffic
classification results in packets being queued in the L-queue while the
C-queue is empty, this leads to a NULL skb dereference during limit or
memlimit enforcement.
This is fixed by first dequeuing from the C-queue path if it is non-empty.
Once the C-queue is empty, packets are dequeued directly from the L-queue.
Return values from qdisc_dequeue_internal() are checked for both queues. When
dequeuing from the L-queue, the parent qdisc qlen and backlog counters are
updated explicitly to keep overall qdisc statistics consistent.
Fixes: 320d031ad6e4 ("sched: Struct definition and parsing of dualpi2 qdisc")
Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
---
net/sched/sch_dualpi2.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index 6d7e6389758d..56d4422970b6 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -872,11 +872,25 @@ static int dualpi2_change(struct Qdisc *sch, struct nlattr *opt,
old_backlog = sch->qstats.backlog;
while (qdisc_qlen(sch) > sch->limit ||
q->memory_used > q->memory_limit) {
- struct sk_buff *skb = qdisc_dequeue_internal(sch, true);
-
- q->memory_used -= skb->truesize;
- qdisc_qstats_backlog_dec(sch, skb);
- rtnl_qdisc_drop(skb, sch);
+ int c_len = qdisc_qlen(sch) - qdisc_qlen(q->l_queue);
+ struct sk_buff *skb = NULL;
+
+ if (c_len) {
+ skb = qdisc_dequeue_internal(sch, true);
+ if (!skb)
+ break;
+ q->memory_used -= skb->truesize;
+ rtnl_qdisc_drop(skb, sch);
+ } else if (qdisc_qlen(q->l_queue)) {
+ skb = qdisc_dequeue_internal(q->l_queue, true);
+ if (!skb)
+ break;
+ q->memory_used -= skb->truesize;
+ rtnl_qdisc_drop(skb, q->l_queue);
+ /* Keep the overall qdisc stats consistent */
+ --sch->q.qlen;
+ qdisc_qstats_backlog_dec(sch, skb);
+ }
}
qdisc_tree_reduce_backlog(sch, old_qlen - qdisc_qlen(sch),
old_backlog - sch->qstats.backlog);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net] net: usb: cdc_ncm: reject negative chained NDP offsets
From: Bjørn Mork @ 2026-04-13 16:20 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Oliver Neukum, linux-usb, netdev, linux-kernel, Oliver Neukum,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, stable
In-Reply-To: <2026041355-designate-spiritual-e785@gregkh>
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Mon, Apr 13, 2026 at 02:11:50PM +0200, Oliver Neukum wrote:
>> On 13.04.26 12:43, Greg Kroah-Hartman wrote:
>> > On Mon, Apr 13, 2026 at 10:36:19AM +0200, Oliver Neukum wrote:
>> > >
>> > >
>> > > On 11.04.26 12:53, Greg Kroah-Hartman wrote:
>> > > > cdc_ncm_rx_fixup() reads dwNextNdpIndex from each NDP32 to chain to the
>> > > > next one. The 32-bit value from the device is stored into the signed
>> > > > int ndpoffset so that means values with the high bit set become
>> > >
>> > > Well, then isn't the problem rather that you should not store an
>> > > unsigned value in a signed variable?
>> >
>> > No. well, yes. but no.
>> >
>> > cdc_ncm_rx_verify_nth16() returns an int, and is negative if something
>> > went wrong, so we need it that way, and then we need to check it, like
>> > we properly do at the top of the loop, it's just that at the bottom of
>> > the loop we also need to do the same exact thing.
>>
>> Doesn't that suggest that cdc_ncm_rx_verify_nth16() is the problem?
>> To be precise, the way it indicates errors?
>> As this is an offset into a buffer and the header must be at the start
>> of the buffer, isn't 0 the natural indication of an error?
>
> Maybe? I really don't know, sorry, parsing the cdc_ncm buffer is not
> something I looked too deeply into :)
Oliver is correct AFAICS. These functions could use 0 to indicate
errors. This would make the code simpler and cleaner.
The negative error return is just a sloppy choice I made at a time we
only supported the 16bit versions. Didn't anticipate 32bit support
since it is optional and pointless. But as usual, hardware vendors do
surprising things.
Note that cdc_mbim.c must be updated if cdc_ncm_rx_verify_nth16() is
changed.
Bjørn
^ permalink raw reply
* Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler
From: Sebastian Andrzej Siewior @ 2026-04-13 16:10 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Marek Vasut, netdev, stable, David S. Miller, Andrew Lunn,
Eric Dumazet, Nicolai Buchwitz, Paolo Abeni, Ronald Wahl,
Yicong Hui, linux-kernel, Thomas Gleixner
In-Reply-To: <20260413084445.59fe28d6@kernel.org>
On 2026-04-13 08:44:45 [-0700], Jakub Kicinski wrote:
> On Mon, 13 Apr 2026 14:57:44 +0200 Sebastian Andrzej Siewior wrote:
> > On 2026-04-12 10:51:25 [-0700], Jakub Kicinski wrote:
> > > > Does the backtrace make the problem clearer, with the annotation above ?
> > >
> > > Sebastian, do you have any recommendation here? tl;dr is that the driver does
> > …
> >
> > What about this:
>
> Thanks for taking a look (according to you auto-reply immediately after
> a vacation ;))
;)
> TBH changing the driver feels like a workaround / invitation for a
> whack-a-mole game. I'd prefer to fix the skb allocation.
The problem is that _irq() implicitly disables bh processing but this
does not happen. Forcing this is possible but expensive.
However, I did remove lock from bh_disable() on RT.
Marek: from which kernel version was this backtrace?
> Is there any way we can check if any locks which were _irq() on non-RT
> are held?
lockdep has a list of locks which are acquired but it does not see if it
is _irq() or not. It only records it was acquired.
Sebastian
^ permalink raw reply
* Re: [PATCH bpf-next v2 1/1] bpf: Refactor dynptr mutability tracking
From: Mykyta Yatsenko @ 2026-04-13 16:05 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, memxor, eddyz87,
yatsenko, martin.lau, kernel-team
In-Reply-To: <20260402065013.884228-2-ameryhung@gmail.com>
On 4/2/26 7:50 AM, Amery Hung wrote:
> Redefine dynptr mutability and fix inconsistency in the verifier and
> kfunc signatures. Dynptr mutability is at two levels. The first is
> the bpf_dynptr structure and the second is the memory the dynptr points
> to. The verifer currently tracks the mutability of the bpf_dynptr struct
> through helper and kfunc prototypes, where "const struct bpf_dynptr *"
> means the structure itself is immutable. The second level is tracked
> in upper bit of bpf_dynptr->size in runtime and is not changed in this
> patch.
>
> There are two type of inconsistency in the verfier regarding the
> mutability of the bpf_dynptr struct. First, there are many existing
> kfuncs whose prototypes are wrong. For example, bpf_dynptr_adjust()
> mutates a dynptr's start and offset but marks the argument as a const
> pointer. At the same time many other kfuncs that does not mutate the
> dynptr but mark themselves as mutable. Second, the verifier currently
> does not honor the const qualifier in kfunc prototypes as it determines
> whether tagging the arg_type with MEM_RDONLY or not based on the register
> state.
>
> Since all the verifier care is to prevent CONST_PTR_TO_DYNPTR from
> being destroyed in callback and global subprogram, redefine the
> mutability at the bpf_dynptr level to just bpf_dynptr_kern->data. Then,
> explicitly prohibit passing CONST_PTR_TO_DYNPTR to an argument tagged
> with MEM_UNINIT or OBJ_RELEASE. The mutability of a dynptr's view is not
> really interesting so drop MEM_RDONLY annotation for dynptr from the
> helpers and kfuncs. Plus, if the mutability of the entire bpf_dynptr
> were to be done correctly, it would kill the bpf_dynptr_adjust() usage
> in callback and global subporgram.
>
> Implementation wise
>
> - First, make sure all kfunc arg are correctly tagged: Tag the dynptr
> argument of bpf_dynptr_file_discard() with OBJ_RELEASE.
> - Then, in process_dynptr_func(), make sure CONST_PTR_TO_DYNPTR cannot
> be passed to argument tagged with MEM_UNINIT or OBJ_RELEASE. For
> MEM_UNINIT, it is already checked by is_dynptr_reg_valid_uninit().
> For OBJ_RELEASE, check against OBJ_RELEASE instead of MEM_RDONLY and
> drop a now identical check in umark_stack_slots_dynptr().
> - Remove the mutual exclusive check between MEM_UNINIT and MEM_RDONLY,
> but don't add a MEM_UNINIT and OBJ_RELEASE version as it is obviously
> wrong.
>
> Note that while this patch stops following the C semantic for the
> mutability of bpf_dynptr, the prototype of kfuncs are still fixed to
> maintain the correct C semantics in the helper implementation. Adding or
> removing the const qualifier does not break backward compatibility.
>
> In test_kfunc_dynptr_param.c, initialize dynptr to 0 to avoid
> -Wuninitialized-const-pointer warning.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
> fs/verity/measure.c | 2 +-
> include/linux/bpf.h | 8 +--
> kernel/bpf/btf.c | 2 +-
> kernel/bpf/helpers.c | 18 ++---
> kernel/bpf/verifier.c | 68 +++++--------------
> kernel/trace/bpf_trace.c | 18 ++---
> tools/testing/selftests/bpf/bpf_kfuncs.h | 8 +--
> .../selftests/bpf/progs/dynptr_success.c | 6 +-
> .../bpf/progs/test_kfunc_dynptr_param.c | 9 +--
> 9 files changed, 51 insertions(+), 88 deletions(-)
>
> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> index 6a35623ebdf0..3840436e4510 100644
> --- a/fs/verity/measure.c
> +++ b/fs/verity/measure.c
> @@ -118,7 +118,7 @@ __bpf_kfunc_start_defs();
> *
> * Return: 0 on success, a negative value on error.
> */
> -__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr *digest_p)
> +__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, const struct bpf_dynptr *digest_p)
> {
> struct bpf_dynptr_kern *digest_ptr = (struct bpf_dynptr_kern *)digest_p;
maybe we can make this digest_ptr const as well, otherwise it's a little
bit strange to introduce const, but cast to non-const kernel struct
immediately. I think we can apply this in other kfuncs.
> const struct inode *inode = file_inode(file);
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 05b34a6355b0..329b78940b79 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3621,8 +3621,8 @@ static inline int bpf_fd_reuseport_array_update_elem(struct bpf_map *map,
> struct bpf_key *bpf_lookup_user_key(s32 serial, u64 flags);
> struct bpf_key *bpf_lookup_system_key(u64 id);
> void bpf_key_put(struct bpf_key *bkey);
> -int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p,
> - struct bpf_dynptr *sig_p,
> +int bpf_verify_pkcs7_signature(const struct bpf_dynptr *data_p,
> + const struct bpf_dynptr *sig_p,
> struct bpf_key *trusted_keyring);
>
> #else
...
> err = mark_stack_slots_dynptr(env, reg, arg_type, insn_idx, clone_ref_obj_id);
> - } else /* MEM_RDONLY and None case from above */ {
> + } else /* OBJ_RELEASE and None case from above */ {
> /* For the reg->type == PTR_TO_STACK case, bpf_dynptr is never const */
> - if (reg->type == CONST_PTR_TO_DYNPTR && !(arg_type & MEM_RDONLY)) {
> - verbose(env, "cannot pass pointer to const bpf_dynptr, the helper mutates it\n");
> + if (reg->type == CONST_PTR_TO_DYNPTR && (arg_type & OBJ_RELEASE)) {
> + verbose(env, "CONST_PTR_TO_DYNPTR cannot be released");
\n is missing in the verbose.
> return -EINVAL;
> }
>
> @@ -8958,7 +8929,7 @@ static int process_dynptr_func(struct bpf_verifier_env *env, int regno, int insn
> return -EINVAL;
> }
>
> - /* Fold modifiers (in this case, MEM_RDONLY) when checking expected type */
> + /* Fold modifiers (in this case, OBJ_RELEASE) when checking expected type */
> if (!is_dynptr_type_expected(env, reg, arg_type & ~MEM_RDONLY)) {
Do we need to update the `is_dynptr_type_expected(env, reg, arg_type &
~MEM_RDONLY)` as MEM_RDONLY is no longer applied to the dynptr?
> verbose(env,
> "Expected a dynptr of type %s as arg #%d\n",
> @@ -10803,7 +10774,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> bpf_log(log, "R%d is not a pointer to arena or scalar.\n", regno);
> return -EINVAL;
> }
> - } else if (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) {
> + } else if (arg->arg_type == ARG_PTR_TO_DYNPTR) {
> ret = check_func_arg_reg_off(env, reg, regno, ARG_PTR_TO_DYNPTR);
> if (ret)
> return ret;
> @@ -13718,9 +13689,6 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> enum bpf_arg_type dynptr_arg_type = ARG_PTR_TO_DYNPTR;
> int clone_ref_obj_id = 0;
>
> - if (reg->type == CONST_PTR_TO_DYNPTR)
> - dynptr_arg_type |= MEM_RDONLY;
> -
> if (is_kfunc_arg_uninit(btf, &args[i]))
> dynptr_arg_type |= MEM_UNINIT;
>
> @@ -13733,7 +13701,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
> } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_from_file]) {
> dynptr_arg_type |= DYNPTR_TYPE_FILE;
> } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_file_discard]) {
> - dynptr_arg_type |= DYNPTR_TYPE_FILE;
> + dynptr_arg_type |= DYNPTR_TYPE_FILE | OBJ_RELEASE;
> meta->release_regno = regno;
> } else if (meta->func_id == special_kfunc_list[KF_bpf_dynptr_clone] &&
> (dynptr_arg_type & MEM_UNINIT)) {
> @@ -24785,7 +24753,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
> } else if (arg->arg_type == ARG_ANYTHING) {
> reg->type = SCALAR_VALUE;
> mark_reg_unknown(env, regs, i);
> - } else if (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) {
> + } else if (arg->arg_type == ARG_PTR_TO_DYNPTR) {
> /* assume unspecial LOCAL dynptr type */
> __mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, ++env->id_gen);
> } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 0b040a417442..5f35ecdd5341 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -3393,7 +3393,7 @@ typedef int (*copy_fn_t)(void *dst, const void *src, u32 size, struct task_struc
> * direct calls into all the specific callback implementations
> * (copy_user_data_sleepable, copy_user_data_nofault, and so on)
> */
> -static __always_inline int __bpf_dynptr_copy_str(struct bpf_dynptr *dptr, u64 doff, u64 size,
> +static __always_inline int __bpf_dynptr_copy_str(const struct bpf_dynptr *dptr, u64 doff, u64 size,
> const void *unsafe_src,
> copy_fn_t str_copy_fn,
> struct task_struct *tsk)
> @@ -3535,49 +3535,49 @@ __bpf_kfunc int bpf_send_signal_task(struct task_struct *task, int sig, enum pid
> return bpf_send_signal_common(sig, type, task, value);
> }
>
> -__bpf_kfunc int bpf_probe_read_user_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_probe_read_user_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void __user *unsafe_ptr__ign)
> {
> return __bpf_dynptr_copy(dptr, off, size, (const void __force *)unsafe_ptr__ign,
> copy_user_data_nofault, NULL);
> }
>
> -__bpf_kfunc int bpf_probe_read_kernel_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void *unsafe_ptr__ign)
> {
> return __bpf_dynptr_copy(dptr, off, size, unsafe_ptr__ign,
> copy_kernel_data_nofault, NULL);
> }
>
> -__bpf_kfunc int bpf_probe_read_user_str_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_probe_read_user_str_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void __user *unsafe_ptr__ign)
> {
> return __bpf_dynptr_copy_str(dptr, off, size, (const void __force *)unsafe_ptr__ign,
> copy_user_str_nofault, NULL);
> }
>
> -__bpf_kfunc int bpf_probe_read_kernel_str_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_probe_read_kernel_str_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void *unsafe_ptr__ign)
> {
> return __bpf_dynptr_copy_str(dptr, off, size, unsafe_ptr__ign,
> copy_kernel_str_nofault, NULL);
> }
>
> -__bpf_kfunc int bpf_copy_from_user_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_copy_from_user_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void __user *unsafe_ptr__ign)
> {
> return __bpf_dynptr_copy(dptr, off, size, (const void __force *)unsafe_ptr__ign,
> copy_user_data_sleepable, NULL);
> }
>
> -__bpf_kfunc int bpf_copy_from_user_str_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_copy_from_user_str_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void __user *unsafe_ptr__ign)
> {
> return __bpf_dynptr_copy_str(dptr, off, size, (const void __force *)unsafe_ptr__ign,
> copy_user_str_sleepable, NULL);
> }
>
> -__bpf_kfunc int bpf_copy_from_user_task_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_copy_from_user_task_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void __user *unsafe_ptr__ign,
> struct task_struct *tsk)
> {
> @@ -3585,7 +3585,7 @@ __bpf_kfunc int bpf_copy_from_user_task_dynptr(struct bpf_dynptr *dptr, u64 off,
> copy_user_data_sleepable, tsk);
> }
>
> -__bpf_kfunc int bpf_copy_from_user_task_str_dynptr(struct bpf_dynptr *dptr, u64 off,
> +__bpf_kfunc int bpf_copy_from_user_task_str_dynptr(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void __user *unsafe_ptr__ign,
> struct task_struct *tsk)
> {
> diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h
> index 7dad01439391..ae71e9b69051 100644
> --- a/tools/testing/selftests/bpf/bpf_kfuncs.h
> +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h
> @@ -40,7 +40,7 @@ extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, __u64 offset,
> extern void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr *ptr, __u64 offset, void *buffer,
> __u64 buffer__szk) __ksym __weak;
>
> -extern int bpf_dynptr_adjust(const struct bpf_dynptr *ptr, __u64 start, __u64 end) __ksym __weak;
> +extern int bpf_dynptr_adjust(struct bpf_dynptr *ptr, __u64 start, __u64 end) __ksym __weak;
> extern bool bpf_dynptr_is_null(const struct bpf_dynptr *ptr) __ksym __weak;
> extern bool bpf_dynptr_is_rdonly(const struct bpf_dynptr *ptr) __ksym __weak;
> extern __u64 bpf_dynptr_size(const struct bpf_dynptr *ptr) __ksym __weak;
> @@ -70,13 +70,13 @@ extern void *bpf_rdonly_cast(const void *obj, __u32 btf_id) __ksym __weak;
>
> extern int bpf_get_file_xattr(struct file *file, const char *name,
> struct bpf_dynptr *value_ptr) __ksym;
> -extern int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr *digest_ptr) __ksym;
> +extern int bpf_get_fsverity_digest(struct file *file, const struct bpf_dynptr *digest_ptr) __ksym;
>
> extern struct bpf_key *bpf_lookup_user_key(__s32 serial, __u64 flags) __ksym;
> extern struct bpf_key *bpf_lookup_system_key(__u64 id) __ksym;
> extern void bpf_key_put(struct bpf_key *key) __ksym;
> -extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,
> - struct bpf_dynptr *sig_ptr,
> +extern int bpf_verify_pkcs7_signature(const struct bpf_dynptr *data_ptr,
> + const struct bpf_dynptr *sig_ptr,
> struct bpf_key *trusted_keyring) __ksym;
>
> struct dentry;
> diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
> index e0d672d93adf..e0745b6e467e 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_success.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
> @@ -914,7 +914,7 @@ void *user_ptr;
> char expected_str[384];
> __u32 test_len[7] = {0/* placeholder */, 0, 1, 2, 255, 256, 257};
>
> -typedef int (*bpf_read_dynptr_fn_t)(struct bpf_dynptr *dptr, u64 off,
> +typedef int (*bpf_read_dynptr_fn_t)(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void *unsafe_ptr);
>
> /* Returns the offset just before the end of the maximum sized xdp fragment.
> @@ -1106,7 +1106,7 @@ int test_copy_from_user_str_dynptr(void *ctx)
> return 0;
> }
>
> -static int bpf_copy_data_from_user_task(struct bpf_dynptr *dptr, u64 off,
> +static int bpf_copy_data_from_user_task(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void *unsafe_ptr)
> {
> struct task_struct *task = bpf_get_current_task_btf();
> @@ -1114,7 +1114,7 @@ static int bpf_copy_data_from_user_task(struct bpf_dynptr *dptr, u64 off,
> return bpf_copy_from_user_task_dynptr(dptr, off, size, unsafe_ptr, task);
> }
>
> -static int bpf_copy_data_from_user_task_str(struct bpf_dynptr *dptr, u64 off,
> +static int bpf_copy_data_from_user_task_str(const struct bpf_dynptr *dptr, u64 off,
> u64 size, const void *unsafe_ptr)
> {
> struct task_struct *task = bpf_get_current_task_btf();
> diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
> index d249113ed657..1c6cfd0888ba 100644
> --- a/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
> +++ b/tools/testing/selftests/bpf/progs/test_kfunc_dynptr_param.c
> @@ -11,12 +11,7 @@
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> #include "bpf_misc.h"
> -
> -extern struct bpf_key *bpf_lookup_system_key(__u64 id) __ksym;
> -extern void bpf_key_put(struct bpf_key *key) __ksym;
> -extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,
> - struct bpf_dynptr *sig_ptr,
> - struct bpf_key *trusted_keyring) __ksym;
> +#include "bpf_kfuncs.h"
>
> struct {
> __uint(type, BPF_MAP_TYPE_RINGBUF);
> @@ -38,7 +33,7 @@ SEC("?lsm.s/bpf")
> __failure __msg("cannot pass in dynptr at an offset=-8")
> int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size, bool kernel)
> {
> - unsigned long val;
> + unsigned long val = 0;
>
> return bpf_verify_pkcs7_signature((struct bpf_dynptr *)&val,
> (struct bpf_dynptr *)&val, NULL);
^ permalink raw reply
* Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler
From: Sebastian Andrzej Siewior @ 2026-04-13 16:03 UTC (permalink / raw)
To: Marek Vasut
Cc: Jakub Kicinski, netdev, stable, David S. Miller, Andrew Lunn,
Eric Dumazet, Nicolai Buchwitz, Paolo Abeni, Ronald Wahl,
Yicong Hui, linux-kernel, Thomas Gleixner
In-Reply-To: <16fdeec9-9208-4c9b-b228-d6c6e045e116@nabladev.com>
On 2026-04-13 17:31:34 [+0200], Marek Vasut wrote:
> > I don't see why it needs to disable interrupts.
>
> Because when the lock is held, the PAR code shouldn't be interrupted by an
> interrupt, otherwise it would completely mess up the state of the KS8851
> MAC. The spinlock does not protect only the IRQ handler, it protects also
> ks8851_start_xmit_par() and ks8851_write_mac_addr() and
> ks8851_read_mac_addr() and ks8851_net_open() and ks8851_net_stop() and other
> sites which call ks8851_lock()/ks8851_unlock() which cannot be executed
> concurrently, but where BHs can be enabled.
I need check this once brain is at full power again. But which
interrupt? Your interrupt is threaded. So that should be okay.
> > ? This seems to be used by
> > the _par driver and the _common part. The comments refer to DMA but I
> > see only FIFO access.
>
> The KS8851 does its own internal DMA into the SRAM, from which the data are
> copied by the driver into system DRAM.
So this no interrupt involved as "dma completed" and you do your manual
"memcpy".
> > And while at it, I would recommend to
> >
> > diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
> > index 8048770958d60..f1c662887646c 100644
> > --- a/drivers/net/ethernet/micrel/ks8851_common.c
> > +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> > @@ -378,9 +378,12 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
> > if (status & IRQ_LCI)
> > mii_check_link(&ks->mii);
> > - if (status & IRQ_RXI)
> > + if (status & IRQ_RXI) {
> > + local_bh_disable();
> > while ((skb = __skb_dequeue(&rxq)))
> > netif_rx(skb);
> > + local_bh_enable();
> > + }
> > return IRQ_HANDLED;
> > }
> >
> > Because otherwise it will kick-off backlog NAPI after every packet if
> > multiple packets are available.
> I think this patch will do the same, but the above should be done for the
> SPI part ?
Yes, both. This the SPI/ Mutex part does not matter. You inject one
packet into netif_rx() then if will add it to its internal NAPI and
schedule a softirq, process it. It would be more efficient to queue
multiple packets and process them all at the local_bh_enable() time.
Sebastian
^ permalink raw reply
* Re: [PATCH 5/5] selftests: net: add rss_multiqueue test variant to iou-zcrx
From: Juanlu Herrero @ 2026-04-13 16:01 UTC (permalink / raw)
To: David Wei; +Cc: netdev
In-Reply-To: <d3777e86-2859-491d-8071-77871df13c08@davidwei.uk>
On Fri, Apr 10, 2026 at 03:26:54PM -0600, David Wei wrote:
> On 2026-04-08 09:38, Juanlu Herrero wrote:
> > Add multi-port support to the iou-zcrx test binary and a new
> > rss_multiqueue Python test variant that exercises multi-queue zero-copy
> > receive with per-port flow rule steering.
> >
> > In multi-port mode, the server creates N listening sockets on
> > consecutive ports (cfg_port, cfg_port+1, ...) and uses epoll to accept
> > one connection per socket. Each client thread connects to its
> > corresponding port. Per-port ntuple flow rules steer traffic to
> > different NIC hardware queues, each with its own zcrx instance.
> >
> > For single-thread mode (the default), behavior is unchanged: one socket
> > on cfg_port, one thread, one queue.
> >
> > Signed-off-by: Juanlu Herrero <juanlu@fastmail.com>
> > ---
> > .../selftests/drivers/net/hw/iou-zcrx.c | 81 ++++++++++++++-----
> > .../selftests/drivers/net/hw/iou-zcrx.py | 45 ++++++++++-
> > 2 files changed, 104 insertions(+), 22 deletions(-)
> >
> > diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.c b/tools/testing/selftests/drivers/net/hw/iou-zcrx.c
> > index 646682167bb0..1f33d7127185 100644
> > --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.c
> > +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.c
>
> Please make all changes in iou-zcrx.c in a single patch. Then patch 5
> only changes the Python selftest.
>
> [...]
> > @@ -397,12 +410,36 @@ static void run_server(void)
> > if (cfg_dry_run)
> > goto join;
> > + epfd = epoll_create1(0);
> > + if (epfd < 0)
> > + error(1, 0, "epoll_create1()");
> > +
> > for (i = 0; i < cfg_num_threads; i++) {
> > - ctxs[i].connfd = accept(fd, NULL, NULL);
> > - if (ctxs[i].connfd < 0)
> > - error(1, 0, "accept()");
> > + ev.events = EPOLLIN;
> > + ev.data.u32 = i;
> > + if (epoll_ctl(epfd, EPOLL_CTL_ADD, fds[i], &ev) < 0)
> > + error(1, 0, "epoll_ctl()");
> > }
> > + accepted = 0;
> > + while (accepted < cfg_num_threads) {
>
> You're using epoll here but it is still accepting a fixed nr of
> connections. The server should be able to accept an arbitrary nr of
> connections, dispatching them to the server worker threads.
>
> Also with multiple queues, connections must be dispatched according to
> their NAPI IDs to the correct server workers.
>
> > + nfds = epoll_wait(epfd, events, 64, 5000);
> > + if (nfds < 0)
> > + error(1, 0, "epoll_wait()");
> > + if (nfds == 0)
> > + error(1, 0, "epoll_wait() timeout");
> > +
> > + for (i = 0; i < nfds; i++) {
> > + int idx = events[i].data.u32;
> > +
> > + ctxs[idx].connfd = accept(fds[idx], NULL, NULL);
> > + if (ctxs[idx].connfd < 0)
> > + error(1, 0, "accept()");
> > + accepted++;
> > + }
> > + }
> > +
> > + close(epfd);
> > pthread_barrier_wait(&barrier);
> > join:
Makes sense, I will re-work the patches and address the epoll & NAPI
id feedback. Thanks!
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH iwl-net] ice: fix NULL pointer dereference in ice_reset_all_vfs()
From: Romanowski, Rafal @ 2026-04-13 15:53 UTC (permalink / raw)
To: Oros, Petr, netdev@vger.kernel.org
Cc: Kitszel, Przemyslaw, Brett Creeley, Eric Dumazet,
linux-kernel@vger.kernel.org, Andrew Lunn, Nguyen, Anthony L,
intel-wired-lan@lists.osuosl.org, Jakub Kicinski, Paolo Abeni,
David S. Miller
In-Reply-To: <20260401110937.83497-1-poros@redhat.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Petr
> Oros
> Sent: Wednesday, April 1, 2026 1:10 PM
> To: netdev@vger.kernel.org
> Cc: Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Brett Creeley
> <brett.creeley@intel.com>; Eric Dumazet <edumazet@google.com>; linux-
> kernel@vger.kernel.org; Andrew Lunn <andrew+netdev@lunn.ch>; Nguyen,
> Anthony L <anthony.l.nguyen@intel.com>; intel-wired-lan@lists.osuosl.org;
> Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S.
> Miller <davem@davemloft.net>
> Subject: [Intel-wired-lan] [PATCH iwl-net] ice: fix NULL pointer dereference in
> ice_reset_all_vfs()
>
> ice_reset_all_vfs() ignores the return value of ice_vf_rebuild_vsi().
> When the VSI rebuild fails (e.g. during NVM firmware update via nvmupdate64e),
> ice_vsi_rebuild() tears down the VSI on its error path, leaving txq_map and
> rxq_map as NULL. The subsequent unconditional call to ice_vf_post_vsi_rebuild()
> leads to a NULL pointer dereference in
> ice_ena_vf_q_mappings() when it accesses vsi->txq_map[0].
>
> The single-VF reset path in ice_reset_vf() already handles this correctly by
> checking the return value of ice_vf_reconfig_vsi() and skipping
> ice_vf_post_vsi_rebuild() on failure.
>
> Apply the same pattern to ice_reset_all_vfs(): check the return value of
> ice_vf_rebuild_vsi() and skip ice_vf_post_vsi_rebuild() and
> ice_eswitch_attach_vf() on failure. The VF is left safely disabled
> (ICE_VF_STATE_INIT not set, VFGEN_RSTAT not set to VFACTIVE) and can be
> recovered via a VFLR triggered by a PCI reset of the VF (sysfs reset or driver
> rebind).
>
> Note that this patch does not prevent the VF VSI rebuild from failing during NVM
> update — the underlying cause is firmware being in a transitional state while the
> EMP reset is processed, which can cause Admin Queue commands (ice_add_vsi,
> ice_cfg_vsi_lan) to fail. This patch only prevents the subsequent NULL pointer
> dereference that crashes the kernel when the rebuild does fail.
>
> crash> bt
> PID: 50795 TASK: ff34c9ee708dc680 CPU: 1 COMMAND:
> "kworker/u512:5"
> #0 [ff72159bcfe5bb50] machine_kexec at ffffffffaa8850ee
> #1 [ff72159bcfe5bba8] __crash_kexec at ffffffffaaa15fba
> #2 [ff72159bcfe5bc68] crash_kexec at ffffffffaaa16540
> #3 [ff72159bcfe5bc70] oops_end at ffffffffaa837eda
> #4 [ff72159bcfe5bc90] page_fault_oops at ffffffffaa893997
> #5 [ff72159bcfe5bce8] exc_page_fault at ffffffffab528595
> #6 [ff72159bcfe5bd10] asm_exc_page_fault at ffffffffab600bb2
> [exception RIP: ice_ena_vf_q_mappings+0x79]
> RIP: ffffffffc0a85b29 RSP: ff72159bcfe5bdc8 RFLAGS: 00010206
> RAX: 00000000000f0000 RBX: ff34c9efc9c00000 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000010 RDI: ff34c9efc9c00000
> RBP: ff34c9efc27d4828 R8: 0000000000000093 R9: 0000000000000040
> R10: ff34c9efc27d4828 R11: 0000000000000040 R12: 0000000000100000
> R13: 0000000000000010 R14: R15:
> ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018
> #7 [ff72159bcfe5bdf8] ice_sriov_post_vsi_rebuild at ffffffffc0a85e2e [ice]
> #8 [ff72159bcfe5be08] ice_reset_all_vfs at ffffffffc0a920b4 [ice]
> #9 [ff72159bcfe5be48] ice_service_task at ffffffffc0a31519 [ice]
> #10 [ff72159bcfe5be88] process_one_work at ffffffffaa93dca4
> #11 [ff72159bcfe5bec8] worker_thread at ffffffffaa93e9de
> #12 [ff72159bcfe5bf18] kthread at ffffffffaa946663
> #13 [ff72159bcfe5bf50] ret_from_fork at ffffffffaa8086b9
>
> The panic occurs attempting to dereference the NULL pointer in RDX at
> ice_sriov.c:294, which loads vsi->txq_map (offset 0x4b8 in ice_vsi).
>
> The faulting VSI is an allocated slab object but not fully initialized after a failed
> ice_vsi_rebuild():
>
> crash> struct ice_vsi 0xff34c9efc27d4828
> netdev = 0x0,
> rx_rings = 0x0,
> tx_rings = 0x0,
> q_vectors = 0x0,
> txq_map = 0x0,
> rxq_map = 0x0,
> alloc_txq = 0x10,
> num_txq = 0x10,
> alloc_rxq = 0x10,
> num_rxq = 0x10,
>
> The nvmupdate64e process was performing NVM firmware update:
>
> crash> bt 0xff34c9edd1a30000
> PID: 49858 TASK: ff34c9edd1a30000 CPU: 1 COMMAND: "nvmupdate64e"
> #0 [ff72159bcd617618] __schedule at ffffffffab5333f8
> #4 [ff72159bcd617750] ice_sq_send_cmd at ffffffffc0a35347 [ice]
> #5 [ff72159bcd6177a8] ice_sq_send_cmd_retry at ffffffffc0a35b47 [ice]
> #6 [ff72159bcd617810] ice_aq_send_cmd at ffffffffc0a38018 [ice]
> #7 [ff72159bcd617848] ice_aq_read_nvm at ffffffffc0a40254 [ice]
> #8 [ff72159bcd6178b8] ice_read_flat_nvm at ffffffffc0a4034c [ice]
> #9 [ff72159bcd617918] ice_devlink_nvm_snapshot at ffffffffc0a6ffa5 [ice]
>
> dmesg:
> ice 0000:13:00.0: firmware recommends not updating fw.mgmt, as it
> may result in a downgrade. continuing anyways
> ice 0000:13:00.1: ice_init_nvm failed -5
> ice 0000:13:00.1: Rebuild failed, unload and reload driver
>
> Fixes: 12bb018c538c ("ice: Refactor VF reset")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
> drivers/net/ethernet/intel/ice/ice_vf_lib.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> index c8bc952f05cdb5..51259a4fdda4b9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c
> @@ -804,7 +804,12 @@ void ice_reset_all_vfs(struct ice_pf *pf)
> ice_vf_ctrl_invalidate_vsi(vf);
>
> ice_vf_pre_vsi_rebuild(vf);
> - ice_vf_rebuild_vsi(vf);
> + if (ice_vf_rebuild_vsi(vf)) {
> + dev_err(dev, "VF %u VSI rebuild failed, leaving VF
> disabled\n",
> + vf->vf_id);
> + mutex_unlock(&vf->cfg_lock);
> + continue;
> + }
> ice_vf_post_vsi_rebuild(vf);
>
> ice_eswitch_attach_vf(pf, vf);
> --
> 2.52.0
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH iwl-next] ice: fix FDIR CTRL VSI resource leak in ice_reset_all_vfs()
From: Romanowski, Rafal @ 2026-04-13 15:51 UTC (permalink / raw)
To: Simon Horman, Loktionov, Aleksandr
Cc: intel-wired-lan@lists.osuosl.org, Nguyen, Anthony L,
netdev@vger.kernel.org, Dawid Osuchowski
In-Reply-To: <20260403115255.GA60103@horms.kernel.org>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Simon
> Horman
> Sent: Friday, April 3, 2026 1:57 PM
> To: Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; netdev@vger.kernel.org; Dawid Osuchowski
> <dawid.osuchowski@linux.intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next] ice: fix FDIR CTRL VSI resource
> leak in ice_reset_all_vfs()
>
> On Fri, Mar 27, 2026 at 08:22:32AM +0100, Aleksandr Loktionov wrote:
> > From: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
> >
> > Resetting all VFs causes resource leak on VFs with FDIR filters
> > enabled as CTRL VSIs are only invalidated and not freed. Fix by using
> > ice_vf_ctrl_vsi_release() instead of ice_vf_ctrl_invalidate_vsi()
> > which aligns behavior with the ice_reset_vf() function.
> >
> > Reproduction:
> > echo 1 > /sys/class/net/$pf/device/sriov_numvfs
> > ethtool -N $vf flow-type ether proto 0x9000 action 0
> > echo 1 > /sys/class/net/$pf/device/reset
> >
> > Fixes: da62c5ff9dcd ("ice: Add support for per VF ctrl VSI enabling")
> > Signed-off-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
>
> Reviewed-by: Simon Horman <horms@kernel.org>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply
* Re: [PATCH iwl-next v2] igb: use ktime_get_real helpers in igb_ptp_reset()
From: Simon Horman @ 2026-04-13 15:50 UTC (permalink / raw)
To: Aleksandr Loktionov
Cc: intel-wired-lan, anthony.l.nguyen, netdev, Jacob Keller,
Paul Menzel
In-Reply-To: <20260409075523.3728506-1-aleksandr.loktionov@intel.com>
On Thu, Apr 09, 2026 at 09:55:23AM +0200, Aleksandr Loktionov wrote:
> Replace ktime_to_ns(ktime_get_real()) with the direct equivalent
> ktime_get_real_ns() and ktime_to_timespec64(ktime_get_real()) with
> ktime_get_real_ts64() in igb_ptp_reset(). Using the combined helpers
> makes the intent clearer.
>
> Suggested-by: Jacob Keller <jacob.e.keller@intel.com>
> Suggested-by: Simon Horman <horms@kernel.org>
> Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler
From: Jakub Kicinski @ 2026-04-13 15:44 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Marek Vasut, netdev, stable, David S. Miller, Andrew Lunn,
Eric Dumazet, Nicolai Buchwitz, Paolo Abeni, Ronald Wahl,
Yicong Hui, linux-kernel, Thomas Gleixner
In-Reply-To: <20260413125744.TVKkZcEK@linutronix.de>
On Mon, 13 Apr 2026 14:57:44 +0200 Sebastian Andrzej Siewior wrote:
> On 2026-04-12 10:51:25 [-0700], Jakub Kicinski wrote:
> > > Does the backtrace make the problem clearer, with the annotation above ?
> >
> > Sebastian, do you have any recommendation here? tl;dr is that the driver does
> …
>
> What about this:
Thanks for taking a look (according to you auto-reply immediately after
a vacation ;))
TBH changing the driver feels like a workaround / invitation for a
whack-a-mole game. I'd prefer to fix the skb allocation.
Is there any way we can check if any locks which were _irq() on non-RT
are held?
^ permalink raw reply
* Re: [PATCH 2/5] selftests: net: add multithread client support to iou-zcrx
From: Juanlu Herrero @ 2026-04-13 15:44 UTC (permalink / raw)
To: David Wei; +Cc: netdev
In-Reply-To: <8fa08d73-28a3-4521-bcfb-ec81869c24f3@davidwei.uk>
On Thu, Apr 09, 2026 at 08:51:11AM -0600, David Wei wrote:
> On 2026-04-08 09:38, Juanlu Herrero wrote:
> > Add pthreads to the iou-zcrx client so that multiple connections can be
> > established simultaneously. Each client thread connects to the server
> > and sends its payload independently.
> >
> > Introduce struct thread_ctx and the -t option to control the number of
> > threads (default 1), preserving backwards compatibility with existing
> > tests.
> >
> > Signed-off-by: Juanlu Herrero <juanlu@fastmail.com>
> > ---
> > .../testing/selftests/drivers/net/hw/Makefile | 2 +-
> > .../selftests/drivers/net/hw/iou-zcrx.c | 46 +++++++++++++++++--
> > 2 files changed, 44 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
> > index deeca3f8d080..227adfec706c 100644
> > --- a/tools/testing/selftests/drivers/net/hw/Makefile
> > +++ b/tools/testing/selftests/drivers/net/hw/Makefile
> > @@ -80,5 +80,5 @@ include ../../../net/ynl.mk
> > include ../../../net/bpf.mk
> > ifeq ($(HAS_IOURING_ZCRX),y)
> > -$(OUTPUT)/iou-zcrx: LDLIBS += -luring
> > +$(OUTPUT)/iou-zcrx: LDLIBS += -luring -lpthread
> > endif
> > diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.c b/tools/testing/selftests/drivers/net/hw/iou-zcrx.c
> > index 334985083f61..de2eea78a5b6 100644
> > --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.c
> > +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.c
> > @@ -4,6 +4,7 @@
> > #include <error.h>
> > #include <fcntl.h>
> > #include <limits.h>
> > +#include <pthread.h>
> > #include <stdbool.h>
> > #include <stdint.h>
> > #include <stdio.h>
> > @@ -85,8 +86,14 @@ static int cfg_send_size = SEND_SIZE;
> > static struct sockaddr_in6 cfg_addr;
> > static unsigned int cfg_rx_buf_len;
> > static bool cfg_dry_run;
> > +static int cfg_num_threads = 1;
> > static char *payload;
> > +
> > +struct thread_ctx {
> > + int thread_id;
>
> This is set here and in patch 4 but I don't see it being used.
Makes sense, will remove in v2.
^ permalink raw reply
* Re: [net-next PATCH v5 1/4] octeontx2-af: npa: cn20k: Add NPA Halo support
From: Alexander Lobakin @ 2026-04-13 15:32 UTC (permalink / raw)
To: Subbaraya Sundeep
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, sgoutham, gakula,
bbhushan2, netdev, linux-kernel, Linu Cherian
In-Reply-To: <20260410101150.GA1783722@kernel-ep2>
From: Subbaraya Sundeep <sbhatta@marvell.com>
Date: Fri, 10 Apr 2026 15:41:50 +0530
> On 2026-04-10 at 15:06:56, Alexander Lobakin (aleksander.lobakin@intel.com) wrote:
>> From: Subbaraya Sundeep <sbhatta@marvell.com>
>> Date: Fri, 10 Apr 2026 15:05:36 +0530
>>
>>> On 2026-04-09 at 20:39:02, Alexander Lobakin (aleksander.lobakin@intel.com) wrote:
>>>> From: Subbaraya Sundeep <sbhatta@marvell.com>
>>>> Date: Thu, 9 Apr 2026 15:23:21 +0530
>>>>
>>>>> From: Linu Cherian <lcherian@marvell.com>
>>>>>
>>>>> CN20K silicon implements unified aura and pool context
>>>>> type called Halo for better resource usage. Add support to
>>>>> handle Halo context type operations.
>>>>>
>>>>> Signed-off-by: Linu Cherian <lcherian@marvell.com>
>>>>> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
>>>>> index 763f6cabd7c2..2364bafd329d 100644
>>>>> --- a/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
>>>>> +++ b/drivers/net/ethernet/marvell/octeontx2/af/cn20k/struct.h
>>>>> @@ -377,4 +377,85 @@ struct npa_cn20k_pool_s {
>>>>>
>>>>> static_assert(sizeof(struct npa_cn20k_pool_s) == NIX_MAX_CTX_SIZE);
>>>>>
>>>>> +struct npa_cn20k_halo_s {
>>>>> + u64 stack_base : 64;
>>>>
>>>> It's redundant to add : 64 to a 64-bit field.
>>> Agreed. But this is for readability, it helps when checking HRM. For
>>> instance HRM says [703:640] and we define as u64 reserved_640_703 : 64;
>>> so that we do not have to count bits in mind.
>>>> Moreover, on 32-bit systems, the compilers sometimes complain on
>>>> bitfields > 32 bits.
>>> This driver depends on 64BIT.
>>>>
>>>>> + u64 ena : 1;
>>>>> + u64 nat_align : 1;
>>>>> + u64 reserved_66_67 : 2;
>>>>> + u64 stack_caching : 1;
>>>>> + u64 reserved_69_71 : 3;
>>>>> + u64 aura_drop_ena : 1;
>>>>> + u64 reserved_73_79 : 7;
>>>>> + u64 aura_drop : 8;
>>>>> + u64 buf_offset : 12;
>>>>> + u64 reserved_100_103 : 4;
>>>>> + u64 buf_size : 12;
>>>>> + u64 reserved_116_119 : 4;
>>>>> + u64 ref_cnt_prof : 3;
>>>>> + u64 reserved_123_127 : 5;
>>>>> + u64 stack_max_pages : 32;
>>>>> + u64 stack_pages : 32;
>>>>> + u64 bp_0 : 7;
>>>>> + u64 bp_1 : 7;
>>>>> + u64 bp_2 : 7;
>>>>> + u64 bp_3 : 7;
>>>>> + u64 bp_4 : 7;
>>>>> + u64 bp_5 : 7;
>>>>> + u64 bp_6 : 7;
>>>>> + u64 bp_7 : 7;
>>>>> + u64 bp_ena_0 : 1;
>>>>> + u64 bp_ena_1 : 1;
>>>>> + u64 bp_ena_2 : 1;
>>>>> + u64 bp_ena_3 : 1;
>>>>> + u64 bp_ena_4 : 1;
>>>>> + u64 bp_ena_5 : 1;
>>>>> + u64 bp_ena_6 : 1;
>>>>> + u64 bp_ena_7 : 1;
>>>>> + u64 stack_offset : 4;
>>>>> + u64 reserved_260_263 : 4;
>>>>> + u64 shift : 6;
>>>>> + u64 reserved_270_271 : 2;
>>>>> + u64 avg_level : 8;
>>>>> + u64 avg_con : 9;
>>>>> + u64 fc_ena : 1;
>>>>> + u64 fc_stype : 2;
>>>>> + u64 fc_hyst_bits : 4;
>>>>> + u64 fc_up_crossing : 1;
>>>>> + u64 reserved_297_299 : 3;
>>>>> + u64 update_time : 16;
>>>>> + u64 reserved_316_319 : 4;
>>>>> + u64 fc_addr : 64;
>>>>> + u64 ptr_start : 64;
>>>>> + u64 ptr_end : 64;
>>>>> + u64 bpid_0 : 12;
>>>>> + u64 reserved_524_535 : 12;
>>>>> + u64 err_int : 8;
>>>>> + u64 err_int_ena : 8;
>>>>> + u64 thresh_int : 1;
>>>>> + u64 thresh_int_ena : 1;
>>>>> + u64 thresh_up : 1;
>>>>> + u64 reserved_555 : 1;
>>>>> + u64 thresh_qint_idx : 7;
>>>>> + u64 reserved_563 : 1;
>>>>> + u64 err_qint_idx : 7;
>>>>> + u64 reserved_571_575 : 5;
>>>>> + u64 thresh : 36;
>>>>> + u64 reserved_612_615 : 4;
>>>>> + u64 fc_msh_dst : 11;
>>>>> + u64 reserved_627_630 : 4;
>>>>> + u64 op_dpc_ena : 1;
>>>>> + u64 op_dpc_set : 5;
>>>>> + u64 reserved_637_637 : 1;
>>>>> + u64 stream_ctx : 1;
>>>>> + u64 unified_ctx : 1;
>>>>> + u64 reserved_640_703 : 64;
>>>>> + u64 reserved_704_767 : 64;
>>>>> + u64 reserved_768_831 : 64;
>>>>> + u64 reserved_832_895 : 64;
>>>>> + u64 reserved_896_959 : 64;
>>>>> + u64 reserved_960_1023 : 64;
>>>>> +};
>>>>> +
>>>>> +static_assert(sizeof(struct npa_cn20k_halo_s) == NIX_MAX_CTX_SIZE);
>>>>
>>>> Now the main question:
>>>>
>>>> Is mailbox's Endianness fixed (LE/BE)? Or is it always the same as the
>>>> host's ones (I doubt so)?
>>>> If not, these need to be __le{8,16,32,64} (or __be if it's Big Endian)
>>>> and you need to handle the conversions manually.
>>>>
>>> Yes endianness is LE and fixed. This is NOT a host side driver for an
>>> endpoint card. This is driver for on chip PCI device of CN20K soc.
>>> Hope I answered your question wrt host.
>>
>> But the mailbox is shared between the SoC and the host or HW or not? Is
> In hardware it is just shared DDR region between two on chip devices and both
> devices access shared region using their BARs.
>> it possible that one client of the mailbox will have LE and the second
>> will have BE?
> No not possible.
Okay, so seems like it's safe to use Endianness-agnostic types without
messing with `__le`/`__be`, thanks for explaining.
Thanks,
Olek
^ permalink raw reply
* Re: [patch 15/38] ptp: ptp_vmclock: Replace get_cycles() usage
From: David Woodhouse @ 2026-04-13 15:33 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: Arnd Bergmann, x86, Lu Baolu, iommu, Michael Grzeschik, netdev,
linux-wireless, Herbert Xu, linux-crypto, Vlastimil Babka,
linux-mm, Bernie Thompson, linux-fbdev, Theodore Tso, linux-ext4,
Andrew Morton, Uladzislau Rezki, Marco Elver, Dmitry Vyukov,
kasan-dev, Andrey Ryabinin, Thomas Sailer, linux-hams,
Jason A. Donenfeld, Richard Henderson, linux-alpha, Russell King,
linux-arm-kernel, Catalin Marinas, Huacai Chen, loongarch,
Geert Uytterhoeven, linux-m68k, Dinh Nguyen, Jonas Bonn,
linux-openrisc, Helge Deller, linux-parisc, Michael Ellerman,
linuxppc-dev, Paul Walmsley, linux-riscv, Heiko Carstens,
linux-s390, David S. Miller, sparclinux
In-Reply-To: <20260410120318.592237447@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
On Fri, 2026-04-10 at 14:19 +0200, Thomas Gleixner wrote:
> get_cycles() is not really well defined and similar to other usaage of the
> underlying hardware CPU counters the PTP vmclock should use an explicit
> interface as well.
>
> Implement ptp_vmclock_read_cpu_counter() in arm64 and x86 and simplify the
> Kconfig selection while at it.
>
> No functional change.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: David Woodhouse <dwmw2@infradead.org>
Acked-by: David Woodhouse <dwmw@amazon.co.uk>
Although I might follow up with a change to make this...
> +static inline u64 ptp_vmclock_read_cpu_counter(void)
> +{
> + return cpu_feature_enabled(X86_FEATURE_TSC) ? rdtsc() : 0;
> +}
> +
... depend on TSC_RELIABLE¹, since if the guest doesn't believe that it
is, then the guest shouldn't be trying to use it as the basis for
precise timing.
¹ (Or... one of the other zoo of TSC flags for the gradually reducing
brokenness over the years...)
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [net,PATCH v2] net: ks8851: Reinstate disabling of BHs around IRQ handler
From: Marek Vasut @ 2026-04-13 15:31 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Jakub Kicinski
Cc: netdev, stable, David S. Miller, Andrew Lunn, Eric Dumazet,
Nicolai Buchwitz, Paolo Abeni, Ronald Wahl, Yicong Hui,
linux-kernel, Thomas Gleixner
In-Reply-To: <20260413125744.TVKkZcEK@linutronix.de>
On 4/13/26 2:57 PM, Sebastian Andrzej Siewior wrote:
> On 2026-04-12 10:51:25 [-0700], Jakub Kicinski wrote:
>>> Does the backtrace make the problem clearer, with the annotation above ?
>>
>> Sebastian, do you have any recommendation here? tl;dr is that the driver does
> …
>
> What about this:
>
> --- a/drivers/net/ethernet/micrel/ks8851_par.c
> +++ b/drivers/net/ethernet/micrel/ks8851_par.c
> @@ -63,7 +63,7 @@ static void ks8851_lock_par(struct ks8851_net *ks, unsigned long *flags)
> {
> struct ks8851_net_par *ksp = to_ks8851_par(ks);
>
> - spin_lock_irqsave(&ksp->lock, *flags);
> + spin_lock_bh(&ksp->lock);
> }
>
> /**
> @@ -77,7 +77,7 @@ static void ks8851_unlock_par(struct ks8851_net *ks, unsigned long *flags)
> {
> struct ks8851_net_par *ksp = to_ks8851_par(ks);
>
> - spin_unlock_irqrestore(&ksp->lock, *flags);
> + spin_unlock_bh(&ksp->lock);
> }
>
> /**
>
>
> I don't see why it needs to disable interrupts.
Because when the lock is held, the PAR code shouldn't be interrupted by
an interrupt, otherwise it would completely mess up the state of the
KS8851 MAC. The spinlock does not protect only the IRQ handler, it
protects also ks8851_start_xmit_par() and ks8851_write_mac_addr() and
ks8851_read_mac_addr() and ks8851_net_open() and ks8851_net_stop() and
other sites which call ks8851_lock()/ks8851_unlock() which cannot be
executed concurrently, but where BHs can be enabled.
> ? This seems to be used by
> the _par driver and the _common part. The comments refer to DMA but I
> see only FIFO access.
The KS8851 does its own internal DMA into the SRAM, from which the data
are copied by the driver into system DRAM.
> And while at it, I would recommend to
>
> diff --git a/drivers/net/ethernet/micrel/ks8851_common.c b/drivers/net/ethernet/micrel/ks8851_common.c
> index 8048770958d60..f1c662887646c 100644
> --- a/drivers/net/ethernet/micrel/ks8851_common.c
> +++ b/drivers/net/ethernet/micrel/ks8851_common.c
> @@ -378,9 +378,12 @@ static irqreturn_t ks8851_irq(int irq, void *_ks)
> if (status & IRQ_LCI)
> mii_check_link(&ks->mii);
>
> - if (status & IRQ_RXI)
> + if (status & IRQ_RXI) {
> + local_bh_disable();
> while ((skb = __skb_dequeue(&rxq)))
> netif_rx(skb);
> + local_bh_enable();
> + }
>
> return IRQ_HANDLED;
> }
>
> Because otherwise it will kick-off backlog NAPI after every packet if
> multiple packets are available.
I think this patch will do the same, but the above should be done for
the SPI part ?
^ permalink raw reply
* Re: [patch 10/38] arcnet: Remove function timing code
From: David Woodhouse @ 2026-04-13 15:29 UTC (permalink / raw)
To: Thomas Gleixner, LKML
Cc: Michael Grzeschik, netdev, Arnd Bergmann, x86, Lu Baolu, iommu,
linux-wireless, Herbert Xu, linux-crypto, Vlastimil Babka,
linux-mm, Bernie Thompson, linux-fbdev, Theodore Tso, linux-ext4,
Andrew Morton, Uladzislau Rezki, Marco Elver, Dmitry Vyukov,
kasan-dev, Andrey Ryabinin, Thomas Sailer, linux-hams,
Jason A. Donenfeld, Richard Henderson, linux-alpha, Russell King,
linux-arm-kernel, Catalin Marinas, Huacai Chen, loongarch,
Geert Uytterhoeven, linux-m68k, Dinh Nguyen, Jonas Bonn,
linux-openrisc, Helge Deller, linux-parisc, Michael Ellerman,
linuxppc-dev, Paul Walmsley, linux-riscv, Heiko Carstens,
linux-s390, David S. Miller, sparclinux
In-Reply-To: <20260410120318.253872322@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 778 bytes --]
On Fri, 2026-04-10 at 14:19 +0200, Thomas Gleixner wrote:
> ARCNET is a museums piece and the function timing can be done with
> ftrace. Remove the cruft.
>
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
> Cc: netdev@vger.kernel.org
> ---
> drivers/net/arcnet/arc-rimi.c | 4 ++--
> drivers/net/arcnet/arcdevice.h | 20 +-------------------
> drivers/net/arcnet/com20020.c | 6 ++----
> drivers/net/arcnet/com90io.c | 6 ++----
> drivers/net/arcnet/com90xx.c | 4 ++--
> 5 files changed, 9 insertions(+), 31 deletions(-)
Acked-by: David Woodhouse <dwmw2@infradead.org>
By coincidence, I took the last of my ARCNET cards to the tip just this
morning...
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
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