From: Leon Romanovsky <leon@kernel.org>
To: Tariq Toukan <tariqt@nvidia.com>
Cc: Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Saeed Mahameed <saeedm@nvidia.com>,
Mark Bloch <mbloch@nvidia.com>,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-kernel@vger.kernel.org, Gal Pressman <gal@nvidia.com>,
Parav Pandit <parav@nvidia.com>, Shay Drory <shayd@nvidia.com>
Subject: Re: [PATCH net-next 1/4] net/mlx5: Refactor devcom to use match attributes
Date: Mon, 15 Sep 2025 12:08:02 +0300 [thread overview]
Message-ID: <20250915090802.GC9353@unreal> (raw)
In-Reply-To: <1757572267-601785-2-git-send-email-tariqt@nvidia.com>
On Thu, Sep 11, 2025 at 09:31:04AM +0300, Tariq Toukan wrote:
> From: Shay Drory <shayd@nvidia.com>
>
> Refactor the devcom interface to use a match attribute structure instead
> of passing raw keys. This change lays the groundwork for extending devcom
> matching logic with additional fields like net namespace, improving its
> flexibility and robustness.
>
> No functional changes.
>
> Signed-off-by: Shay Drory <shayd@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/en_main.c | 6 +++-
> .../net/ethernet/mellanox/mlx5/core/en_tc.c | 7 +++--
> .../net/ethernet/mellanox/mlx5/core/eswitch.h | 7 +++--
> .../mellanox/mlx5/core/eswitch_offloads.c | 5 +--
> .../ethernet/mellanox/mlx5/core/lib/clock.c | 14 ++++++---
> .../ethernet/mellanox/mlx5/core/lib/devcom.c | 31 +++++++++++++------
> .../ethernet/mellanox/mlx5/core/lib/devcom.h | 10 +++++-
> .../net/ethernet/mellanox/mlx5/core/lib/sd.c | 4 ++-
> .../net/ethernet/mellanox/mlx5/core/main.c | 7 +++--
> 9 files changed, 65 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 714cce595692..fe5f5ae433b7 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -235,9 +235,13 @@ static int mlx5e_devcom_event_mpv(int event, void *my_data, void *event_data)
>
> static int mlx5e_devcom_init_mpv(struct mlx5e_priv *priv, u64 *data)
> {
> + struct mlx5_devcom_match_attr attr = {
> + .key.val = *data,
> + };
> +
> priv->devcom = mlx5_devcom_register_component(priv->mdev->priv.devc,
> MLX5_DEVCOM_MPV,
> - *data,
> + &attr,
> mlx5e_devcom_event_mpv,
> priv);
> if (IS_ERR(priv->devcom))
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 32c07a8b03d1..9874a15c6fba 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -5387,12 +5387,13 @@ void mlx5e_tc_ht_cleanup(struct rhashtable *tc_ht)
> int mlx5e_tc_esw_init(struct mlx5_rep_uplink_priv *uplink_priv)
> {
> const size_t sz_enc_opts = sizeof(struct tunnel_match_enc_opts);
> + struct mlx5_devcom_match_attr attr = {};
> struct netdev_phys_item_id ppid;
> struct mlx5e_rep_priv *rpriv;
> struct mapping_ctx *mapping;
> struct mlx5_eswitch *esw;
> struct mlx5e_priv *priv;
> - u64 mapping_id, key;
> + u64 mapping_id;
> int err = 0;
>
> rpriv = container_of(uplink_priv, struct mlx5e_rep_priv, uplink_priv);
> @@ -5448,8 +5449,8 @@ int mlx5e_tc_esw_init(struct mlx5_rep_uplink_priv *uplink_priv)
>
> err = netif_get_port_parent_id(priv->netdev, &ppid, false);
> if (!err) {
> - memcpy(&key, &ppid.id, sizeof(key));
> - mlx5_esw_offloads_devcom_init(esw, key);
> + memcpy(&attr.key.val, &ppid.id, sizeof(attr.key.val));
> + mlx5_esw_offloads_devcom_init(esw, &attr);
> }
>
> return 0;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> index 4fe285ce32aa..df3756d7e52e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> @@ -433,7 +433,8 @@ int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs);
> void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw, bool clear_vf);
> void mlx5_eswitch_disable_locked(struct mlx5_eswitch *esw);
> void mlx5_eswitch_disable(struct mlx5_eswitch *esw);
> -void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw, u64 key);
> +void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw,
> + const struct mlx5_devcom_match_attr *attr);
> void mlx5_esw_offloads_devcom_cleanup(struct mlx5_eswitch *esw);
> bool mlx5_esw_offloads_devcom_is_ready(struct mlx5_eswitch *esw);
> int mlx5_eswitch_set_vport_mac(struct mlx5_eswitch *esw,
> @@ -928,7 +929,9 @@ static inline void mlx5_eswitch_cleanup(struct mlx5_eswitch *esw) {}
> static inline int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs) { return 0; }
> static inline void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw, bool clear_vf) {}
> static inline void mlx5_eswitch_disable(struct mlx5_eswitch *esw) {}
> -static inline void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw, u64 key) {}
> +static inline void
> +mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw,
> + const struct mlx5_devcom_match_attr *attr) {}
> static inline void mlx5_esw_offloads_devcom_cleanup(struct mlx5_eswitch *esw) {}
> static inline bool mlx5_esw_offloads_devcom_is_ready(struct mlx5_eswitch *esw) { return false; }
> static inline bool mlx5_eswitch_is_funcs_handler(struct mlx5_core_dev *dev) { return false; }
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index d57f86d297ab..bc9838dc5bf8 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -3104,7 +3104,8 @@ static int mlx5_esw_offloads_devcom_event(int event,
> return err;
> }
>
> -void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw, u64 key)
> +void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw,
> + const struct mlx5_devcom_match_attr *attr)
> {
> int i;
>
> @@ -3123,7 +3124,7 @@ void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw, u64 key)
> esw->num_peers = 0;
> esw->devcom = mlx5_devcom_register_component(esw->dev->priv.devc,
> MLX5_DEVCOM_ESW_OFFLOADS,
> - key,
> + attr,
> mlx5_esw_offloads_devcom_event,
> esw);
> if (IS_ERR(esw->devcom))
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> index 7ad3baca99de..8f2ad45bec9f 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
> @@ -1435,14 +1435,20 @@ static int mlx5_clock_alloc(struct mlx5_core_dev *mdev, bool shared)
> static void mlx5_shared_clock_register(struct mlx5_core_dev *mdev, u64 key)
> {
> struct mlx5_core_dev *peer_dev, *next = NULL;
> + struct mlx5_devcom_match_attr attr = {
> + .key.val = key,
> + };
> + struct mlx5_devcom_comp_dev *compd;
> struct mlx5_devcom_comp_dev *pos;
>
> - mdev->clock_state->compdev = mlx5_devcom_register_component(mdev->priv.devc,
> - MLX5_DEVCOM_SHARED_CLOCK,
> - key, NULL, mdev);
> - if (IS_ERR(mdev->clock_state->compdev))
> + compd = mlx5_devcom_register_component(mdev->priv.devc,
> + MLX5_DEVCOM_SHARED_CLOCK,
> + &attr, NULL, mdev);
> + if (IS_ERR(compd))
> return;
>
> + mdev->clock_state->compdev = compd;
> +
> mlx5_devcom_comp_lock(mdev->clock_state->compdev);
> mlx5_devcom_for_each_peer_entry(mdev->clock_state->compdev, peer_dev, pos) {
> if (peer_dev->clock) {
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
> index 7b0766c89f4c..1ab9de316deb 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
> @@ -22,11 +22,15 @@ struct mlx5_devcom_dev {
> struct kref ref;
> };
>
> +struct mlx5_devcom_key {
> + union mlx5_devcom_match_key key;
> +};
> +
> struct mlx5_devcom_comp {
> struct list_head comp_list;
> enum mlx5_devcom_component id;
> - u64 key;
> struct list_head comp_dev_list_head;
> + struct mlx5_devcom_key key;
> mlx5_devcom_event_handler_t handler;
> struct kref ref;
> bool ready;
> @@ -108,7 +112,8 @@ void mlx5_devcom_unregister_device(struct mlx5_devcom_dev *devc)
> }
>
> static struct mlx5_devcom_comp *
> -mlx5_devcom_comp_alloc(u64 id, u64 key, mlx5_devcom_event_handler_t handler)
> +mlx5_devcom_comp_alloc(u64 id, const struct mlx5_devcom_match_attr *attr,
> + mlx5_devcom_event_handler_t handler)
> {
> struct mlx5_devcom_comp *comp;
>
> @@ -117,7 +122,7 @@ mlx5_devcom_comp_alloc(u64 id, u64 key, mlx5_devcom_event_handler_t handler)
> return ERR_PTR(-ENOMEM);
>
> comp->id = id;
> - comp->key = key;
> + comp->key.key = attr->key;
> comp->handler = handler;
> init_rwsem(&comp->sem);
> lockdep_register_key(&comp->lock_key);
> @@ -180,21 +185,27 @@ devcom_free_comp_dev(struct mlx5_devcom_comp_dev *devcom)
> static bool
> devcom_component_equal(struct mlx5_devcom_comp *devcom,
> enum mlx5_devcom_component id,
> - u64 key)
> + const struct mlx5_devcom_match_attr *attr)
> {
> - return devcom->id == id && devcom->key == key;
> + if (devcom->id != id)
> + return false;
> +
> + if (memcmp(&devcom->key.key, &attr->key, sizeof(devcom->key.key)))
> + return false;
> +
> + return true;
> }
>
> static struct mlx5_devcom_comp *
> devcom_component_get(struct mlx5_devcom_dev *devc,
> enum mlx5_devcom_component id,
> - u64 key,
> + const struct mlx5_devcom_match_attr *attr,
> mlx5_devcom_event_handler_t handler)
> {
> struct mlx5_devcom_comp *comp;
>
> devcom_for_each_component(comp) {
> - if (devcom_component_equal(comp, id, key)) {
> + if (devcom_component_equal(comp, id, attr)) {
> if (handler == comp->handler) {
> kref_get(&comp->ref);
> return comp;
> @@ -212,7 +223,7 @@ devcom_component_get(struct mlx5_devcom_dev *devc,
> struct mlx5_devcom_comp_dev *
> mlx5_devcom_register_component(struct mlx5_devcom_dev *devc,
> enum mlx5_devcom_component id,
> - u64 key,
> + const struct mlx5_devcom_match_attr *attr,
> mlx5_devcom_event_handler_t handler,
> void *data)
> {
> @@ -223,14 +234,14 @@ mlx5_devcom_register_component(struct mlx5_devcom_dev *devc,
> return ERR_PTR(-EINVAL);
>
> mutex_lock(&comp_list_lock);
> - comp = devcom_component_get(devc, id, key, handler);
> + comp = devcom_component_get(devc, id, attr, handler);
> if (IS_ERR(comp)) {
> devcom = ERR_PTR(-EINVAL);
> goto out_unlock;
> }
>
> if (!comp) {
> - comp = mlx5_devcom_comp_alloc(id, key, handler);
> + comp = mlx5_devcom_comp_alloc(id, attr, handler);
> if (IS_ERR(comp)) {
> devcom = ERR_CAST(comp);
> goto out_unlock;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h
> index c79699b94a02..f350d2395707 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.h
> @@ -6,6 +6,14 @@
>
> #include <linux/mlx5/driver.h>
>
> +union mlx5_devcom_match_key {
> + u64 val;
> +};
> +
> +struct mlx5_devcom_match_attr {
> + union mlx5_devcom_match_key key;
> +};
> +
> enum mlx5_devcom_component {
> MLX5_DEVCOM_ESW_OFFLOADS,
> MLX5_DEVCOM_MPV,
> @@ -25,7 +33,7 @@ void mlx5_devcom_unregister_device(struct mlx5_devcom_dev *devc);
> struct mlx5_devcom_comp_dev *
> mlx5_devcom_register_component(struct mlx5_devcom_dev *devc,
> enum mlx5_devcom_component id,
> - u64 key,
> + const struct mlx5_devcom_match_attr *attr,
> mlx5_devcom_event_handler_t handler,
> void *data);
> void mlx5_devcom_unregister_component(struct mlx5_devcom_comp_dev *devcom);
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> index eeb0b7ea05f1..d4015328ba65 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/sd.c
> @@ -210,13 +210,15 @@ static void sd_cleanup(struct mlx5_core_dev *dev)
> static int sd_register(struct mlx5_core_dev *dev)
> {
> struct mlx5_devcom_comp_dev *devcom, *pos;
> + struct mlx5_devcom_match_attr attr = {};
> struct mlx5_core_dev *peer, *primary;
> struct mlx5_sd *sd, *primary_sd;
> int err, i;
>
> sd = mlx5_get_sd(dev);
> + attr.key.val = sd->group_id;
> devcom = mlx5_devcom_register_component(dev->priv.devc, MLX5_DEVCOM_SD_GROUP,
> - sd->group_id, NULL, dev);
> + &attr, NULL, dev);
> if (IS_ERR(devcom))
> return PTR_ERR(devcom);
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> index 0951c7cc1b5f..1f7942202e14 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
> @@ -975,6 +975,10 @@ static void mlx5_pci_close(struct mlx5_core_dev *dev)
>
> static void mlx5_register_hca_devcom_comp(struct mlx5_core_dev *dev)
> {
> + struct mlx5_devcom_match_attr attr = {
> + .key.val = mlx5_query_nic_system_image_guid(dev),
> + };
> +
> /* This component is use to sync adding core_dev to lag_dev and to sync
> * changes of mlx5_adev_devices between LAG layer and other layers.
> */
> @@ -983,8 +987,7 @@ static void mlx5_register_hca_devcom_comp(struct mlx5_core_dev *dev)
>
> dev->priv.hca_devcom_comp =
> mlx5_devcom_register_component(dev->priv.devc, MLX5_DEVCOM_HCA_PORTS,
> - mlx5_query_nic_system_image_guid(dev),
> - NULL, dev);
> + &attr, NULL, dev);
> if (IS_ERR(dev->priv.hca_devcom_comp))
Because you are changing mlx5_devcom_register_component() signature
anyway, please do one step more and make sure that mlx5_devcom_register_component()
returns or NULL or successful devcom pointer. The thing is that devcom
pointer has error code which is stored for whole lifetime of the driver
is causing issues in mlx5_ib IPsec cleanup for multi-port devices.
Thanks
> mlx5_core_err(dev, "Failed to register devcom HCA component\n");
> }
> --
> 2.31.1
>
next prev parent reply other threads:[~2025-09-15 9:08 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-11 6:31 [PATCH net-next 0/4] net/mlx5: Refactor devcom and add net namespace support Tariq Toukan
2025-09-11 6:31 ` [PATCH net-next 1/4] net/mlx5: Refactor devcom to use match attributes Tariq Toukan
2025-09-12 14:11 ` Simon Horman
2025-09-15 9:08 ` Leon Romanovsky [this message]
2025-09-11 6:31 ` [PATCH net-next 2/4] net/mlx5: Lag, move devcom registration to LAG layer Tariq Toukan
2025-09-12 14:11 ` Simon Horman
2025-09-11 6:31 ` [PATCH net-next 3/4] net/mlx5: Add net namespace support to devcom Tariq Toukan
2025-09-12 14:11 ` Simon Horman
2025-09-11 6:31 ` [PATCH net-next 4/4] net/mlx5: Lag, add net namespace support Tariq Toukan
2025-09-12 14:09 ` Simon Horman
2025-09-13 1:52 ` Mark Bloch
2025-09-15 8:51 ` Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250915090802.GC9353@unreal \
--to=leon@kernel.org \
--cc=andrew+netdev@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gal@nvidia.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mbloch@nvidia.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=parav@nvidia.com \
--cc=saeedm@nvidia.com \
--cc=shayd@nvidia.com \
--cc=tariqt@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).