lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250915090802.GC9353@unreal>
Date: Mon, 15 Sep 2025 12:08:02 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Tariq Toukan <tariqt@...dia.com>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Saeed Mahameed <saeedm@...dia.com>, Mark Bloch <mbloch@...dia.com>,
	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org, Gal Pressman <gal@...dia.com>,
	Parav Pandit <parav@...dia.com>, Shay Drory <shayd@...dia.com>
Subject: Re: [PATCH net-next 1/4] net/mlx5: Refactor devcom to use match
 attributes

On Thu, Sep 11, 2025 at 09:31:04AM +0300, Tariq Toukan wrote:
> From: Shay Drory <shayd@...dia.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@...dia.com>
> Reviewed-by: Mark Bloch <mbloch@...dia.com>
> Signed-off-by: Tariq Toukan <tariqt@...dia.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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ