* Re: [PATCH net-next V9 04/14] devlink: Decouple rate storage from associated devlink object
From: Jakub Kicinski @ 2026-03-31 2:08 UTC (permalink / raw)
To: tariqt
Cc: Jakub Kicinski, edumazet, pabeni, andrew+netdev, davem,
donald.hunter, horms, jiri, corbet, skhan, saeedm, leon, mbloch,
shuah, chuck.lever, matttbe, cjubran, cratiu, dtatulea,
jacob.e.keller, shshitrit, daniel.zahka, parav, ajayachandra,
kees, shayd, danielj, moshe, willemb, dw, petrm, sdf, daniel, joe,
razor, vadim.fedorenko, mst, antonio, allison.henderson,
minhquangbui99, noren, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, gal, jiri
In-Reply-To: <20260326065949.44058-5-tariqt@nvidia.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
devlink: Decouple rate storage from associated devlink object
Devlink rate leafs and nodes were stored in their respective devlink
objects pointed to by devlink_rate->devlink.
> diff --git a/net/devlink/rate.c b/net/devlink/rate.c
> index 4781429109195..9ebbc72130c6a 100644
> --- a/net/devlink/rate.c
> +++ b/net/devlink/rate.c
[ ... ]
> @@ -648,8 +702,9 @@ int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
> goto err_rate_set;
>
> refcount_set(&rate_node->refcnt, 1);
> - list_add(&rate_node->list, &devlink->rate_list);
> + list_add(&rate_node->list, &rate_devlink->rate_list);
> devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
> + devl_rate_unlock(devlink);
> return 0;
>
> err_rate_set:
> ops->rate_node_del(rate_node, rate_node->priv, info->extack);
> @@ -658,6 +713,8 @@ int devlink_nl_rate_new_doit(struct sk_buff *skb, struct genl_info *info)
> kfree(rate_node->name);
> err_strdup:
> kfree(rate_node);
> +unlock:
> + devl_rate_unlock(devlink);
> return err;
> }
If devlink_nl_rate_set() fails after it successfully sets the parent node
and increments the parent's refcount, does this error path permanently
leak that reference since it frees the node without decrementing it?
[ ... ]
> @@ -716,14 +785,20 @@ devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name,
> struct devlink_rate *parent)
> {
> struct devlink_rate *rate_node;
> + struct devlink *rate_devlink;
>
> + rate_devlink = devl_rate_lock(devlink);
> rate_node = devlink_rate_node_get_by_name(devlink, node_name);
> - if (!IS_ERR(rate_node))
> - return ERR_PTR(-EEXIST);
> + if (!IS_ERR(rate_node)) {
> + rate_node = ERR_PTR(-EEXIST);
> + goto unlock;
> + }
>
> rate_node = kzalloc_obj(*rate_node);
> - if (!rate_node)
> - return ERR_PTR(-ENOMEM);
> + if (!rate_node) {
> + rate_node = ERR_PTR(-ENOMEM);
> + goto unlock;
> + }
>
> if (parent) {
> rate_node->parent = parent;
> @@ -737,12 +812,15 @@ devl_rate_node_create(struct devlink *devlink, void *priv, char *node_name,
> rate_node->name = kstrdup(node_name, GFP_KERNEL);
> if (!rate_node->name) {
> kfree(rate_node);
> - return ERR_PTR(-ENOMEM);
> + rate_node = ERR_PTR(-ENOMEM);
> + goto unlock;
> }
Similarly, if kstrdup() fails here, does this leak the parent's refcount
since it was already incremented earlier in the function?
^ permalink raw reply
* Re: [PATCH net-next V9 07/14] devlink: Allow rate node parents from other devlinks
From: Jakub Kicinski @ 2026-03-31 2:08 UTC (permalink / raw)
To: tariqt
Cc: Jakub Kicinski, edumazet, pabeni, andrew+netdev, davem,
donald.hunter, horms, jiri, corbet, skhan, saeedm, leon, mbloch,
shuah, chuck.lever, matttbe, cjubran, cratiu, dtatulea,
jacob.e.keller, shshitrit, daniel.zahka, parav, ajayachandra,
kees, shayd, danielj, moshe, willemb, dw, petrm, sdf, daniel, joe,
razor, vadim.fedorenko, mst, antonio, allison.henderson,
minhquangbui99, noren, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, gal, jiri
In-Reply-To: <20260326065949.44058-8-tariqt@nvidia.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
devlink: Allow rate node parents from other devlinks
This commit makes use of the building blocks previously added to
implement cross-device rate nodes.
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 3038af6ec017e..8d5ad5d4f1d05 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1585,6 +1585,11 @@ struct devlink_ops {
> struct devlink_rate *parent,
> void *priv_child, void *priv_parent,
> struct netlink_ext_ack *extack);
> + /* Indicates if cross-device rate nodes are supported.
> + * This also requires a shared common ancestor object all devices that
> + * could share rate nodes are nested in.
> + */
> + bool supported_cross_device_rate_nodes;
When a device acting as a cross-device rate parent is destroyed,
devl_rate_nodes_destroy() iterates the shared rate_list to detach its
children. If it encounters a child rate node belonging to a different device,
it invokes the child driver's ops->rate_node_parent_set or
ops->rate_leaf_parent_set callbacks.
Since the cleanup path only holds the parent's devlink lock and the shared
root devlink lock, does this allow the child driver's rate operations to
execute concurrently with other operations on the child device without holding
the child's devlink instance lock?
[ ... ]
> diff --git a/net/devlink/rate.c b/net/devlink/rate.c
> index 1949746fab291..f243cccc95bee 100644
> --- a/net/devlink/rate.c
> +++ b/net/devlink/rate.c
> @@ -30,19 +30,53 @@ devlink_rate_leaf_get_from_info(struct devlink *devlink, struct genl_info *info)
> return devlink_rate ?: ERR_PTR(-ENODEV);
> }
>
> +/* Repeatedly locks the nested-in devlink instances while cross device rate
> + * nodes are supported. Returns the devlink instance where rates should be
> + * stored.
> + */
> static struct devlink *devl_rate_lock(struct devlink *devlink)
> {
> - return devlink;
> + struct devlink *rate_devlink = devlink;
> +
> + while (rate_devlink->ops &&
> + rate_devlink->ops->supported_cross_device_rate_nodes) {
> + devlink = devlink_nested_in_get_lock(rate_devlink->rel);
> + if (!devlink)
> + break;
> + rate_devlink = devlink;
> + }
> + return rate_devlink;
> }
[ ... ]
> +/* Repeatedly unlocks the nested-in devlink instances of 'devlink' while cross
> + * device nodes are supported.
> + */
> static void devl_rate_unlock(struct devlink *devlink)
> {
> + if (!devlink || !devlink->ops ||
> + !devlink->ops->supported_cross_device_rate_nodes)
> + return;
> +
> + devl_rate_unlock(devlink_nested_in_get_locked(devlink->rel));
> + devlink_nested_in_put_unlock(devlink->rel);
> }
If a parent devlink is not yet registered, devlink_nested_in_get_lock() can
return NULL in devl_rate_lock() and break the loop without taking a lock.
If another thread completes registering the parent devlink concurrently,
devlink_nested_in_get_locked() in devl_rate_unlock() will find it.
Could this lead to unlocking a mutex that was never acquired and dropping a
reference that was never taken, resulting in a use-after-free or lock
corruption?
[ ... ]
> @@ -320,13 +371,14 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
> struct genl_info *info,
> struct nlattr *nla_parent)
> {
> - struct devlink *devlink = devlink_rate->devlink;
> + struct devlink *devlink = devlink_rate->devlink, *parent_devlink;
> const char *parent_name = nla_data(nla_parent);
> const struct devlink_ops *ops = devlink->ops;
> size_t len = strlen(parent_name);
> struct devlink_rate *parent;
> int err = -EOPNOTSUPP;
>
> + parent_devlink = devlink_nl_ctx(info)->parent_devlink ? : devlink;
> parent = devlink_rate->parent;
>
> if (parent && !len) {
> @@ -344,7 +396,13 @@ devlink_nl_rate_parent_node_set(struct devlink_rate *devlink_rate,
> refcount_dec(&parent->refcnt);
> devlink_rate->parent = NULL;
> } else if (len) {
> - parent = devlink_rate_node_get_by_name(devlink, parent_name);
> + /* parent_devlink (when different than devlink) isn't locked,
> + * but the rate node devlink instance is, so nobody from the
> + * same group of devices sharing rates could change the used
> + * fields or unregister the parent.
> + */
> + parent = devlink_rate_node_get_by_name(parent_devlink,
> + parent_name);
Is parent_devlink validated to ensure it shares the same common rate root
ancestor as devlink?
If an arbitrary unrelated parent_devlink is specified,
devlink_rate_node_get_by_name() could traverse an unrelated parent's
rate_list that was never locked by devl_rate_lock(), leading to a data race.
Additionally, if a rate node is found, its priv pointer is passed to the
target driver's ops->rate_node_parent_set. Could this cause the target driver
to cast a foreign priv pointer to its own private struct type, resulting in
type confusion and memory corruption?
^ permalink raw reply
* Re: [PATCH net-next V9 11/14] net/mlx5: qos: Remove qos domains and use shd lock
From: Jakub Kicinski @ 2026-03-31 2:08 UTC (permalink / raw)
To: tariqt
Cc: Jakub Kicinski, edumazet, pabeni, andrew+netdev, davem,
donald.hunter, horms, jiri, corbet, skhan, saeedm, leon, mbloch,
shuah, chuck.lever, matttbe, cjubran, cratiu, dtatulea,
jacob.e.keller, shshitrit, daniel.zahka, parav, ajayachandra,
kees, shayd, danielj, moshe, willemb, dw, petrm, sdf, daniel, joe,
razor, vadim.fedorenko, mst, antonio, allison.henderson,
minhquangbui99, noren, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, gal, jiri
In-Reply-To: <20260326065949.44058-12-tariqt@nvidia.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/mlx5: qos: Remove qos domains and use shd lock
This commit removes QoS domains and switches to using the shd lock to
protect against concurrent QoS modifications. It also enables the
supported_cross_device_rate_nodes devlink ops attribute.
Link: https://lore.kernel.org/netdev/20250213180134.323929-1-tariqt@nvidia.com/
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> index 6698ac55a4bfa..c051605fecd23 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
> @@ -385,6 +385,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
> .rate_node_del = mlx5_esw_devlink_rate_node_del,
> .rate_leaf_parent_set = mlx5_esw_devlink_rate_leaf_parent_set,
> .rate_node_parent_set = mlx5_esw_devlink_rate_node_parent_set,
> + .supported_cross_device_rate_nodes = true,
The commit message states that enabling supported_cross_device_rate_nodes
now is safe because mlx5_esw_qos_vport_update_parent rejects cross-esw
parent updates.
When a user provides a DEVLINK_ATTR_PARENT_DEV belonging to an unrelated
device, the devlink core (devlink_nl_rate_parent_node_set) drops the lock on
the parent devlink and calls devlink_rate_node_get_by_name(). This function
traverses the rate_list of the unlocked parent devlink instance.
Does this leave the devlink core exposed to data races and potential
use-after-free vulnerabilities, since the driver's rejection callback
executes only after this lockless list traversal has occurred?
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> index 0be516003bcde..f67f99428959f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> @@ -1110,7 +1068,7 @@ void mlx5_esw_qos_vport_disable(struct mlx5_vport *vport)
> struct mlx5_esw_sched_node *parent;
>
> lockdep_assert_held(&esw->state_lock);
> - esw_qos_lock(esw);
> + devl_lock(esw->dev->shd);
Could esw->dev->shd be NULL here?
In mlx5_shd_init(), initialization is explicitly skipped for non-PF devices,
meaning Sub-Functions (SFs) acting as E-Switch managers will have a NULL
shd pointer. If the device lacks VPD data, pci_vpd_alloc() returns -ENODEV,
causing mlx5_shd_init() to return 0 but leaving dev->shd as NULL.
When QoS operations are attempted on these devices, calling devl_lock()
dereferences the pointer, which could cause an immediate NULL pointer
dereference and kernel panic.
Additionally, this acquires the devlink lock while already holding
esw->state_lock, establishing the lock order:
state_lock -> devlink->lock.
Conversely, the devlink core invokes driver callbacks with the devlink
instance lock held, and the driver subsequently acquires esw->state_lock
inside these callbacks, creating the reverse order:
devlink->lock -> state_lock.
Does this introduce a circular locking dependency splat between state_lock
and the devlink instance lock?
> @@ -1159,26 +1117,25 @@ int mlx5_esw_qos_set_vport_rate(struct mlx5_vport *vport, u32 max_rate, u32 min_
> struct mlx5_eswitch *esw = vport->dev->priv.eswitch;
> int err;
>
> - esw_qos_lock(esw);
> + devl_lock(esw->dev->shd);
This function is invoked by mlx5_eswitch_set_vport_rate(), which acts as the
handler for the ndo_set_vf_rate netdev callback. The core network stack
executes ndo_set_vf_rate with the rtnl_lock held, establishing the lock
order: rtnl_lock -> devlink->lock.
However, the devlink subsystem enforces the opposite locking order
(devlink->lock -> rtnl_lock). Because all devlink locks share the same
lockdep class, does this inversion trigger a circular locking dependency
lockdep splat and pose an ABBA deadlock risk?
[ ... ]
> @@ -1859,13 +1763,15 @@ mlx5_esw_qos_vport_update_parent(struct mlx5_vport *vport,
> parent ? : esw->qos.root,
> extack);
> }
> - esw_qos_unlock(esw);
> +
> return err;
> }
>
> void mlx5_esw_qos_vport_clear_parent(struct mlx5_vport *vport)
> {
> + devl_lock(vport->dev->shd);
> mlx5_esw_qos_vport_update_parent(vport, NULL, NULL);
> + devl_unlock(vport->dev->shd);
> }
During device teardown, execution paths such as mlx5_unload_one_devl_locked()
hold the primary devlink instance lock. This path invokes
mlx5_eswitch_unload_vf_vports() -> mlx5_esw_offloads_devlink_port_unregister()
-> mlx5_esw_qos_vport_clear_parent().
Because both the primary devlink instance lock and the shd devlink instance
lock are initialized from the same site (devlink_alloc), they share the same
lockdep class.
Does calling devl_lock() directly (instead of using the devlink core's
mutex_lock_nested capability) while a lock of the same class is already held
trigger a recursive locking lockdep warning?
^ permalink raw reply
* Re: [PATCH net-next V9 12/14] net/mlx5: qos: Support cross-device tx scheduling
From: Jakub Kicinski @ 2026-03-31 2:08 UTC (permalink / raw)
To: tariqt
Cc: Jakub Kicinski, edumazet, pabeni, andrew+netdev, davem,
donald.hunter, horms, jiri, corbet, skhan, saeedm, leon, mbloch,
shuah, chuck.lever, matttbe, cjubran, cratiu, dtatulea,
jacob.e.keller, shshitrit, daniel.zahka, parav, ajayachandra,
kees, shayd, danielj, moshe, willemb, dw, petrm, sdf, daniel, joe,
razor, vadim.fedorenko, mst, antonio, allison.henderson,
minhquangbui99, noren, netdev, linux-kernel, linux-doc,
linux-rdma, linux-kselftest, gal, jiri
In-Reply-To: <20260326065949.44058-13-tariqt@nvidia.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/mlx5: qos: Support cross-device tx scheduling
This commit relaxes the restriction that rate groups can only contain vports
from the same E-Switch. It enables cross-device TX scheduling if the hardware
supports it, LAG is enabled, and the E-Switches share a devlink device.
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
[ ... ]
> @@ -1738,18 +1761,44 @@ int mlx5_esw_devlink_rate_node_del(struct devlink_rate *rate_node, void *priv,
> return 0;
> }
>
> +static int
> +mlx5_esw_validate_cross_esw_scheduling(struct mlx5_eswitch *esw,
> + struct mlx5_esw_sched_node *parent,
> + struct netlink_ext_ack *extack)
> +{
[ ... ]
> + if (!mlx5_lag_is_active(esw->dev)) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "Cross E-Switch scheduling requires LAG to be activated");
> + return -EOPNOTSUPP;
> + }
What happens to the cross-device rate scheduling if LAG is deactivated
after the configuration is applied?
When LAG is deactivated, the LAG state is torn down in hardware, but there
does not appear to be a corresponding cleanup or update triggered for the
cross-eswitch QoS scheduling elements.
Could this result in a regression where the hardware TSAR tree and devlink
rate configuration remain pointing to a now-invalid VHCA association,
bypassing the state validation and causing traffic drops or firmware errors?
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH bpf v4 1/2] bpf: tcp: Reject non-TCP skb in bpf_sk_assign_tcp_reqsk()
From: Martin KaFai Lau @ 2026-03-31 3:18 UTC (permalink / raw)
To: Kuniyuki Iwashima, Jiayuan Chen
Cc: bot+bpf-ci, bpf, clm, daniel, john.fastabend, sdf, ast, andrii,
eddyz87, song, yonghong.song, kpsingh, haoluo, jolsa, davem,
edumazet, kuba, pabeni, horms, shuah, netdev, linux-kernel,
linux-kselftest, martin.lau, ihor.solodrai
In-Reply-To: <CAAVpQUD1n7L6LkP32H_Ct71RLNpJ2RQWFc8w08+c8h=U_tDjpQ@mail.gmail.com>
On 3/30/26 6:35 PM, Kuniyuki Iwashima wrote:
> On Mon, Mar 30, 2026 at 2:26 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>>
>>
>> On 3/30/26 5:00 PM, bot+bpf-ci@kernel.org wrote:
>>>> #if IS_BUILTIN(CONFIG_IPV6)
>>>> case htons(ETH_P_IPV6):
>>>> + if (ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)
>>>> + return -EINVAL;
>>>> +
>>>> ops = &tcp6_request_sock_ops;
>>> For IPv6, ipv6_hdr(skb)->nexthdr gives the type of the header
>>> immediately following the base IPv6 header. If extension headers
>>> are present (hop-by-hop options, routing, etc.), nexthdr would be
>>> the extension header type rather than IPPROTO_TCP, even when the
>>> packet is a valid TCP segment.
>>>
>>> Would it be worth using ipv6_find_hdr() here, similar to
>>> bpf_update_srh_state() in the same file, to walk past any extension
>>> headers? The IPv4 check above is fine since ip_hdr(skb)->protocol
>>> always identifies the transport protocol directly.
>>>
>>> In practice this is very unlikely to matter since TCP SYN packets
>>> with IPv6 extension headers are essentially non-existent, but the
>>> check as written would reject them.
>>>
>>>
>>> ---
>>> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
>>> See:https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>>>
>>> CI run summary:https://github.com/kernel-patches/bpf/actions/runs/23735111188
>>
>>
>> Thanks for the analysis.
>>
>> There are many places in the kernel that check nexthdr directly without
>> walking extension headers.
>>
>> I'd prefer to keep it simple for now and leave ipv6_find_hdr() as a
>> potential future improvement if needed.
>
> +1.
>
> Given this feature is to create a reqsk to process on this running
> host, it does not make sense to support such ext options.
While at header reading, does it need a pskb_may_pull() before reading?
^ permalink raw reply
* [PATCH net-next] pppoe: update Kconfig URLs
From: Qingfang Deng @ 2026-03-31 3:33 UTC (permalink / raw)
To: linux-ppp, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Eric Biggers, netdev, linux-kernel
Cc: Paul Mackerras, Dianne Skoll, Jaco Kroon, James Carlson
Both links are no longer valid. Update them to the correct URLs.
Signed-off-by: Qingfang Deng <dqfext@gmail.com>
---
drivers/net/ppp/Kconfig | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ppp/Kconfig b/drivers/net/ppp/Kconfig
index a1806b4b84be..c751959c53a1 100644
--- a/drivers/net/ppp/Kconfig
+++ b/drivers/net/ppp/Kconfig
@@ -122,11 +122,10 @@ config PPPOE
help
Support for PPP over Ethernet.
- This driver requires the latest version of pppd from the CVS
- repository at cvs.samba.org. Alternatively, see the
- RoaringPenguin package (<http://www.roaringpenguin.com/pppoe>)
- which contains instruction on how to use this driver (under
- the heading "Kernel mode PPPoE").
+ This driver requires the latest version of pppd at
+ <https://ppp.samba.org>.
+ Alternatively, see the out-of-tree RP-PPPoE plugin at
+ <https://dianne.skoll.ca/projects/rp-pppoe/>.
choice
prompt "Number of PPPoE hash bits"
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v25 01/11] sfc: add cxl support
From: Dan Williams @ 2026-03-31 3:37 UTC (permalink / raw)
To: alejandro.lucero-palau, linux-cxl, netdev, dave.jiang,
dan.j.williams, edward.cree, davem, kuba, pabeni, edumazet
Cc: Alejandro Lucero, Jonathan Cameron, Edward Cree, Alison Schofield
In-Reply-To: <20260330143827.1278677-2-alejandro.lucero-palau@amd.com>
Hi Alejandro, similar to the approach taken with the CXL port error
handling series [1], I propose that additional changes to this
implementation be handled incrementally.
There may still be something that should be rebased out of the history,
but we have passed the point where fixes on top of stable commits starts
to be a better working model.
[1]: http://lore.kernel.org/69c98caef1348_178904100e0@dwillia2-mobl4.notmuch
One warning below...
alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
>
> Add CXL initialization based on new CXL API for accel drivers and make
> it dependent on kernel CXL configuration.
>
> Signed-off-by: Alejandro Lucero <alucerop@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Acked-by: Edward Cree <ecree.xilinx@gmail.com>
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
[..]
> + /* Create a cxl_dev_state embedded in the cxl struct using cxl core api
> + * specifying no mbox available.
> + */
> + cxl = devm_cxl_dev_state_create(&pci_dev->dev, CXL_DEVTYPE_DEVMEM,
> + pci_dev->dev.id, dvsec, struct efx_cxl,
> + cxlds, false);
> +
> + if (!cxl)
> + return -ENOMEM;
> +
> + probe_data->cxl = cxl;
> +
> + return 0;
> +}
> +
> +void efx_cxl_exit(struct efx_probe_data *probe_data)
> +{
> +}
Not a driver I maintain so feel free to disregard, but if you are going
to have an exit handler, then you also want an explicit
devm_cxl_dev_state_destroy() API.
As a reviewer I do not want to always remember that some of this
driver's resources are cleaned up in the ->remove() callback and others
are cleaned up post ->remove() by the devres core. It is either 100%
explicit release or 100% implicit release if I have my druthers.
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH iwl-net v1] ixgbe: stop re-reading flash on every get_drvinfo for e610
From: Rinitha, SX @ 2026-03-31 3:45 UTC (permalink / raw)
To: Loktionov, Aleksandr, intel-wired-lan@lists.osuosl.org,
Nguyen, Anthony L, Loktionov, Aleksandr
Cc: netdev@vger.kernel.org, Jagielski, Jedrzej
In-Reply-To: <20260304084232.2937498-1-aleksandr.loktionov@intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Aleksandr Loktionov
> Sent: 04 March 2026 14:13
> To: intel-wired-lan@lists.osuosl.org; Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Cc: netdev@vger.kernel.org; Jagielski, Jedrzej <jedrzej.jagielski@intel.com>
> Subject: [Intel-wired-lan] [PATCH iwl-net v1] ixgbe: stop re-reading flash on every get_drvinfo for e610
>
> ixgbe_get_drvinfo() calls ixgbe_refresh_fw_version() on every ethtool query for e610 adapters. That ends up in ixgbe_discover_flash_size(), which bisects the full 16 MB NVM space issuing one ACI command per step (~20 ms each, ~24 steps total = ~500 ms).
>
> Profiling on an idle E610-XAT2 system with telegraf scraping ethtool stats every 10 seconds:
>
> kretprobe:ixgbe_get_drvinfo took 527603 us
> kretprobe:ixgbe_get_drvinfo took 523978 us
> kretprobe:ixgbe_get_drvinfo took 552975 us
> kretprobe:ice_get_drvinfo took 3 us
> kretprobe:igb_get_drvinfo took 2 us
> kretprobe:i40e_get_drvinfo took 5 us
>
> The half-second stall happens under the RTNL lock, causing visible latency on ip-link and friends.
>
> The FW version can only change after an EMPR reset. All flash data is already populated at probe time and the cached adapter->eeprom_id is what get_drvinfo should be returning. The only place that needs to trigger a re-read is ixgbe_devlink_reload_empr_finish(), right after the EMPR completes and new firmware is running. Additionally, refresh the FW version in ixgbe_reinit_locked() so that any PF that undergoes a reinit after an EMPR (e.g. triggered by another PF's devlink reload) also picks up the new version in adapter->eeprom_id.
>
> ixgbe_devlink_info_get() keeps its refresh call for explicit "devlink dev info" queries, which is fine given those are user-initiated.
>
> Fixes: c9e563cae19e ("ixgbe: add support for devlink reload")
> Co-developed-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/devlink/devlink.c | 2 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 13 +++++++------
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 10 ++++++++++
> 4 files changed, 19 insertions(+), 8 deletions(-)
>
Tested-by: Rinitha S <sx.rinitha@intel.com> (A Contingent worker at Intel)
^ permalink raw reply
* Re: [PATCH v25 04/11] cxl: Prepare memdev creation for type2
From: Dan Williams @ 2026-03-31 3:46 UTC (permalink / raw)
To: alejandro.lucero-palau, linux-cxl, netdev, dave.jiang,
dan.j.williams, edward.cree, davem, kuba, pabeni, edumazet
Cc: Alejandro Lucero, Ben Cheatham, Jonathan Cameron,
Alison Schofield
In-Reply-To: <20260330143827.1278677-5-alejandro.lucero-palau@amd.com>
alejandro.lucero-palau@ wrote:
> From: Alejandro Lucero <alucerop@amd.com>
>
> Current cxl core is relying on a CXL_DEVTYPE_CLASSMEM type device when
> creating a memdev leading to problems when obtaining cxl_memdev_state
> references from a CXL_DEVTYPE_DEVMEM type.
>
> Modify check for obtaining cxl_memdev_state adding CXL_DEVTYPE_DEVMEM
> support.
>
> Make devm_cxl_add_memdev accessible from an accel driver.
>
[..]
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index fcffe24dcb42..ff858318091f 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -65,6 +65,26 @@ static int cxl_debugfs_poison_clear(void *data, u64 dpa)
> DEFINE_DEBUGFS_ATTRIBUTE(cxl_poison_clear_fops, NULL,
> cxl_debugfs_poison_clear, "%llx\n");
>
> +static void cxl_memdev_poison_enable(struct cxl_memdev_state *mds,
> + struct cxl_memdev *cxlmd,
> + struct dentry *dentry)
> +{
> + /*
> + * Avoid poison debugfs for DEVMEM aka accelerators as they rely on
> + * cxl_memdev_state.
> + */
There is nothing stopping an accelerator from having a CXL mailbox, so
should probably circle back to either make this comment more generic, or
just delete it.
^ permalink raw reply
* RE: [PATCH net v2] tipc: fix UAF race in tipc_mon_peer_up/down/remove_peer vs bearer teardown
From: Tung Quang Nguyen @ 2026-03-31 3:46 UTC (permalink / raw)
To: Kai Zen; +Cc: stable@vger.kernel.org, jmaloy@redhat.com, netdev@vger.kernel.org
In-Reply-To: <CALynFi5d0DuGW50xq7xQnsDPdEuN5jBGTqh8bcsUwxk6L-FAdA@mail.gmail.com>
>Subject: [PATCH net v2] tipc: fix UAF race in
>tipc_mon_peer_up/down/remove_peer vs bearer teardown
>
>CVE-2025-40280 fixed tipc_mon_reinit_self() accessing monitors[] from a
>workqueue without RTNL. That patch closed the workqueue path by adding
>rtnl_lock() around the call.
>
>However, three additional functions in the same subsystem access tipc_net-
>>monitors[] from softirq context with no RCU protection at all:
>
> tipc_mon_peer_up() - called from tipc_node_write_unlock()
> tipc_mon_peer_down() - called from tipc_node_write_unlock()
> tipc_mon_remove_peer() - called from tipc_node_link_down()
>
>These three are invoked from the packet receive path (tipc_rcv ->
>tipc_node_write_unlock / tipc_node_link_down) and hold only the per-node
>rwlock, not RTNL.
>
>Concurrently, bearer_disable() -- which always holds RTNL per its own inline
>documentation -- calls tipc_mon_delete(), which:
>
> 1. acquires mon->lock
> 2. sets tn->monitors[bearer_id] = NULL
> 3. frees all peer entries
> 4. releases mon->lock
> 5. calls kfree(mon) <-- no synchronize_rcu()
>
>The race is structural: there is no shared lock between the data-path reader
>(which reads monitors[id] then acquires mon->lock) and the teardown path
>(which acquires mon->lock, NULLs the slot, then frees).
>A softirq thread can read a non-NULL mon pointer, get preempted, and resume
>after kfree(mon) has run on another CPU, then call
>write_lock_bh(&mon->lock) on freed memory:
>
> CPU 0 (softirq / tipc_rcv) CPU 1 (RTNL / bearer_disable)
> tipc_mon_peer_up()
> mon = tipc_monitor(net, id)
> [mon is non-NULL]
> tipc_mon_delete()
> write_lock_bh(&mon->lock)
> tn->monitors[id] = NULL
> ...
> write_unlock_bh(&mon->lock)
> kfree(mon)
> write_lock_bh(&mon->lock) <-- UAF
>
Can you reproduce above scenario and capture the stack trace when UAF happens ?
>The fix mirrors the existing bearer_list[] pattern in the same module:
>convert monitors[] to __rcu, use rcu_assign_pointer() on creation,
>RCU_INIT_POINTER() + synchronize_rcu() on deletion (before the kfree), and
>the appropriate rcu_dereference_bh() vs rtnl_dereference() variant at each
>read site depending on execution context.
>
>synchronize_rcu() in tipc_mon_delete() is placed after the
>write_unlock_bh() and before timer_shutdown_sync() + kfree() to ensure all
>softirq-context readers that already observed the old pointer have completed
>before the memory is freed.
>
Not sure why your patch does not implement your above solution. I see only one change in core.h.
>Fixes: 35c55c9877f8 ("tipc: add neighbor monitoring framework")
>Cc: stable@vger.kernel.org
>Signed-off-by: Kai Aizen <kai.aizen.dev@gmail.com>
>---
>v2: Resubmit targeting mainline via netdev per stable-kernel-rules (Option 1).
> No code changes from v1.
>
> net/tipc/core.h | 2 +-
> net/tipc/monitor.c | 51 +++++++++++++++++++++++++++++++++--------------
> 2 files changed, 37 insertions(+), 16 deletions(-)
>
>diff --git a/net/tipc/core.h b/net/tipc/core.h
>--- a/net/tipc/core.h
>+++ b/net/tipc/core.h
>@@ -109,7 +109,7 @@
> u32 num_links;
> /* Neighbor monitoring list */
>- struct tipc_monitor *monitors[MAX_BEARERS];
>+ struct tipc_monitor __rcu *monito[MAX_BEARERS];
> rs
>+
^ permalink raw reply
* Re: [PATCH v8 net-next 6/6] octeontx2-af: npc: Support for custom KPU profile from filesystem
From: Ratheesh Kannoth @ 2026-03-31 3:43 UTC (permalink / raw)
To: Simon Horman
Cc: netdev, linux-kernel, sgoutham, davem, edumazet, kuba, pabeni,
donald.hunter, jiri, chuck.lever, matttbe, cjubran, shshitrit,
dtatulea, tariqt
In-Reply-To: <20260327133036.GE567789@horms.kernel.org>
On 2026-03-27 at 19:00:36, Simon Horman (horms@kernel.org) wrote:
> AI review flags that:
>
> npc_load_kpu_profile_from_fs() calls npc_apply_custom_kpu(),
> which returns early if the following condition is met.
>
> if (fw->kpus > profile->kpus)
>
> Does npc_prepare_default_kpu() need to be called before
> npc_load_kpu_profile_from_fs() to initialise profile->kpus,
> which is 0 by default due to profile being allocated using devm_kzalloc()?
simon,
Thank you very much for your time.
The v9 AI review has raised the following concern again:
https://netdev-ai.bots.linux.dev/ai-review.html?id=f29b81fe-bb3f-4902-85de-8271effbb41c
"In the filesystem loading path, does profile->kpus need to be initialized
before calling npc_load_kpu_profile_from_fs()?
Simon Horman raised this concern in v8 review on lore:
https://lore.kernel.org/netdev/20260327133036.GE567789@horms.kernel.org/"
would like to clarify that npc_prepare_default_kpu(rvu, profile) is invoked
very early in the flow, and therefore applies to all execution paths
(default, filesystem loading, and firmware loading). Additionally,
if the filesystem loading fails, npc_prepare_default_kpu() is invoked again to
ensure that all values are reset to their defaults before attempting to load from firmware.
Could you please advise on the best way to address the AI review comment?
Specifically, would you recommend:
1. Adding a clarifying comment in the code, or
2. Invoking npc_prepare_default_kpu() again just before npc_load_kpu_profile_from_fs()
to make this behavior clearer?
Thank you once again !
^ permalink raw reply
* [PATCH net-next 0/4] ynl/ethtool/netlink: warn nla_len overflow for large string sets
From: Hangbin Liu @ 2026-03-31 3:56 UTC (permalink / raw)
To: Donald Hunter, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Andrew Lunn
Cc: netdev, linux-kernel, Hangbin Liu
This series addresses a silent data corruption issue triggered when ynl
retrieves string sets from NICs with a large number of statistics entries
(e.g. mlx5_core with thousands of ETH_SS_STATS strings).
The root cause is that struct nlattr.nla_len is a __u16 (max 65535
bytes). When a NIC exports enough statistics strings, the
ETHTOOL_A_STRINGSET_STRINGS nest built by strset_fill_set() exceeds
this limit. nla_nest_end() silently truncates the length on assignment,
producing a corrupted netlink message. In the doit path the corrupted
message is delivered to userspace without any error; in the dumpit path
an -EMSGSIZE may be returned if the data does not fit in the dump skb.
Patch 1 improves the userspace tool: rename the doit/dumpit helpers
to do_set/do_get and convert do_get to use ynl.do() with an
explicit device header instead of a full dump with client-side filtering.
Patch 2 adds a --dbg-small-recv option to the YNL ethtool tool,
matching the same option already present in cli.py, to aid debugging
of netlink message size issues.
Patch 3 is the kernel fix: check whether the strings_attr nest would
exceed U16_MAX before calling nla_nest_end() in strset_fill_set(),
and return -EMSGSIZE early if so.
Patch 4 adds a generic WARN_ON_ONCE() in nla_nest_end() itself, so
that any future caller hitting the same overflow is immediately visible
in the kernel log rather than silently corrupting data.
---
Hangbin Liu (4):
tools: ynl: ethtool: use doit instead of dumpit for per-device GET
tools: ynl: ethtool: add --dbg-small-recv option
ethtool: strset: check nla_len overflow before nla_nest_end
netlink: warn on nla_len overflow in nla_nest_end()
include/net/netlink.h | 1 +
net/ethtool/strset.c | 4 +++
tools/net/ynl/pyynl/ethtool.py | 77 ++++++++++++++++++++++--------------------
3 files changed, 46 insertions(+), 36 deletions(-)
---
base-commit: dc9e9d61e301c087bcd990dbf2fa18ad3e2e1429
change-id: 20260324-b4-ynl_ethtool-f87cd42f572c
Best regards,
--
Hangbin Liu <liuhangbin@gmail.com>
^ permalink raw reply
* [PATCH net-next 1/4] tools: ynl: ethtool: use doit instead of dumpit for per-device GET
From: Hangbin Liu @ 2026-03-31 3:56 UTC (permalink / raw)
To: Donald Hunter, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Andrew Lunn
Cc: netdev, linux-kernel, Hangbin Liu
In-Reply-To: <20260331-b4-ynl_ethtool-v1-0-dda2a9b55df8@gmail.com>
Rename the local helper doit() to do_set() and dumpit() to do_get() to
better reflect their purpose.
Convert do_get() to use ynl.do() with an explicit device header instead
of ynl.dump() followed by client-side filtering. This is more efficient
as the kernel only processes and returns data for the requested device,
rather than dumping all devices across the netns.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
tools/net/ynl/pyynl/ethtool.py | 68 ++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/tools/net/ynl/pyynl/ethtool.py b/tools/net/ynl/pyynl/ethtool.py
index f1a2a2a89985..8bf234d594b3 100755
--- a/tools/net/ynl/pyynl/ethtool.py
+++ b/tools/net/ynl/pyynl/ethtool.py
@@ -84,9 +84,9 @@ def print_speed(name, value):
speed = [ k for k, v in value.items() if v and speed_re.match(k) ]
print(f'{name}: {" ".join(speed)}')
-def doit(ynl, args, op_name):
+def do_set(ynl, args, op_name):
"""
- Prepare request header, parse arguments and doit.
+ Prepare request header, parse arguments and do a set operation.
"""
req = {
'header': {
@@ -97,26 +97,24 @@ def doit(ynl, args, op_name):
args_to_req(ynl, op_name, args.args, req)
ynl.do(op_name, req)
-def dumpit(ynl, args, op_name, extra=None):
+def do_get(ynl, args, op_name, extra=None):
"""
- Prepare request header, parse arguments and dumpit (filtering out the
- devices we're not interested in).
+ Prepare request header and get info for a specific device using doit.
"""
extra = extra or {}
- reply = ynl.dump(op_name, { 'header': {} } | extra)
+ req = {'header': {'dev-name': args.device}}
+ req['header'].update(extra.pop('header', {}))
+ req.update(extra)
+
+ reply = ynl.do(op_name, req)
if not reply:
return {}
- for msg in reply:
- if msg['header']['dev-name'] == args.device:
- if args.json:
- pprint.PrettyPrinter().pprint(msg)
- sys.exit(0)
- msg.pop('header', None)
- return msg
-
- print(f"Not supported for device {args.device}")
- sys.exit(1)
+ if args.json:
+ pprint.PrettyPrinter().pprint(reply)
+ sys.exit(0)
+ reply.pop('header', None)
+ return reply
def bits_to_dict(attr):
"""
@@ -181,15 +179,15 @@ def main():
return
if args.set_eee:
- doit(ynl, args, 'eee-set')
+ do_set(ynl, args, 'eee-set')
return
if args.set_pause:
- doit(ynl, args, 'pause-set')
+ do_set(ynl, args, 'pause-set')
return
if args.set_coalesce:
- doit(ynl, args, 'coalesce-set')
+ do_set(ynl, args, 'coalesce-set')
return
if args.set_features:
@@ -198,20 +196,20 @@ def main():
return
if args.set_channels:
- doit(ynl, args, 'channels-set')
+ do_set(ynl, args, 'channels-set')
return
if args.set_ring:
- doit(ynl, args, 'rings-set')
+ do_set(ynl, args, 'rings-set')
return
if args.show_priv_flags:
- flags = bits_to_dict(dumpit(ynl, args, 'privflags-get')['flags'])
+ flags = bits_to_dict(do_get(ynl, args, 'privflags-get')['flags'])
print_field(flags)
return
if args.show_eee:
- eee = dumpit(ynl, args, 'eee-get')
+ eee = do_get(ynl, args, 'eee-get')
ours = bits_to_dict(eee['modes-ours'])
peer = bits_to_dict(eee['modes-peer'])
@@ -232,18 +230,18 @@ def main():
return
if args.show_pause:
- print_field(dumpit(ynl, args, 'pause-get'),
+ print_field(do_get(ynl, args, 'pause-get'),
('autoneg', 'Autonegotiate', 'bool'),
('rx', 'RX', 'bool'),
('tx', 'TX', 'bool'))
return
if args.show_coalesce:
- print_field(dumpit(ynl, args, 'coalesce-get'))
+ print_field(do_get(ynl, args, 'coalesce-get'))
return
if args.show_features:
- reply = dumpit(ynl, args, 'features-get')
+ reply = do_get(ynl, args, 'features-get')
available = bits_to_dict(reply['hw'])
requested = bits_to_dict(reply['wanted']).keys()
active = bits_to_dict(reply['active']).keys()
@@ -270,7 +268,7 @@ def main():
return
if args.show_channels:
- reply = dumpit(ynl, args, 'channels-get')
+ reply = do_get(ynl, args, 'channels-get')
print(f'Channel parameters for {args.device}:')
print('Pre-set maximums:')
@@ -290,7 +288,7 @@ def main():
return
if args.show_ring:
- reply = dumpit(ynl, args, 'channels-get')
+ reply = do_get(ynl, args, 'channels-get')
print(f'Ring parameters for {args.device}:')
@@ -319,7 +317,7 @@ def main():
print('NIC statistics:')
# TODO: pass id?
- strset = dumpit(ynl, args, 'strset-get')
+ strset = do_get(ynl, args, 'strset-get')
pprint.PrettyPrinter().pprint(strset)
req = {
@@ -338,7 +336,7 @@ def main():
},
}
- rsp = dumpit(ynl, args, 'stats-get', req)
+ rsp = do_get(ynl, args, 'stats-get', req)
pprint.PrettyPrinter().pprint(rsp)
return
@@ -349,7 +347,7 @@ def main():
},
}
- tsinfo = dumpit(ynl, args, 'tsinfo-get', req)
+ tsinfo = do_get(ynl, args, 'tsinfo-get', req)
print(f'Time stamping parameters for {args.device}:')
@@ -377,7 +375,7 @@ def main():
return
print(f'Settings for {args.device}:')
- linkmodes = dumpit(ynl, args, 'linkmodes-get')
+ linkmodes = do_get(ynl, args, 'linkmodes-get')
ours = bits_to_dict(linkmodes['ours'])
supported_ports = ('TP', 'AUI', 'BNC', 'MII', 'FIBRE', 'Backplane')
@@ -425,7 +423,7 @@ def main():
5: 'Directly Attached Copper',
0xef: 'None',
}
- linkinfo = dumpit(ynl, args, 'linkinfo-get')
+ linkinfo = do_get(ynl, args, 'linkinfo-get')
print(f'Port: {ports.get(linkinfo["port"], "Other")}')
print_field(linkinfo, ('phyaddr', 'PHYAD'))
@@ -447,11 +445,11 @@ def main():
mdix = mdix_ctrl.get(linkinfo['tp-mdix'], 'Unknown (auto)')
print(f'MDI-X: {mdix}')
- debug = dumpit(ynl, args, 'debug-get')
+ debug = do_get(ynl, args, 'debug-get')
msgmask = bits_to_dict(debug.get("msgmask", [])).keys()
print(f'Current message level: {" ".join(msgmask)}')
- linkstate = dumpit(ynl, args, 'linkstate-get')
+ linkstate = do_get(ynl, args, 'linkstate-get')
detected_states = {
0: 'no',
1: 'yes',
--
Git-155)
^ permalink raw reply related
* [PATCH net-next 2/4] tools: ynl: ethtool: add --dbg-small-recv option
From: Hangbin Liu @ 2026-03-31 3:56 UTC (permalink / raw)
To: Donald Hunter, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Andrew Lunn
Cc: netdev, linux-kernel, Hangbin Liu
In-Reply-To: <20260331-b4-ynl_ethtool-v1-0-dda2a9b55df8@gmail.com>
Add a --dbg-small-recv debug option to control the recv() buffer size
used by YNL, matching the same option already present in cli.py. This
is useful if user need to get large netlink message.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
tools/net/ynl/pyynl/ethtool.py | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/tools/net/ynl/pyynl/ethtool.py b/tools/net/ynl/pyynl/ethtool.py
index 8bf234d594b3..6c50368de967 100755
--- a/tools/net/ynl/pyynl/ethtool.py
+++ b/tools/net/ynl/pyynl/ethtool.py
@@ -166,12 +166,19 @@ def main():
parser.add_argument('device', metavar='device', type=str)
parser.add_argument('args', metavar='args', type=str, nargs='*')
+ dbg_group = parser.add_argument_group('Debug options')
+ dbg_group.add_argument('--dbg-small-recv', default=0, const=4000,
+ action='store', nargs='?', type=int, metavar='INT',
+ help="Length of buffers used for recv()")
+
args = parser.parse_args()
spec = os.path.join(spec_dir(), 'ethtool.yaml')
schema = os.path.join(schema_dir(), 'genetlink-legacy.yaml')
- ynl = YnlFamily(spec, schema)
+ ynl = YnlFamily(spec, schema, recv_size=args.dbg_small_recv)
+ if args.dbg_small_recv:
+ ynl.set_recv_dbg(True)
if args.set_priv_flags:
# TODO: parse the bitmask
--
Git-155)
^ permalink raw reply related
* [PATCH net-next 3/4] ethtool: strset: check nla_len overflow before nla_nest_end
From: Hangbin Liu @ 2026-03-31 3:56 UTC (permalink / raw)
To: Donald Hunter, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Andrew Lunn
Cc: netdev, linux-kernel, Hangbin Liu
In-Reply-To: <20260331-b4-ynl_ethtool-v1-0-dda2a9b55df8@gmail.com>
The netlink attribute length field nla_len is a __u16, which can only
represent values up to 65535 bytes. NICs with a large number of
statistics strings (e.g. mlx5_core with thousands of ETH_SS_STATS
entries) can produce a ETHTOOL_A_STRINGSET_STRINGS nest that exceeds
this limit.
When nla_nest_end() writes the actual nest size back to nla_len, the
value is silently truncated. This results in a corrupted netlink message
being sent to userspace: the parser reads a wrong (truncated) attribute
length and misaligns all subsequent attribute boundaries, causing decode
errors.
Fix this by checking whether the size of strings_attr would exceed
U16_MAX after all strings have been written, and give up nla put if so.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/ethtool/strset.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index f6a67109beda..9c502b290f5c 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -441,6 +441,10 @@ static int strset_fill_set(struct sk_buff *skb,
if (strset_fill_string(skb, set_info, i) < 0)
goto nla_put_failure;
}
+
+ if (skb_tail_pointer(skb) - (unsigned char *)strings_attr > U16_MAX)
+ goto nla_put_failure;
+
nla_nest_end(skb, strings_attr);
}
--
Git-155)
^ permalink raw reply related
* [PATCH net-next 4/4] netlink: warn on nla_len overflow in nla_nest_end()
From: Hangbin Liu @ 2026-03-31 3:56 UTC (permalink / raw)
To: Donald Hunter, Jakub Kicinski, David S. Miller, Eric Dumazet,
Paolo Abeni, Simon Horman, Andrew Lunn
Cc: netdev, linux-kernel, Hangbin Liu
In-Reply-To: <20260331-b4-ynl_ethtool-v1-0-dda2a9b55df8@gmail.com>
The nla_len field in struct nlattr is a __u16, which can only hold
values up to 65535. If a nested attribute grows beyond this limit,
nla_nest_end() silently truncates the length, producing a corrupted
netlink message with no indication of the problem.
Since this is unlikely to happen, to avoid unnecessary checking every
time on the production system, add a DEBUG_NET_WARN_ON_ONCE() before
the assignment to make this overflow visible in the debug kernel log.
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
include/net/netlink.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 1a8356ca4b78..00ea52dc08c4 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -2260,6 +2260,7 @@ static inline struct nlattr *nla_nest_start(struct sk_buff *skb, int attrtype)
*/
static inline int nla_nest_end(struct sk_buff *skb, struct nlattr *start)
{
+ DEBUG_NET_WARN_ON_ONCE(skb_tail_pointer(skb) - (unsigned char *)start > U16_MAX);
start->nla_len = skb_tail_pointer(skb) - (unsigned char *)start;
return skb->len;
}
--
Git-155)
^ permalink raw reply related
* Re: clarification: PCI device not getting enumerated
From: Ratheesh Kannoth @ 2026-03-31 4:08 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: vidyas, bhelgaas, netdev, linux-kernel, linux-pci, sbhatta
In-Reply-To: <abojekUtFrMxM1RB@rkannoth-OptiPlex-7090>
On 2026-03-18 at 09:30:58, Ratheesh Kannoth (rkannoth@marvell.com) wrote:
> On 2026-03-18 at 02:36:40, Bjorn Helgaas (helgaas@kernel.org) wrote:
> > > Hi,
> > >
> > > Below commit breaks PCI enumeration of a marvell PCI endpoint (177d:a0e2).
> > > I am not familiar with PCI code, could you please help in debugging/fixing this ?
> > >
> > > Before the commit, the kernel set PCI_REASSIGN_ALL_BUS. So pcibios_assign_all_busses() was true,
> > > and in pci_scan_bridge_extend() the kernel took the “reassign” path and used EA
> > > (and pci_ea_fixed_busnrs()) to assign bus numbers.
> > >
> > > commit 7246a4520b4bf1494d7d030166a11b5226f6d508
> > > Author: Vidya Sagar <vidyas@nvidia.com>
> > > Date: Wed May 8 23:11:38 2024 +0530
> > >
> > > PCI: Use preserve_config in place of pci_flags
> > >
> > > Use preserve_config in place of checking for PCI_PROBE_ONLY flag to enable
> > > support for "linux,pci-probe-only" on a per host bridge basis.
> > >
> > > This also obviates the use of adding PCI_REASSIGN_ALL_BUS flag if
> > > !PCI_PROBE_ONLY, as pci_assign_unassigned_root_bus_resources() takes care
> > > of reassigning the resources that are not already claimed.
> >
> > This commit appeared in v6.11. Apparently on v6.6, 0002:1b:00.0 is
> > detected, and in the v6.12 dmesg below, we don't enumerate it.
> >
> > It looks like 7246a4520b4b can still be reverted cleanly from
> > v7.0-rc1. Can you collect the complete dmesg logs from v7.0-rc1
> > (where I assume we won't see 0002:1b:00.0) and from v7.0-rc1 with
> > 7246a4520b4b reverted (where I assume we *will* see it) and output of
> > "sudo lspci -vv"? Use the same kernel config and DT for both, of
> > course.
> Please find dmesg, lspci -vv and device tree output for v7.0-rc1 and v7.0-rc1 with patch reverted
> at bottom of this email. Used same kernel config and DT for both.
Sorry to bother.
Could you please advise on the preferred approach to address this issue? Alternatively, if further
testing or additional data would be helpful, I would be happy to carry that out and share the results.
Thanks a ton for your time !
^ permalink raw reply
* [PATCH net v3] ipv6: fix data race in fib6_metric_set() using cmpxchg
From: Hangbin Liu @ 2026-03-31 4:17 UTC (permalink / raw)
To: David S. Miller, David Ahern, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Jiayuan Chen
Cc: David Ahern, netdev, linux-kernel, Fei Liu, Hangbin Liu
fib6_metric_set() may be called concurrently from softirq context without
holding the FIB table lock. A typical path is:
ndisc_router_discovery()
spin_unlock_bh(&table->tb6_lock) <- lock released
fib6_metric_set(rt, RTAX_HOPLIMIT, ...) <- lockless call
When two CPUs process Router Advertisement packets for the same router
simultaneously, they can both arrive at fib6_metric_set() with the same
fib6_info pointer whose fib6_metrics still points to dst_default_metrics.
if (f6i->fib6_metrics == &dst_default_metrics) { /* both CPUs: true */
struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC);
refcount_set(&p->refcnt, 1);
f6i->fib6_metrics = p; /* CPU1 overwrites CPU0's p -> p0 leaked */
}
The dst_metrics allocated by the losing CPU has refcnt=1 but no pointer
to it anywhere in memory, producing a kmemleak report:
unreferenced object 0xff1100025aca1400 (size 96):
comm "softirq", pid 0, jiffies 4299271239
backtrace:
kmalloc_trace+0x28a/0x380
fib6_metric_set+0xcd/0x180
ndisc_router_discovery+0x12dc/0x24b0
icmpv6_rcv+0xc16/0x1360
Fix this by:
- Set val for p->metrics before published via cmpxchg() so the metrics
value is ready before the pointer becomes visible to other CPUs.
- Replace the plain pointer store with cmpxchg() and free the allocation
safely when competition failed.
- Add READ_ONCE()/WRITE_ONCE() for metrics[] setting in the non-default
metrics path to prevent compiler-based data races.
Fixes: d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
Reported-by: Fei Liu <feliu@redhat.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
Changes in v3:
- move variable declarations to the top of function (Jiayuan Chen)
- Link to v2: https://lore.kernel.org/r/20260327-b4-fib6_metric_set-kmemleak-v2-1-366b2c78b5c2@gmail.com
Changes in v2:
- Set val for p->metrics before published via cmpxchg() (Eric Dumazet)
- Add READ_ONCE()/WRITE_ONCE() for metrics[] setting (Jiayuan Chen)
- Link to v1: https://lore.kernel.org/r/20260326-b4-fib6_metric_set-kmemleak-v1-1-c89fc1b312c0@gmail.com
---
net/ipv6/ip6_fib.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index dd26657b6a4a..45ef4d65dcbc 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -727,20 +727,28 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
void fib6_metric_set(struct fib6_info *f6i, int metric, u32 val)
{
+ struct dst_metrics *m;
+
if (!f6i)
return;
- if (f6i->fib6_metrics == &dst_default_metrics) {
+ if (READ_ONCE(f6i->fib6_metrics) == &dst_default_metrics) {
+ struct dst_metrics *dflt = (struct dst_metrics *)&dst_default_metrics;
struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC);
if (!p)
return;
+ p->metrics[metric - 1] = val;
refcount_set(&p->refcnt, 1);
- f6i->fib6_metrics = p;
+ if (cmpxchg(&f6i->fib6_metrics, dflt, p) != dflt)
+ kfree(p);
+ else
+ return;
}
- f6i->fib6_metrics->metrics[metric - 1] = val;
+ m = READ_ONCE(f6i->fib6_metrics);
+ WRITE_ONCE(m->metrics[metric - 1], val);
}
/*
---
base-commit: c4ea7d8907cf72b259bf70bd8c2e791e1c4ff70f
change-id: 20260326-b4-fib6_metric_set-kmemleak-7aa51978284a
Best regards,
--
Hangbin Liu <liuhangbin@gmail.com>
^ permalink raw reply related
* [PATCH net v3 1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-31 5:02 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
The old-method path in fw_classify() calls tcf_block_q() and
dereferences q->handle. Shared blocks leave block->q NULL, causing a
NULL deref when an empty cls_fw filter is attached to a shared block
and a packet with a nonzero major skb mark is classified.
Reject the configuration in fw_change() when the old method (no
TCA_OPTIONS) is used on a shared block, since fw_classify()'s
old-method path needs block->q which is NULL for shared blocks.
The fixed null-ptr-deref calling stack:
KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
RIP: 0010:fw_classify (net/sched/cls_fw.c:81)
Call Trace:
tcf_classify (./include/net/tc_wrapper.h:197 net/sched/cls_api.c:1764 net/sched/cls_api.c:1860)
tc_run (net/core/dev.c:4401)
__dev_queue_xmit (net/core/dev.c:4535 net/core/dev.c:4790)
Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
v2: Correct 3/3 selftest case
v3: Avoid the bug earlier in fw_change
net/sched/cls_fw.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index be81c108179d..23884ef8b80c 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -247,8 +247,18 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
struct nlattr *tb[TCA_FW_MAX + 1];
int err;
- if (!opt)
- return handle ? -EINVAL : 0; /* Succeed if it is old method. */
+ if (!opt) {
+ if (handle)
+ return -EINVAL;
+
+ if (tcf_block_shared(tp->chain->block)) {
+ NL_SET_ERR_MSG(extack,
+ "Must specify mark when attaching fw filter to block");
+ return -EINVAL;
+ }
+
+ return 0; /* Succeed if it is old method. */
+ }
err = nla_parse_nested_deprecated(tb, TCA_FW_MAX, opt, fw_policy,
NULL);
--
2.43.0
^ permalink raw reply related
* [PATCH net v3 2/3] net/sched: cls_flow: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-31 5:02 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
In-Reply-To: <20260331050217.504278-1-xmei5@asu.edu>
flow_change() calls tcf_block_q() and dereferences q->handle to derive
a default baseclass. Shared blocks leave block->q NULL, causing a NULL
deref when a flow filter without a fully qualified baseclass is created
on a shared block.
Check tcf_block_shared() before accessing block->q and return -EINVAL
for shared blocks. This avoids the null-deref shown below:
=======================================================================
KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
RIP: 0010:flow_change (net/sched/cls_flow.c:508)
Call Trace:
tc_new_tfilter (net/sched/cls_api.c:2432)
rtnetlink_rcv_msg (net/core/rtnetlink.c:6980)
[...]
=======================================================================
Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
Reported-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
v2: Correct 3/3 selftest case
v3: add error message
net/sched/cls_flow.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 339c664beff6..ab364e4e4686 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -503,8 +503,16 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
}
if (TC_H_MAJ(baseclass) == 0) {
- struct Qdisc *q = tcf_block_q(tp->chain->block);
+ struct tcf_block *block = tp->chain->block;
+ struct Qdisc *q;
+ if (tcf_block_shared(block)) {
+ NL_SET_ERR_MSG(extack,
+ "Must specify baseclass when attaching flow filter to block");
+ goto err2;
+ }
+
+ q = tcf_block_q(block);
baseclass = TC_H_MAKE(q->handle, baseclass);
}
if (TC_H_MIN(baseclass) == 0)
--
2.43.0
^ permalink raw reply related
* [PATCH net v3 3/3] selftests/tc-testing: add tests for cls_fw and cls_flow on shared blocks
From: Xiang Mei @ 2026-03-31 5:02 UTC (permalink / raw)
To: netdev; +Cc: jhs, jiri, davem, edumazet, kuba, horms, shuah, bestswngs,
Xiang Mei
In-Reply-To: <20260331050217.504278-1-xmei5@asu.edu>
Regression tests for the shared-block NULL derefs fixed in the previous
two patches:
- fw: attempt to attach an empty fw filter to a shared block and
verify the configuration is rejected with EINVAL.
- flow: create a flow filter on a shared block without a baseclass
and verify the configuration is rejected with EINVAL.
Signed-off-by: Xiang Mei <xmei5@asu.edu>
---
v2: Correct 3/3 selftest case
v3: make b7e3 forcus on fw_change
.../tc-testing/tc-tests/infra/filter.json | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json b/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json
index 8d10042b489b..dbce6436ed26 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/infra/filter.json
@@ -22,5 +22,49 @@
"teardown": [
"$TC qdisc del dev $DUMMY root handle 1: htb default 1"
]
+ },
+ {
+ "id": "b7e3",
+ "name": "Empty fw filter on shared block - rejected at config time",
+ "category": [
+ "filter",
+ "fw"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 egress_block 1 clsact"
+ ],
+ "cmdUnderTest": "$TC filter add block 1 protocol ip prio 1 fw",
+ "expExitCode": "2",
+ "verifyCmd": "$TC filter show block 1",
+ "matchPattern": "fw",
+ "matchCount": "0",
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
+ },
+ {
+ "id": "c8f4",
+ "name": "Flow filter on shared block without baseclass - rejected at config time",
+ "category": [
+ "filter",
+ "flow"
+ ],
+ "plugins": {
+ "requires": "nsPlugin"
+ },
+ "setup": [
+ "$TC qdisc add dev $DEV1 ingress_block 1 clsact"
+ ],
+ "cmdUnderTest": "$TC filter add block 1 protocol ip prio 1 handle 1 flow map key dst",
+ "expExitCode": "2",
+ "verifyCmd": "$TC filter show block 1",
+ "matchPattern": "flow",
+ "matchCount": "0",
+ "teardown": [
+ "$TC qdisc del dev $DEV1 clsact"
+ ]
}
]
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net 1/3] net/sched: cls_fw: fix NULL pointer dereference on shared blocks
From: Xiang Mei @ 2026-03-31 5:04 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, jiri, davem, edumazet, kuba, horms, shuah, bestswngs
In-Reply-To: <CAM0EoMnNp2tV1Gn7a8KFMhUnQAWs0FDzb6bMUwhHtfPz-Uiqmg@mail.gmail.com>
Thanks for the review and advice. It's a better patch. I have sent the v3:
- for cls_fw and cls_flow, we check in fw/flow_change
- Adjust the tests
Thanks,
Xiang
On Mon, Mar 30, 2026 at 9:46 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Sun, Mar 29, 2026 at 2:52 PM Xiang Mei <xmei5@asu.edu> wrote:
> >
> > On Sun, Mar 29, 2026 at 08:15:17AM -0400, Jamal Hadi Salim wrote:
> > > On Fri, Mar 27, 2026 at 10:11 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > >
> > > > On Fri, Mar 27, 2026 at 2:13 AM Xiang Mei <xmei5@asu.edu> wrote:
> > > > >
> > > > > The old-method path in fw_classify() calls tcf_block_q() and
> > > > > dereferences q->handle. Shared blocks leave block->q NULL, causing a
> > > > > NULL deref when an empty cls_fw filter is attached to a shared block
> > > > > and a packet with a nonzero major skb mark is classified.
> > > > >
> > > > > Check tcf_block_shared() before accessing block->q and return -1 (no
> > > > > match) for shared blocks, consistent with cls_u32's tc_u_common_ptr().
> > > > >
> > > > > The fixed null-ptr-deref calling stack:
> > > > > KASAN: null-ptr-deref in range [0x0000000000000038-0x000000000000003f]
> > > > > RIP: 0010:fw_classify (net/sched/cls_fw.c:81)
> > > > > Call Trace:
> > > > > tcf_classify (./include/net/tc_wrapper.h:197 net/sched/cls_api.c:1764 net/sched/cls_api.c:1860)
> > > > > tc_run (net/core/dev.c:4401)
> > > > > __dev_queue_xmit (net/core/dev.c:4535 net/core/dev.c:4790)
> > > > >
> > > > > Fixes: 1abf272022cf ("net: sched: tcindex, fw, flow: use tcf_block_q helper to get struct Qdisc")
> > > > > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > > > > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > > >
> > > > Thanks for also providing the tdc tests. Looks like a bizarre bug—and
> > > > in my mind the question is does fw or flow even factor into tc blocks?
> > > > Please give me time to review if the approach makes sense - perhaps
> > > > this weekend.
> > > >
> > > Since the fix is exactly as what u32 does - why not move
> > > tc_u_common_ptr() to a header file (pkt_cls.h)? then reuse it in u32,
> > > fw, and flow.
> >
> > Thank you for the suggestion, I appreciate the review.
> >
>
> Actually on second thought - why dont you fix the fw one so it catches
> things at control time? Something like:
>
> --- a/net/sched/cls_fw.c
> +++ b/net/sched/cls_fw.c
> @@ -247,8 +247,18 @@ static int fw_change(struct net *net, struct
> sk_buff *in_skb,
> struct nlattr *tb[TCA_FW_MAX + 1];
> int err;
>
> - if (!opt)
> - return handle ? -EINVAL : 0; /* Succeed if it is old method. */
> + if (!opt) {
> + if (handle)
> + return -EINVAL;
> +
> + if (tcf_block_shared(tp->chain->block)) {
> + NL_SET_ERR_MSG(extack,
> + "Must specify mark when
> attaching fw filter to block");
> + return -EINVAL;
> + }
> +
> + return 0; /* Succeed if it is old method. */
> + }
>
> err = nla_parse_nested_deprecated(tb, TCA_FW_MAX, opt, fw_policy,
> NULL);
>
> Also for the flow one, can you add an extack like above.
> Note: This will change the nature of your tdc test, you dont need to
> send an skb marked packet anymore since your config will be reject the
> setup..
>
> cheers,
> jamal
>
> > I spent some time exploring how to reuse tc_u_common_ptr() in cls_fw and
> > cls_flow, but I'm not sure I see a clean way to make it work across all
> > three classifiers. The core issue is that they need different things
> > when the block is shared:
> >
> > - cls_u32 needs a pointer value (block or block->q) as a hash key for
> > tc_u_common lookup
> > - cls_fw and cls_flow need to bail out and return -1 (no match)
> >
> > If we keep tc_u_common_ptr() as-is (returning void *), the usage in
> > fw/flow would be tc_u_common_ptr(tp) == block to detect shared blocks,
> > which is less clear than tcf_block_shared(block) and doesn't save any
> > code.
> >
> > If we rework it to return struct Qdisc * (NULL for shared), it would
> > break cls_u32's current usage where it needs the block pointer itself
> > as a hash key for the shared case.
> >
> > I may be misunderstanding your suggestion though — could you share
> > what you had in mind?
> >
> > Thank you again for your time,
> > Xiang
> > >
> > > I am still questioning the value of using blocks with these two
> > > classifiers and unfortunately the commit message of 1abf272022cf was
> > > not helpful.
> > > The testcases look good. Please add a letter head, perhaps quote the
> > > comment in tc_u_common_ptr() since it gives a good description of this
> > > whole dance.
> > >
> > > cheers,
> > > jamal
^ permalink raw reply
* [PATCH net-next 0/7] Decouple receive and transmit enablement in team driver
From: Marc Harvey @ 2026-03-31 5:33 UTC (permalink / raw)
To: jiri, andrew+netdev
Cc: willemb, maheshb, netdev, linux-kselftest, Marc Harvey
Allow independent control over receive and transmit enablement states
for aggregated ports in the team driver.
The motivation is that IEE 802.3ad LACP "independent control" can't
be implemented for the team driver currently. This was added to the
bonding driver in commit 240fd405528b ("bonding: Add independent
control state machine").
This series also has a few patches that add tests to show that the old
coupled enablement still works and that the new decoupled enalbment
works as intended (4, 5, and 7).
There are three patches with small fixes as well, with the goal of
making the final decouplement patch clearer (1, 2, and 3).
Marc Harvey (7):
net: team: Annotate reads and writes for mixed lock accessed values
net: team: Remove unused team_mode_op, port_enabled
net: team: Rename port_disabled team mode op to port_tx_disabled
selftests: net: Add tests for failover of team-aggregated ports
selftests: net: Add test for enablement of ports with teamd
net: team: Decouple rx and tx enablement in the team driver
selftests: net: Add tests for team driver decoupled tx and rx control
drivers/net/team/team_core.c | 241 +++++++++++++++---
drivers/net/team/team_mode_loadbalance.c | 8 +-
drivers/net/team/team_mode_random.c | 4 +-
drivers/net/team/team_mode_roundrobin.c | 2 +-
include/linux/if_team.h | 63 +++--
.../selftests/drivers/net/team/Makefile | 3 +
.../testing/selftests/drivers/net/team/config | 4 +
.../drivers/net/team/decoupled_enablement.sh | 200 +++++++++++++++
.../selftests/drivers/net/team/options.sh | 96 +++++++
.../selftests/drivers/net/team/team_lib.sh | 145 +++++++++++
.../drivers/net/team/teamd_activebackup.sh | 208 +++++++++++++++
.../drivers/net/team/transmit_failover.sh | 155 +++++++++++
tools/testing/selftests/net/forwarding/lib.sh | 13 +-
tools/testing/selftests/net/lib.sh | 13 +
14 files changed, 1084 insertions(+), 71 deletions(-)
create mode 100755 tools/testing/selftests/drivers/net/team/decoupled_enablement.sh
create mode 100644 tools/testing/selftests/drivers/net/team/team_lib.sh
create mode 100755 tools/testing/selftests/drivers/net/team/teamd_activebackup.sh
create mode 100755 tools/testing/selftests/drivers/net/team/transmit_failover.sh
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply
* [PATCH net-next 1/7] net: team: Annotate reads and writes for mixed lock accessed values
From: Marc Harvey @ 2026-03-31 5:33 UTC (permalink / raw)
To: jiri, andrew+netdev
Cc: willemb, maheshb, netdev, linux-kselftest, Marc Harvey
In-Reply-To: <20260331053353.2504254-1-marcharvey@google.com>
The team_port's "index" and the team's "en_port_count" are read in
the hot transmit path, but are only written to when holding the rtnl
lock.
Use READ_ONCE() for all lockless reads of these values, and use
WRITE_ONCE() for all writes.
Signed-off-by: Marc Harvey <marcharvey@google.com>
---
drivers/net/team/team_core.c | 11 ++++++-----
drivers/net/team/team_mode_random.c | 2 +-
include/linux/if_team.h | 4 ++--
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index 566a5d102c23..becd066279a6 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -938,7 +938,8 @@ static void team_port_enable(struct team *team,
{
if (team_port_enabled(port))
return;
- port->index = team->en_port_count++;
+ WRITE_ONCE(port->index, team->en_port_count);
+ WRITE_ONCE(team->en_port_count, team->en_port_count + 1);
hlist_add_head_rcu(&port->hlist,
team_port_index_hash(team, port->index));
team_adjust_ops(team);
@@ -958,7 +959,7 @@ static void __reconstruct_port_hlist(struct team *team, int rm_index)
for (i = rm_index + 1; i < team->en_port_count; i++) {
port = team_get_port_by_index(team, i);
hlist_del_rcu(&port->hlist);
- port->index--;
+ WRITE_ONCE(port->index, port->index - 1);
hlist_add_head_rcu(&port->hlist,
team_port_index_hash(team, port->index));
}
@@ -973,8 +974,8 @@ static void team_port_disable(struct team *team,
team->ops.port_disabled(team, port);
hlist_del_rcu(&port->hlist);
__reconstruct_port_hlist(team, port->index);
- port->index = -1;
- team->en_port_count--;
+ WRITE_ONCE(port->index, -1);
+ WRITE_ONCE(team->en_port_count, team->en_port_count - 1);
team_queue_override_port_del(team, port);
team_adjust_ops(team);
team_lower_state_changed(port);
@@ -1245,7 +1246,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev,
netif_addr_unlock_bh(dev);
}
- port->index = -1;
+ WRITE_ONCE(port->index, -1);
list_add_tail_rcu(&port->list, &team->port_list);
team_port_enable(team, port);
netdev_compute_master_upper_features(dev, true);
diff --git a/drivers/net/team/team_mode_random.c b/drivers/net/team/team_mode_random.c
index 53d0ce34b8ce..169a7bc865b2 100644
--- a/drivers/net/team/team_mode_random.c
+++ b/drivers/net/team/team_mode_random.c
@@ -16,7 +16,7 @@ static bool rnd_transmit(struct team *team, struct sk_buff *skb)
struct team_port *port;
int port_index;
- port_index = get_random_u32_below(team->en_port_count);
+ port_index = get_random_u32_below(READ_ONCE(team->en_port_count));
port = team_get_port_by_index_rcu(team, port_index);
if (unlikely(!port))
goto drop;
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index ccb5327de26d..06f4d7400c1e 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -77,7 +77,7 @@ static inline struct team_port *team_port_get_rcu(const struct net_device *dev)
static inline bool team_port_enabled(struct team_port *port)
{
- return port->index != -1;
+ return READ_ONCE(port->index) != -1;
}
static inline bool team_port_txable(struct team_port *port)
@@ -272,7 +272,7 @@ static inline struct team_port *team_get_port_by_index_rcu(struct team *team,
struct hlist_head *head = team_port_index_hash(team, port_index);
hlist_for_each_entry_rcu(port, head, hlist)
- if (port->index == port_index)
+ if (READ_ONCE(port->index) == port_index)
return port;
return NULL;
}
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH net-next 2/7] net: team: Remove unused team_mode_op, port_enabled
From: Marc Harvey @ 2026-03-31 5:33 UTC (permalink / raw)
To: jiri, andrew+netdev
Cc: willemb, maheshb, netdev, linux-kselftest, Marc Harvey
In-Reply-To: <20260331053353.2504254-1-marcharvey@google.com>
This team_mode_op wasn't used by any of the team modes, so remove it.
Signed-off-by: Marc Harvey <marcharvey@google.com>
---
drivers/net/team/team_core.c | 2 --
include/linux/if_team.h | 1 -
2 files changed, 3 deletions(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index becd066279a6..e54bd21bd068 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -944,8 +944,6 @@ static void team_port_enable(struct team *team,
team_port_index_hash(team, port->index));
team_adjust_ops(team);
team_queue_override_port_add(team, port);
- if (team->ops.port_enabled)
- team->ops.port_enabled(team, port);
team_notify_peers(team);
team_mcast_rejoin(team);
team_lower_state_changed(port);
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index 06f4d7400c1e..a761f5282bcf 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -121,7 +121,6 @@ struct team_mode_ops {
int (*port_enter)(struct team *team, struct team_port *port);
void (*port_leave)(struct team *team, struct team_port *port);
void (*port_change_dev_addr)(struct team *team, struct team_port *port);
- void (*port_enabled)(struct team *team, struct team_port *port);
void (*port_disabled)(struct team *team, struct team_port *port);
};
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox