* Re: [PATCH net-next V2 4/7] net/mlx5: E-Switch, serialize representor lifecycle
From: Mark Bloch @ 2026-05-02 20:05 UTC (permalink / raw)
To: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Andrew Lunn, David S. Miller
Cc: Leon Romanovsky, Jason Gunthorpe, Saeed Mahameed, Shay Drory,
Or Har-Toov, Edward Srouji, Maher Sanalla, Simon Horman,
Gerd Bayer, Moshe Shemesh, Kees Cook, Patrisious Haddad,
Parav Pandit, Carolina Jubran, Cosmin Ratiu, linux-rdma,
linux-kernel, netdev, Gal Pressman, Dragos Tatulea
In-Reply-To: <20260501041633.231662-5-tariqt@nvidia.com>
On 01/05/2026 7:16, Tariq Toukan wrote:
> From: Mark Bloch <mbloch@nvidia.com>
>
> Representor callbacks can be registered and unregistered while the
> E-Switch is already in switchdev mode, and the same E-Switch may also be
> reconfigured by devlink, VF changes and SF changes. Serialize these paths
> with the per-E-Switch representor mutex instead of relying on ad-hoc bit
> state and wait queues.
>
> Take the representor lock around the mode transition, VF/SF representor
> changes and representor ops registration. Keep mode_lock and the
> representor lock unnested by using the operation flag while the mode lock
> is dropped. During mode changes, drop the representor lock around the
> auxiliary bus rescan because driver bind/unbind may register or unregister
> representor ops.
>
> Split representor ops registration into locked public wrappers and blocked
> internal helpers, clear the ops pointer on unregister, and add nested
> wrappers for the shared-FDB master IB path that registers peer
> representor ops while another E-Switch representor lock is already held.
>
> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> drivers/infiniband/hw/mlx5/ib_rep.c | 6 +-
> .../net/ethernet/mellanox/mlx5/core/eswitch.c | 10 ++
> .../mellanox/mlx5/core/eswitch_offloads.c | 102 ++++++++++++++++--
> .../ethernet/mellanox/mlx5/core/sf/devlink.c | 5 +
> include/linux/mlx5/eswitch.h | 6 ++
> 5 files changed, 119 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
> index 1709b628702e..65d8767d1830 100644
> --- a/drivers/infiniband/hw/mlx5/ib_rep.c
> +++ b/drivers/infiniband/hw/mlx5/ib_rep.c
> @@ -262,9 +262,10 @@ mlx5_ib_vport_rep_unload(struct mlx5_eswitch_rep *rep)
> struct mlx5_core_dev *peer_mdev;
> struct mlx5_eswitch *esw;
>
> + /* Called while the master E-Switch reps_lock is held. */
> mlx5_lag_for_each_peer_mdev(mdev, peer_mdev, i) {
> esw = peer_mdev->priv.eswitch;
> - mlx5_eswitch_unregister_vport_reps(esw, REP_IB);
> + mlx5_eswitch_unregister_vport_reps_nested(esw, REP_IB);
> }
> mlx5_ib_release_transport(mdev);
> }
> @@ -284,9 +285,10 @@ static void mlx5_ib_register_peer_vport_reps(struct mlx5_core_dev *mdev)
> struct mlx5_eswitch *esw;
> int i;
>
> + /* Called while the master E-Switch reps_lock is held. */
> mlx5_lag_for_each_peer_mdev(mdev, peer_mdev, i) {
> esw = peer_mdev->priv.eswitch;
> - mlx5_eswitch_register_vport_reps(esw, &rep_ops, REP_IB);
> + mlx5_eswitch_register_vport_reps_nested(esw, &rep_ops, REP_IB);
> }
> }
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index 66a773a99876..f70737437954 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -1712,6 +1712,7 @@ int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs)
> mlx5_lag_disable_change(esw->dev);
>
> mlx5_eswitch_invalidate_wq(esw);
> + mlx5_esw_reps_block(esw);
>
> if (!mlx5_esw_is_fdb_created(esw)) {
> ret = mlx5_eswitch_enable_locked(esw, num_vfs);
> @@ -1735,6 +1736,8 @@ int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs)
> }
> }
>
> + mlx5_esw_reps_unblock(esw);
> +
> if (toggle_lag)
> mlx5_lag_enable_change(esw->dev);
>
> @@ -1759,6 +1762,7 @@ void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw, bool clear_vf)
> esw->esw_funcs.num_vfs, esw->esw_funcs.num_ec_vfs, esw->enabled_vports);
>
> mlx5_eswitch_invalidate_wq(esw);
> + mlx5_esw_reps_block(esw);
>
> if (!mlx5_core_is_ecpf(esw->dev)) {
> mlx5_eswitch_unload_vf_vports(esw, esw->esw_funcs.num_vfs);
> @@ -1770,6 +1774,8 @@ void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw, bool clear_vf)
> mlx5_eswitch_clear_ec_vf_vports_info(esw);
> }
>
> + mlx5_esw_reps_unblock(esw);
> +
> if (esw->mode == MLX5_ESWITCH_OFFLOADS) {
> struct devlink *devlink = priv_to_devlink(esw->dev);
>
> @@ -1825,7 +1831,11 @@ void mlx5_eswitch_disable(struct mlx5_eswitch *esw)
>
> devl_assert_locked(priv_to_devlink(esw->dev));
> mlx5_lag_disable_change(esw->dev);
> +
> + mlx5_esw_reps_block(esw);
> mlx5_eswitch_disable_locked(esw);
> + mlx5_esw_reps_unblock(esw);
> +
> esw->mode = MLX5_ESWITCH_LEGACY;
> mlx5_lag_enable_change(esw->dev);
> }
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 6a5143b63dfd..d4ac07c995b9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -36,6 +36,7 @@
> #include <linux/mlx5/mlx5_ifc.h>
> #include <linux/mlx5/vport.h>
> #include <linux/mlx5/fs.h>
> +#include <linux/lockdep.h>
> #include "mlx5_core.h"
> #include "eswitch.h"
> #include "esw/indir_table.h"
> @@ -2413,11 +2414,21 @@ static int esw_create_restore_table(struct mlx5_eswitch *esw)
> return err;
> }
>
> +static void mlx5_esw_assert_reps_locked(struct mlx5_eswitch *esw)
> +{
> + lockdep_assert_held(&esw->offloads.reps_lock);
> +}
> +
> void mlx5_esw_reps_block(struct mlx5_eswitch *esw)
> {
> mutex_lock(&esw->offloads.reps_lock);
> }
>
> +static void mlx5_esw_reps_block_nested(struct mlx5_eswitch *esw)
> +{
> + mutex_lock_nested(&esw->offloads.reps_lock, SINGLE_DEPTH_NESTING);
> +}
> +
> void mlx5_esw_reps_unblock(struct mlx5_eswitch *esw)
> {
> mutex_unlock(&esw->offloads.reps_lock);
> @@ -2425,21 +2436,22 @@ void mlx5_esw_reps_unblock(struct mlx5_eswitch *esw)
>
> static void esw_mode_change(struct mlx5_eswitch *esw, u16 mode)
> {
> + mlx5_esw_reps_unblock(esw);
> mlx5_devcom_comp_lock(esw->dev->priv.hca_devcom_comp);
> if (esw->dev->priv.flags & MLX5_PRIV_FLAGS_DISABLE_IB_ADEV ||
> mlx5_core_mp_enabled(esw->dev)) {
> esw->mode = mode;
> - mlx5_rescan_drivers_locked(esw->dev);
> - mlx5_devcom_comp_unlock(esw->dev->priv.hca_devcom_comp);
> - return;
> + goto out;
> }
>
> esw->dev->priv.flags |= MLX5_PRIV_FLAGS_DISABLE_IB_ADEV;
> mlx5_rescan_drivers_locked(esw->dev);
> esw->mode = mode;
> esw->dev->priv.flags &= ~MLX5_PRIV_FLAGS_DISABLE_IB_ADEV;
> +out:
> mlx5_rescan_drivers_locked(esw->dev);
> mlx5_devcom_comp_unlock(esw->dev->priv.hca_devcom_comp);
> + mlx5_esw_reps_block(esw);
> }
>
> static void mlx5_esw_fdb_drop_destroy(struct mlx5_eswitch *esw)
> @@ -2776,6 +2788,8 @@ void esw_offloads_cleanup(struct mlx5_eswitch *esw)
> static int __esw_offloads_load_rep(struct mlx5_eswitch *esw,
> struct mlx5_eswitch_rep *rep, u8 rep_type)
> {
> + mlx5_esw_assert_reps_locked(esw);
> +
> if (atomic_cmpxchg(&rep->rep_data[rep_type].state,
> REP_REGISTERED, REP_LOADED) == REP_REGISTERED)
> return esw->offloads.rep_ops[rep_type]->load(esw->dev, rep);
> @@ -2786,6 +2800,8 @@ static int __esw_offloads_load_rep(struct mlx5_eswitch *esw,
> static void __esw_offloads_unload_rep(struct mlx5_eswitch *esw,
> struct mlx5_eswitch_rep *rep, u8 rep_type)
> {
> + mlx5_esw_assert_reps_locked(esw);
> +
> if (atomic_cmpxchg(&rep->rep_data[rep_type].state,
> REP_LOADED, REP_REGISTERED) == REP_LOADED) {
> if (rep_type == REP_ETH)
> @@ -3691,6 +3707,7 @@ static void esw_vfs_changed_event_handler(struct mlx5_eswitch *esw)
> if (new_num_vfs == esw->esw_funcs.num_vfs || host_pf_disabled)
> goto free;
>
> + mlx5_esw_reps_block(esw);
> /* Number of VFs can only change from "0 to x" or "x to 0". */
> if (esw->esw_funcs.num_vfs > 0) {
> mlx5_eswitch_unload_vf_vports(esw, esw->esw_funcs.num_vfs);
> @@ -3700,9 +3717,11 @@ static void esw_vfs_changed_event_handler(struct mlx5_eswitch *esw)
> err = mlx5_eswitch_load_vf_vports(esw, new_num_vfs,
> MLX5_VPORT_UC_ADDR_CHANGE);
> if (err)
> - goto free;
> + goto unblock;
> }
> esw->esw_funcs.num_vfs = new_num_vfs;
> +unblock:
> + mlx5_esw_reps_unblock(esw);
> free:
> kvfree(out);
> }
> @@ -4188,9 +4207,14 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
> goto unlock;
> }
>
> + /* Keep mode_lock and reps_lock unnested. The operation flag excludes
> + * mode users while mode_lock is dropped before taking reps_lock.
> + */
> esw->eswitch_operation_in_progress = true;
> up_write(&esw->mode_lock);
>
> + mlx5_esw_reps_block(esw);
> +
> if (mlx5_mode == MLX5_ESWITCH_OFFLOADS &&
> !mlx5_devlink_netdev_netns_immutable_set(devlink, true)) {
> NL_SET_ERR_MSG_MOD(extack,
> @@ -4223,6 +4247,10 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
> skip:
> if (mlx5_mode == MLX5_ESWITCH_OFFLOADS && err)
> mlx5_devlink_netdev_netns_immutable_set(devlink, false);
> + /* Reconfiguration is done; drop reps_lock before taking mode_lock again
> + * to clear the operation flag.
> + */
> + mlx5_esw_reps_unblock(esw);
> down_write(&esw->mode_lock);
> esw->eswitch_operation_in_progress = false;
> unlock:
> @@ -4496,9 +4524,10 @@ mlx5_eswitch_vport_has_rep(const struct mlx5_eswitch *esw, u16 vport_num)
> return true;
> }
>
> -void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
> - const struct mlx5_eswitch_rep_ops *ops,
> - u8 rep_type)
> +static void
> +mlx5_eswitch_register_vport_reps_blocked(struct mlx5_eswitch *esw,
> + const struct mlx5_eswitch_rep_ops *ops,
> + u8 rep_type)
> {
> struct mlx5_eswitch_rep_data *rep_data;
> struct mlx5_eswitch_rep *rep;
> @@ -4513,9 +4542,40 @@ void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
> }
> }
> }
> +
> +static void
> +mlx5_eswitch_register_vport_reps_locked(struct mlx5_eswitch *esw,
> + const struct mlx5_eswitch_rep_ops *ops,
> + u8 rep_type, bool nested)
> +{
> + if (nested)
> + mlx5_esw_reps_block_nested(esw);
> + else
> + mlx5_esw_reps_block(esw);
> + mlx5_eswitch_register_vport_reps_blocked(esw, ops, rep_type);
> + mlx5_esw_reps_unblock(esw);
> +}
> +
> +void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
> + const struct mlx5_eswitch_rep_ops *ops,
> + u8 rep_type)
> +{
> + mlx5_eswitch_register_vport_reps_locked(esw, ops, rep_type, false);
> +}
> EXPORT_SYMBOL(mlx5_eswitch_register_vport_reps);
>
> -void mlx5_eswitch_unregister_vport_reps(struct mlx5_eswitch *esw, u8 rep_type)
> +void
> +mlx5_eswitch_register_vport_reps_nested(struct mlx5_eswitch *esw,
> + const struct mlx5_eswitch_rep_ops *ops,
> + u8 rep_type)
> +{
> + mlx5_eswitch_register_vport_reps_locked(esw, ops, rep_type, true);
> +}
> +EXPORT_SYMBOL(mlx5_eswitch_register_vport_reps_nested);
> +
> +static void
> +mlx5_eswitch_unregister_vport_reps_blocked(struct mlx5_eswitch *esw,
> + u8 rep_type)
> {
> struct mlx5_eswitch_rep *rep;
> unsigned long i;
> @@ -4525,9 +4585,35 @@ void mlx5_eswitch_unregister_vport_reps(struct mlx5_eswitch *esw, u8 rep_type)
>
> mlx5_esw_for_each_rep(esw, i, rep)
> atomic_set(&rep->rep_data[rep_type].state, REP_UNREGISTERED);
> +
> + esw->offloads.rep_ops[rep_type] = NULL;
sashiko.dev says:
"
Could this assignment cause a NULL pointer dereference in concurrent readers?
In mlx5_eswitch_get_proto_dev(), the state is checked before accessing the ops
pointer without holding reps_lock:
if (atomic_read(&rep->rep_data[rep_type].state) == REP_LOADED &&
esw->offloads.rep_ops[rep_type]->get_proto_dev)
return esw->offloads.rep_ops[rep_type]->get_proto_dev(rep);
If a thread in mlx5_eswitch_get_proto_dev() evaluates the state check to true
and is then preempted, can the unregister path execute and set rep_ops to NULL?
When the preempted thread resumes, it might dereference the now-NULL pointer.
Also, since the ops pointer isn't fetched into a local variable using
READ_ONCE(), could the compiler emit multiple loads, further widening the
race window?
"
The REP_LOADED check is not the only protection here, get_proto_dev()
is only reached from representor-owned contexts, and unregister first
unloads all reps under reps_lock. That unload tears down the users
that can call into this helper before the state is set to REP_UNREGISTERED
and before rep_ops is cleared. So clearing rep_ops does not create a new
live-reader window; it only removes the stale ops pointer after the
representor lifecycle is already quiesced.
Mark
> +}
> +
> +static void
> +mlx5_eswitch_unregister_vport_reps_locked(struct mlx5_eswitch *esw,
> + u8 rep_type, bool nested)
> +{
> + if (nested)
> + mlx5_esw_reps_block_nested(esw);
> + else
> + mlx5_esw_reps_block(esw);
> + mlx5_eswitch_unregister_vport_reps_blocked(esw, rep_type);
> + mlx5_esw_reps_unblock(esw);
> +}
> +
> +void mlx5_eswitch_unregister_vport_reps(struct mlx5_eswitch *esw, u8 rep_type)
> +{
> + mlx5_eswitch_unregister_vport_reps_locked(esw, rep_type, false);
> }
> EXPORT_SYMBOL(mlx5_eswitch_unregister_vport_reps);
>
> +void mlx5_eswitch_unregister_vport_reps_nested(struct mlx5_eswitch *esw,
> + u8 rep_type)
> +{
> + mlx5_eswitch_unregister_vport_reps_locked(esw, rep_type, true);
> +}
> +EXPORT_SYMBOL(mlx5_eswitch_unregister_vport_reps_nested);
> +
> void *mlx5_eswitch_get_uplink_priv(struct mlx5_eswitch *esw, u8 rep_type)
> {
> struct mlx5_eswitch_rep *rep;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
> index 8503e532f423..2fc69897e35b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
> @@ -245,8 +245,10 @@ static int mlx5_sf_add(struct mlx5_core_dev *dev, struct mlx5_sf_table *table,
> if (IS_ERR(sf))
> return PTR_ERR(sf);
>
> + mlx5_esw_reps_block(esw);
> err = mlx5_eswitch_load_sf_vport(esw, sf->hw_fn_id, MLX5_VPORT_UC_ADDR_CHANGE,
> &sf->dl_port, new_attr->controller, new_attr->sfnum);
> + mlx5_esw_reps_unblock(esw);
> if (err)
> goto esw_err;
> *dl_port = &sf->dl_port.dl_port;
> @@ -367,7 +369,10 @@ int mlx5_devlink_sf_port_del(struct devlink *devlink,
> struct mlx5_sf_table *table = dev->priv.sf_table;
> struct mlx5_sf *sf = mlx5_sf_by_dl_port(dl_port);
>
> + mlx5_esw_reps_block(dev->priv.eswitch);
> mlx5_sf_del(table, sf);
> + mlx5_esw_reps_unblock(dev->priv.eswitch);
> +
> return 0;
> }
>
> diff --git a/include/linux/mlx5/eswitch.h b/include/linux/mlx5/eswitch.h
> index 3b29a3c6794d..a0dd162baa78 100644
> --- a/include/linux/mlx5/eswitch.h
> +++ b/include/linux/mlx5/eswitch.h
> @@ -63,7 +63,13 @@ struct mlx5_eswitch_rep {
> void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
> const struct mlx5_eswitch_rep_ops *ops,
> u8 rep_type);
> +void
> +mlx5_eswitch_register_vport_reps_nested(struct mlx5_eswitch *esw,
> + const struct mlx5_eswitch_rep_ops *ops,
> + u8 rep_type);
> void mlx5_eswitch_unregister_vport_reps(struct mlx5_eswitch *esw, u8 rep_type);
> +void mlx5_eswitch_unregister_vport_reps_nested(struct mlx5_eswitch *esw,
> + u8 rep_type);
> void *mlx5_eswitch_get_proto_dev(struct mlx5_eswitch *esw,
> u16 vport_num,
> u8 rep_type);
^ permalink raw reply
* Re: [PATCH net-next V2 5/7] net/mlx5: E-Switch, unwind only newly loaded representor types
From: Mark Bloch @ 2026-05-02 20:06 UTC (permalink / raw)
To: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Andrew Lunn, David S. Miller
Cc: Leon Romanovsky, Jason Gunthorpe, Saeed Mahameed, Shay Drory,
Or Har-Toov, Edward Srouji, Maher Sanalla, Simon Horman,
Gerd Bayer, Moshe Shemesh, Kees Cook, Patrisious Haddad,
Parav Pandit, Carolina Jubran, Cosmin Ratiu, linux-rdma,
linux-kernel, netdev, Gal Pressman, Dragos Tatulea
In-Reply-To: <20260501041633.231662-6-tariqt@nvidia.com>
On 01/05/2026 7:16, Tariq Toukan wrote:
> From: Mark Bloch <mbloch@nvidia.com>
>
> __esw_offloads_load_rep() may return success without invoking the
> representor load callback when the representor type is already loaded.
>
> On a later load failure, mlx5_esw_offloads_rep_load() unconditionally
> unloaded all previously iterated representor types. This could unload
> representor types that were already loaded before this load attempt.
>
> Track which representor types were actually loaded by the current call and
> unwind only those on error. Also restore the representor state back to
> REP_REGISTERED when the load callback itself fails.
>
> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
sashiko.dev says:
"
Note: Due to recitation restrictions, the standard inline patch quoting format
has been altered to a free-form summary.
commit ca89c8e618c910a6810fe7127853404ce151ae6b
Author: Mark Bloch <mbloch@nvidia.com>
net/mlx5: E-Switch, unwind only newly loaded representor types
This commit updates the representor load error handling to track which
representor types were actually loaded by the current call and unwind only
those on error. It also restores the state to REP_REGISTERED if the load
callback fails.
In the modified __esw_offloads_load_rep() function, the representor state
is set to REP_LOADED via atomic_cmpxchg() before the actual load() callback
is invoked. Does this sequence expose a race condition with lockless readers?
If lockless readers, such as mlx5_eswitch_get_proto_dev() or
mlx5_esw_ipsec_restore_dest_uplink(), check the state and observe REP_LOADED
while the load callback is still executing, can they access uninitialized
private data resulting in a NULL pointer dereference?
Additionally, if the load() callback subsequently fails and frees the
allocated private data, could a lockless reader that already observed the
"
The REP_LOADED before load() ordering is pre-existing and orthogonal
to this unwind fix. I’ll look into it and if needed address it in
a separate patch.
Mark
> .../mellanox/mlx5/core/eswitch_offloads.c | 38 ++++++++++++++-----
> 1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index d4ac07c995b9..8f656253981b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -2786,13 +2786,28 @@ void esw_offloads_cleanup(struct mlx5_eswitch *esw)
> }
>
> static int __esw_offloads_load_rep(struct mlx5_eswitch *esw,
> - struct mlx5_eswitch_rep *rep, u8 rep_type)
> + struct mlx5_eswitch_rep *rep,
> + u8 rep_type, bool *newly_loaded)
> {
> + int err;
> +
> mlx5_esw_assert_reps_locked(esw);
>
> + if (newly_loaded)
> + *newly_loaded = false;
> +
> if (atomic_cmpxchg(&rep->rep_data[rep_type].state,
> - REP_REGISTERED, REP_LOADED) == REP_REGISTERED)
> - return esw->offloads.rep_ops[rep_type]->load(esw->dev, rep);
> + REP_REGISTERED, REP_LOADED) != REP_REGISTERED)
> + return 0;
> +
> + err = esw->offloads.rep_ops[rep_type]->load(esw->dev, rep);
> + if (err) {
> + atomic_set(&rep->rep_data[rep_type].state, REP_REGISTERED);
> + return err;
> + }
> +
> + if (newly_loaded)
> + *newly_loaded = true;
>
> return 0;
> }
> @@ -2822,22 +2837,27 @@ static void __unload_reps_all_vport(struct mlx5_eswitch *esw, u8 rep_type)
> static int mlx5_esw_offloads_rep_load(struct mlx5_eswitch *esw, u16 vport_num)
> {
> struct mlx5_eswitch_rep *rep;
> + unsigned long loaded = 0;
> + bool newly_loaded;
> int rep_type;
> int err;
>
> rep = mlx5_eswitch_get_rep(esw, vport_num);
> for (rep_type = 0; rep_type < NUM_REP_TYPES; rep_type++) {
> - err = __esw_offloads_load_rep(esw, rep, rep_type);
> + err = __esw_offloads_load_rep(esw, rep, rep_type,
> + &newly_loaded);
> if (err)
> goto err_reps;
> + if (newly_loaded)
> + loaded |= BIT(rep_type);
> }
>
> return 0;
>
> err_reps:
> - atomic_set(&rep->rep_data[rep_type].state, REP_REGISTERED);
> - for (--rep_type; rep_type >= 0; rep_type--)
> - __esw_offloads_unload_rep(esw, rep, rep_type);
> + while (--rep_type >= 0)
> + if (test_bit(rep_type, &loaded))
> + __esw_offloads_unload_rep(esw, rep, rep_type);
> return err;
> }
>
> @@ -3591,13 +3611,13 @@ int mlx5_eswitch_reload_ib_reps(struct mlx5_eswitch *esw)
> if (atomic_read(&rep->rep_data[REP_ETH].state) != REP_LOADED)
> return 0;
>
> - ret = __esw_offloads_load_rep(esw, rep, REP_IB);
> + ret = __esw_offloads_load_rep(esw, rep, REP_IB, NULL);
> if (ret)
> return ret;
>
> mlx5_esw_for_each_rep(esw, i, rep) {
> if (atomic_read(&rep->rep_data[REP_ETH].state) == REP_LOADED)
> - __esw_offloads_load_rep(esw, rep, REP_IB);
> + __esw_offloads_load_rep(esw, rep, REP_IB, NULL);
> }
>
> return 0;
^ permalink raw reply
* [PATCH net v5 0/8] xsk: fix bugs around xsk skb allocation
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
There are rare issues around xsk_build_skb(). Some of them
were founded by Sashiko[1][2].
[1]: https://lore.kernel.org/all/20260415082654.21026-1-kerneljasonxing@gmail.com/
[2]: https://lore.kernel.org/all/20260418045644.28612-1-kerneljasonxing@gmail.com/
---
v5
Link: https://lore.kernel.org/all/20260424053816.27965-1-kerneljasonxing@gmail.com/
1. reword patch 3
2. adjust the order of patch 6 that is now moved up to be patch 2)
3. use helper in patch (Stan)
v4
Link: https://lore.kernel.org/all/20260422033650.68457-1-kerneljasonxing@gmail.com/
1. fix 32-bit arch issue in patch 8 (Alexander && Maciej)
2. add acked-by tags (Stan)
v3
Link: https://lore.kernel.org/all/20260420082805.14844-1-kerneljasonxing@gmail.com/
1. use !xs->skb as the indicator of first frag in patch 3 (Stan)
2. move init skb after it's allocated successfully, so patch 4,5,6 have
been adjusted accordingly (Stan)
3. do not support 32-bit arch (Stan)
4. add acked-by tags (Stan)
v2
Link: https://lore.kernel.org/all/20260418045644.28612-1-kerneljasonxing@gmail.com/#t
1. add four patches spotted by sashiko to fix buggy pre-existing
behavior
2. adjust the order of 8 patches.
Jason Xing (8):
xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
xsk: handle NULL dereference of the skb without frags issue
xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
xsk: avoid skb leak in XDP_TX_METADATA case
xsk: fix xsk_addrs slab leak on multi-buffer error path
xsk: fix u64 descriptor address truncation on 32-bit architectures
net/xdp/xsk.c | 115 ++++++++++++++++++++++++++--------------
net/xdp/xsk_buff_pool.c | 3 ++
2 files changed, 77 insertions(+), 41 deletions(-)
--
2.41.3
^ permalink raw reply
* [PATCH net v5 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
skb_checksum_help() is a common helper that writes the folded
16-bit checksum back via skb->data + csum_start + csum_offset,
i.e. it relies on the skb's linear head and fails (with WARN_ONCE
and -EINVAL) when skb_headlen() is 0.
AF_XDP generic xmit takes two very different paths depending on the
netdev. Drivers that advertise IFF_TX_SKB_NO_LINEAR (e.g. virtio_net)
skip the "copy payload into a linear head" step on purpose as a
performance optimisation: xsk_build_skb_zerocopy() only attaches UMEM
pages as frags and never calls skb_put(), so skb_headlen() stays 0
for the whole skb. For these skbs there is simply no linear area for
skb_checksum_help() to write the csum into - the sw-csum fallback is
structurally inapplicable.
The patch tries to catch this and reject the combination with error at
setup time. Rejecting at bind() converts this silent per-packet failure
into a synchronous, actionable -EOPNOTSUPP at setup time. HW csum and
launch_time metadata on IFF_TX_SKB_NO_LINEAR drivers are unaffected
because they do not call skb_checksum_help().
Without the patch, every descriptor carrying 'XDP_TX_METADATA |
XDP_TXMD_FLAGS_CHECKSUM' produces:
1) a WARN_ONCE "offset (N) >= skb_headlen() (0)" from skb_checksum_help(),
2) sendmsg() returning -EINVAL without consuming the descriptor
(invalid_descs is not incremented),
3) a wedged TX ring: __xsk_generic_xmit() does not advance the
consumer on non-EOVERFLOW errors, so the next sendmsg() re-reads
the same descriptor and re-hits the same WARN until the socket
is closed.
Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/#t
Fixes: 30c3055f9c0d ("xsk: wrap generic metadata handling onto separate function")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk_buff_pool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 37b7a68b89b3..c2521b6547e3 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -169,6 +169,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
if (force_zc && force_copy)
return -EINVAL;
+ if (pool->tx_sw_csum && (netdev->priv_flags & IFF_TX_SKB_NO_LINEAR))
+ return -EOPNOTSUPP;
+
if (xsk_get_pool_from_qid(netdev, queue_id))
return -EBUSY;
--
2.41.3
^ permalink raw reply related
* [PATCH net v5 2/8] xsk: free the skb when hitting the upper bound MAX_SKB_FRAGS
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
Fix it by explicitly adding kfree_skb() before returning back to its
caller.
How to reproduce it in virtio_net:
1. the current skb is the first one (which means xs->skb is NULL) and
hit the limit MAX_SKB_FRAGS.
2. xsk_build_skb_zerocopy() returns -EOVERFLOW.
3. the caller xsk_build_skb() clears skb by using 'skb = NULL;'. This
is why bug can be triggered.
4. there is no chance to free this skb anymore.
Note that if in this case the xs->skb is not NULL, xsk_build_skb() will
call xsk_drop_skb(xs->skb) to do the right thing.
Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 6149f6a79897..46660943c81e 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -776,8 +776,11 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
addr = buffer - pool->addrs;
for (copied = 0, i = skb_shinfo(skb)->nr_frags; copied < len; i++) {
- if (unlikely(i >= MAX_SKB_FRAGS))
+ if (unlikely(i >= MAX_SKB_FRAGS)) {
+ if (!xs->skb)
+ kfree_skb(skb);
return ERR_PTR(-EOVERFLOW);
+ }
page = pool->umem->pgs[addr >> PAGE_SHIFT];
get_page(page);
--
2.41.3
^ permalink raw reply related
* [PATCH net v5 3/8] xsk: handle NULL dereference of the skb without frags issue
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
When a first descriptor (xs->skb == NULL) triggers -EOVERFLOW in
xsk_build_skb_zerocopy() (e.g., MAX_SKB_FRAGS exceeded), the
free_err -EOVERFLOW handler unconditionally dereferences xs->skb
via xsk_inc_num_desc(xs->skb) and xsk_drop_skb(xs->skb), causing
a NULL pointer dereference.
Fix this by guarding the existing xsk_inc_num_desc()/xsk_drop_skb()
calls with an xs->skb check (for the continuation case), and add
an else branch for the first-descriptor case that manually cancels
the one reserved CQ slot and increments invalid_descs by one to
account for the single invalid descriptor.
Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 46660943c81e..ff1eade29aa6 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -896,9 +896,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
kfree_skb(skb);
if (err == -EOVERFLOW) {
- /* Drop the packet */
- xsk_inc_num_desc(xs->skb);
- xsk_drop_skb(xs->skb);
+ if (xs->skb) {
+ /* Drop the packet */
+ xsk_inc_num_desc(xs->skb);
+ xsk_drop_skb(xs->skb);
+ } else {
+ xsk_cq_cancel_locked(xs->pool, 1);
+ xs->tx->invalid_descs++;
+ }
xskq_cons_release(xs->tx);
} else {
/* Let application retry */
--
2.41.3
^ permalink raw reply related
* [PATCH net v5 4/8] xsk: fix use-after-free of xs->skb in xsk_build_skb() free_err path
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
When xsk_build_skb() processes multi-buffer packets in copy mode, the
first descriptor stores data into the skb linear area without adding
any frags, so nr_frags stays at 0. The caller then sets xs->skb = skb
to accumulate subsequent descriptors.
If a continuation descriptor fails (e.g. alloc_page returns NULL with
-EAGAIN), we jump to free_err where the condition:
if (skb && !skb_shinfo(skb)->nr_frags)
kfree_skb(skb);
evaluates to true because nr_frags is still 0 (the first descriptor
used the linear area, not frags). This frees the skb while xs->skb
still points to it, creating a dangling pointer. On the next transmit
attempt or socket close, xs->skb is dereferenced, causing a
use-after-free or double-free.
Fix by using a !xs->skb check to handle first frag situation, ensuring
we only free skbs that were freshly allocated in this call
(xs->skb is NULL) and never free an in-progress multi-buffer skb that
the caller still references.
Closes: https://lore.kernel.org/all/20260415082654.21026-4-kerneljasonxing@gmail.com/
Fixes: 6b9c129c2f93 ("xsk: remove @first_frag from xsk_build_skb()")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index ff1eade29aa6..ae59d1c1d2f8 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -892,7 +892,7 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
return skb;
free_err:
- if (skb && !skb_shinfo(skb)->nr_frags)
+ if (skb && !xs->skb)
kfree_skb(skb);
if (err == -EOVERFLOW) {
--
2.41.3
^ permalink raw reply related
* [PATCH net v5 5/8] xsk: prevent CQ desync when freeing half-built skbs in xsk_build_skb()
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
Once xsk_skb_init_misc() has been called on an skb, its destructor is
set to xsk_destruct_skb(), which submits the descriptor address(es) to
the completion queue and advances the CQ producer. If such an skb is
subsequently freed via kfree_skb() along an error path - before the
skb has ever been handed to the driver - the destructor still runs and
submits a bogus, half-initialized address to the CQ.
Postpone the init phase when we believe the allocation of first frag is
successfully completed. Before this init, skb can be safely freed by
kfree_skb().
Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/
Fixes: c30d084960cf ("xsk: avoid overwriting skb fields for multi-buffer traffic")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index ae59d1c1d2f8..13bacc11fa9d 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -739,8 +739,6 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
return ERR_PTR(err);
skb_reserve(skb, hr);
-
- xsk_skb_init_misc(skb, xs, desc->addr);
if (desc->options & XDP_TX_METADATA) {
err = xsk_skb_metadata(skb, buffer, desc, pool, hr);
if (unlikely(err))
@@ -837,7 +835,6 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
if (unlikely(err))
goto free_err;
- xsk_skb_init_misc(skb, xs, desc->addr);
if (desc->options & XDP_TX_METADATA) {
err = xsk_skb_metadata(skb, buffer, desc,
xs->pool, hr);
@@ -887,6 +884,8 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
}
}
+ if (!xs->skb)
+ xsk_skb_init_misc(skb, xs, desc->addr);
xsk_inc_num_desc(skb);
return skb;
--
2.41.3
^ permalink raw reply related
* Re: [PATCH net-next V2 6/7] net/mlx5: E-switch, load reps via work queue after registration
From: Mark Bloch @ 2026-05-02 20:07 UTC (permalink / raw)
To: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Andrew Lunn, David S. Miller
Cc: Leon Romanovsky, Jason Gunthorpe, Saeed Mahameed, Shay Drory,
Or Har-Toov, Edward Srouji, Maher Sanalla, Simon Horman,
Gerd Bayer, Moshe Shemesh, Kees Cook, Patrisious Haddad,
Parav Pandit, Carolina Jubran, Cosmin Ratiu, linux-rdma,
linux-kernel, netdev, Gal Pressman, Dragos Tatulea
In-Reply-To: <20260501041633.231662-7-tariqt@nvidia.com>
On 01/05/2026 7:16, Tariq Toukan wrote:
> From: Mark Bloch <mbloch@nvidia.com>
>
> mlx5_eswitch_register_vport_reps() only installs representor callbacks and
> marks the rep type as registered. If the E-Switch is already in switchdev
> mode, the newly registered rep type must then be loaded for already enabled
> vports.
>
> That load path needs to run under the devlink lock, which is not held by
> the auxiliary driver registration context. Queue the reload to the E-Switch
> workqueue, whose handler acquires the devlink lock, and load the relevant
> representors from there.
>
> The unregister path is unchanged and still unloads representors
> synchronously while tearing down the registered callbacks.
>
> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
sashiko.dev says:
"
[Note: The format of this report is altered due to recitation restrictions. Code quoting and inline diffs have been omitted.]
commit f4f3e4872e599ef6d13a1f2bacffd52e6dbc8d37
Author: Mark Bloch <mbloch@nvidia.com>
net/mlx5: E-switch, load reps via work queue after registration
This commit defers the loading of representors for already enabled vports to
a workqueue when the E-Switch is in switchdev mode. This ensures the load
path runs under the devlink lock, which is not held during auxiliary driver
registration.
I have a couple of questions regarding error handling in this update.
Looking at mlx5_eswitch_register_vport_reps_locked(), the return value of
mlx5_esw_add_work() is not checked. Since mlx5_esw_add_work() uses
kzalloc_obj() with GFP_ATOMIC, it can fail under memory pressure and return
-ENOMEM. If this allocation fails, the background work to load the
representors is never queued, but the representors are already marked as
REP_REGISTERED. Could this result in a NULL pointer dereference if callers
attempt to access the representor data under the assumption that it was
successfully initialized?
Additionally, in mlx5_eswitch_reload_reps_blocked(), failures from
mlx5_esw_offloads_rep_load() appear to be silently ignored. If the call
fails for MLX5_VPORT_UPLINK, the function aborts early without loading the
other vports. Within the loop for the remaining vports, the return value is
also ignored. Does this leave the system in a partially loaded and
inconsistent state, where representors are registered but uninitialized for
some vports?
"
REP_REGISTERED does not imply initialized representor data. Users that need
private data only proceed on REP_LOADED, and failed loads leave the state
REP_REGISTERED. The queued reload is for late rep-ops registration while already
in switchdev, where there is no synchronous error path back to the auxiliary
probe, so it is intentionally opportunistic. The strict path is still the
switchdev transition: there, representor load errors are propagated and the
mode change is aborted, so this does not change the consistency model.
Mark
> .../mellanox/mlx5/core/eswitch_offloads.c | 34 +++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 8f656253981b..f26d1652dd05 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -4563,6 +4563,38 @@ mlx5_eswitch_register_vport_reps_blocked(struct mlx5_eswitch *esw,
> }
> }
>
> +static void mlx5_eswitch_reload_reps_blocked(struct mlx5_eswitch *esw)
> +{
> + struct mlx5_vport *vport;
> + unsigned long i;
> +
> + if (esw->mode != MLX5_ESWITCH_OFFLOADS)
> + return;
> +
> + if (mlx5_esw_offloads_rep_load(esw, MLX5_VPORT_UPLINK))
> + return;
> +
> + mlx5_esw_for_each_vport(esw, i, vport) {
> + if (!vport)
> + continue;
> + if (!vport->enabled)
> + continue;
> + if (vport->vport == MLX5_VPORT_UPLINK)
> + continue;
> + if (!mlx5_eswitch_vport_has_rep(esw, vport->vport))
> + continue;
> +
> + mlx5_esw_offloads_rep_load(esw, vport->vport);
> + }
> +}
> +
> +static void mlx5_eswitch_reload_reps(struct mlx5_eswitch *esw)
> +{
> + mlx5_esw_reps_block(esw);
> + mlx5_eswitch_reload_reps_blocked(esw);
> + mlx5_esw_reps_unblock(esw);
> +}
> +
> static void
> mlx5_eswitch_register_vport_reps_locked(struct mlx5_eswitch *esw,
> const struct mlx5_eswitch_rep_ops *ops,
> @@ -4574,6 +4606,8 @@ mlx5_eswitch_register_vport_reps_locked(struct mlx5_eswitch *esw,
> mlx5_esw_reps_block(esw);
> mlx5_eswitch_register_vport_reps_blocked(esw, ops, rep_type);
> mlx5_esw_reps_unblock(esw);
> +
> + mlx5_esw_add_work(esw, mlx5_eswitch_reload_reps);
> }
>
> void mlx5_eswitch_register_vport_reps(struct mlx5_eswitch *esw,
^ permalink raw reply
* [PATCH net v5 6/8] xsk: avoid skb leak in XDP_TX_METADATA case
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
Fix it by explicitly adding kfree_skb() before returning back to its
caller.
How to reproduce it in virtio_net:
1. the current skb is the first one (which means no frag and xs->skb is
NULL) and users enable metadata feature.
2. xsk_skb_metadata() returns a error code.
3. the caller xsk_build_skb() clears skb by using 'skb = NULL;'.
4. there is no chance to free this skb anymore.
Closes: https://lore.kernel.org/all/20260415085204.3F87AC19424@smtp.kernel.org/
Fixes: 30c3055f9c0d ("xsk: wrap generic metadata handling onto separate function")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 13bacc11fa9d..5e6326e076ab 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -741,8 +741,10 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
skb_reserve(skb, hr);
if (desc->options & XDP_TX_METADATA) {
err = xsk_skb_metadata(skb, buffer, desc, pool, hr);
- if (unlikely(err))
+ if (unlikely(err)) {
+ kfree_skb(skb);
return ERR_PTR(err);
+ }
}
} else {
struct xsk_addrs *xsk_addr;
--
2.41.3
^ permalink raw reply related
* [PATCH net v5 7/8] xsk: fix xsk_addrs slab leak on multi-buffer error path
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
When xsk_build_skb() / xsk_build_skb_zerocopy() sees the first
continuation descriptor, it promotes destructor_arg from an inlined
address to a freshly allocated xsk_addrs (num_descs = 1). The counter
is bumped to >= 2 only at the very end of a successful build (by calling
xsk_inc_num_desc()).
If the build fails in between (e.g. alloc_page() returns NULL with
-EAGAIN, or the MAX_SKB_FRAGS overflow hits), we jump to free_err, skip
calling xsk_inc_num_desc() to increment num_descs and leave the half-built
skb attached to xs->skb for the app to retry. The skb now has
1) destructor_arg = a real xsk_addrs pointer,
2) num_descs = 1
If the app never retries and just close()s the socket, xsk_release()
calls xsk_drop_skb() -> xsk_consume_skb(), which decides whether to
free xsk_addrs by testing num_descs > 1:
if (unlikely(num_descs > 1))
kmem_cache_free(xsk_tx_generic_cache, destructor_arg);
Because num_descs is exactly 1 the branch is skipped and the
xsk_addrs object is leaked to the xsk_tx_generic_cache slab.
Fix it by directly testing if destructor_arg is still addr. Or else it
is modified and used to store the newly allocated memory from
xsk_tx_generic_cache regardless of increment of num_desc, which we
need to handle.
Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/
Fixes: 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 5e6326e076ab..ed96f6ec8ff2 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -605,7 +605,7 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
spin_lock_irqsave(&pool->cq_prod_lock, flags);
idx = xskq_get_prod(pool->cq);
- if (unlikely(num_descs > 1)) {
+ if (unlikely(!xsk_skb_destructor_is_addr(skb))) {
xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
for (i = 0; i < num_descs; i++) {
@@ -660,7 +660,7 @@ static void xsk_consume_skb(struct sk_buff *skb)
u32 num_descs = xsk_get_num_desc(skb);
struct xsk_addrs *xsk_addr;
- if (unlikely(num_descs > 1)) {
+ if (unlikely(!xsk_skb_destructor_is_addr(skb))) {
xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
kmem_cache_free(xsk_tx_generic_cache, xsk_addr);
}
--
2.41.3
^ permalink raw reply related
* [PATCH net v5 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
From: Jason Xing @ 2026-05-02 20:07 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, horms, andrew+netdev
Cc: bpf, netdev, Jason Xing
In-Reply-To: <20260502200722.53960-1-kerneljasonxing@gmail.com>
From: Jason Xing <kernelxing@tencent.com>
In copy mode TX, xsk_skb_destructor_set_addr() stores the 64-bit
descriptor address into skb_shinfo(skb)->destructor_arg (void *) via a
uintptr_t cast:
skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
On 32-bit architectures uintptr_t is 32 bits, so the upper 32 bits of
the descriptor address are silently dropped. In XDP_ZEROCOPY unaligned
mode the chunk offset is encoded in bits 48-63 of the descriptor
address (XSK_UNALIGNED_BUF_OFFSET_SHIFT = 48), meaning the offset is
lost entirely. The completion queue then returns a truncated address to
userspace, making buffer recycling impossible.
Fix this by handling the 32-bit case directly in
xsk_skb_destructor_set_addr(): when !CONFIG_64BIT, allocate an
xsk_addrs struct (the same path already used for multi-descriptor
SKBs) to store the full u64 address. The existing tagged-pointer logic
in xsk_skb_destructor_is_addr() stays unchanged: slab pointers returned
from kmem_cache_zalloc() are always word-aligned and therefore have
bit 0 clear, which correctly identifies them as a struct pointer
rather than an inline tagged address on every architecture.
Factor the shared kmem_cache_zalloc + destructor_arg assignment into
__xsk_addrs_alloc() and add a wrapper xsk_addrs_alloc() that handles
the inline-to-list upgrade (is_addr check + get_addr + num_descs = 1).
The three former open-coded kmem_cache_zalloc call sites now reduce to
a single call each.
Propagate the -ENOMEM from xsk_skb_destructor_set_addr() through
xsk_skb_init_misc() so the caller can clean up the skb via kfree_skb()
before skb->destructor is installed.
The overhead is one extra kmem_cache_zalloc per first descriptor on
32-bit only; 64-bit builds are completely unchanged.
Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/
Fixes: 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
net/xdp/xsk.c | 88 ++++++++++++++++++++++++++++++++-------------------
1 file changed, 56 insertions(+), 32 deletions(-)
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index ed96f6ec8ff2..6bcd77068e52 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -566,9 +566,42 @@ static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
return (u64)((uintptr_t)skb_shinfo(skb)->destructor_arg & ~0x1UL);
}
-static void xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
+static struct xsk_addrs *__xsk_addrs_alloc(struct sk_buff *skb, u64 addr)
{
- skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
+ struct xsk_addrs *xsk_addr;
+
+ xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
+ if (unlikely(!xsk_addr))
+ return NULL;
+
+ xsk_addr->addrs[0] = addr;
+ skb_shinfo(skb)->destructor_arg = (void *)xsk_addr;
+ return xsk_addr;
+}
+
+static struct xsk_addrs *xsk_addrs_alloc(struct sk_buff *skb)
+{
+ struct xsk_addrs *xsk_addr;
+
+ if (!xsk_skb_destructor_is_addr(skb))
+ return (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
+
+ xsk_addr = __xsk_addrs_alloc(skb, xsk_skb_destructor_get_addr(skb));
+ if (likely(xsk_addr))
+ xsk_addr->num_descs = 1;
+ return xsk_addr;
+}
+
+static int xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
+{
+ if (IS_ENABLED(CONFIG_64BIT)) {
+ skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
+ return 0;
+ }
+
+ if (unlikely(!__xsk_addrs_alloc(skb, addr)))
+ return -ENOMEM;
+ return 0;
}
static void xsk_inc_num_desc(struct sk_buff *skb)
@@ -644,14 +677,20 @@ void xsk_destruct_skb(struct sk_buff *skb)
sock_wfree(skb);
}
-static void xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
- u64 addr)
+static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
+ u64 addr)
{
+ int err;
+
+ err = xsk_skb_destructor_set_addr(skb, addr);
+ if (unlikely(err))
+ return err;
+
skb->dev = xs->dev;
skb->priority = READ_ONCE(xs->sk.sk_priority);
skb->mark = READ_ONCE(xs->sk.sk_mark);
skb->destructor = xsk_destruct_skb;
- xsk_skb_destructor_set_addr(skb, addr);
+ return 0;
}
static void xsk_consume_skb(struct sk_buff *skb)
@@ -749,18 +788,9 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
} else {
struct xsk_addrs *xsk_addr;
- if (xsk_skb_destructor_is_addr(skb)) {
- xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache,
- GFP_KERNEL);
- if (!xsk_addr)
- return ERR_PTR(-ENOMEM);
-
- xsk_addr->num_descs = 1;
- xsk_addr->addrs[0] = xsk_skb_destructor_get_addr(skb);
- skb_shinfo(skb)->destructor_arg = (void *)xsk_addr;
- } else {
- xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
- }
+ xsk_addr = xsk_addrs_alloc(skb);
+ if (!xsk_addr)
+ return ERR_PTR(-ENOMEM);
/* in case of -EOVERFLOW that could happen below,
* xsk_consume_skb() will release this node as whole skb
@@ -849,19 +879,10 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
struct page *page;
u8 *vaddr;
- if (xsk_skb_destructor_is_addr(skb)) {
- xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache,
- GFP_KERNEL);
- if (!xsk_addr) {
- err = -ENOMEM;
- goto free_err;
- }
-
- xsk_addr->num_descs = 1;
- xsk_addr->addrs[0] = xsk_skb_destructor_get_addr(skb);
- skb_shinfo(skb)->destructor_arg = (void *)xsk_addr;
- } else {
- xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
+ xsk_addr = xsk_addrs_alloc(skb);
+ if (!xsk_addr) {
+ err = -ENOMEM;
+ goto free_err;
}
if (unlikely(nr_frags == (MAX_SKB_FRAGS - 1) && xp_mb_desc(desc))) {
@@ -886,8 +907,11 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
}
}
- if (!xs->skb)
- xsk_skb_init_misc(skb, xs, desc->addr);
+ if (!xs->skb) {
+ err = xsk_skb_init_misc(skb, xs, desc->addr);
+ if (unlikely(err))
+ goto free_err;
+ }
xsk_inc_num_desc(skb);
return skb;
--
2.41.3
^ permalink raw reply related
* Re: [PATCH net-next V2 7/7] net/mlx5: Add profile to auto-enable switchdev mode at device init
From: Mark Bloch @ 2026-05-02 20:08 UTC (permalink / raw)
To: Tariq Toukan, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Andrew Lunn, David S. Miller
Cc: Leon Romanovsky, Jason Gunthorpe, Saeed Mahameed, Shay Drory,
Or Har-Toov, Edward Srouji, Maher Sanalla, Simon Horman,
Gerd Bayer, Moshe Shemesh, Kees Cook, Patrisious Haddad,
Parav Pandit, Carolina Jubran, Cosmin Ratiu, linux-rdma,
linux-kernel, netdev, Gal Pressman, Dragos Tatulea
In-Reply-To: <20260501041633.231662-8-tariqt@nvidia.com>
On 01/05/2026 7:16, Tariq Toukan wrote:
> From: Mark Bloch <mbloch@nvidia.com>
>
> Deployments that always operate in switchdev mode currently require
> manual devlink configuration after driver probe, which complicates
> automated provisioning.
>
> Introduce MLX5_PROF_MASK_DEF_SWITCHDEV, a new profile mask bit, and
> profile index 8. When a device is initialized or reloaded with this
> profile, the driver automatically switches the e-switch to switchdev
> mode by calling mlx5_devlink_eswitch_mode_set() immediately after
> bringing the device online.
>
> A no-op stub of mlx5_devlink_eswitch_mode_set() is added for builds
> without CONFIG_MLX5_ESWITCH.
>
> Signed-off-by: Mark Bloch <mbloch@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/eswitch.h | 6 +++
> .../net/ethernet/mellanox/mlx5/core/main.c | 43 ++++++++++++++++++-
> include/linux/mlx5/driver.h | 2 +
> 3 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> index 3858690e09b4..cfb9595f9de8 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> @@ -1049,6 +1049,12 @@ mlx5_esw_lag_demux_rule_create(struct mlx5_eswitch *esw, u16 vport_num,
> return ERR_PTR(-EOPNOTSUPP);
> }
>
> +static inline int
> +mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
> + struct netlink_ext_ack *extack)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif /* CONFIG_MLX5_ESWITCH */
>
> #endif /* __MLX5_ESWITCH_H__ */
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 74827e8ca125..4cdda15ed7f5 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -86,7 +86,7 @@ MODULE_PARM_DESC(debug_mask, "debug mask: 1 = dump cmd data, 2 = dump cmd exec t
>
> static unsigned int prof_sel = MLX5_DEFAULT_PROF;
> module_param_named(prof_sel, prof_sel, uint, 0444);
> -MODULE_PARM_DESC(prof_sel, "profile selector. Valid range 0 - 2");
> +MODULE_PARM_DESC(prof_sel, "profile selector. Valid range 0 - 3 and 8");
sashiko.dev says:
"
Does using a module parameter to configure e-switch mode bypass the standard
devlink uAPI?
The networking subsystem typically standardizes this configuration via
netlink/devlink interfaces, and relying on a module parameter for this might
fragment automated provisioning workflows.
"
The target here is DPU side deployments where switchdev is the only valid
operating mode, and the goal is to boot directly into that mode instead
of relying on userspace scripts after probe. I agree devlink remains the
runtime uapi. This profile is only an opt-in transition step so we can
avoid forcing switchdev in code before users have a way to reject that behavior.
This does not replace devlink provisioning, it provides a DPU side default for
environments that explicitly choose this profile.
>
> static u32 sw_owner_id[4];
> #define MAX_SW_VHCA_ID (BIT(__mlx5_bit_sz(cmd_hca_cap_2, sw_vhca_id)) - 1)
> @@ -99,6 +99,8 @@ enum {
>
> #define LOG_MAX_SUPPORTED_QPS 0xff
>
> +#define MLX5_PROF_SEL_LAST_NIC 3
> +#define MLX5_PROF_SEL_FIRST_ESW 8
> static struct mlx5_profile profile[] = {
> [0] = {
> .mask = 0,
> @@ -120,6 +122,11 @@ static struct mlx5_profile profile[] = {
> .log_max_qp = LOG_MAX_SUPPORTED_QPS,
> .num_cmd_caches = 0,
> },
> + [8] = {
> + .mask = MLX5_PROF_MASK_DEF_SWITCHDEV | MLX5_PROF_MASK_QP_SIZE,
> + .log_max_qp = LOG_MAX_SUPPORTED_QPS,
> + .num_cmd_caches = MLX5_NUM_COMMAND_CACHES,
> + },
> };
>
> static int wait_fw_init(struct mlx5_core_dev *dev, u32 max_wait_mili,
> @@ -1385,6 +1392,22 @@ static void mlx5_unload(struct mlx5_core_dev *dev)
> mlx5_free_bfreg(dev, &dev->priv.bfreg);
> }
>
> +static void mlx5_set_default_switchdev(struct mlx5_core_dev *dev)
> +{
> + int err;
> +
> + /* Default switchdev is best-effort; keep the device usable on
> + * failure.
> + */
> + err = mlx5_devlink_eswitch_mode_set(priv_to_devlink(dev),
> + DEVLINK_ESWITCH_MODE_SWITCHDEV,
> + NULL);
> + if (err && err != -EOPNOTSUPP)
> + mlx5_core_warn(dev,
> + "Failed to set switchdev as default, continuing in current mode, err(%d)\n",
> + err);
> +}
> +
> int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
> {
> bool light_probe = mlx5_dev_is_lightweight(dev);
> @@ -1431,6 +1454,10 @@ int mlx5_init_one_devl_locked(struct mlx5_core_dev *dev)
> mlx5_core_err(dev, "mlx5_hwmon_dev_register failed with error code %d\n", err);
>
> mutex_unlock(&dev->intf_state_mutex);
> +
> + if (dev->profile.mask & MLX5_PROF_MASK_DEF_SWITCHDEV)
> + mlx5_set_default_switchdev(dev);
> +
> return 0;
sashiko.dev says:
"
If a user explicitly sets the e-switch mode to legacy via devlink after
initialization, will this override their setting during driver reload or
firmware error recovery?
Since mlx5_set_default_switchdev() is called unconditionally here based on
the profile mask, it seems like it could silently revert the device back to
switchdev mode, discarding the active user configuration.
"
Yes, with this profile selected, switchdev is intentionally reapplied after
reload or recovery. A devlink change to legacy affects the current lifetime,
but the selected profile defines the default mode after the device is
reinitialized, users that want legacy to persist should not use this profile.
>
> err_register:
> @@ -1532,6 +1559,10 @@ int mlx5_load_one_devl_locked(struct mlx5_core_dev *dev, bool recovery)
> goto err_attach;
>
> mutex_unlock(&dev->intf_state_mutex);
> +
> + if (dev->profile.mask & MLX5_PROF_MASK_DEF_SWITCHDEV)
> + mlx5_set_default_switchdev(dev);
> +
> return 0;
>
> err_attach:
> @@ -2314,6 +2345,16 @@ static void mlx5_core_verify_params(void)
> MLX5_DEFAULT_PROF);
> prof_sel = MLX5_DEFAULT_PROF;
> }
> +
> + if (prof_sel > MLX5_PROF_SEL_LAST_NIC &&
> + prof_sel < MLX5_PROF_SEL_FIRST_ESW) {
> + pr_warn("mlx5_core: WARNING: Invalid module parameter prof_sel %d invalid range %d - %d, changing back to default (%d)\n",
> + prof_sel,
> + MLX5_PROF_SEL_LAST_NIC + 1,
> + MLX5_PROF_SEL_FIRST_ESW - 1,
> + MLX5_DEFAULT_PROF);
> + prof_sel = MLX5_DEFAULT_PROF;
> + }
> }
>
> static int __init mlx5_init(void)
> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> index 04b96c5abb57..65298c07df4d 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -705,6 +705,8 @@ struct mlx5_st;
>
> enum {
> MLX5_PROF_MASK_QP_SIZE = (u64)1 << 0,
> + MLX5_PROF_MASK_MR_CACHE = (u64)1 << 1,
> + MLX5_PROF_MASK_DEF_SWITCHDEV = (u64)1 << 2,
> };
sashiko.dev says:
"
This isn't a bug, but it looks like MLX5_PROF_MASK_MR_CACHE is introduced
here but never used in the driver code. Is this mask intended for a future
patch?
"
Before I respin for the unrelated MR_CACHE cleanup, I’d like to confirm
whether the opt-in profile approach is acceptable at all. Regardless
of this last patch, the first 6 patches fix real representor/LAG locking
issues and are needed independently, so I’d like to keep those moving toward
acceptance as soon as possible.
Mark
>
> struct mlx5_profile {
^ permalink raw reply
* Re: [PATCH net v4 8/8] xsk: fix u64 descriptor address truncation on 32-bit architectures
From: Jason Xing @ 2026-05-02 20:10 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend, aleksander.lobakin, bpf, netdev, Jason Xing
In-Reply-To: <afQdzWlMzjgpAUq7@devvm7509.cco0.facebook.com>
On Fri, May 1, 2026 at 6:29 AM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
>
> On 04/29, Jason Xing wrote:
> > On Wed, Apr 29, 2026 at 6:14 PM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> > >
> > > On 04/29, Jason Xing wrote:
> > > > On Wed, Apr 29, 2026 at 2:11 AM Stanislav Fomichev <sdf.kernel@gmail.com> wrote:
> > > > >
> > > > > On 04/24, Jason Xing wrote:
> > > > > > From: Jason Xing <kernelxing@tencent.com>
> > > > > >
> > > > > > In copy mode TX, xsk_skb_destructor_set_addr() stores the 64-bit
> > > > > > descriptor address into skb_shinfo(skb)->destructor_arg (void *) via a
> > > > > > uintptr_t cast:
> > > > > >
> > > > > > skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
> > > > > >
> > > > > > On 32-bit architectures uintptr_t is 32 bits, so the upper 32 bits of
> > > > > > the descriptor address are silently dropped. In unaligned mode the chunk
> > > > > > offset is encoded in bits 48-63 of the descriptor address
> > > > > > (XSK_UNALIGNED_BUF_OFFSET_SHIFT = 48), meaning the offset is lost
> > > > > > entirely. The completion queue then returns a truncated address to
> > > > > > userspace, making buffer recycling impossible.
> > > > > >
> > > > > > Fix this by handling the 32-bit case in the destructor_arg helpers:
> > > > > >
> > > > > > - xsk_skb_destructor_set_addr(): on !CONFIG_64BIT, allocate an
> > > > > > xsk_addrs struct via kmem_cache_zalloc() to store the full u64
> > > > > > address. Leave num_descs as 0 (zalloc) so that the subsequent
> > > > > > xsk_inc_num_desc() brings it to the correct count of 1.
> > > > > >
> > > > > > - xsk_skb_destructor_is_addr(): on !CONFIG_64BIT, return true only
> > > > > > when destructor_arg is NULL (not yet set), false when it points to
> > > > > > an xsk_addrs struct.
> > > > > >
> > > > > > - xsk_skb_init_misc(): call xsk_skb_destructor_set_addr() first
> > > > > > before touching any other skb fields; on failure return early so
> > > > > > the skb destructor is never changed from sock_wfree.
> > > > > >
> > > > > > The existing xsk_consume_skb() already handles 32-bit correctly after
> > > > > > these changes: xsk_skb_destructor_is_addr() returns false for any
> > > > > > allocated xsk_addrs, so the kmem_cache_free path is always taken.
> > > > > >
> > > > > > The overhead is one extra kmem_cache_zalloc per first descriptor on
> > > > > > 32-bit only; 64-bit builds are completely unchanged.
> > > > > >
> > > > > > Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/
> > > > > > Fixes: 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number")
> > > > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > > > ---
> > > > > > net/xdp/xsk.c | 38 +++++++++++++++++++++++++++++++-------
> > > > > > 1 file changed, 31 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > > > > > index ed96f6ec8ff2..fe88f47741b5 100644
> > > > > > --- a/net/xdp/xsk.c
> > > > > > +++ b/net/xdp/xsk.c
> > > > > > @@ -558,7 +558,10 @@ static int xsk_cq_reserve_locked(struct xsk_buff_pool *pool)
> > > > > >
> > > > > > static bool xsk_skb_destructor_is_addr(struct sk_buff *skb)
> > > > > > {
> > > > > > - return (uintptr_t)skb_shinfo(skb)->destructor_arg & 0x1UL;
> > > > > > + if (IS_ENABLED(CONFIG_64BIT))
> > > > > > + return (uintptr_t)skb_shinfo(skb)->destructor_arg & 0x1UL;
> > > > > > + else
> > > > > > + return !skb_shinfo(skb)->destructor_arg;
> > > > >
> > > > > Don't understand why we need to special case CONFIG_64BIT here?
> > > > > Shouldn't the same existing condition work on 32bit?
> > > >
> > > > Because 0x1UL is the particular semantic applied on a 64-bit arch.
> > > > xsk_skb_destructor_set_addr() sets it while
> > > > xsk_skb_destructor_is_addr() recognizes it. They are a pair.
> > > >
> > > > As you noticed, one liner works but is not that appropriate: on a
> > > > 32-bit arch, this member should be either a NULL point or a valid
> > > > pointer pointing to a memory region. Testing if it's NULL can be
> > > > helpful as to the long term maintenance because of its readability and
> > > > robustness/safety.
> > > >
> > > > The error path in allocation of skb is really complex, which is why
> > > > I'm so cautious to take care of it :)
> > >
> > > Let's cleanup the error path instead of adding more complexity? Similar to what
> > > you do with your "xsk: fix xsk_addrs slab leak on multi-buffer error path",
> > > but maybe add a few NULL checks?
> >
> > Good suggestion. I think I can cook a follow up patch to do such a
> > thing targetting net-next tree. This patch 8 belongs to net material
> > which means it will be backported to the older stable kernel as soon
> > as it gets merged. IIUC, the better way is to make it as simple as
> > possible?
> >
> > >
> > > Instead of 32 vs 64, I'd like to reason about whether destructor_arg
> > > is an address or an allocated array (not whether we have 1 or >1
> > > descriptors). And we special case 32 bit by always allocating it.
> > >
> > > Haven't checked, but maybe this is all you need (besides your _set_addr
> > > changes)?
> > >
> > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> > > index 6149f6a79897..03f217e85d31 100644
> > > --- a/net/xdp/xsk.c
> > > +++ b/net/xdp/xsk.c
> > > @@ -589,6 +589,8 @@ static u32 xsk_get_num_desc(struct sk_buff *skb)
> > > return 1;
> > >
> > > xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
> > > + if (!xsk_addr)
> > > + return 0;
> > >
> > > return xsk_addr->num_descs;
> > > }
> >
> > Right, as I mentioned, how about posting a new cleanup patch with your
> > suggested-by tag?
> >
> > >
> > > > I've noticed the status has been changed to 'changes requested'. Does
> > > > that mean one way or another I have to post a new version?
> > >
> > > That wasn't me :-) From my POW, patches 1-7 are good to go..
> >
> > Great! Thanks for the review. My hope is to get this series merged
> > soon in the net tree.
> >
> > >
> > > > >
> > > > > > }
> > > > > >
> > > > > > static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
> > > > > > @@ -566,9 +569,21 @@ static u64 xsk_skb_destructor_get_addr(struct sk_buff *skb)
> > > > > > return (u64)((uintptr_t)skb_shinfo(skb)->destructor_arg & ~0x1UL);
> > > > > > }
> > > > > >
> > > > > > -static void xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
> > > > > > +static int xsk_skb_destructor_set_addr(struct sk_buff *skb, u64 addr)
> > > > > > {
> > > > >
> > > > > [..]
> > > > >
> > > > > > + if (!IS_ENABLED(CONFIG_64BIT)) {
> > > > > > + struct xsk_addrs *xsk_addr;
> > > > > > +
> > > > > > + xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
> > > > > > + if (!xsk_addr)
> > > > > > + return -ENOMEM;
> > > > > > + xsk_addr->addrs[0] = addr;
> > > > > > + skb_shinfo(skb)->destructor_arg = (void *)xsk_addr;
> > > > > > + return 0;
> > > > > > + }
> > > > > > +
> > > > > > skb_shinfo(skb)->destructor_arg = (void *)((uintptr_t)addr | 0x1UL);
> > > > > > + return 0;
> > > > >
> > > > > I think this is gonna be a 3rd copy paste of the same logic? Let's
> > > > > move to a new helper and replace existing kmem_cache_zalloc places?
> > > > >
> > > > > xsk_skb_destructor_alloc_list(prev_addr) ?
> > >
> > > Any comments on this?
> >
> > I didn't comment on this because I thought I was not that sure if we
> > needed to wrap it up in the stable kernels :)
> >
> > Of course, it would be easier for me to work on the net-next tree to
> > make the code look
> > more neat and elegant.
>
> Are you concerned that you're gonna break something in the net tree? Why not
My primary concern is this patch would be a little bit more
complicated (more insertions/deletions). IIUC, the fix should be as
simple as possible to easily get backported while we can do some
cleanup work on the next branch.
Please see this new version (v5) with your advice applied. Does it look good?
https://lore.kernel.org/all/20260502200722.53960-9-kerneljasonxing@gmail.com/
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH RFC net-next 0/4] net: pse-pd: decouple controller lookup from MDIO probe
From: Carlo Szelinsky @ 2026-05-02 20:10 UTC (permalink / raw)
To: Corey Leavitt, Kory Maincent, Oleksij Rempel, Andrew Lunn
Cc: Andrew Lunn, Heiner Kallweit, Russell King, Jakub Kicinski,
David S . Miller, Eric Dumazet, Paolo Abeni, Simon Horman, netdev,
linux-kernel, Carlo Szelinsky
In-Reply-To: <20260424144124.131837c7@kmaincent-XPS-13-7390>
Hi,
I tested the series on a Hasivo S1100WP-8GT-SE with hs104 PSE
chips. Boot is clean, no more probe loop. ethtool --set-pse on/off
works, and a PD plugged into a port gets power. So the fix works
for me.
One thing I noticed: rmmod alone does not work while ports are
attached. Each PHY that holds a psec keeps a ref on the PSE
driver, so the module count stays above 0 and rmmod fails.
Unbinding the i2c devices first works:
echo 0-000d > /sys/bus/i2c/drivers/hasivo-hs104/unbind
echo 0-0015 > /sys/bus/i2c/drivers/hasivo-hs104/unbind
rmmod hasivo-hs104
After unbind your PSE_UNREGISTERED notifier fires, the PHYs drop
the psec, and the module count goes to 0. Then rmmod works.
Not a big deal, but maybe worth a short note in the cover letter
so people know they need to unbind first. Or if there is a clean
way to skip the per-consumer module_get and just rely on the
unbind path, that would be nice too. What do other folks think?
One thing my test does not cover yet is the SFP path change in
sfp.c, since the S1100WP-8GT-SE is copper only. I do have an
S600WP-5GT-2SX-SE that uses the same hs104 chips plus SFP+ cages,
but it is not on my desk right now. I can run that part on it
later.
Anyway, thanks for the work on this. Happy to test a v2.
Carlo
Tested-by: Carlo Szelinsky <github@szelinsky.de>
^ permalink raw reply
* [PATCH] rxrpc: always copy skb on in-place decrypt to avoid pagecache aliasing
From: Ziyi Guo @ 2026-05-02 21:13 UTC (permalink / raw)
To: David Howells, Marc Dionne, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: linux-afs, netdev, linux-kernel, Ziyi Guo
skb_cloned() does not detect frag-level page sharing introduced by splice()
with MSG_SPLICE_PAGES. When such an skb reaches the rxrpc DATA path,
in-place decryption (rxkad/rxgk) writes plaintext into file
pagecache pages still aliased to the source file, leading to
corrupt the page cache of any readable file.
Drop the skb_cloned() gate so skb_copy() is performed unconditionally
for DATA packets, severing the alias before decrypt.
Fixes: d0d5c0cd1e71 ("rxrpc: Use skb_unshare() rather than skb_cow_data()")
Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
---
net/rxrpc/call_event.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index fdd683261226..39d19a354172 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -333,10 +333,12 @@ bool rxrpc_input_call_event(struct rxrpc_call *call)
saw_ack |= sp->hdr.type == RXRPC_PACKET_TYPE_ACK;
if (sp->hdr.type == RXRPC_PACKET_TYPE_DATA &&
- sp->hdr.securityIndex != 0 &&
- skb_cloned(skb)) {
- /* Unshare the packet so that it can be
- * modified by in-place decryption.
+ sp->hdr.securityIndex != 0) {
+ /* Always unshare: skb_cloned() does not
+ * detect frag-level page sharing introduced
+ * by splice() with MSG_SPLICE_PAGES, so
+ * in-place decryption could otherwise corrupt
+ * the source file's page cache.
*/
struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH net 1/2] net/sched: sch_cake: annotate data-races in cake_dump_class_stats (I)
From: Toke Høiland-Jørgensen @ 2026-05-02 21:32 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Jamal Hadi Salim, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
In-Reply-To: <20260430061610.3503483-2-edumazet@google.com>
Eric Dumazet <edumazet@google.com> writes:
> cake_dump_class_stats() runs without qdisc spinlock being held.
>
> In this first patch, I add READ_ONCE()/WRITE_ONCE() annotations for:
>
> - flow->head
> - flow->dropped
> - b->backlogs[]
>
> Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
^ permalink raw reply
* Re: [PATCH net 2/2] net/sched: sch_cake: annotate data-races in cake_dump_class_stats (II)
From: Toke Høiland-Jørgensen @ 2026-05-02 21:35 UTC (permalink / raw)
To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Jamal Hadi Salim, Jiri Pirko, netdev, eric.dumazet,
Eric Dumazet
In-Reply-To: <20260430061610.3503483-3-edumazet@google.com>
Eric Dumazet <edumazet@google.com> writes:
> cake_dump_class_stats() runs without qdisc spinlock being held.
>
> In this second patch, I add READ_ONCE()/WRITE_ONCE() annotations for:
>
> - flow->deficit
> - flow->cvars.dropping
> - flow->cvars.count
> - flow->cvars.p_drop
> - flow->cvars.blue_timer
> - flow->cvars.drop_next
>
> Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Toke Høiland-Jørgensen <toke@toke.dk>
^ permalink raw reply
* [PATCH net-next 0/3] net: dsa: yt921x: Add port qdisc tbf support
From: David Yang @ 2026-05-02 21:53 UTC (permalink / raw)
To: netdev
Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim,
Jiri Pirko, Simon Horman, linux-kernel
v0: https://lore.kernel.org/r/20260409171209.2575583-1-mmyangfl@gmail.com
- picked from old series
- add extack to the offload struct
- add all params to the offload struct
David Yang (3):
net: sched: tbf: add extack to offload params
net: sched: tbf: pass all params to offload users
net: dsa: yt921x: Add port qdisc tbf support
drivers/net/dsa/yt921x.c | 138 +++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/yt921x.h | 66 ++++++++++++++++++-
include/net/pkt_cls.h | 7 +-
net/sched/sch_tbf.c | 15 ++++-
4 files changed, 221 insertions(+), 5 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH net-next 1/3] net: sched: tbf: add extack to offload params
From: David Yang @ 2026-05-02 21:53 UTC (permalink / raw)
To: netdev
Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim,
Jiri Pirko, Simon Horman, linux-kernel
In-Reply-To: <20260502215314.917687-1-mmyangfl@gmail.com>
Drivers might have error messages to propagate to user space. Propagate
the netlink extack so that they can inform user space in a verbal way of
their limitations.
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
include/net/pkt_cls.h | 1 +
net/sched/sch_tbf.c | 9 +++++++--
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 99ac747b7906..3bd08d7f39c1 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -1046,6 +1046,7 @@ struct tc_tbf_qopt_offload_replace_params {
};
struct tc_tbf_qopt_offload {
+ struct netlink_ext_ack *extack;
enum tc_tbf_command command;
u32 handle;
u32 parent;
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index f2340164f579..4576111fe075 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -139,7 +139,8 @@ static u64 psched_ns_t2l(const struct psched_ratecfg *r,
return len;
}
-static void tbf_offload_change(struct Qdisc *sch)
+static void tbf_offload_change(struct Qdisc *sch,
+ struct netlink_ext_ack *extack)
{
struct tbf_sched_data *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch);
@@ -148,6 +149,7 @@ static void tbf_offload_change(struct Qdisc *sch)
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
return;
+ qopt.extack = extack;
qopt.command = TC_TBF_REPLACE;
qopt.handle = sch->handle;
qopt.parent = sch->parent;
@@ -166,6 +168,7 @@ static void tbf_offload_destroy(struct Qdisc *sch)
if (!tc_can_offload(dev) || !dev->netdev_ops->ndo_setup_tc)
return;
+ qopt.extack = NULL;
qopt.command = TC_TBF_DESTROY;
qopt.handle = sch->handle;
qopt.parent = sch->parent;
@@ -176,6 +179,7 @@ static int tbf_offload_dump(struct Qdisc *sch)
{
struct tc_tbf_qopt_offload qopt;
+ qopt.extack = NULL;
qopt.command = TC_TBF_STATS;
qopt.handle = sch->handle;
qopt.parent = sch->parent;
@@ -193,6 +197,7 @@ static void tbf_offload_graft(struct Qdisc *sch, struct Qdisc *new,
.parent = sch->parent,
.child_handle = new->handle,
.command = TC_TBF_GRAFT,
+ .extack = extack,
};
qdisc_offload_graft_helper(qdisc_dev(sch), sch, new, old,
@@ -477,7 +482,7 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt,
qdisc_put(old);
err = 0;
- tbf_offload_change(sch);
+ tbf_offload_change(sch, extack);
done:
return err;
}
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 2/3] net: sched: tbf: pass all params to offload users
From: David Yang @ 2026-05-02 21:53 UTC (permalink / raw)
To: netdev
Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim,
Jiri Pirko, Simon Horman, linux-kernel
In-Reply-To: <20260502215314.917687-1-mmyangfl@gmail.com>
Drivers might have fine-grained control over queues. Pass all available
params so they can make use of them.
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
include/net/pkt_cls.h | 6 +++++-
net/sched/sch_tbf.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 3bd08d7f39c1..d73c04e3d919 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -1040,8 +1040,12 @@ enum tc_tbf_command {
};
struct tc_tbf_qopt_offload_replace_params {
+ u32 limit;
+ u32 max_size;
+ s64 buffer;
+ s64 mtu;
struct psched_ratecfg rate;
- u32 max_size;
+ struct psched_ratecfg peak;
struct gnet_stats_queue *qstats;
};
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 4576111fe075..8a1110d59dec 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -153,8 +153,12 @@ static void tbf_offload_change(struct Qdisc *sch,
qopt.command = TC_TBF_REPLACE;
qopt.handle = sch->handle;
qopt.parent = sch->parent;
- qopt.replace_params.rate = q->rate;
+ qopt.replace_params.limit = q->limit;
qopt.replace_params.max_size = q->max_size;
+ qopt.replace_params.buffer = q->buffer;
+ qopt.replace_params.mtu = q->mtu;
+ qopt.replace_params.rate = q->rate;
+ qopt.replace_params.peak = q->peak;
qopt.replace_params.qstats = &sch->qstats;
dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_QDISC_TBF, &qopt);
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 3/3] net: dsa: yt921x: Add port qdisc tbf support
From: David Yang @ 2026-05-02 21:53 UTC (permalink / raw)
To: netdev
Cc: David Yang, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim,
Jiri Pirko, Simon Horman, linux-kernel
In-Reply-To: <20260502215314.917687-1-mmyangfl@gmail.com>
Enable port shaping and support limiting the rate of outgoing traffic.
Signed-off-by: David Yang <mmyangfl@gmail.com>
---
drivers/net/dsa/yt921x.c | 138 +++++++++++++++++++++++++++++++++++++++
drivers/net/dsa/yt921x.h | 66 ++++++++++++++++++-
2 files changed, 203 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/yt921x.c b/drivers/net/dsa/yt921x.c
index fd1fdcd5f9a3..9560fb2b8657 100644
--- a/drivers/net/dsa/yt921x.c
+++ b/drivers/net/dsa/yt921x.c
@@ -24,6 +24,7 @@
#include <net/dsa.h>
#include <net/dscp.h>
#include <net/ieee8021q.h>
+#include <net/pkt_cls.h>
#include "yt921x.h"
@@ -1272,6 +1273,19 @@ yt921x_marker_tfm_police(struct yt921x_marker *marker,
priv, port, extack);
}
+static int
+yt921x_marker_tfm_shape(struct yt921x_marker *marker, u64 rate, u64 burst,
+ unsigned int flags, bool queue,
+ struct yt921x_priv *priv, int port,
+ struct netlink_ext_ack *extack)
+{
+ return yt921x_marker_tfm(marker, rate, burst, flags,
+ queue ? priv->queue_shape_slot_ns :
+ priv->port_shape_slot_ns, YT921X_SHAPE_CIR_MAX,
+ YT921X_SHAPE_CBS_MAX, YT921X_SHAPE_UNIT_MAX,
+ priv, port, extack);
+}
+
static int
yt921x_police_validate(const struct flow_action_police *police,
const struct flow_action *action,
@@ -1378,6 +1392,115 @@ yt921x_dsa_port_policer_add(struct dsa_switch *ds, int port,
return res;
}
+static int
+yt921x_tbf_validate(struct yt921x_priv *priv,
+ const struct tc_tbf_qopt_offload *qopt, int *queuep)
+{
+ struct netlink_ext_ack *extack = qopt->extack;
+ int queue = -1;
+
+ /* TODO: queue support */
+ if (qopt->parent != TC_H_ROOT) {
+ NL_SET_ERR_MSG_MOD(extack, "Parent should be \"root\"");
+ return -EOPNOTSUPP;
+ }
+
+ switch (qopt->command) {
+ case TC_TBF_REPLACE: {
+ const struct tc_tbf_qopt_offload_replace_params *p;
+
+ p = &qopt->replace_params;
+
+ if (p->mtu || p->peak.rate_bytes_ps) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when mtu/peakrate is configured");
+ return -EOPNOTSUPP;
+ }
+
+ if (!p->rate.mpu) {
+ NL_SET_ERR_MSG_MOD(extack, "Assuming mpu = 64");
+ } else if (p->rate.mpu != 64) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "Offload not supported when mpu is other than 64");
+ return -EOPNOTSUPP;
+ }
+
+ break;
+ }
+ default:
+ break;
+ }
+
+ *queuep = queue;
+ return 0;
+}
+
+static int
+yt921x_dsa_port_setup_tc_tbf_port(struct dsa_switch *ds, int port,
+ const struct tc_tbf_qopt_offload *qopt)
+{
+ struct yt921x_priv *priv = to_yt921x_priv(ds);
+ u32 ctrls[2];
+ int res;
+
+ switch (qopt->command) {
+ case TC_TBF_DESTROY:
+ ctrls[0] = 0;
+ ctrls[1] = 0;
+ break;
+ case TC_TBF_REPLACE: {
+ const struct tc_tbf_qopt_offload_replace_params *p;
+ struct yt921x_marker marker;
+
+ p = &qopt->replace_params;
+
+ res = yt921x_marker_tfm_shape(&marker, p->rate.rate_bytes_ps,
+ p->max_size,
+ YT921X_MARKER_SINGLE_BUCKET,
+ false, priv, port, NULL);
+ if (res)
+ return res;
+
+ ctrls[0] = YT921X_PORT_SHAPE_CTRLa_CIR(marker.cir) |
+ YT921X_PORT_SHAPE_CTRLa_CBS(marker.cbs);
+ ctrls[1] = YT921X_PORT_SHAPE_CTRLb_UNIT(marker.unit) |
+ YT921X_PORT_SHAPE_CTRLb_EN;
+ break;
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ mutex_lock(&priv->reg_lock);
+ res = yt921x_reg64_write(priv, YT921X_PORTn_SHAPE_CTRL(port), ctrls);
+ mutex_unlock(&priv->reg_lock);
+
+ return res;
+}
+
+static int
+yt921x_dsa_port_setup_tc(struct dsa_switch *ds, int port,
+ enum tc_setup_type type, void *type_data)
+{
+ struct yt921x_priv *priv = to_yt921x_priv(ds);
+ int res;
+
+ switch (type) {
+ case TC_SETUP_QDISC_TBF: {
+ const struct tc_tbf_qopt_offload *qopt = type_data;
+ int queue;
+
+ res = yt921x_tbf_validate(priv, qopt, &queue);
+ if (res)
+ return res;
+
+ return yt921x_dsa_port_setup_tc_tbf_port(ds, port, qopt);
+ }
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static int
yt921x_mirror_del(struct yt921x_priv *priv, int port, bool ingress)
{
@@ -3524,6 +3647,20 @@ static int yt921x_chip_setup_tc(struct yt921x_priv *priv)
return res;
priv->meter_slot_ns = ctrl * op_ns;
+ ctrl = max(priv->port_shape_slot_ns / op_ns,
+ YT921X_PORT_SHAPE_SLOT_MIN);
+ res = yt921x_reg_write(priv, YT921X_PORT_SHAPE_SLOT, ctrl);
+ if (res)
+ return res;
+ priv->port_shape_slot_ns = ctrl * op_ns;
+
+ ctrl = max(priv->queue_shape_slot_ns / op_ns,
+ YT921X_QUEUE_SHAPE_SLOT_MIN);
+ res = yt921x_reg_write(priv, YT921X_QUEUE_SHAPE_SLOT, ctrl);
+ if (res)
+ return res;
+ priv->queue_shape_slot_ns = ctrl * op_ns;
+
return 0;
}
@@ -3680,6 +3817,7 @@ static const struct dsa_switch_ops yt921x_dsa_switch_ops = {
/* rate */
.port_policer_del = yt921x_dsa_port_policer_del,
.port_policer_add = yt921x_dsa_port_policer_add,
+ .port_setup_tc = yt921x_dsa_port_setup_tc,
/* hsr */
.port_hsr_leave = dsa_port_simple_hsr_leave,
.port_hsr_join = dsa_port_simple_hsr_join,
diff --git a/drivers/net/dsa/yt921x.h b/drivers/net/dsa/yt921x.h
index 546b12a8994a..0114a89b58b8 100644
--- a/drivers/net/dsa/yt921x.h
+++ b/drivers/net/dsa/yt921x.h
@@ -521,6 +521,12 @@ enum yt921x_app_selector {
#define YT921X_PORT_VLAN_CTRL1_CVLAN_DROP_TAGGED BIT(1)
#define YT921X_PORT_VLAN_CTRL1_CVLAN_DROP_UNTAGGED BIT(0)
+#define YT921X_PORTn_PRIO_UCAST_QUEUE(port) (0x300200 + 4 * (port))
+#define YT921X_PORT_PRIOm_UCAST_QUEUE_M(m) (7 << (3 * (m)))
+#define YT921X_PORT_PRIOm_UCAST_QUEUE(m, x) ((x) << (3 * (m)))
+#define YT921X_PORTn_PRIO_MCAST_QUEUE(port) (0x300280 + 4 * (port))
+#define YT921X_PORT_PRIOm_MCAST_QUEUE_M(m) (3 << (2 * (m)))
+#define YT921X_PORT_PRIOm_MCAST_QUEUE(m, x) ((x) << (2 * (m)))
#define YT921X_MIRROR 0x300300
#define YT921X_MIRROR_IGR_PORTS_M GENMASK(26, 16)
#define YT921X_MIRROR_IGR_PORTS(x) FIELD_PREP(YT921X_MIRROR_IGR_PORTS_M, (x))
@@ -531,6 +537,48 @@ enum yt921x_app_selector {
#define YT921X_MIRROR_PORT_M GENMASK(3, 0)
#define YT921X_MIRROR_PORT(x) FIELD_PREP(YT921X_MIRROR_PORT_M, (x))
+#define YT921X_QUEUE_SHAPE_SLOT 0x340008
+#define YT921X_QUEUE_SHAPE_SLOT_SLOT_M GENMASK(11, 0)
+#define YT921X_PORT_SHAPE_SLOT 0x34000c
+#define YT921X_PORT_SHAPE_SLOT_SLOT_M GENMASK(11, 0)
+#define YT921X_QUEUEn_SCH(x) (0x341000 + 4 * (x))
+#define YT921X_QUEUE_SCH_E_DWRR_M GENMASK(27, 18)
+#define YT921X_QUEUE_SCH_E_DWRR(x) FIELD_PREP(YT921X_QUEUE_SCH_E_DWRR_M, (x))
+#define YT921X_QUEUE_SCH_C_DWRR_M GENMASK(17, 8)
+#define YT921X_QUEUE_SCH_C_DWRR(x) FIELD_PREP(YT921X_QUEUE_SCH_C_DWRR_M, (x))
+#define YT921X_QUEUE_SCH_E_PRIO_M GENMASK(7, 4)
+#define YT921X_QUEUE_SCH_E_PRIO(x) FIELD_PREP(YT921X_QUEUE_SCH_E_PRIO_M, (x))
+#define YT921X_QUEUE_SCH_C_PRIO_M GENMASK(3, 0)
+#define YT921X_QUEUE_SCH_C_PRIO(x) FIELD_PREP(YT921X_QUEUE_SCH_C_PRIO_M, (x))
+#define YT921X_C_DWRRn(x) (0x342000 + 4 * (x))
+#define YT921X_E_DWRRn(x) (0x343000 + 4 * (x))
+#define YT921X_DWRR_PKT_MODE BIT(0) /* 0: byte rate mode */
+#define YT921X_QUEUEn_SHAPE_CTRL(x) (0x34c000 + 0x10 * (x))
+#define YT921X_QUEUE_SHAPE_CTRLc_TOKEN_OVERFLOW_EN BIT(6)
+#define YT921X_QUEUE_SHAPE_CTRLc_E_EN BIT(5)
+#define YT921X_QUEUE_SHAPE_CTRLc_C_EN BIT(4)
+#define YT921X_QUEUE_SHAPE_CTRLc_PKT_MODE BIT(3) /* 0: byte rate mode */
+#define YT921X_QUEUE_SHAPE_CTRLc_UNIT_M GENMASK(2, 0)
+#define YT921X_QUEUE_SHAPE_CTRLc_UNIT(x) FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLc_UNIT_M, (x))
+#define YT921X_QUEUE_SHAPE_CTRLb_EBS_M GENMASK(31, 18)
+#define YT921X_QUEUE_SHAPE_CTRLb_EBS(x) FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLb_EBS_M, (x))
+#define YT921X_QUEUE_SHAPE_CTRLb_EIR_M GENMASK(17, 0)
+#define YT921X_QUEUE_SHAPE_CTRLb_EIR(x) FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLb_EIR_M, (x))
+#define YT921X_QUEUE_SHAPE_CTRLa_CBS_M GENMASK(31, 18)
+#define YT921X_QUEUE_SHAPE_CTRLa_CBS(x) FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLa_CBS_M, (x))
+#define YT921X_QUEUE_SHAPE_CTRLa_CIR_M GENMASK(17, 0)
+#define YT921X_QUEUE_SHAPE_CTRLa_CIR(x) FIELD_PREP(YT921X_QUEUE_SHAPE_CTRLa_CIR_M, (x))
+#define YT921X_PORTn_SHAPE_CTRL(port) (0x354000 + 8 * (port))
+#define YT921X_PORT_SHAPE_CTRLb_EN BIT(4)
+#define YT921X_PORT_SHAPE_CTRLb_PKT_MODE BIT(3) /* 0: byte rate mode */
+#define YT921X_PORT_SHAPE_CTRLb_UNIT_M GENMASK(2, 0)
+#define YT921X_PORT_SHAPE_CTRLb_UNIT(x) FIELD_PREP(YT921X_PORT_SHAPE_CTRLb_UNIT_M, (x))
+#define YT921X_PORT_SHAPE_CTRLa_CBS_M GENMASK(31, 18)
+#define YT921X_PORT_SHAPE_CTRLa_CBS(x) FIELD_PREP(YT921X_PORT_SHAPE_CTRLa_CBS_M, (x))
+#define YT921X_PORT_SHAPE_CTRLa_CIR_M GENMASK(17, 0)
+#define YT921X_PORT_SHAPE_CTRLa_CIR(x) FIELD_PREP(YT921X_PORT_SHAPE_CTRLa_CIR_M, (x))
+#define YT921X_PORTn_SHAPE_STAT(port) (0x356000 + 4 * (port))
+
#define YT921X_EDATA_EXTMODE 0xfb
#define YT921X_EDATA_LEN 0x100
@@ -556,6 +604,11 @@ enum yt921x_fdb_entry_status {
#define YT921X_METER_UNIT_MAX ((1 << 3) - 1)
#define YT921X_METER_CIR_MAX ((1 << 18) - 1)
#define YT921X_METER_CBS_MAX ((1 << 16) - 1)
+#define YT921X_PORT_SHAPE_SLOT_MIN 80
+#define YT921X_QUEUE_SHAPE_SLOT_MIN 132
+#define YT921X_SHAPE_UNIT_MAX ((1 << 3) - 1)
+#define YT921X_SHAPE_CIR_MAX ((1 << 18) - 1)
+#define YT921X_SHAPE_CBS_MAX ((1 << 14) - 1)
#define YT921X_LAG_NUM 2
#define YT921X_LAG_PORT_NUM 4
@@ -573,7 +626,16 @@ enum yt921x_fdb_entry_status {
#define YT921X_TAG_LEN 8
/* 8 internal + 2 external + 1 mcu */
-#define YT921X_PORT_NUM 11
+#define YT921X_PORT_NUM 11
+#define YT921X_UCAST_QUEUE_NUM 8
+#define YT921X_MCAST_QUEUE_NUM 4
+#define YT921X_PORT_QUEUE_NUM \
+ (YT921X_UCAST_QUEUE_NUM + YT921X_MCAST_QUEUE_NUM)
+#define YT921X_UCAST_QUEUE_ID(port, queue) \
+ (YT921X_UCAST_QUEUE_NUM * (port) + (queue))
+#define YT921X_MCAST_QUEUE_ID(port, queue) \
+ (YT921X_UCAST_QUEUE_NUM * YT921X_PORT_NUM + \
+ YT921X_MCAST_QUEUE_NUM * (port) + (queue))
#define yt921x_port_is_internal(port) ((port) < 8)
#define yt921x_port_is_external(port) (8 <= (port) && (port) < 9)
@@ -652,6 +714,8 @@ struct yt921x_priv {
const struct yt921x_info *info;
unsigned int meter_slot_ns;
+ unsigned int port_shape_slot_ns;
+ unsigned int queue_shape_slot_ns;
/* cache of dsa_cpu_ports(ds) */
u16 cpu_ports_mask;
unsigned char cycle_ns;
--
2.53.0
^ permalink raw reply related
* [PATCH net-next 1/5] net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode
From: Ciprian Regus via B4 Relay @ 2026-05-02 23:24 UTC (permalink / raw)
To: Parthiban Veerasooran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Andrew Lunn, Heiner Kallweit, Russell King,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: netdev, linux-kernel, linux-doc, devicetree, Ciprian Regus
In-Reply-To: <20260503-adin1140-driver-v1-0-dd043cdd88f0@analog.com>
From: Ciprian Regus <ciprian.regus@analog.com>
Implement the OA TC6 standard defined protected mode for control (register
access) transactions. In addition to the current register access formats
the oa_tc6 driver handles, 1's complement values of the data field
are included (by both the host and the MACPHY) in the SPI transfer frames.
This feature acts as an integrity check.
Control write transactions look like this:
|<- 32 bits ->|<--- data_size --->|<- 32 bits ->|
MOSI: | ctrl header | reg write data | ignored |
MISO: | (discard) | echoed ctrl hdr | echoed data |
data_size (LEN = number of registers to read in a sequence):
Unprotected: 32 x (LEN + 1) bits
Protected: 2 x 32 x (LEN + 1) bits
Control read transaction:
|<- 32 bits ->|<--- 32 bits --> |<- data_size ->|
MOSI: | ctrl header | ignored ... |
MISO: | (discard) | echoed ctrl hdr | reg read data |
data_size (LEN = number of registers to read in a sequence):
Unprotected: 32 x (LEN + 1) bits
Protected: 2 x 32 x (LEN + 1) bits
Register data format ("reg write data" and "reg read data"):
Unprotected:
| W1 (normal) | W2 (normal) | ... | Wx (normal) |
Protected:
| W1 (normal) | W1 (complement) | ... | Wx (normal) | Wx (complement)|
The protected mode state can be read from the bit 5 of CONFIG0 (0x4)
register, and this setting is usually only configured during the
MACPHY's reset (depending on the device it can be done by setting the
state of a pin). We can read the protected mode configuration before any
other register access and since the SPI transfer is initially sized for an
unprotected read, the MACPHY's complement words are never clocked out
and no checking is required. The data transactions (Ethernet frames)
remain unchanged.
Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
---
drivers/net/ethernet/oa_tc6.c | 105 ++++++++++++++++++++++++++++++++++++------
1 file changed, 92 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
index 91a906a7918a..546ca652d974 100644
--- a/drivers/net/ethernet/oa_tc6.c
+++ b/drivers/net/ethernet/oa_tc6.c
@@ -24,6 +24,7 @@
#define OA_TC6_REG_CONFIG0 0x0004
#define CONFIG0_SYNC BIT(15)
#define CONFIG0_ZARFE_ENABLE BIT(12)
+#define CONFIG0_PROTE BIT(5)
/* Status Register #0 */
#define OA_TC6_REG_STATUS0 0x0008
@@ -87,6 +88,7 @@
#define OA_TC6_PHY_C45_AUTO_NEG_MMS5 5 /* MMD 7 */
#define OA_TC6_PHY_C45_POWER_UNIT_MMS6 6 /* MMD 13 */
+#define OA_TC6_CTRL_PROT_REPLY_SIZE 4
#define OA_TC6_CTRL_HEADER_SIZE 4
#define OA_TC6_CTRL_REG_VALUE_SIZE 4
#define OA_TC6_CTRL_IGNORED_SIZE 4
@@ -95,6 +97,13 @@
(OA_TC6_CTRL_MAX_REGISTERS *\
OA_TC6_CTRL_REG_VALUE_SIZE) +\
OA_TC6_CTRL_IGNORED_SIZE)
+
+#define OA_TC6_CTRL_SPI_BUF_PROT_SIZE (OA_TC6_CTRL_HEADER_SIZE +\
+ (OA_TC6_CTRL_MAX_REGISTERS *\
+ (OA_TC6_CTRL_REG_VALUE_SIZE +\
+ OA_TC6_CTRL_PROT_REPLY_SIZE)) +\
+ OA_TC6_CTRL_IGNORED_SIZE)
+
#define OA_TC6_CHUNK_PAYLOAD_SIZE 64
#define OA_TC6_DATA_HEADER_SIZE 4
#define OA_TC6_CHUNK_SIZE (OA_TC6_DATA_HEADER_SIZE +\
@@ -129,6 +138,7 @@ struct oa_tc6 {
u8 rx_chunks_available;
bool rx_buf_overflow;
bool int_flag;
+ bool prot_ctrl;
};
enum oa_tc6_header_type {
@@ -212,25 +222,36 @@ static void oa_tc6_update_ctrl_write_data(struct oa_tc6 *tc6, u32 value[],
{
__be32 *tx_buf = tc6->spi_ctrl_tx_buf + OA_TC6_CTRL_HEADER_SIZE;
- for (int i = 0; i < length; i++)
+ for (int i = 0; i < length; i++) {
*tx_buf++ = cpu_to_be32(value[i]);
+ if (tc6->prot_ctrl)
+ *tx_buf++ = cpu_to_be32(~value[i]);
+ }
}
-static u16 oa_tc6_calculate_ctrl_buf_size(u8 length)
+static u16 oa_tc6_calculate_ctrl_buf_size(u8 length, bool ctrl_prot)
{
+ u32 reply_size = OA_TC6_CTRL_REG_VALUE_SIZE;
+
+ if (ctrl_prot)
+ reply_size += OA_TC6_CTRL_PROT_REPLY_SIZE;
+
/* Control command consists 4 bytes header + 4 bytes register value for
- * each register + 4 bytes ignored value.
+ * each register (+ 4 bytes for the register value complement in case
+ * protected mode is used) + 4 bytes ignored value.
*/
- return OA_TC6_CTRL_HEADER_SIZE + OA_TC6_CTRL_REG_VALUE_SIZE * length +
+ return OA_TC6_CTRL_HEADER_SIZE + reply_size * length +
OA_TC6_CTRL_IGNORED_SIZE;
}
static void oa_tc6_prepare_ctrl_spi_buf(struct oa_tc6 *tc6, u32 address,
u32 value[], u8 length,
- enum oa_tc6_register_op reg_op)
+ enum oa_tc6_register_op reg_op,
+ u16 buf_size)
{
__be32 *tx_buf = tc6->spi_ctrl_tx_buf;
+ memset(tx_buf, 0, buf_size);
*tx_buf = oa_tc6_prepare_ctrl_header(address, length, reg_op);
if (reg_op == OA_TC6_CTRL_REG_WRITE)
@@ -253,10 +274,12 @@ static int oa_tc6_check_ctrl_write_reply(struct oa_tc6 *tc6, u8 size)
return 0;
}
-static int oa_tc6_check_ctrl_read_reply(struct oa_tc6 *tc6, u8 size)
+static int oa_tc6_check_ctrl_read_reply(struct oa_tc6 *tc6, u8 length)
{
- u32 *rx_buf = tc6->spi_ctrl_rx_buf + OA_TC6_CTRL_IGNORED_SIZE;
- u32 *tx_buf = tc6->spi_ctrl_tx_buf;
+ __be32 *rx_buf = tc6->spi_ctrl_rx_buf + OA_TC6_CTRL_IGNORED_SIZE;
+ __be32 *tx_buf = tc6->spi_ctrl_tx_buf;
+ u32 complement;
+ u32 reply;
/* The echoed control read header must match with the one that was
* transmitted.
@@ -264,6 +287,20 @@ static int oa_tc6_check_ctrl_read_reply(struct oa_tc6 *tc6, u8 size)
if (*tx_buf != *rx_buf)
return -EPROTO;
+ if (tc6->prot_ctrl) {
+ /* Skip past the echoed header to the value/complement pairs */
+ rx_buf += 1;
+ for (int i = 0; i < length; i++) {
+ reply = be32_to_cpu(rx_buf[0]);
+ complement = be32_to_cpu(rx_buf[1]);
+
+ if (complement != ~reply)
+ return -EPROTO;
+
+ rx_buf += 2;
+ }
+ }
+
return 0;
}
@@ -273,8 +310,13 @@ static void oa_tc6_copy_ctrl_read_data(struct oa_tc6 *tc6, u32 value[],
__be32 *rx_buf = tc6->spi_ctrl_rx_buf + OA_TC6_CTRL_IGNORED_SIZE +
OA_TC6_CTRL_HEADER_SIZE;
- for (int i = 0; i < length; i++)
+ for (int i = 0; i < length; i++) {
value[i] = be32_to_cpu(*rx_buf++);
+
+ /* skip complement word */
+ if (tc6->prot_ctrl)
+ rx_buf++;
+ }
}
static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
@@ -283,10 +325,10 @@ static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
u16 size;
int ret;
- /* Prepare control command and copy to SPI control buffer */
- oa_tc6_prepare_ctrl_spi_buf(tc6, address, value, length, reg_op);
+ size = oa_tc6_calculate_ctrl_buf_size(length, tc6->prot_ctrl);
- size = oa_tc6_calculate_ctrl_buf_size(length);
+ /* Prepare control command and copy to SPI control buffer */
+ oa_tc6_prepare_ctrl_spi_buf(tc6, address, value, length, reg_op, size);
/* Perform SPI transfer */
ret = oa_tc6_spi_transfer(tc6, OA_TC6_CTRL_HEADER, size);
@@ -301,7 +343,7 @@ static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
return oa_tc6_check_ctrl_write_reply(tc6, size);
/* Check echoed/received control read command reply for errors */
- ret = oa_tc6_check_ctrl_read_reply(tc6, size);
+ ret = oa_tc6_check_ctrl_read_reply(tc6, length);
if (ret)
return ret;
@@ -1224,6 +1266,20 @@ netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(oa_tc6_start_xmit);
+static int oa_tc6_check_ctrl_protection(struct oa_tc6 *tc6)
+{
+ u32 regval;
+ int ret;
+
+ ret = oa_tc6_read_register(tc6, OA_TC6_REG_CONFIG0, ®val);
+ if (ret)
+ return ret;
+
+ tc6->prot_ctrl = FIELD_GET(CONFIG0_PROTE, regval);
+
+ return 0;
+}
+
/**
* oa_tc6_init - allocates and initializes oa_tc6 structure.
* @spi: device with which data will be exchanged.
@@ -1276,6 +1332,29 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
if (!tc6->spi_data_rx_buf)
return NULL;
+ ret = oa_tc6_check_ctrl_protection(tc6);
+ if (ret) {
+ dev_err(&tc6->spi->dev,
+ "Failed to check the protection mode: %d\n", ret);
+ return NULL;
+ }
+
+ if (tc6->prot_ctrl) {
+ tc6->spi_ctrl_tx_buf = devm_krealloc(&tc6->spi->dev,
+ tc6->spi_ctrl_tx_buf,
+ OA_TC6_CTRL_SPI_BUF_PROT_SIZE,
+ GFP_KERNEL);
+ if (!tc6->spi_ctrl_tx_buf)
+ return NULL;
+
+ tc6->spi_ctrl_rx_buf = devm_krealloc(&tc6->spi->dev,
+ tc6->spi_ctrl_rx_buf,
+ OA_TC6_CTRL_SPI_BUF_PROT_SIZE,
+ GFP_KERNEL);
+ if (!tc6->spi_ctrl_rx_buf)
+ return NULL;
+ }
+
ret = oa_tc6_sw_reset_macphy(tc6);
if (ret) {
dev_err(&tc6->spi->dev,
--
2.43.0
^ permalink raw reply related
* [PATCH net-next 0/5] net: Add ADIN1140 support
From: Ciprian Regus via B4 Relay @ 2026-05-02 23:24 UTC (permalink / raw)
To: Parthiban Veerasooran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Andrew Lunn, Heiner Kallweit, Russell King,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: netdev, linux-kernel, linux-doc, devicetree, Ciprian Regus
This series introduces support for the ADIN1140 (also called AD3306)
10BASE-T1S single port MACPHY. The device integrates the MAC and PHY in
the same package. The communication with the host CPU is done through an
SPI interface, using the Open Alliance TC6 protocol for control and data
transactions. As a result, the oa_tc6 framework is used to implement
the communication with the device (register accesses and Ethernet frame
RX/TX).
The MAC and PHY are connected internally using an MII and MDIO bus.
The PHY is a half duplex 10Mbps device, which implements both the PLCA
RS (IEEE 802.3 clause 148) and CSMA/CD methods of accessing the Ethernet
medium. The 10BASE-T1S standard allows multiple PHY devices to be
connected (in parallel) on the same single twisted pair network segment,
so PLCA can be configured in order to provide a fair access scheme to
all the nodes and reduce the jitter introduced by the unordered CSMA/CD
transmits. The PHY's internal register map can be accessed using the
direct MDIO mode of the OA TC6. The control, status, phy id 1 & 2 C22
registers are mapped to the 0xFF00 - 0xFF03 range. As for C45
addressable devices, the PHY has PCS, PMA and PLCA blocks.
The first 2 patches in the series are changes to the oa_tc6, that would
make the framework usable by the subsequent ADIN1140 MAC driver.
The first commit is required because the ADIN1140 only allows protected
mode OA TC6 control transactions, which the oa_tc6 framework doesn't
currently implement.
The second commit is required in order to allow the MAC driver to have a
custom implementation for the mii_bus access methods as a workaround for
hardware issues:
1. The OA TC6 standard defines the direct and indirect access modes for
MDIO transactions. The ADIN1140 incorrectly advertises indirect mode
only (supported capabilities register - 0x2, bit 9), while actually
implementing just the direct mode. We cannot rely on the CAP register
to choose an access method (which oa_tc6 does by default, even though
it only implements the direct mode), so the driver has to use its
own.
2. The ADIN1140 cannot access the C22 register space of the internal
PHY, while the PHY is busy receiving frames. If that happens, the
CONFIG0 and CONFIG2 registers of the MAC will get corrupted and the
data transfer will stop. Those two registers configure settings for
the transfer protocol between the MAC and host, so the value for some
of their subfields shouldn't be changed while the netdev is up.
Since we know the PHY is internal, the MAC driver can implement a
custom mii_bus, which can intercept C22 accesses. Most of the
registers mapped in the 0x0 - 0x3 range (the only ones the PHY offers)
are read only, and their value can be read from somewhere else (e.g
the PHYID 1 & 2 have the same value as 0x1 in the MAC memory map).
C45 accesses do not cause this issue, so we can properly implement
them.
Even though they have different driver, the MAC one cannot function
without the PHY driver, since the PHY is not compatible with the generic
c22 driver. As such CONFIG_ADIN1140 selects CONFIG_ADIN1140_PHY.
Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
---
Ciprian Regus (5):
net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode
net: ethernet: oa_tc6: Allow custom mii_bus
net: phy: Add support for the ADIN1140 PHY
net: ethernet: adi: Add a driver for the ADIN1140 MACPHY
dt-bindings: net: Add bindings for the ADIN1140
.../devicetree/bindings/net/adi,adin1140.yaml | 69 ++
Documentation/networking/oa-tc6-framework.rst | 3 +-
MAINTAINERS | 15 +
drivers/net/ethernet/adi/Kconfig | 12 +
drivers/net/ethernet/adi/Makefile | 1 +
drivers/net/ethernet/adi/adin1140.c | 805 +++++++++++++++++++++
drivers/net/ethernet/microchip/lan865x/lan865x.c | 6 +-
drivers/net/ethernet/oa_tc6.c | 194 +++--
drivers/net/phy/Kconfig | 6 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/adin1140.c | 102 +++
include/linux/oa_tc6.h | 9 +-
12 files changed, 1173 insertions(+), 50 deletions(-)
---
base-commit: fbf6f64a4322cfeb0d98f39baf8ce18246dd12c0
change-id: 20260429-adin1140-driver-93ae0d376318
Best regards,
--
Ciprian Regus <ciprian.regus@analog.com>
^ permalink raw reply
* [PATCH net-next 3/5] net: phy: Add support for the ADIN1140 PHY
From: Ciprian Regus via B4 Relay @ 2026-05-02 23:24 UTC (permalink / raw)
To: Parthiban Veerasooran, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Jonathan Corbet,
Shuah Khan, Andrew Lunn, Heiner Kallweit, Russell King,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: netdev, linux-kernel, linux-doc, devicetree, Ciprian Regus
In-Reply-To: <20260503-adin1140-driver-v1-0-dd043cdd88f0@analog.com>
From: Ciprian Regus <ciprian.regus@analog.com>
Add a driver for the ADIN1140's internal 10BASE-T1S PHY. The device
doesn't implement autonegotiation, so the link is always reported as
being up. Since the PHY has no link-change interrupts and the link is
always up, we set phydev->irq = PHY_MAC_INTERRUPT to prevent phylib from
polling the link state.
The device implements both C22 and C45 MDIO access methods, but can only
be discovered over C22, since the C45 MMD devices lack the MDIO_DEVID1 and
MDIO_DEVID2 registers. The indirect C45 over C22 feature is not
supported.
Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
---
MAINTAINERS | 7 ++++
drivers/net/phy/Kconfig | 6 +++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/adin1140.c | 102 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 116 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 27a073f53cea..1e58da5ef47a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1843,6 +1843,13 @@ S: Supported
W: https://ez.analog.com/linux-software-drivers
F: drivers/dma/dma-axi-dmac.c
+ANALOG DEVICES INC ETHERNET PHY DRIVERS
+M: Ciprian Regus <ciprian.regus@analog.com>
+L: netdev@vger.kernel.org
+S: Maintained
+W: https://ez.analog.com/linux-software-drivers
+F: drivers/net/phy/adin1140.c
+
ANALOG DEVICES INC IIO DRIVERS
M: Lars-Peter Clausen <lars@metafoo.de>
M: Michael Hennerich <Michael.Hennerich@analog.com>
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index b5ee338b620d..fa5cd59a3825 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -124,6 +124,12 @@ config ADIN1100_PHY
Currently supports the:
- ADIN1100 - Robust,Industrial, Low Power 10BASE-T1L Ethernet PHY
+config ADIN1140_PHY
+ tristate "Analog Devices ADIN1140 10BASE-T1S PHY"
+ help
+ Adds support for the Analog Devices, Inc. ADIN1140's internal
+ 10BASE-T1S PHY.
+
config AMCC_QT2025_PHY
tristate "AMCC QT2025 PHY"
depends on RUST_PHYLIB_ABSTRACTIONS
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 05e4878af27a..2519364bc334 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -29,6 +29,7 @@ obj-y += $(sfp-obj-y) $(sfp-obj-m)
obj-$(CONFIG_ADIN_PHY) += adin.o
obj-$(CONFIG_ADIN1100_PHY) += adin1100.o
+obj-$(CONFIG_ADIN1140_PHY) += adin1140.o
obj-$(CONFIG_AIR_EN8811H_PHY) += air_en8811h.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_AMCC_QT2025_PHY) += qt2025.o
diff --git a/drivers/net/phy/adin1140.c b/drivers/net/phy/adin1140.c
new file mode 100644
index 000000000000..3244107ce9ef
--- /dev/null
+++ b/drivers/net/phy/adin1140.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Analog Devices, Inc. ADIN1140 10BASE-T1S PHY
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/phy.h>
+
+#define ADIN1140_PHY_ID 0x0283be00
+
+#define ADIN1140_PCS_CTRL 0x08f3
+#define ADIN1140_PCS_CTRL_LOOPBACK BIT(14)
+
+static int adin1140_phy_read_mmd(struct phy_device *phydev, int devnum,
+ u16 regnum)
+{
+ struct mii_bus *bus = phydev->mdio.bus;
+ int addr = phydev->mdio.addr;
+
+ return __mdiobus_c45_read(bus, addr, devnum, regnum);
+}
+
+static int adin1140_phy_write_mmd(struct phy_device *phydev, int devnum,
+ u16 regnum, u16 val)
+{
+ struct mii_bus *bus = phydev->mdio.bus;
+ int addr = phydev->mdio.addr;
+
+ return __mdiobus_c45_write(bus, addr, devnum, regnum, val);
+}
+
+static int adin1140_config_init(struct phy_device *phydev)
+{
+ /* The link status of the PHY doesn't need to be polled, because
+ * the device doesn't implement AN and there is no other mechanism
+ * to report the link state.
+ */
+ phydev->irq = PHY_MAC_INTERRUPT;
+
+ return 0;
+}
+
+static int adin1140_config_aneg(struct phy_device *phydev)
+{
+ /* phylib tries to clear BIT(12) in MDIO_CTRL1, since AN is disabled.
+ * However, on the ADIN1140, that field is non-standard, being used
+ * to control the reset status of the PHY (thus it needs to remain set).
+ */
+ return 0;
+}
+
+static int adin1140_loopback(struct phy_device *phydev, bool enable, int speed)
+{
+ if (enable && speed)
+ return -EOPNOTSUPP;
+
+ return phy_modify_mmd(phydev, MDIO_MMD_PCS, ADIN1140_PCS_CTRL,
+ ADIN1140_PCS_CTRL_LOOPBACK,
+ enable ? ADIN1140_PCS_CTRL_LOOPBACK : 0);
+}
+
+static int adin1140_read_status(struct phy_device *phydev)
+{
+ phydev->link = 1;
+ phydev->duplex = DUPLEX_HALF;
+ phydev->speed = SPEED_10;
+ phydev->autoneg = AUTONEG_DISABLE;
+
+ return 0;
+}
+
+static struct phy_driver adin1140_driver[] = {
+ {
+ PHY_ID_MATCH_EXACT(ADIN1140_PHY_ID),
+ .name = "ADIN1140",
+ .features = PHY_BASIC_T1S_P2MP_FEATURES,
+ .read_status = adin1140_read_status,
+ .config_init = adin1140_config_init,
+ .config_aneg = adin1140_config_aneg,
+ .set_loopback = adin1140_loopback,
+ .read_mmd = adin1140_phy_read_mmd,
+ .write_mmd = adin1140_phy_write_mmd,
+ .get_plca_cfg = genphy_c45_plca_get_cfg,
+ .set_plca_cfg = genphy_c45_plca_set_cfg,
+ .get_plca_status = genphy_c45_plca_get_status,
+ },
+};
+module_phy_driver(adin1140_driver);
+
+static const struct mdio_device_id __maybe_unused adin1140_tbl[] = {
+ { PHY_ID_MATCH_EXACT(ADIN1140_PHY_ID) },
+ { }
+};
+
+MODULE_DEVICE_TABLE(mdio, adin1140_tbl);
+
+MODULE_DESCRIPTION("Analog Devices, Inc. ADIN1140 10BASE-T1S PHY");
+MODULE_AUTHOR("Ciprian Regus <ciprian.regus@analog.com>");
+MODULE_LICENSE("GPL");
--
2.43.0
^ 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