[PATCH net-next v3 0/2] devlink: net/mlx5: implement swp_l4_csum_mode via devlink params

[PATCH net-next v3 0/2] devlink: net/mlx5: implement swp_l4_csum_mode via devlink params

[Daniel Zahka]

This series contains two patches. The first is a pure refactor that
passes through the extack to devlink_param::get() implementations. The
second introduces a permanent devlink param to the mlx5 driver for
controlling tx csum behavior.

Enabling extack for devlink_param::get() allows drivers to provide
more information in cases when reading parameters from hardware can
result in errors or unexpected values.

The mlx5 swp_l4_csum_mode devlink param is necessary for initializing
PSP on CX7 NICs.

CHANGES:
v3:
  - fix warnings about undocumented param in intel ice driver
v2: https://lore.kernel.org/netdev/20251103194554.3203178-1-daniel.zahka@gmail.com/
  - fix indentation issue in new mlx5.rst entry
  - use extack in mlx5_nv_param_devlink_swp_l4_csum_mode_get()
  - introduce extack patch.
v1: https://lore.kernel.org/netdev/20251022190932.1073898-1-daniel.zahka@gmail.com/

Daniel Zahka (2):
  devlink: pass extack through to devlink_param::get()
  net/mlx5: implement swp_l4_csum_mode via devlink params

 Documentation/networking/devlink/mlx5.rst     |   9 +
 .../marvell/octeontx2/otx2_cpt_devlink.c      |   6 +-
 drivers/net/ethernet/amd/pds_core/core.h      |   3 +-
 drivers/net/ethernet/amd/pds_core/devlink.c   |   3 +-
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |   6 +-
 .../net/ethernet/intel/ice/devlink/devlink.c  |  14 +-
 .../marvell/octeontx2/af/rvu_devlink.c        |  15 +-
 .../marvell/octeontx2/nic/otx2_devlink.c      |   6 +-
 drivers/net/ethernet/mellanox/mlx4/main.c     |   6 +-
 .../net/ethernet/mellanox/mlx5/core/devlink.h |   3 +-
 .../net/ethernet/mellanox/mlx5/core/eswitch.c |   3 +-
 .../mellanox/mlx5/core/eswitch_offloads.c     |   3 +-
 .../net/ethernet/mellanox/mlx5/core/fs_core.c |   3 +-
 .../ethernet/mellanox/mlx5/core/fw_reset.c    |   3 +-
 .../mellanox/mlx5/core/lib/nv_param.c         | 170 +++++++++++++++++-
 .../mellanox/mlxsw/spectrum_acl_tcam.c        |   3 +-
 .../ethernet/netronome/nfp/devlink_param.c    |   3 +-
 drivers/net/ethernet/qlogic/qed/qed_devlink.c |   3 +-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |   3 +-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |   3 +-
 drivers/net/ethernet/ti/cpsw_new.c            |   6 +-
 drivers/net/wwan/iosm/iosm_ipc_devlink.c      |   3 +-
 include/net/devlink.h                         |   3 +-
 include/net/dsa.h                             |   3 +-
 net/devlink/param.c                           |  19 +-
 net/dsa/devlink.c                             |   3 +-
 26 files changed, 259 insertions(+), 46 deletions(-)

-- 
2.47.3

[PATCH net-next v3 1/2] devlink: pass extack through to devlink_param::get()

[Daniel Zahka]

Allow devlink_param::get() handlers to report error messages via
extack. This function is called in a few different contexts, but not
all of them will have an valid extack to use.

When devlink_param::get() is called from param_get_doit or
param_get_dumpit contexts, pass the extack through so that drivers can
report errors when retrieving param values. devlink_param::get() is
called from the context of devlink_param_notify(), pass NULL in for
the extack.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---

Notes:
    v3:
    - fix warnings about undocumented param in intel ice driver

 .../marvell/octeontx2/otx2_cpt_devlink.c      |  6 ++++--
 drivers/net/ethernet/amd/pds_core/core.h      |  3 ++-
 drivers/net/ethernet/amd/pds_core/devlink.c   |  3 ++-
 .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  6 ++++--
 .../net/ethernet/intel/ice/devlink/devlink.c  | 14 ++++++++++----
 .../marvell/octeontx2/af/rvu_devlink.c        | 15 ++++++++++-----
 .../marvell/octeontx2/nic/otx2_devlink.c      |  6 ++++--
 drivers/net/ethernet/mellanox/mlx4/main.c     |  6 ++++--
 .../net/ethernet/mellanox/mlx5/core/eswitch.c |  3 ++-
 .../mellanox/mlx5/core/eswitch_offloads.c     |  3 ++-
 .../net/ethernet/mellanox/mlx5/core/fs_core.c |  3 ++-
 .../ethernet/mellanox/mlx5/core/fw_reset.c    |  3 ++-
 .../mellanox/mlx5/core/lib/nv_param.c         |  9 ++++++---
 .../mellanox/mlxsw/spectrum_acl_tcam.c        |  3 ++-
 .../ethernet/netronome/nfp/devlink_param.c    |  3 ++-
 drivers/net/ethernet/qlogic/qed/qed_devlink.c |  3 ++-
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  3 ++-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  3 ++-
 drivers/net/ethernet/ti/cpsw_new.c            |  6 ++++--
 drivers/net/wwan/iosm/iosm_ipc_devlink.c      |  3 ++-
 include/net/devlink.h                         |  3 ++-
 include/net/dsa.h                             |  3 ++-
 net/devlink/param.c                           | 19 +++++++++++--------
 net/dsa/devlink.c                             |  3 ++-
 24 files changed, 87 insertions(+), 45 deletions(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
index 215a1a8ba7e9..07a74f702c3a 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
@@ -24,7 +24,8 @@ static int otx2_cpt_dl_egrp_delete(struct devlink *dl, u32 id,
 }
 
 static int otx2_cpt_dl_uc_info(struct devlink *dl, u32 id,
-			       struct devlink_param_gset_ctx *ctx)
+			       struct devlink_param_gset_ctx *ctx,
+			       struct netlink_ext_ack *extack)
 {
 	ctx->val.vstr[0] = '\0';
 
@@ -32,7 +33,8 @@ static int otx2_cpt_dl_uc_info(struct devlink *dl, u32 id,
 }
 
 static int otx2_cpt_dl_t106_mode_get(struct devlink *dl, u32 id,
-				     struct devlink_param_gset_ctx *ctx)
+				     struct devlink_param_gset_ctx *ctx,
+				     struct netlink_ext_ack *extack)
 {
 	struct otx2_cpt_devlink *cpt_dl = devlink_priv(dl);
 	struct otx2_cptpf_dev *cptpf = cpt_dl->cptpf;
diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
index 0b53a1fab46d..4a6b35c84dab 100644
--- a/drivers/net/ethernet/amd/pds_core/core.h
+++ b/drivers/net/ethernet/amd/pds_core/core.h
@@ -255,7 +255,8 @@ int pdsc_dl_flash_update(struct devlink *dl,
 			 struct devlink_flash_update_params *params,
 			 struct netlink_ext_ack *extack);
 int pdsc_dl_enable_get(struct devlink *dl, u32 id,
-		       struct devlink_param_gset_ctx *ctx);
+		       struct devlink_param_gset_ctx *ctx,
+		       struct netlink_ext_ack *extack);
 int pdsc_dl_enable_set(struct devlink *dl, u32 id,
 		       struct devlink_param_gset_ctx *ctx,
 		       struct netlink_ext_ack *extack);
diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
index d8dc39da4161..b576be626a29 100644
--- a/drivers/net/ethernet/amd/pds_core/devlink.c
+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
@@ -22,7 +22,8 @@ pdsc_viftype *pdsc_dl_find_viftype_by_id(struct pdsc *pdsc,
 }
 
 int pdsc_dl_enable_get(struct devlink *dl, u32 id,
-		       struct devlink_param_gset_ctx *ctx)
+		       struct devlink_param_gset_ctx *ctx,
+		       struct netlink_ext_ack *extack)
 {
 	struct pdsc *pdsc = devlink_priv(dl);
 	struct pdsc_viftype *vt_entry;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 67ca02d84c97..15de802bbac4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -1086,7 +1086,8 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
 }
 
 static int bnxt_dl_nvm_param_get(struct devlink *dl, u32 id,
-				 struct devlink_param_gset_ctx *ctx)
+				 struct devlink_param_gset_ctx *ctx,
+				 struct netlink_ext_ack *extack)
 {
 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
 	struct hwrm_nvm_get_variable_input *req;
@@ -1168,7 +1169,8 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
 }
 
 static int bnxt_remote_dev_reset_get(struct devlink *dl, u32 id,
-				     struct devlink_param_gset_ctx *ctx)
+				     struct devlink_param_gset_ctx *ctx,
+				     struct netlink_ext_ack *extack)
 {
 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
 
diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
index 938914abbe06..d88b7f3fd1f9 100644
--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
@@ -610,11 +610,13 @@ static int ice_update_tx_topo_user_sel(struct ice_pf *pf, int layers)
  * @devlink: pointer to the devlink instance
  * @id: the parameter ID to set
  * @ctx: context to store the parameter value
+ * @extack: netlink extended ACK structure
  *
  * Return: zero on success and negative value on failure.
  */
 static int ice_devlink_tx_sched_layers_get(struct devlink *devlink, u32 id,
-					   struct devlink_param_gset_ctx *ctx)
+					   struct devlink_param_gset_ctx *ctx,
+					   struct netlink_ext_ack *extack)
 {
 	struct ice_pf *pf = devlink_priv(devlink);
 	int err;
@@ -1349,7 +1351,8 @@ static const struct devlink_ops ice_sf_devlink_ops;
 
 static int
 ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
+			    struct devlink_param_gset_ctx *ctx,
+			    struct netlink_ext_ack *extack)
 {
 	struct ice_pf *pf = devlink_priv(devlink);
 	struct iidc_rdma_core_dev_info *cdev;
@@ -1415,7 +1418,8 @@ ice_devlink_enable_roce_validate(struct devlink *devlink, u32 id,
 
 static int
 ice_devlink_enable_iw_get(struct devlink *devlink, u32 id,
-			  struct devlink_param_gset_ctx *ctx)
+			  struct devlink_param_gset_ctx *ctx,
+			  struct netlink_ext_ack *extack)
 {
 	struct ice_pf *pf = devlink_priv(devlink);
 	struct iidc_rdma_core_dev_info *cdev;
@@ -1522,11 +1526,13 @@ static int ice_devlink_local_fwd_str_to_mode(const char *mode_str)
  * @devlink: Pointer to the devlink instance.
  * @id: The parameter ID to set.
  * @ctx: Context to store the parameter value.
+ * @extack: netlink extended ACK structure
  *
  * Return: Zero.
  */
 static int ice_devlink_local_fwd_get(struct devlink *devlink, u32 id,
-				     struct devlink_param_gset_ctx *ctx)
+				     struct devlink_param_gset_ctx *ctx,
+				     struct netlink_ext_ack *extack)
 {
 	struct ice_pf *pf = devlink_priv(devlink);
 	struct ice_port_info *pi;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
index 3735372539bd..0f9953eaf1b0 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
@@ -1233,7 +1233,8 @@ static int rvu_af_dl_dwrr_mtu_set(struct devlink *devlink, u32 id,
 }
 
 static int rvu_af_dl_dwrr_mtu_get(struct devlink *devlink, u32 id,
-				  struct devlink_param_gset_ctx *ctx)
+				  struct devlink_param_gset_ctx *ctx,
+				  struct netlink_ext_ack *extack)
 {
 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
 	struct rvu *rvu = rvu_dl->rvu;
@@ -1259,7 +1260,8 @@ enum rvu_af_dl_param_id {
 };
 
 static int rvu_af_npc_exact_feature_get(struct devlink *devlink, u32 id,
-					struct devlink_param_gset_ctx *ctx)
+					struct devlink_param_gset_ctx *ctx,
+					struct netlink_ext_ack *extack)
 {
 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
 	struct rvu *rvu = rvu_dl->rvu;
@@ -1314,7 +1316,8 @@ static int rvu_af_npc_exact_feature_validate(struct devlink *devlink, u32 id,
 }
 
 static int rvu_af_dl_npc_mcam_high_zone_percent_get(struct devlink *devlink, u32 id,
-						    struct devlink_param_gset_ctx *ctx)
+						    struct devlink_param_gset_ctx *ctx,
+						    struct netlink_ext_ack *extack)
 {
 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
 	struct rvu *rvu = rvu_dl->rvu;
@@ -1376,7 +1379,8 @@ static int rvu_af_dl_npc_mcam_high_zone_percent_validate(struct devlink *devlink
 }
 
 static int rvu_af_dl_npc_def_rule_cntr_get(struct devlink *devlink, u32 id,
-					   struct devlink_param_gset_ctx *ctx)
+					   struct devlink_param_gset_ctx *ctx,
+					   struct netlink_ext_ack *extack)
 {
 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
 	struct rvu *rvu = rvu_dl->rvu;
@@ -1402,7 +1406,8 @@ static int rvu_af_dl_npc_def_rule_cntr_set(struct devlink *devlink, u32 id,
 }
 
 static int rvu_af_dl_nix_maxlf_get(struct devlink *devlink, u32 id,
-				   struct devlink_param_gset_ctx *ctx)
+				   struct devlink_param_gset_ctx *ctx,
+				   struct netlink_ext_ack *extack)
 {
 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
 	struct rvu *rvu = rvu_dl->rvu;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
index e13ae5484c19..a72694219df4 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
@@ -48,7 +48,8 @@ static int otx2_dl_mcam_count_set(struct devlink *devlink, u32 id,
 }
 
 static int otx2_dl_mcam_count_get(struct devlink *devlink, u32 id,
-				  struct devlink_param_gset_ctx *ctx)
+				  struct devlink_param_gset_ctx *ctx,
+				  struct netlink_ext_ack *extack)
 {
 	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
 	struct otx2_nic *pfvf = otx2_dl->pfvf;
@@ -84,7 +85,8 @@ static int otx2_dl_ucast_flt_cnt_set(struct devlink *devlink, u32 id,
 }
 
 static int otx2_dl_ucast_flt_cnt_get(struct devlink *devlink, u32 id,
-				     struct devlink_param_gset_ctx *ctx)
+				     struct devlink_param_gset_ctx *ctx,
+				     struct netlink_ext_ack *extack)
 {
 	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
 	struct otx2_nic *pfvf = otx2_dl->pfvf;
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 03d2fc7d9b09..2de226951e19 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -174,7 +174,8 @@ MODULE_PARM_DESC(port_type_array, "Array of port types: HW_DEFAULT (0) is defaul
 static atomic_t pf_loading = ATOMIC_INIT(0);
 
 static int mlx4_devlink_ierr_reset_get(struct devlink *devlink, u32 id,
-				       struct devlink_param_gset_ctx *ctx)
+				       struct devlink_param_gset_ctx *ctx,
+				       struct netlink_ext_ack *extack)
 {
 	ctx->val.vbool = !!mlx4_internal_err_reset;
 	return 0;
@@ -189,7 +190,8 @@ static int mlx4_devlink_ierr_reset_set(struct devlink *devlink, u32 id,
 }
 
 static int mlx4_devlink_crdump_snapshot_get(struct devlink *devlink, u32 id,
-					    struct devlink_param_gset_ctx *ctx)
+					    struct devlink_param_gset_ctx *ctx,
+					    struct netlink_ext_ack *extack)
 {
 	struct mlx4_priv *priv = devlink_priv(devlink);
 	struct mlx4_dev *dev = &priv->dev;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index e2ffb87b94cb..308429175bb2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1978,7 +1978,8 @@ static int mlx5_devlink_esw_multiport_set(struct devlink *devlink, u32 id,
 }
 
 static int mlx5_devlink_esw_multiport_get(struct devlink *devlink, u32 id,
-					  struct devlink_param_gset_ctx *ctx)
+					  struct devlink_param_gset_ctx *ctx,
+					  struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 4092ea29c630..a4dd1239514f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -2492,7 +2492,8 @@ static int esw_port_metadata_set(struct devlink *devlink, u32 id,
 }
 
 static int esw_port_metadata_get(struct devlink *devlink, u32 id,
-				 struct devlink_param_gset_ctx *ctx)
+				 struct devlink_param_gset_ctx *ctx,
+				 struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index 2db3ffb0a2b2..88dc2023fca5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -3774,7 +3774,8 @@ static int mlx5_fs_mode_set(struct devlink *devlink, u32 id,
 }
 
 static int mlx5_fs_mode_get(struct devlink *devlink, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
+			    struct devlink_param_gset_ctx *ctx,
+			    struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index 89e399606877..2bceb42c98cc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -73,7 +73,8 @@ static int mlx5_fw_reset_enable_remote_dev_reset_set(struct devlink *devlink, u3
 }
 
 static int mlx5_fw_reset_enable_remote_dev_reset_get(struct devlink *devlink, u32 id,
-						     struct devlink_param_gset_ctx *ctx)
+						     struct devlink_param_gset_ctx *ctx,
+						     struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 	struct mlx5_fw_reset *fw_reset;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
index f0ab5ef95fc2..3d2195338d39 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
@@ -200,7 +200,8 @@ static const char *const
 
 static int
 mlx5_nv_param_devlink_cqe_compress_get(struct devlink *devlink, u32 id,
-				       struct devlink_param_gset_ctx *ctx)
+				       struct devlink_param_gset_ctx *ctx,
+				       struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
@@ -302,7 +303,8 @@ static int mlx5_nv_param_read_per_host_pf_conf(struct mlx5_core_dev *dev,
 }
 
 static int mlx5_devlink_enable_sriov_get(struct devlink *devlink, u32 id,
-					 struct devlink_param_gset_ctx *ctx)
+					 struct devlink_param_gset_ctx *ctx,
+					 struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
@@ -413,7 +415,8 @@ static int mlx5_devlink_enable_sriov_set(struct devlink *devlink, u32 id,
 }
 
 static int mlx5_devlink_total_vfs_get(struct devlink *devlink, u32 id,
-				      struct devlink_param_gset_ctx *ctx)
+				      struct devlink_param_gset_ctx *ctx,
+				      struct netlink_ext_ack *extack)
 {
 	struct mlx5_core_dev *dev = devlink_priv(devlink);
 	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
index b1d08e958bf9..69f9da9fb305 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
@@ -1489,7 +1489,8 @@ mlxsw_sp_acl_tcam_vregion_rehash(struct mlxsw_sp *mlxsw_sp,
 
 static int
 mlxsw_sp_acl_tcam_region_rehash_intrvl_get(struct devlink *devlink, u32 id,
-					   struct devlink_param_gset_ctx *ctx)
+					   struct devlink_param_gset_ctx *ctx,
+					   struct netlink_ext_ack *extack)
 {
 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
 	struct mlxsw_sp_acl_tcam *tcam;
diff --git a/drivers/net/ethernet/netronome/nfp/devlink_param.c b/drivers/net/ethernet/netronome/nfp/devlink_param.c
index 0e1a3800f371..85e3b19e6165 100644
--- a/drivers/net/ethernet/netronome/nfp/devlink_param.c
+++ b/drivers/net/ethernet/netronome/nfp/devlink_param.c
@@ -81,7 +81,8 @@ static const struct nfp_devlink_param_u8_arg nfp_devlink_u8_args[] = {
 
 static int
 nfp_devlink_param_u8_get(struct devlink *devlink, u32 id,
-			 struct devlink_param_gset_ctx *ctx)
+			 struct devlink_param_gset_ctx *ctx,
+			 struct netlink_ext_ack *extack)
 {
 	const struct nfp_devlink_param_u8_arg *arg;
 	struct nfp_pf *pf = devlink_priv(devlink);
diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
index 94c5689b5abd..0c5278c0598c 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
@@ -121,7 +121,8 @@ void qed_fw_reporters_destroy(struct devlink *devlink)
 }
 
 static int qed_dl_param_get(struct devlink *dl, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
+			    struct devlink_param_gset_ctx *ctx,
+			    struct netlink_ext_ack *extack)
 {
 	struct qed_devlink *qed_dl = devlink_priv(dl);
 	struct qed_dev *cdev;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index ccf383b355e7..e46d443b9da1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7488,7 +7488,8 @@ static int stmmac_dl_ts_coarse_set(struct devlink *dl, u32 id,
 }
 
 static int stmmac_dl_ts_coarse_get(struct devlink *dl, u32 id,
-				   struct devlink_param_gset_ctx *ctx)
+				   struct devlink_param_gset_ctx *ctx,
+				   struct netlink_ext_ack *extack)
 {
 	struct stmmac_devlink_priv *dl_priv = devlink_priv(dl);
 	struct stmmac_priv *priv = dl_priv->stmmac_priv;
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index d5f358ec9820..5924db6be3fe 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -3068,7 +3068,8 @@ static void am65_cpsw_init_host_port_emac(struct am65_cpsw_common *common)
 }
 
 static int am65_cpsw_dl_switch_mode_get(struct devlink *dl, u32 id,
-					struct devlink_param_gset_ctx *ctx)
+					struct devlink_param_gset_ctx *ctx,
+					struct netlink_ext_ack *extack)
 {
 	struct am65_cpsw_devlink *dl_priv = devlink_priv(dl);
 	struct am65_cpsw_common *common = dl_priv->common;
diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
index 8b9e2078c602..ab88d4c02cbd 100644
--- a/drivers/net/ethernet/ti/cpsw_new.c
+++ b/drivers/net/ethernet/ti/cpsw_new.c
@@ -1618,7 +1618,8 @@ static const struct devlink_ops cpsw_devlink_ops = {
 };
 
 static int cpsw_dl_switch_mode_get(struct devlink *dl, u32 id,
-				   struct devlink_param_gset_ctx *ctx)
+				   struct devlink_param_gset_ctx *ctx,
+				   struct netlink_ext_ack *extack)
 {
 	struct cpsw_devlink *dl_priv = devlink_priv(dl);
 	struct cpsw_common *cpsw = dl_priv->cpsw;
@@ -1753,7 +1754,8 @@ static int cpsw_dl_switch_mode_set(struct devlink *dl, u32 id,
 }
 
 static int cpsw_dl_ale_ctrl_get(struct devlink *dl, u32 id,
-				struct devlink_param_gset_ctx *ctx)
+				struct devlink_param_gset_ctx *ctx,
+				struct netlink_ext_ack *extack)
 {
 	struct cpsw_devlink *dl_priv = devlink_priv(dl);
 	struct cpsw_common *cpsw = dl_priv->cpsw;
diff --git a/drivers/net/wwan/iosm/iosm_ipc_devlink.c b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
index 33d6342124bc..301a9d294d30 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_devlink.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
@@ -21,7 +21,8 @@ static struct iosm_coredump_file_info list[IOSM_NOF_CD_REGION] = {
 
 /* Get the param values for the specific param ID's */
 static int ipc_devlink_get_param(struct devlink *dl, u32 id,
-				 struct devlink_param_gset_ctx *ctx)
+				 struct devlink_param_gset_ctx *ctx,
+				 struct netlink_ext_ack *extack)
 {
 	struct iosm_devlink *ipc_devlink = devlink_priv(dl);
 
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 9e824f61e40f..6d942597d07d 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -490,7 +490,8 @@ struct devlink_param {
 	enum devlink_param_type type;
 	unsigned long supported_cmodes;
 	int (*get)(struct devlink *devlink, u32 id,
-		   struct devlink_param_gset_ctx *ctx);
+		   struct devlink_param_gset_ctx *ctx,
+		   struct netlink_ext_ack *extack);
 	int (*set)(struct devlink *devlink, u32 id,
 		   struct devlink_param_gset_ctx *ctx,
 		   struct netlink_ext_ack *extack);
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 2df2e2ead9a8..85c1b938f2c4 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -1251,7 +1251,8 @@ struct dsa_switch_ops {
 			     dsa_devlink_param_get, dsa_devlink_param_set, NULL)
 
 int dsa_devlink_param_get(struct devlink *dl, u32 id,
-			  struct devlink_param_gset_ctx *ctx);
+			  struct devlink_param_gset_ctx *ctx,
+			  struct netlink_ext_ack *extack);
 int dsa_devlink_param_set(struct devlink *dl, u32 id,
 			  struct devlink_param_gset_ctx *ctx,
 			  struct netlink_ext_ack *extack);
diff --git a/net/devlink/param.c b/net/devlink/param.c
index 70e69523412c..8bae3f733bde 100644
--- a/net/devlink/param.c
+++ b/net/devlink/param.c
@@ -169,11 +169,12 @@ devlink_param_cmode_is_supported(const struct devlink_param *param,
 
 static int devlink_param_get(struct devlink *devlink,
 			     const struct devlink_param *param,
-			     struct devlink_param_gset_ctx *ctx)
+			     struct devlink_param_gset_ctx *ctx,
+			     struct netlink_ext_ack *extack)
 {
 	if (!param->get)
 		return -EOPNOTSUPP;
-	return param->get(devlink, param->id, ctx);
+	return param->get(devlink, param->id, ctx, extack);
 }
 
 static int devlink_param_set(struct devlink *devlink,
@@ -245,7 +246,8 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 				 unsigned int port_index,
 				 struct devlink_param_item *param_item,
 				 enum devlink_command cmd,
-				 u32 portid, u32 seq, int flags)
+				 u32 portid, u32 seq, int flags,
+				 struct netlink_ext_ack *extack)
 {
 	union devlink_param_value param_value[DEVLINK_PARAM_CMODE_MAX + 1];
 	bool param_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
@@ -270,7 +272,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 				return -EOPNOTSUPP;
 		} else {
 			ctx.cmode = i;
-			err = devlink_param_get(devlink, param, &ctx);
+			err = devlink_param_get(devlink, param, &ctx, extack);
 			if (err)
 				return err;
 			param_value[i] = ctx.val;
@@ -352,7 +354,7 @@ static void devlink_param_notify(struct devlink *devlink,
 	if (!msg)
 		return;
 	err = devlink_nl_param_fill(msg, devlink, port_index, param_item, cmd,
-				    0, 0, 0);
+				    0, 0, 0, NULL);
 	if (err) {
 		nlmsg_free(msg);
 		return;
@@ -395,7 +397,8 @@ static int devlink_nl_param_get_dump_one(struct sk_buff *msg,
 		err = devlink_nl_param_fill(msg, devlink, 0, param_item,
 					    DEVLINK_CMD_PARAM_GET,
 					    NETLINK_CB(cb->skb).portid,
-					    cb->nlh->nlmsg_seq, flags);
+					    cb->nlh->nlmsg_seq, flags,
+					    cb->extack);
 		if (err == -EOPNOTSUPP) {
 			err = 0;
 		} else if (err) {
@@ -504,8 +507,8 @@ int devlink_nl_param_get_doit(struct sk_buff *skb,
 		return -ENOMEM;
 
 	err = devlink_nl_param_fill(msg, devlink, 0, param_item,
-				    DEVLINK_CMD_PARAM_GET,
-				    info->snd_portid, info->snd_seq, 0);
+				    DEVLINK_CMD_PARAM_GET, info->snd_portid,
+				    info->snd_seq, 0, info->extack);
 	if (err) {
 		nlmsg_free(msg);
 		return err;
diff --git a/net/dsa/devlink.c b/net/dsa/devlink.c
index f41f9fc2194e..ed342f345692 100644
--- a/net/dsa/devlink.c
+++ b/net/dsa/devlink.c
@@ -182,7 +182,8 @@ static const struct devlink_ops dsa_devlink_ops = {
 };
 
 int dsa_devlink_param_get(struct devlink *dl, u32 id,
-			  struct devlink_param_gset_ctx *ctx)
+			  struct devlink_param_gset_ctx *ctx,
+			  struct netlink_ext_ack *extack)
 {
 	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
 
-- 
2.47.3

[PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Daniel Zahka]

swp_l4_csum_mode controls how L4 transmit checksums are computed when
using Software Parser (SWP) hints for header locations.

Supported values:
  1. device_default: use device default setting.
  2. full_csum: calculate L4 checksum with the pseudo-header.
  3. l4_only: calculate L4 checksum without the pseudo-header. Only
     available when swp_l4_csum_mode_l4_only is set in
     mlx5_ifc_nv_sw_offload_cap_bits.

The l4_only setting is a dependency for PSP initialization in
mlx5e_psp_init().

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---

Notes:
    v2:
    - use extack in mlx5_nv_param_devlink_swp_l4_csum_mode_get()
    - fix indentation issue in mlx5.rst entry

 Documentation/networking/devlink/mlx5.rst     |   9 +
 .../net/ethernet/mellanox/mlx5/core/devlink.h |   3 +-
 .../mellanox/mlx5/core/lib/nv_param.c         | 161 ++++++++++++++++++
 3 files changed, 172 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/devlink/mlx5.rst b/Documentation/networking/devlink/mlx5.rst
index 0e5f9c76e514..675b5a1ec625 100644
--- a/Documentation/networking/devlink/mlx5.rst
+++ b/Documentation/networking/devlink/mlx5.rst
@@ -218,6 +218,15 @@ parameters.
        * ``balanced`` : Merges fewer CQEs, resulting in a moderate compression ratio but maintaining a balance between bandwidth savings and performance
        * ``aggressive`` : Merges more CQEs into a single entry, achieving a higher compression rate and maximizing performance, particularly under high traffic loads
 
+   * - ``swp_l4_csum_mode``
+     - string
+     - permanent
+     - Configure how the L4 checksum is calculated by the device when using
+       Software Parser (SWP) hints for header locations.
+       * ``device_default`` : Use the device's default checksum calculation mode
+       * ``full_csum`` : Calculate full checksum including the pseudo-header
+       * ``l4_only`` : Calculate L4-only checksum, excluding the pseudo-header
+
 The ``mlx5`` driver supports reloading via ``DEVLINK_CMD_RELOAD``
 
 Info versions
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/devlink.h
index c9555119a661..43b9bf8829cf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.h
@@ -26,7 +26,8 @@ enum mlx5_devlink_param_id {
 	MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_HIGH,
 	MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_LOW,
 	MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH,
-	MLX5_DEVLINK_PARAM_ID_CQE_COMPRESSION_TYPE
+	MLX5_DEVLINK_PARAM_ID_CQE_COMPRESSION_TYPE,
+	MLX5_DEVLINK_PARAM_ID_SWP_L4_CSUM_MODE,
 };
 
 struct mlx5_trap_ctx {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
index 3d2195338d39..3dc5b899a5fb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
@@ -8,6 +8,8 @@ enum {
 	MLX5_CLASS_0_CTRL_ID_NV_GLOBAL_PCI_CONF               = 0x80,
 	MLX5_CLASS_0_CTRL_ID_NV_GLOBAL_PCI_CAP                = 0x81,
 	MLX5_CLASS_0_CTRL_ID_NV_SW_OFFLOAD_CONFIG             = 0x10a,
+	MLX5_CLASS_0_CTRL_ID_NV_SW_OFFLOAD_CAP                = 0x10b,
+	MLX5_CLASS_0_CTRL_ID_NV_SW_ACCELERATE_CONF            = 0x11d,
 
 	MLX5_CLASS_3_CTRL_ID_NV_PF_PCI_CONF                   = 0x80,
 };
@@ -123,6 +125,17 @@ struct mlx5_ifc_nv_sw_offload_conf_bits {
 	u8         lro_log_timeout0[0x4];
 };
 
+struct mlx5_ifc_nv_sw_offload_cap_bits {
+	u8         reserved_at_0[0x19];
+	u8         swp_l4_csum_mode_l4_only[0x1];
+	u8         reserved_at_1a[0x6];
+};
+
+struct mlx5_ifc_nv_sw_accelerate_conf_bits {
+	u8         swp_l4_csum_mode[0x2];
+	u8         reserved_at_2[0x3e];
+};
+
 #define MNVDA_HDR_SZ \
 	(MLX5_ST_SZ_BYTES(mnvda_reg) - \
 	 MLX5_BYTE_OFF(mnvda_reg, configuration_item_data))
@@ -195,6 +208,30 @@ mlx5_nv_param_read_sw_offload_conf(struct mlx5_core_dev *dev, void *mnvda,
 	return mlx5_nv_param_read(dev, mnvda, len);
 }
 
+static int
+mlx5_nv_param_read_sw_offload_cap(struct mlx5_core_dev *dev, void *mnvda,
+				  size_t len)
+{
+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, type_class, 0);
+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, parameter_index,
+			       MLX5_CLASS_0_CTRL_ID_NV_SW_OFFLOAD_CAP);
+	MLX5_SET_CFG_HDR_LEN(mnvda, nv_sw_offload_cap);
+
+	return mlx5_nv_param_read(dev, mnvda, len);
+}
+
+static int
+mlx5_nv_param_read_sw_accelerate_conf(struct mlx5_core_dev *dev, void *mnvda,
+				      size_t len)
+{
+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, type_class, 0);
+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, parameter_index,
+			       MLX5_CLASS_0_CTRL_ID_NV_SW_ACCELERATE_CONF);
+	MLX5_SET_CFG_HDR_LEN(mnvda, nv_sw_accelerate_conf);
+
+	return mlx5_nv_param_read(dev, mnvda, len);
+}
+
 static const char *const
 	cqe_compress_str[] = { "balanced", "aggressive" };
 
@@ -269,6 +306,124 @@ mlx5_nv_param_devlink_cqe_compress_set(struct devlink *devlink, u32 id,
 	return mlx5_nv_param_write(dev, mnvda, sizeof(mnvda));
 }
 
+enum swp_l4_csum_mode {
+	SWP_L4_CSUM_MODE_DEVICE_DEFAULT = 0,
+	SWP_L4_CSUM_MODE_FULL_CSUM = 1,
+	SWP_L4_CSUM_MODE_L4_ONLY = 2,
+};
+
+static const char *const
+	swp_l4_csum_mode_str[] = { "device_default", "full_csum", "l4_only" };
+
+static int
+mlx5_nv_param_devlink_swp_l4_csum_mode_get(struct devlink *devlink, u32 id,
+					   struct devlink_param_gset_ctx *ctx,
+					   struct netlink_ext_ack *extack)
+{
+	struct mlx5_core_dev *dev = devlink_priv(devlink);
+	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
+	u8 value = U8_MAX;
+	void *data;
+	int err;
+
+	err = mlx5_nv_param_read_sw_accelerate_conf(dev, mnvda, sizeof(mnvda));
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Failed to read sw_accelerate_conf mnvda reg");
+		return err;
+	}
+
+	data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
+	value = MLX5_GET(nv_sw_accelerate_conf, data, swp_l4_csum_mode);
+
+	if (value >= ARRAY_SIZE(swp_l4_csum_mode_str)) {
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Invalid swp_l4_csum_mode value %u read from device",
+				       value);
+		return -EINVAL;
+	}
+
+	strscpy(ctx->val.vstr, swp_l4_csum_mode_str[value],
+		sizeof(ctx->val.vstr));
+	return 0;
+}
+
+static int
+mlx5_nv_param_devlink_swp_l4_csum_mode_validate(struct devlink *devlink, u32 id,
+						union devlink_param_value val,
+						struct netlink_ext_ack *extack)
+{
+	struct mlx5_core_dev *dev = devlink_priv(devlink);
+	u32 cap[MLX5_ST_SZ_DW(mnvda_reg)] = {};
+	void *data;
+	int err, i;
+
+	for (i = 0; i < ARRAY_SIZE(swp_l4_csum_mode_str); i++) {
+		if (!strcmp(val.vstr, swp_l4_csum_mode_str[i]))
+			break;
+	}
+
+	if (i >= ARRAY_SIZE(swp_l4_csum_mode_str)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Invalid value, supported values are device_default/full_csum/l4_only");
+		return -EINVAL;
+	}
+
+	if (i == SWP_L4_CSUM_MODE_L4_ONLY) {
+		err = mlx5_nv_param_read_sw_offload_cap(dev, cap, sizeof(cap));
+		if (err) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Failed to read sw_offload_cap");
+			return err;
+		}
+
+		data = MLX5_ADDR_OF(mnvda_reg, cap, configuration_item_data);
+		if (!MLX5_GET(nv_sw_offload_cap, data, swp_l4_csum_mode_l4_only)) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "l4_only mode is not supported on this device");
+			return -EOPNOTSUPP;
+		}
+	}
+
+	return 0;
+}
+
+static int
+mlx5_nv_param_devlink_swp_l4_csum_mode_set(struct devlink *devlink, u32 id,
+					   struct devlink_param_gset_ctx *ctx,
+					   struct netlink_ext_ack *extack)
+{
+	struct mlx5_core_dev *dev = devlink_priv(devlink);
+	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
+	void *data;
+	u8 value;
+	int err;
+
+	if (!strcmp(ctx->val.vstr, "device_default"))
+		value = SWP_L4_CSUM_MODE_DEVICE_DEFAULT;
+	else if (!strcmp(ctx->val.vstr, "full_csum"))
+		value = SWP_L4_CSUM_MODE_FULL_CSUM;
+	else
+		value = SWP_L4_CSUM_MODE_L4_ONLY;
+
+	err = mlx5_nv_param_read_sw_accelerate_conf(dev, mnvda, sizeof(mnvda));
+	if (err) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Failed to read sw_accelerate_conf mnvda reg");
+		return err;
+	}
+
+	data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
+	MLX5_SET(nv_sw_accelerate_conf, data, swp_l4_csum_mode, value);
+
+	err = mlx5_nv_param_write(dev, mnvda, sizeof(mnvda));
+	if (err)
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Failed to write sw_accelerate_conf mnvda reg");
+
+	return err;
+}
+
 static int mlx5_nv_param_read_global_pci_conf(struct mlx5_core_dev *dev,
 					      void *mnvda, size_t len)
 {
@@ -548,6 +703,12 @@ static const struct devlink_param mlx5_nv_param_devlink_params[] = {
 			     mlx5_nv_param_devlink_cqe_compress_get,
 			     mlx5_nv_param_devlink_cqe_compress_set,
 			     mlx5_nv_param_devlink_cqe_compress_validate),
+	DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_SWP_L4_CSUM_MODE,
+			     "swp_l4_csum_mode", DEVLINK_PARAM_TYPE_STRING,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     mlx5_nv_param_devlink_swp_l4_csum_mode_get,
+			     mlx5_nv_param_devlink_swp_l4_csum_mode_set,
+			     mlx5_nv_param_devlink_swp_l4_csum_mode_validate),
 };
 
 int mlx5_nv_param_register_dl_params(struct devlink *devlink)
-- 
2.47.3

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Saeed Mahameed]

On 07 Nov 12:43, Daniel Zahka wrote:
>swp_l4_csum_mode controls how L4 transmit checksums are computed when
>using Software Parser (SWP) hints for header locations.
>
>Supported values:
>  1. device_default: use device default setting.
>  2. full_csum: calculate L4 checksum with the pseudo-header.
>  3. l4_only: calculate L4 checksum without the pseudo-header. Only
>     available when swp_l4_csum_mode_l4_only is set in
>     mlx5_ifc_nv_sw_offload_cap_bits.
>
>The l4_only setting is a dependency for PSP initialization in
>mlx5e_psp_init().
>
>Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
>---
>
>Notes:
>    v2:
>    - use extack in mlx5_nv_param_devlink_swp_l4_csum_mode_get()
>    - fix indentation issue in mlx5.rst entry
>
> Documentation/networking/devlink/mlx5.rst     |   9 +
> .../net/ethernet/mellanox/mlx5/core/devlink.h |   3 +-
> .../mellanox/mlx5/core/lib/nv_param.c         | 161 ++++++++++++++++++
> 3 files changed, 172 insertions(+), 1 deletion(-)
>
>diff --git a/Documentation/networking/devlink/mlx5.rst b/Documentation/networking/devlink/mlx5.rst
>index 0e5f9c76e514..675b5a1ec625 100644
>--- a/Documentation/networking/devlink/mlx5.rst
>+++ b/Documentation/networking/devlink/mlx5.rst
>@@ -218,6 +218,15 @@ parameters.
>        * ``balanced`` : Merges fewer CQEs, resulting in a moderate compression ratio but maintaining a balance between bandwidth savings and performance
>        * ``aggressive`` : Merges more CQEs into a single entry, achieving a higher compression rate and maximizing performance, particularly under high traffic loads
>
>+   * - ``swp_l4_csum_mode``
>+     - string
>+     - permanent
>+     - Configure how the L4 checksum is calculated by the device when using
>+       Software Parser (SWP) hints for header locations.
>+       * ``device_default`` : Use the device's default checksum calculation mode

Let's omit the device, just "default". 

So, I checked a couple of flows internally, and it seems this allows
some flexibility in the FW to decide later on which mode to pick,
based on other parameters, which practically means
"user has no preference on this param". Driver can only find out
after boot, when it reads the runtime capabilities, but still
this is a bug, by the time the driver reads this (in devlink), the
default value should've already been determined by FW, so FW must
return the actual runtime value. Which can only be one of the following

>+       * ``full_csum`` : Calculate full checksum including the pseudo-header
>+       * ``l4_only`` : Calculate L4-only checksum, excluding the pseudo-header
>+
> The ``mlx5`` driver supports reloading via ``DEVLINK_CMD_RELOAD``
>
> Info versions
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/devlink.h
>index c9555119a661..43b9bf8829cf 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.h
>@@ -26,7 +26,8 @@ enum mlx5_devlink_param_id {
> 	MLX5_DEVLINK_PARAM_ID_PCIE_CONG_IN_HIGH,
> 	MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_LOW,
> 	MLX5_DEVLINK_PARAM_ID_PCIE_CONG_OUT_HIGH,
>-	MLX5_DEVLINK_PARAM_ID_CQE_COMPRESSION_TYPE
>+	MLX5_DEVLINK_PARAM_ID_CQE_COMPRESSION_TYPE,
>+	MLX5_DEVLINK_PARAM_ID_SWP_L4_CSUM_MODE,
> };
>
> struct mlx5_trap_ctx {
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
>index 3d2195338d39..3dc5b899a5fb 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
>@@ -8,6 +8,8 @@ enum {
> 	MLX5_CLASS_0_CTRL_ID_NV_GLOBAL_PCI_CONF               = 0x80,
> 	MLX5_CLASS_0_CTRL_ID_NV_GLOBAL_PCI_CAP                = 0x81,
> 	MLX5_CLASS_0_CTRL_ID_NV_SW_OFFLOAD_CONFIG             = 0x10a,
>+	MLX5_CLASS_0_CTRL_ID_NV_SW_OFFLOAD_CAP                = 0x10b,
>+	MLX5_CLASS_0_CTRL_ID_NV_SW_ACCELERATE_CONF            = 0x11d,
>
> 	MLX5_CLASS_3_CTRL_ID_NV_PF_PCI_CONF                   = 0x80,
> };
>@@ -123,6 +125,17 @@ struct mlx5_ifc_nv_sw_offload_conf_bits {
> 	u8         lro_log_timeout0[0x4];
> };
>
>+struct mlx5_ifc_nv_sw_offload_cap_bits {
>+	u8         reserved_at_0[0x19];
>+	u8         swp_l4_csum_mode_l4_only[0x1];
>+	u8         reserved_at_1a[0x6];
>+};
>+
>+struct mlx5_ifc_nv_sw_accelerate_conf_bits {
>+	u8         swp_l4_csum_mode[0x2];
>+	u8         reserved_at_2[0x3e];
>+};
>+
> #define MNVDA_HDR_SZ \
> 	(MLX5_ST_SZ_BYTES(mnvda_reg) - \
> 	 MLX5_BYTE_OFF(mnvda_reg, configuration_item_data))
>@@ -195,6 +208,30 @@ mlx5_nv_param_read_sw_offload_conf(struct mlx5_core_dev *dev, void *mnvda,
> 	return mlx5_nv_param_read(dev, mnvda, len);
> }
>
>+static int
>+mlx5_nv_param_read_sw_offload_cap(struct mlx5_core_dev *dev, void *mnvda,
>+				  size_t len)
>+{
>+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, type_class, 0);
>+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, parameter_index,
>+			       MLX5_CLASS_0_CTRL_ID_NV_SW_OFFLOAD_CAP);
>+	MLX5_SET_CFG_HDR_LEN(mnvda, nv_sw_offload_cap);
>+
>+	return mlx5_nv_param_read(dev, mnvda, len);
>+}
>+
>+static int
>+mlx5_nv_param_read_sw_accelerate_conf(struct mlx5_core_dev *dev, void *mnvda,
>+				      size_t len)
>+{
>+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, type_class, 0);
>+	MLX5_SET_CFG_ITEM_TYPE(global, mnvda, parameter_index,
>+			       MLX5_CLASS_0_CTRL_ID_NV_SW_ACCELERATE_CONF);
>+	MLX5_SET_CFG_HDR_LEN(mnvda, nv_sw_accelerate_conf);
>+
>+	return mlx5_nv_param_read(dev, mnvda, len);
>+}
>+
> static const char *const
> 	cqe_compress_str[] = { "balanced", "aggressive" };
>
>@@ -269,6 +306,124 @@ mlx5_nv_param_devlink_cqe_compress_set(struct devlink *devlink, u32 id,
> 	return mlx5_nv_param_write(dev, mnvda, sizeof(mnvda));
> }
>
>+enum swp_l4_csum_mode {
>+	SWP_L4_CSUM_MODE_DEVICE_DEFAULT = 0,
>+	SWP_L4_CSUM_MODE_FULL_CSUM = 1,
>+	SWP_L4_CSUM_MODE_L4_ONLY = 2,
>+};
>+
>+static const char *const
>+	swp_l4_csum_mode_str[] = { "device_default", "full_csum", "l4_only" };
>+
>+static int
>+mlx5_nv_param_devlink_swp_l4_csum_mode_get(struct devlink *devlink, u32 id,
>+					   struct devlink_param_gset_ctx *ctx,
>+					   struct netlink_ext_ack *extack)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
>+	u8 value = U8_MAX;
>+	void *data;
>+	int err;
>+
>+	err = mlx5_nv_param_read_sw_accelerate_conf(dev, mnvda, sizeof(mnvda));
>+	if (err) {
>+		NL_SET_ERR_MSG_MOD(extack,
>+				   "Failed to read sw_accelerate_conf mnvda reg");

Plug in the err, NL_SET_ERR_MSG_FMT_MOD(.., .., err);
other locations as well.

LGTM over all. extack usage is fine, just add the err where applicable.

>+		return err;
>+	}
>+
>+	data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
>+	value = MLX5_GET(nv_sw_accelerate_conf, data, swp_l4_csum_mode);
>+
>+	if (value >= ARRAY_SIZE(swp_l4_csum_mode_str)) {
>+		NL_SET_ERR_MSG_FMT_MOD(extack,
>+				       "Invalid swp_l4_csum_mode value %u read from device",
>+				       value);
>+		return -EINVAL;
>+	}
>+
>+	strscpy(ctx->val.vstr, swp_l4_csum_mode_str[value],
>+		sizeof(ctx->val.vstr));
>+	return 0;
>+}
>+
>+static int
>+mlx5_nv_param_devlink_swp_l4_csum_mode_validate(struct devlink *devlink, u32 id,
>+						union devlink_param_value val,
>+						struct netlink_ext_ack *extack)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+	u32 cap[MLX5_ST_SZ_DW(mnvda_reg)] = {};
>+	void *data;
>+	int err, i;
>+
>+	for (i = 0; i < ARRAY_SIZE(swp_l4_csum_mode_str); i++) {
>+		if (!strcmp(val.vstr, swp_l4_csum_mode_str[i]))
>+			break;
>+	}
>+
>+	if (i >= ARRAY_SIZE(swp_l4_csum_mode_str)) {
>+		NL_SET_ERR_MSG_MOD(extack,
>+				   "Invalid value, supported values are device_default/full_csum/l4_only");
>+		return -EINVAL;
>+	}
>+
>+	if (i == SWP_L4_CSUM_MODE_L4_ONLY) {
>+		err = mlx5_nv_param_read_sw_offload_cap(dev, cap, sizeof(cap));
>+		if (err) {
>+			NL_SET_ERR_MSG_MOD(extack,
>+					   "Failed to read sw_offload_cap");
>+			return err;
>+		}
>+
>+		data = MLX5_ADDR_OF(mnvda_reg, cap, configuration_item_data);
>+		if (!MLX5_GET(nv_sw_offload_cap, data, swp_l4_csum_mode_l4_only)) {
>+			NL_SET_ERR_MSG_MOD(extack,
>+					   "l4_only mode is not supported on this device");
>+			return -EOPNOTSUPP;
>+		}
>+	}
>+
>+	return 0;
>+}
>+
>+static int
>+mlx5_nv_param_devlink_swp_l4_csum_mode_set(struct devlink *devlink, u32 id,
>+					   struct devlink_param_gset_ctx *ctx,
>+					   struct netlink_ext_ack *extack)
>+{
>+	struct mlx5_core_dev *dev = devlink_priv(devlink);
>+	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
>+	void *data;
>+	u8 value;
>+	int err;
>+
>+	if (!strcmp(ctx->val.vstr, "device_default"))
>+		value = SWP_L4_CSUM_MODE_DEVICE_DEFAULT;
>+	else if (!strcmp(ctx->val.vstr, "full_csum"))
>+		value = SWP_L4_CSUM_MODE_FULL_CSUM;
>+	else
>+		value = SWP_L4_CSUM_MODE_L4_ONLY;
>+
>+	err = mlx5_nv_param_read_sw_accelerate_conf(dev, mnvda, sizeof(mnvda));
>+	if (err) {
>+		NL_SET_ERR_MSG_MOD(extack,
>+				   "Failed to read sw_accelerate_conf mnvda reg");
>+		return err;
>+	}
>+
>+	data = MLX5_ADDR_OF(mnvda_reg, mnvda, configuration_item_data);
>+	MLX5_SET(nv_sw_accelerate_conf, data, swp_l4_csum_mode, value);
>+
>+	err = mlx5_nv_param_write(dev, mnvda, sizeof(mnvda));
>+	if (err)
>+		NL_SET_ERR_MSG_MOD(extack,
>+				   "Failed to write sw_accelerate_conf mnvda reg");
>+
>+	return err;
>+}
>+
> static int mlx5_nv_param_read_global_pci_conf(struct mlx5_core_dev *dev,
> 					      void *mnvda, size_t len)
> {
>@@ -548,6 +703,12 @@ static const struct devlink_param mlx5_nv_param_devlink_params[] = {
> 			     mlx5_nv_param_devlink_cqe_compress_get,
> 			     mlx5_nv_param_devlink_cqe_compress_set,
> 			     mlx5_nv_param_devlink_cqe_compress_validate),
>+	DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_SWP_L4_CSUM_MODE,
>+			     "swp_l4_csum_mode", DEVLINK_PARAM_TYPE_STRING,
>+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
>+			     mlx5_nv_param_devlink_swp_l4_csum_mode_get,
>+			     mlx5_nv_param_devlink_swp_l4_csum_mode_set,
>+			     mlx5_nv_param_devlink_swp_l4_csum_mode_validate),
> };
>
> int mlx5_nv_param_register_dl_params(struct devlink *devlink)
>-- 
>2.47.3
>
>

Re: [PATCH net-next v3 1/2] devlink: pass extack through to devlink_param::get()

[Saeed Mahameed]

On 07 Nov 12:43, Daniel Zahka wrote:
>Allow devlink_param::get() handlers to report error messages via
>extack. This function is called in a few different contexts, but not
>all of them will have an valid extack to use.
>
>When devlink_param::get() is called from param_get_doit or
>param_get_dumpit contexts, pass the extack through so that drivers can
>report errors when retrieving param values. devlink_param::get() is
>called from the context of devlink_param_notify(), pass NULL in for
>the extack.
>
>Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
>---
>
>Notes:
>    v3:
>    - fix warnings about undocumented param in intel ice driver
>
> .../marvell/octeontx2/otx2_cpt_devlink.c      |  6 ++++--
> drivers/net/ethernet/amd/pds_core/core.h      |  3 ++-
> drivers/net/ethernet/amd/pds_core/devlink.c   |  3 ++-
> .../net/ethernet/broadcom/bnxt/bnxt_devlink.c |  6 ++++--
> .../net/ethernet/intel/ice/devlink/devlink.c  | 14 ++++++++++----
> .../marvell/octeontx2/af/rvu_devlink.c        | 15 ++++++++++-----
> .../marvell/octeontx2/nic/otx2_devlink.c      |  6 ++++--
> drivers/net/ethernet/mellanox/mlx4/main.c     |  6 ++++--
> .../net/ethernet/mellanox/mlx5/core/eswitch.c |  3 ++-
> .../mellanox/mlx5/core/eswitch_offloads.c     |  3 ++-
> .../net/ethernet/mellanox/mlx5/core/fs_core.c |  3 ++-
> .../ethernet/mellanox/mlx5/core/fw_reset.c    |  3 ++-
> .../mellanox/mlx5/core/lib/nv_param.c         |  9 ++++++---
> .../mellanox/mlxsw/spectrum_acl_tcam.c        |  3 ++-
> .../ethernet/netronome/nfp/devlink_param.c    |  3 ++-
> drivers/net/ethernet/qlogic/qed/qed_devlink.c |  3 ++-
> .../net/ethernet/stmicro/stmmac/stmmac_main.c |  3 ++-
> drivers/net/ethernet/ti/am65-cpsw-nuss.c      |  3 ++-
> drivers/net/ethernet/ti/cpsw_new.c            |  6 ++++--
> drivers/net/wwan/iosm/iosm_ipc_devlink.c      |  3 ++-
> include/net/devlink.h                         |  3 ++-
> include/net/dsa.h                             |  3 ++-
> net/devlink/param.c                           | 19 +++++++++++--------
> net/dsa/devlink.c                             |  3 ++-
> 24 files changed, 87 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
>index 215a1a8ba7e9..07a74f702c3a 100644
>--- a/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
>+++ b/drivers/crypto/marvell/octeontx2/otx2_cpt_devlink.c
>@@ -24,7 +24,8 @@ static int otx2_cpt_dl_egrp_delete(struct devlink *dl, u32 id,
> }
>
> static int otx2_cpt_dl_uc_info(struct devlink *dl, u32 id,
>-			       struct devlink_param_gset_ctx *ctx)
>+			       struct devlink_param_gset_ctx *ctx,
>+			       struct netlink_ext_ack *extack)
> {
> 	ctx->val.vstr[0] = '\0';
>
>@@ -32,7 +33,8 @@ static int otx2_cpt_dl_uc_info(struct devlink *dl, u32 id,
> }
>
> static int otx2_cpt_dl_t106_mode_get(struct devlink *dl, u32 id,
>-				     struct devlink_param_gset_ctx *ctx)
>+				     struct devlink_param_gset_ctx *ctx,
>+				     struct netlink_ext_ack *extack)
> {
> 	struct otx2_cpt_devlink *cpt_dl = devlink_priv(dl);
> 	struct otx2_cptpf_dev *cptpf = cpt_dl->cptpf;
>diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
>index 0b53a1fab46d..4a6b35c84dab 100644
>--- a/drivers/net/ethernet/amd/pds_core/core.h
>+++ b/drivers/net/ethernet/amd/pds_core/core.h
>@@ -255,7 +255,8 @@ int pdsc_dl_flash_update(struct devlink *dl,
> 			 struct devlink_flash_update_params *params,
> 			 struct netlink_ext_ack *extack);
> int pdsc_dl_enable_get(struct devlink *dl, u32 id,
>-		       struct devlink_param_gset_ctx *ctx);
>+		       struct devlink_param_gset_ctx *ctx,
>+		       struct netlink_ext_ack *extack);
> int pdsc_dl_enable_set(struct devlink *dl, u32 id,
> 		       struct devlink_param_gset_ctx *ctx,
> 		       struct netlink_ext_ack *extack);
>diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
>index d8dc39da4161..b576be626a29 100644
>--- a/drivers/net/ethernet/amd/pds_core/devlink.c
>+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
>@@ -22,7 +22,8 @@ pdsc_viftype *pdsc_dl_find_viftype_by_id(struct pdsc *pdsc,
> }
>
> int pdsc_dl_enable_get(struct devlink *dl, u32 id,
>-		       struct devlink_param_gset_ctx *ctx)
>+		       struct devlink_param_gset_ctx *ctx,
>+		       struct netlink_ext_ack *extack)
> {
> 	struct pdsc *pdsc = devlink_priv(dl);
> 	struct pdsc_viftype *vt_entry;
>diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>index 67ca02d84c97..15de802bbac4 100644
>--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
>@@ -1086,7 +1086,8 @@ static int bnxt_hwrm_nvm_req(struct bnxt *bp, u32 param_id, void *msg,
> }
>
> static int bnxt_dl_nvm_param_get(struct devlink *dl, u32 id,
>-				 struct devlink_param_gset_ctx *ctx)
>+				 struct devlink_param_gset_ctx *ctx,
>+				 struct netlink_ext_ack *extack)
> {
> 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
> 	struct hwrm_nvm_get_variable_input *req;
>@@ -1168,7 +1169,8 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
> }
>
> static int bnxt_remote_dev_reset_get(struct devlink *dl, u32 id,
>-				     struct devlink_param_gset_ctx *ctx)
>+				     struct devlink_param_gset_ctx *ctx,
>+				     struct netlink_ext_ack *extack)
> {
> 	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
>
>diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>index 938914abbe06..d88b7f3fd1f9 100644
>--- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
>+++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
>@@ -610,11 +610,13 @@ static int ice_update_tx_topo_user_sel(struct ice_pf *pf, int layers)
>  * @devlink: pointer to the devlink instance
>  * @id: the parameter ID to set
>  * @ctx: context to store the parameter value
>+ * @extack: netlink extended ACK structure
>  *
>  * Return: zero on success and negative value on failure.
>  */
> static int ice_devlink_tx_sched_layers_get(struct devlink *devlink, u32 id,
>-					   struct devlink_param_gset_ctx *ctx)
>+					   struct devlink_param_gset_ctx *ctx,
>+					   struct netlink_ext_ack *extack)
> {
> 	struct ice_pf *pf = devlink_priv(devlink);
> 	int err;
>@@ -1349,7 +1351,8 @@ static const struct devlink_ops ice_sf_devlink_ops;
>
> static int
> ice_devlink_enable_roce_get(struct devlink *devlink, u32 id,
>-			    struct devlink_param_gset_ctx *ctx)
>+			    struct devlink_param_gset_ctx *ctx,
>+			    struct netlink_ext_ack *extack)
> {
> 	struct ice_pf *pf = devlink_priv(devlink);
> 	struct iidc_rdma_core_dev_info *cdev;
>@@ -1415,7 +1418,8 @@ ice_devlink_enable_roce_validate(struct devlink *devlink, u32 id,
>
> static int
> ice_devlink_enable_iw_get(struct devlink *devlink, u32 id,
>-			  struct devlink_param_gset_ctx *ctx)
>+			  struct devlink_param_gset_ctx *ctx,
>+			  struct netlink_ext_ack *extack)
> {
> 	struct ice_pf *pf = devlink_priv(devlink);
> 	struct iidc_rdma_core_dev_info *cdev;
>@@ -1522,11 +1526,13 @@ static int ice_devlink_local_fwd_str_to_mode(const char *mode_str)
>  * @devlink: Pointer to the devlink instance.
>  * @id: The parameter ID to set.
>  * @ctx: Context to store the parameter value.
>+ * @extack: netlink extended ACK structure
>  *
>  * Return: Zero.
>  */
> static int ice_devlink_local_fwd_get(struct devlink *devlink, u32 id,
>-				     struct devlink_param_gset_ctx *ctx)
>+				     struct devlink_param_gset_ctx *ctx,
>+				     struct netlink_ext_ack *extack)
> {
> 	struct ice_pf *pf = devlink_priv(devlink);
> 	struct ice_port_info *pi;
>diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>index 3735372539bd..0f9953eaf1b0 100644
>--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_devlink.c
>@@ -1233,7 +1233,8 @@ static int rvu_af_dl_dwrr_mtu_set(struct devlink *devlink, u32 id,
> }
>
> static int rvu_af_dl_dwrr_mtu_get(struct devlink *devlink, u32 id,
>-				  struct devlink_param_gset_ctx *ctx)
>+				  struct devlink_param_gset_ctx *ctx,
>+				  struct netlink_ext_ack *extack)
> {
> 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
> 	struct rvu *rvu = rvu_dl->rvu;
>@@ -1259,7 +1260,8 @@ enum rvu_af_dl_param_id {
> };
>
> static int rvu_af_npc_exact_feature_get(struct devlink *devlink, u32 id,
>-					struct devlink_param_gset_ctx *ctx)
>+					struct devlink_param_gset_ctx *ctx,
>+					struct netlink_ext_ack *extack)
> {
> 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
> 	struct rvu *rvu = rvu_dl->rvu;
>@@ -1314,7 +1316,8 @@ static int rvu_af_npc_exact_feature_validate(struct devlink *devlink, u32 id,
> }
>
> static int rvu_af_dl_npc_mcam_high_zone_percent_get(struct devlink *devlink, u32 id,
>-						    struct devlink_param_gset_ctx *ctx)
>+						    struct devlink_param_gset_ctx *ctx,
>+						    struct netlink_ext_ack *extack)
> {
> 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
> 	struct rvu *rvu = rvu_dl->rvu;
>@@ -1376,7 +1379,8 @@ static int rvu_af_dl_npc_mcam_high_zone_percent_validate(struct devlink *devlink
> }
>
> static int rvu_af_dl_npc_def_rule_cntr_get(struct devlink *devlink, u32 id,
>-					   struct devlink_param_gset_ctx *ctx)
>+					   struct devlink_param_gset_ctx *ctx,
>+					   struct netlink_ext_ack *extack)
> {
> 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
> 	struct rvu *rvu = rvu_dl->rvu;
>@@ -1402,7 +1406,8 @@ static int rvu_af_dl_npc_def_rule_cntr_set(struct devlink *devlink, u32 id,
> }
>
> static int rvu_af_dl_nix_maxlf_get(struct devlink *devlink, u32 id,
>-				   struct devlink_param_gset_ctx *ctx)
>+				   struct devlink_param_gset_ctx *ctx,
>+				   struct netlink_ext_ack *extack)
> {
> 	struct rvu_devlink *rvu_dl = devlink_priv(devlink);
> 	struct rvu *rvu = rvu_dl->rvu;
>diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
>index e13ae5484c19..a72694219df4 100644
>--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
>+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_devlink.c
>@@ -48,7 +48,8 @@ static int otx2_dl_mcam_count_set(struct devlink *devlink, u32 id,
> }
>
> static int otx2_dl_mcam_count_get(struct devlink *devlink, u32 id,
>-				  struct devlink_param_gset_ctx *ctx)
>+				  struct devlink_param_gset_ctx *ctx,
>+				  struct netlink_ext_ack *extack)
> {
> 	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
> 	struct otx2_nic *pfvf = otx2_dl->pfvf;
>@@ -84,7 +85,8 @@ static int otx2_dl_ucast_flt_cnt_set(struct devlink *devlink, u32 id,
> }
>
> static int otx2_dl_ucast_flt_cnt_get(struct devlink *devlink, u32 id,
>-				     struct devlink_param_gset_ctx *ctx)
>+				     struct devlink_param_gset_ctx *ctx,
>+				     struct netlink_ext_ack *extack)
> {
> 	struct otx2_devlink *otx2_dl = devlink_priv(devlink);
> 	struct otx2_nic *pfvf = otx2_dl->pfvf;
>diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
>index 03d2fc7d9b09..2de226951e19 100644
>--- a/drivers/net/ethernet/mellanox/mlx4/main.c
>+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
>@@ -174,7 +174,8 @@ MODULE_PARM_DESC(port_type_array, "Array of port types: HW_DEFAULT (0) is defaul
> static atomic_t pf_loading = ATOMIC_INIT(0);
>
> static int mlx4_devlink_ierr_reset_get(struct devlink *devlink, u32 id,
>-				       struct devlink_param_gset_ctx *ctx)
>+				       struct devlink_param_gset_ctx *ctx,
>+				       struct netlink_ext_ack *extack)
> {
> 	ctx->val.vbool = !!mlx4_internal_err_reset;
> 	return 0;
>@@ -189,7 +190,8 @@ static int mlx4_devlink_ierr_reset_set(struct devlink *devlink, u32 id,
> }
>
> static int mlx4_devlink_crdump_snapshot_get(struct devlink *devlink, u32 id,
>-					    struct devlink_param_gset_ctx *ctx)
>+					    struct devlink_param_gset_ctx *ctx,
>+					    struct netlink_ext_ack *extack)
> {
> 	struct mlx4_priv *priv = devlink_priv(devlink);
> 	struct mlx4_dev *dev = &priv->dev;
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>index e2ffb87b94cb..308429175bb2 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>@@ -1978,7 +1978,8 @@ static int mlx5_devlink_esw_multiport_set(struct devlink *devlink, u32 id,
> }
>
> static int mlx5_devlink_esw_multiport_get(struct devlink *devlink, u32 id,
>-					  struct devlink_param_gset_ctx *ctx)
>+					  struct devlink_param_gset_ctx *ctx,
>+					  struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>index 4092ea29c630..a4dd1239514f 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>@@ -2492,7 +2492,8 @@ static int esw_port_metadata_set(struct devlink *devlink, u32 id,
> }
>
> static int esw_port_metadata_get(struct devlink *devlink, u32 id,
>-				 struct devlink_param_gset_ctx *ctx)
>+				 struct devlink_param_gset_ctx *ctx,
>+				 struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>index 2db3ffb0a2b2..88dc2023fca5 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
>@@ -3774,7 +3774,8 @@ static int mlx5_fs_mode_set(struct devlink *devlink, u32 id,
> }
>
> static int mlx5_fs_mode_get(struct devlink *devlink, u32 id,
>-			    struct devlink_param_gset_ctx *ctx)
>+			    struct devlink_param_gset_ctx *ctx,
>+			    struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
>index 89e399606877..2bceb42c98cc 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
>@@ -73,7 +73,8 @@ static int mlx5_fw_reset_enable_remote_dev_reset_set(struct devlink *devlink, u3
> }
>
> static int mlx5_fw_reset_enable_remote_dev_reset_get(struct devlink *devlink, u32 id,
>-						     struct devlink_param_gset_ctx *ctx)
>+						     struct devlink_param_gset_ctx *ctx,
>+						     struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
> 	struct mlx5_fw_reset *fw_reset;
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
>index f0ab5ef95fc2..3d2195338d39 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/nv_param.c
>@@ -200,7 +200,8 @@ static const char *const
>
> static int
> mlx5_nv_param_devlink_cqe_compress_get(struct devlink *devlink, u32 id,
>-				       struct devlink_param_gset_ctx *ctx)
>+				       struct devlink_param_gset_ctx *ctx,
>+				       struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
> 	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
>@@ -302,7 +303,8 @@ static int mlx5_nv_param_read_per_host_pf_conf(struct mlx5_core_dev *dev,
> }
>
> static int mlx5_devlink_enable_sriov_get(struct devlink *devlink, u32 id,
>-					 struct devlink_param_gset_ctx *ctx)
>+					 struct devlink_param_gset_ctx *ctx,
>+					 struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
> 	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
>@@ -413,7 +415,8 @@ static int mlx5_devlink_enable_sriov_set(struct devlink *devlink, u32 id,
> }
>
> static int mlx5_devlink_total_vfs_get(struct devlink *devlink, u32 id,
>-				      struct devlink_param_gset_ctx *ctx)
>+				      struct devlink_param_gset_ctx *ctx,
>+				      struct netlink_ext_ack *extack)
> {
> 	struct mlx5_core_dev *dev = devlink_priv(devlink);
> 	u32 mnvda[MLX5_ST_SZ_DW(mnvda_reg)] = {};
>diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
>index b1d08e958bf9..69f9da9fb305 100644
>--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
>+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_acl_tcam.c
>@@ -1489,7 +1489,8 @@ mlxsw_sp_acl_tcam_vregion_rehash(struct mlxsw_sp *mlxsw_sp,
>
> static int
> mlxsw_sp_acl_tcam_region_rehash_intrvl_get(struct devlink *devlink, u32 id,
>-					   struct devlink_param_gset_ctx *ctx)
>+					   struct devlink_param_gset_ctx *ctx,
>+					   struct netlink_ext_ack *extack)
> {
> 	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
> 	struct mlxsw_sp_acl_tcam *tcam;
>diff --git a/drivers/net/ethernet/netronome/nfp/devlink_param.c b/drivers/net/ethernet/netronome/nfp/devlink_param.c
>index 0e1a3800f371..85e3b19e6165 100644
>--- a/drivers/net/ethernet/netronome/nfp/devlink_param.c
>+++ b/drivers/net/ethernet/netronome/nfp/devlink_param.c
>@@ -81,7 +81,8 @@ static const struct nfp_devlink_param_u8_arg nfp_devlink_u8_args[] = {
>
> static int
> nfp_devlink_param_u8_get(struct devlink *devlink, u32 id,
>-			 struct devlink_param_gset_ctx *ctx)
>+			 struct devlink_param_gset_ctx *ctx,
>+			 struct netlink_ext_ack *extack)
> {
> 	const struct nfp_devlink_param_u8_arg *arg;
> 	struct nfp_pf *pf = devlink_priv(devlink);
>diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
>index 94c5689b5abd..0c5278c0598c 100644
>--- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
>+++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
>@@ -121,7 +121,8 @@ void qed_fw_reporters_destroy(struct devlink *devlink)
> }
>
> static int qed_dl_param_get(struct devlink *dl, u32 id,
>-			    struct devlink_param_gset_ctx *ctx)
>+			    struct devlink_param_gset_ctx *ctx,
>+			    struct netlink_ext_ack *extack)
> {
> 	struct qed_devlink *qed_dl = devlink_priv(dl);
> 	struct qed_dev *cdev;
>diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>index ccf383b355e7..e46d443b9da1 100644
>--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>@@ -7488,7 +7488,8 @@ static int stmmac_dl_ts_coarse_set(struct devlink *dl, u32 id,
> }
>
> static int stmmac_dl_ts_coarse_get(struct devlink *dl, u32 id,
>-				   struct devlink_param_gset_ctx *ctx)
>+				   struct devlink_param_gset_ctx *ctx,
>+				   struct netlink_ext_ack *extack)
> {
> 	struct stmmac_devlink_priv *dl_priv = devlink_priv(dl);
> 	struct stmmac_priv *priv = dl_priv->stmmac_priv;
>diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>index d5f358ec9820..5924db6be3fe 100644
>--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>@@ -3068,7 +3068,8 @@ static void am65_cpsw_init_host_port_emac(struct am65_cpsw_common *common)
> }
>
> static int am65_cpsw_dl_switch_mode_get(struct devlink *dl, u32 id,
>-					struct devlink_param_gset_ctx *ctx)
>+					struct devlink_param_gset_ctx *ctx,
>+					struct netlink_ext_ack *extack)
> {
> 	struct am65_cpsw_devlink *dl_priv = devlink_priv(dl);
> 	struct am65_cpsw_common *common = dl_priv->common;
>diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti/cpsw_new.c
>index 8b9e2078c602..ab88d4c02cbd 100644
>--- a/drivers/net/ethernet/ti/cpsw_new.c
>+++ b/drivers/net/ethernet/ti/cpsw_new.c
>@@ -1618,7 +1618,8 @@ static const struct devlink_ops cpsw_devlink_ops = {
> };
>
> static int cpsw_dl_switch_mode_get(struct devlink *dl, u32 id,
>-				   struct devlink_param_gset_ctx *ctx)
>+				   struct devlink_param_gset_ctx *ctx,
>+				   struct netlink_ext_ack *extack)
> {
> 	struct cpsw_devlink *dl_priv = devlink_priv(dl);
> 	struct cpsw_common *cpsw = dl_priv->cpsw;
>@@ -1753,7 +1754,8 @@ static int cpsw_dl_switch_mode_set(struct devlink *dl, u32 id,
> }
>
> static int cpsw_dl_ale_ctrl_get(struct devlink *dl, u32 id,
>-				struct devlink_param_gset_ctx *ctx)
>+				struct devlink_param_gset_ctx *ctx,
>+				struct netlink_ext_ack *extack)
> {
> 	struct cpsw_devlink *dl_priv = devlink_priv(dl);
> 	struct cpsw_common *cpsw = dl_priv->cpsw;
>diff --git a/drivers/net/wwan/iosm/iosm_ipc_devlink.c b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
>index 33d6342124bc..301a9d294d30 100644
>--- a/drivers/net/wwan/iosm/iosm_ipc_devlink.c
>+++ b/drivers/net/wwan/iosm/iosm_ipc_devlink.c
>@@ -21,7 +21,8 @@ static struct iosm_coredump_file_info list[IOSM_NOF_CD_REGION] = {
>
> /* Get the param values for the specific param ID's */
> static int ipc_devlink_get_param(struct devlink *dl, u32 id,
>-				 struct devlink_param_gset_ctx *ctx)
>+				 struct devlink_param_gset_ctx *ctx,
>+				 struct netlink_ext_ack *extack)
> {
> 	struct iosm_devlink *ipc_devlink = devlink_priv(dl);
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 9e824f61e40f..6d942597d07d 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -490,7 +490,8 @@ struct devlink_param {
> 	enum devlink_param_type type;
> 	unsigned long supported_cmodes;
> 	int (*get)(struct devlink *devlink, u32 id,
>-		   struct devlink_param_gset_ctx *ctx);
>+		   struct devlink_param_gset_ctx *ctx,
>+		   struct netlink_ext_ack *extack);
> 	int (*set)(struct devlink *devlink, u32 id,
> 		   struct devlink_param_gset_ctx *ctx,
> 		   struct netlink_ext_ack *extack);
>diff --git a/include/net/dsa.h b/include/net/dsa.h
>index 2df2e2ead9a8..85c1b938f2c4 100644
>--- a/include/net/dsa.h
>+++ b/include/net/dsa.h
>@@ -1251,7 +1251,8 @@ struct dsa_switch_ops {
> 			     dsa_devlink_param_get, dsa_devlink_param_set, NULL)
>
> int dsa_devlink_param_get(struct devlink *dl, u32 id,
>-			  struct devlink_param_gset_ctx *ctx);
>+			  struct devlink_param_gset_ctx *ctx,
>+			  struct netlink_ext_ack *extack);
> int dsa_devlink_param_set(struct devlink *dl, u32 id,
> 			  struct devlink_param_gset_ctx *ctx,
> 			  struct netlink_ext_ack *extack);
>diff --git a/net/devlink/param.c b/net/devlink/param.c
>index 70e69523412c..8bae3f733bde 100644
>--- a/net/devlink/param.c
>+++ b/net/devlink/param.c
>@@ -169,11 +169,12 @@ devlink_param_cmode_is_supported(const struct devlink_param *param,
>
> static int devlink_param_get(struct devlink *devlink,
> 			     const struct devlink_param *param,
>-			     struct devlink_param_gset_ctx *ctx)
>+			     struct devlink_param_gset_ctx *ctx,
>+			     struct netlink_ext_ack *extack)
> {
> 	if (!param->get)
> 		return -EOPNOTSUPP;
>-	return param->get(devlink, param->id, ctx);
>+	return param->get(devlink, param->id, ctx, extack);
> }
>
> static int devlink_param_set(struct devlink *devlink,
>@@ -245,7 +246,8 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
> 				 unsigned int port_index,
> 				 struct devlink_param_item *param_item,
> 				 enum devlink_command cmd,
>-				 u32 portid, u32 seq, int flags)
>+				 u32 portid, u32 seq, int flags,
>+				 struct netlink_ext_ack *extack)

Too many params, I miss David Miller, I think he said once "if you write a function with more
than 5 parameters, you are not my friend".

Anyway I can't think of a creative way to solve this, so LGTM overall.

Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>

> {
> 	union devlink_param_value param_value[DEVLINK_PARAM_CMODE_MAX + 1];
> 	bool param_value_set[DEVLINK_PARAM_CMODE_MAX + 1] = {};
>@@ -270,7 +272,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
> 				return -EOPNOTSUPP;
> 		} else {
> 			ctx.cmode = i;
>-			err = devlink_param_get(devlink, param, &ctx);
>+			err = devlink_param_get(devlink, param, &ctx, extack);
> 			if (err)
> 				return err;
> 			param_value[i] = ctx.val;
>@@ -352,7 +354,7 @@ static void devlink_param_notify(struct devlink *devlink,
> 	if (!msg)
> 		return;
> 	err = devlink_nl_param_fill(msg, devlink, port_index, param_item, cmd,
>-				    0, 0, 0);
>+				    0, 0, 0, NULL);
> 	if (err) {
> 		nlmsg_free(msg);
> 		return;
>@@ -395,7 +397,8 @@ static int devlink_nl_param_get_dump_one(struct sk_buff *msg,
> 		err = devlink_nl_param_fill(msg, devlink, 0, param_item,
> 					    DEVLINK_CMD_PARAM_GET,
> 					    NETLINK_CB(cb->skb).portid,
>-					    cb->nlh->nlmsg_seq, flags);
>+					    cb->nlh->nlmsg_seq, flags,
>+					    cb->extack);
> 		if (err == -EOPNOTSUPP) {
> 			err = 0;
> 		} else if (err) {
>@@ -504,8 +507,8 @@ int devlink_nl_param_get_doit(struct sk_buff *skb,
> 		return -ENOMEM;
>
> 	err = devlink_nl_param_fill(msg, devlink, 0, param_item,
>-				    DEVLINK_CMD_PARAM_GET,
>-				    info->snd_portid, info->snd_seq, 0);
>+				    DEVLINK_CMD_PARAM_GET, info->snd_portid,
>+				    info->snd_seq, 0, info->extack);
> 	if (err) {
> 		nlmsg_free(msg);
> 		return err;
>diff --git a/net/dsa/devlink.c b/net/dsa/devlink.c
>index f41f9fc2194e..ed342f345692 100644
>--- a/net/dsa/devlink.c
>+++ b/net/dsa/devlink.c
>@@ -182,7 +182,8 @@ static const struct devlink_ops dsa_devlink_ops = {
> };
>
> int dsa_devlink_param_get(struct devlink *dl, u32 id,
>-			  struct devlink_param_gset_ctx *ctx)
>+			  struct devlink_param_gset_ctx *ctx,
>+			  struct netlink_ext_ack *extack)
> {
> 	struct dsa_switch *ds = dsa_devlink_to_ds(dl);
>
>-- 
>2.47.3
>
>

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jiri Pirko]

Fri, Nov 07, 2025 at 09:43:46PM +0100, daniel.zahka@gmail.com wrote:
>swp_l4_csum_mode controls how L4 transmit checksums are computed when
>using Software Parser (SWP) hints for header locations.
>
>Supported values:
>  1. device_default: use device default setting.
>  2. full_csum: calculate L4 checksum with the pseudo-header.
>  3. l4_only: calculate L4 checksum without the pseudo-header. Only
>     available when swp_l4_csum_mode_l4_only is set in
>     mlx5_ifc_nv_sw_offload_cap_bits.
>
>The l4_only setting is a dependency for PSP initialization in
>mlx5e_psp_init().
>
>Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

Daniel, I asked twice if this could be a non-driver param. Jakub asked
for clearer definition of this know in that context.

Not sure why you are ignoring this :/

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jiri Pirko]

Sat, Nov 08, 2025 at 07:14:45AM +0100, saeed@kernel.org wrote:
>On 07 Nov 12:43, Daniel Zahka wrote:
>> swp_l4_csum_mode controls how L4 transmit checksums are computed when
>> using Software Parser (SWP) hints for header locations.
>> 
>> Supported values:
>>  1. device_default: use device default setting.
>>  2. full_csum: calculate L4 checksum with the pseudo-header.
>>  3. l4_only: calculate L4 checksum without the pseudo-header. Only
>>     available when swp_l4_csum_mode_l4_only is set in
>>     mlx5_ifc_nv_sw_offload_cap_bits.
>> 
>> The l4_only setting is a dependency for PSP initialization in
>> mlx5e_psp_init().
>> 
>> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
>> ---
>> 
>> Notes:
>>    v2:
>>    - use extack in mlx5_nv_param_devlink_swp_l4_csum_mode_get()
>>    - fix indentation issue in mlx5.rst entry
>> 
>> Documentation/networking/devlink/mlx5.rst     |   9 +
>> .../net/ethernet/mellanox/mlx5/core/devlink.h |   3 +-
>> .../mellanox/mlx5/core/lib/nv_param.c         | 161 ++++++++++++++++++
>> 3 files changed, 172 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/networking/devlink/mlx5.rst b/Documentation/networking/devlink/mlx5.rst
>> index 0e5f9c76e514..675b5a1ec625 100644
>> --- a/Documentation/networking/devlink/mlx5.rst
>> +++ b/Documentation/networking/devlink/mlx5.rst
>> @@ -218,6 +218,15 @@ parameters.
>>        * ``balanced`` : Merges fewer CQEs, resulting in a moderate compression ratio but maintaining a balance between bandwidth savings and performance
>>        * ``aggressive`` : Merges more CQEs into a single entry, achieving a higher compression rate and maximizing performance, particularly under high traffic loads
>> 
>> +   * - ``swp_l4_csum_mode``
>> +     - string
>> +     - permanent
>> +     - Configure how the L4 checksum is calculated by the device when using
>> +       Software Parser (SWP) hints for header locations.
>> +       * ``device_default`` : Use the device's default checksum calculation mode
>
>Let's omit the device, just "default".
>
>So, I checked a couple of flows internally, and it seems this allows
>some flexibility in the FW to decide later on which mode to pick,
>based on other parameters, which practically means
>"user has no preference on this param". Driver can only find out
>after boot, when it reads the runtime capabilities, but still
>this is a bug, by the time the driver reads this (in devlink), the
>default value should've already been determined by FW, so FW must
>return the actual runtime value. Which can only be one of the following

I don't think it is correct to expose the "default" as a value.

On read, user should see the configured value, either "full_csum" or
"l4_only". Reporting "default" to the user does not make any sense.
On write, user should pass either "full_csum" or "l4_only". Why we would
ever want to pass "default"?

Regardless this patch, since this is param to be reflected on fw reboot
(permanent cmode), I think it would be nice to expose indication if
param value passed to user currently affects the fw, or if it is going
to be applied after fw reboot. Perhaps a simple bool attr would do?


>
>> +       * ``full_csum`` : Calculate full checksum including the pseudo-header
>> +       * ``l4_only`` : Calculate L4-only checksum, excluding the pseudo-header

[...]

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Daniel Zahka]

On 11/9/25 5:39 AM, Jiri Pirko wrote:
> Daniel, I asked twice if this could be a non-driver param. Jakub asked
> for clearer definition of this know in that context.
>
> Not sure why you are ignoring this :/
>

My apologies. I think there was a miscommunication. I assumed Jakub's 
question was directed towards you. I have no objection to making it a 
generic param; I will do so in v4. It sounded to me like Jakub was 
wanting more information on what exactly this setting does beyond what I 
was able to provide in the commit message and mlx5 devlink 
documentation. My understanding is that this setting pertains to tx 
csums and how the device expects the driver to prepare partial csums 
when doing tx cso. I don't really know more than that. Especially not 
something like what the FW's role in implementing this is.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Daniel Zahka]

On 11/9/25 5:46 AM, Jiri Pirko wrote:
> Regardless this patch, since this is param to be reflected on fw reboot
> (permanent cmode), I think it would be nice to expose indication if
> param value passed to user currently affects the fw, or if it is going
> to be applied after fw reboot. Perhaps a simple bool attr would do?

I can add something like this. For permanent cmode params, it might make 
the most sense to expose current and next values the way mstconfig does, 
but that would be a more complicated change.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jakub Kicinski]

On Mon, 10 Nov 2025 08:05:57 -0500 Daniel Zahka wrote:
> On 11/9/25 5:39 AM, Jiri Pirko wrote:
> > Daniel, I asked twice if this could be a non-driver param. Jakub asked
> > for clearer definition of this know in that context.
> >
> > Not sure why you are ignoring this :/
> >  
> 
> My apologies. I think there was a miscommunication. I assumed Jakub's 
> question was directed towards you. I have no objection to making it a 
> generic param; I will do so in v4. It sounded to me like Jakub was 
> wanting more information on what exactly this setting does beyond what I 
> was able to provide in the commit message and mlx5 devlink 
> documentation. My understanding is that this setting pertains to tx 
> csums and how the device expects the driver to prepare partial csums 
> when doing tx cso. I don't really know more than that. Especially not 
> something like what the FW's role in implementing this is.

Right, per To: field of my email I as asking Jiri for clarifications.

Since we struggle to understand the semantics nack on making this
generic. Chances are whoever reuses the "generic" param will have a
different interpretation of its meaning, so what's the point of making
it generic.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jakub Kicinski]

On Fri, 7 Nov 2025 22:14:45 -0800 Saeed Mahameed wrote:
> >+	err = mlx5_nv_param_read_sw_accelerate_conf(dev, mnvda, sizeof(mnvda));
> >+	if (err) {
> >+		NL_SET_ERR_MSG_MOD(extack,
> >+				   "Failed to read sw_accelerate_conf mnvda reg");  
> 
> Plug in the err, NL_SET_ERR_MSG_FMT_MOD(.., .., err);
> other locations as well.

Incorrect. extack should basically be passed to perror()
IOW user space will add strerror(errno) after, anyway.
Adding the errno inside the string is pointless and ugly.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jakub Kicinski]

On Sun, 9 Nov 2025 11:46:37 +0100 Jiri Pirko wrote:
> >So, I checked a couple of flows internally, and it seems this allows
> >some flexibility in the FW to decide later on which mode to pick,
> >based on other parameters, which practically means
> >"user has no preference on this param". Driver can only find out
> >after boot, when it reads the runtime capabilities, but still
> >this is a bug, by the time the driver reads this (in devlink), the
> >default value should've already been determined by FW, so FW must
> >return the actual runtime value. Which can only be one of the following  
> 
> I don't think it is correct to expose the "default" as a value.
> 
> On read, user should see the configured value, either "full_csum" or
> "l4_only". Reporting "default" to the user does not make any sense.
> On write, user should pass either "full_csum" or "l4_only". Why we would
> ever want to pass "default"?

FWIW I agree that this feels a bit odd. Should the default be a flag
attr? On get flag being present means the value is the FW default (no
override present). On set passing the flag means user wants to reset
to FW default (remove override)?

> Regardless this patch, since this is param to be reflected on fw reboot
> (permanent cmode), I think it would be nice to expose indication if
> param value passed to user currently affects the fw, or if it is going
> to be applied after fw reboot. Perhaps a simple bool attr would do?

IIUC we're basically talking about user having no information that 
the update is pending? Could this be done by the core? Core can do 
a ->get prior to calling ->set and if the ->set succeeds and 
cmode != runtime record that the update is pending?

That feels very separate from the series tho, there are 3 permanent
params in mlx5, already. Is there something that makes this one special?

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Saeed Mahameed]

On 10 Nov 15:46, Jakub Kicinski wrote:
>On Sun, 9 Nov 2025 11:46:37 +0100 Jiri Pirko wrote:
>> >So, I checked a couple of flows internally, and it seems this allows
>> >some flexibility in the FW to decide later on which mode to pick,
>> >based on other parameters, which practically means
>> >"user has no preference on this param". Driver can only find out
>> >after boot, when it reads the runtime capabilities, but still
>> >this is a bug, by the time the driver reads this (in devlink), the
>> >default value should've already been determined by FW, so FW must
>> >return the actual runtime value. Which can only be one of the following
>>
>> I don't think it is correct to expose the "default" as a value.
>>
>> On read, user should see the configured value, either "full_csum" or
>> "l4_only". Reporting "default" to the user does not make any sense.
>> On write, user should pass either "full_csum" or "l4_only". Why we would
>> ever want to pass "default"?
>
>FWIW I agree that this feels a bit odd. Should the default be a flag
>attr? On get flag being present means the value is the FW default (no
>override present). On set passing the flag means user wants to reset
>to FW default (remove override)?
>
>> Regardless this patch, since this is param to be reflected on fw reboot
>> (permanent cmode), I think it would be nice to expose indication if
>> param value passed to user currently affects the fw, or if it is going
>> to be applied after fw reboot. Perhaps a simple bool attr would do?
>
>IIUC we're basically talking about user having no information that
>the update is pending? Could this be done by the core? Core can do
>a ->get prior to calling ->set and if the ->set succeeds and
>cmode != runtime record that the update is pending?
>

Could work if on GET driver reads 'current' value from FW, then it should
be simpler if GET != SET then 'pending', one problem though is if SET was
done by external tool or value wasn't applied after reboot, then we loose
that information, but do we care? I think we shouldn't.

>That feels very separate from the series tho, there are 3 permanent
>params in mlx5, already. Is there something that makes this one special?

In mlx5 they all have the same behavior, devlink sets 'next' value, 
devlink reads 'next' value. The only special thing about the new param
is that it has a 'device_default' value and when you read that from 
'next' it will always show 'device_default' as the actual value is only
known at run time ,e.g. 'next boot'.

I think the only valid solution for permanent and drv_init params is to
have 'next' and 'current' values reported by driver on read. 
Or maybe go just with  'set' != 'get' then 'pending' as discussed above ?

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Saeed Mahameed]

On 10 Nov 15:01, Jakub Kicinski wrote:
>On Fri, 7 Nov 2025 22:14:45 -0800 Saeed Mahameed wrote:
>> >+	err = mlx5_nv_param_read_sw_accelerate_conf(dev, mnvda, sizeof(mnvda));
>> >+	if (err) {
>> >+		NL_SET_ERR_MSG_MOD(extack,
>> >+				   "Failed to read sw_accelerate_conf mnvda reg");
>>
>> Plug in the err, NL_SET_ERR_MSG_FMT_MOD(.., .., err);
>> other locations as well.
>
>Incorrect. extack should basically be passed to perror()
>IOW user space will add strerror(errno) after, anyway.
>Adding the errno inside the string is pointless and ugly.

ernno set by stack. err set by driver. we can't assume err will propagate
to errno, this is up to the stack.

And not at all ugly, very useful debug hint to the user, unless you
guarantee err == errno.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jiri Pirko]

Tue, Nov 11, 2025 at 04:26:34AM +0100, saeed@kernel.org wrote:
>On 10 Nov 15:46, Jakub Kicinski wrote:
>> On Sun, 9 Nov 2025 11:46:37 +0100 Jiri Pirko wrote:
>> > >So, I checked a couple of flows internally, and it seems this allows
>> > >some flexibility in the FW to decide later on which mode to pick,
>> > >based on other parameters, which practically means
>> > >"user has no preference on this param". Driver can only find out
>> > >after boot, when it reads the runtime capabilities, but still
>> > >this is a bug, by the time the driver reads this (in devlink), the
>> > >default value should've already been determined by FW, so FW must
>> > >return the actual runtime value. Which can only be one of the following
>> > 
>> > I don't think it is correct to expose the "default" as a value.
>> > 
>> > On read, user should see the configured value, either "full_csum" or
>> > "l4_only". Reporting "default" to the user does not make any sense.
>> > On write, user should pass either "full_csum" or "l4_only". Why we would
>> > ever want to pass "default"?
>> 
>> FWIW I agree that this feels a bit odd. Should the default be a flag
>> attr? On get flag being present means the value is the FW default (no
>> override present). On set passing the flag means user wants to reset
>> to FW default (remove override)?
>> 
>> > Regardless this patch, since this is param to be reflected on fw reboot
>> > (permanent cmode), I think it would be nice to expose indication if
>> > param value passed to user currently affects the fw, or if it is going
>> > to be applied after fw reboot. Perhaps a simple bool attr would do?
>> 
>> IIUC we're basically talking about user having no information that
>> the update is pending? Could this be done by the core? Core can do
>> a ->get prior to calling ->set and if the ->set succeeds and
>> cmode != runtime record that the update is pending?
>> 
>
>Could work if on GET driver reads 'current' value from FW, then it should
>be simpler if GET != SET then 'pending', one problem though is if SET was
>done by external tool or value wasn't applied after reboot, then we loose
>that information, but do we care? I think we shouldn't.
>
>> That feels very separate from the series tho, there are 3 permanent
>> params in mlx5, already. Is there something that makes this one special?

Agreed. That is why I wrote "regardless this patch". But I think the
pending indication is definitelly nice to have.


>
>In mlx5 they all have the same behavior, devlink sets 'next' value, devlink
>reads 'next' value. The only special thing about the new param
>is that it has a 'device_default' value and when you read that from 'next' it
>will always show 'device_default' as the actual value is only
>known at run time ,e.g. 'next boot'.
>
>I think the only valid solution for permanent and drv_init params is to
>have 'next' and 'current' values reported by driver on read. Or maybe go just
>with  'set' != 'get' then 'pending' as discussed above ?

Hmm, is it possible to rebind the driver without fw going through
next-boot phase? I'm wondering if it wouldn't be safer to have this
pending flag set to be responsibility of the driver...


>

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jiri Pirko]

Mon, Nov 10, 2025 at 11:58:31PM +0100, kuba@kernel.org wrote:
>On Mon, 10 Nov 2025 08:05:57 -0500 Daniel Zahka wrote:
>> On 11/9/25 5:39 AM, Jiri Pirko wrote:
>> > Daniel, I asked twice if this could be a non-driver param. Jakub asked
>> > for clearer definition of this know in that context.
>> >
>> > Not sure why you are ignoring this :/
>> >  
>> 
>> My apologies. I think there was a miscommunication. I assumed Jakub's 
>> question was directed towards you. I have no objection to making it a 
>> generic param; I will do so in v4. It sounded to me like Jakub was 
>> wanting more information on what exactly this setting does beyond what I 
>> was able to provide in the commit message and mlx5 devlink 
>> documentation. My understanding is that this setting pertains to tx 
>> csums and how the device expects the driver to prepare partial csums 
>> when doing tx cso. I don't really know more than that. Especially not 
>> something like what the FW's role in implementing this is.
>
>Right, per To: field of my email I as asking Jiri for clarifications.
>
>Since we struggle to understand the semantics nack on making this
>generic. Chances are whoever reuses the "generic" param will have a
>different interpretation of its meaning, so what's the point of making
>it generic.

Okay, I don't mind that much.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Daniel Zahka]

On 11/10/25 10:26 PM, Saeed Mahameed wrote:
> On 10 Nov 15:46, Jakub Kicinski wrote:
>> On Sun, 9 Nov 2025 11:46:37 +0100 Jiri Pirko wrote:
>>> >So, I checked a couple of flows internally, and it seems this allows
>>> >some flexibility in the FW to decide later on which mode to pick,
>>> >based on other parameters, which practically means
>>> >"user has no preference on this param". Driver can only find out
>>> >after boot, when it reads the runtime capabilities, but still
>>> >this is a bug, by the time the driver reads this (in devlink), the
>>> >default value should've already been determined by FW, so FW must
>>> >return the actual runtime value. Which can only be one of the 
>>> following
>>>
>>> I don't think it is correct to expose the "default" as a value.
>>>
>>> On read, user should see the configured value, either "full_csum" or
>>> "l4_only". Reporting "default" to the user does not make any sense.
>>> On write, user should pass either "full_csum" or "l4_only". Why we 
>>> would
>>> ever want to pass "default"?
>>
>> FWIW I agree that this feels a bit odd. Should the default be a flag
>> attr? On get flag being present means the value is the FW default (no
>> override present). On set passing the flag means user wants to reset
>> to FW default (remove override)?
>>
>>> Regardless this patch, since this is param to be reflected on fw reboot
>>> (permanent cmode), I think it would be nice to expose indication if
>>> param value passed to user currently affects the fw, or if it is going
>>> to be applied after fw reboot. Perhaps a simple bool attr would do?
>>
>> IIUC we're basically talking about user having no information that
>> the update is pending? Could this be done by the core? Core can do
>> a ->get prior to calling ->set and if the ->set succeeds and
>> cmode != runtime record that the update is pending?
>>
>
> Could work if on GET driver reads 'current' value from FW, then it should
> be simpler if GET != SET then 'pending', one problem though is if SET was
> done by external tool or value wasn't applied after reboot, then we loose
> that information, but do we care? I think we shouldn't.
>
>> That feels very separate from the series tho, there are 3 permanent
>> params in mlx5, already. Is there something that makes this one special?
>
> In mlx5 they all have the same behavior, devlink sets 'next' value, 
> devlink reads 'next' value. The only special thing about the new param
> is that it has a 'device_default' value and when you read that from 
> 'next' it will always show 'device_default' as the actual value is only
> known at run time ,e.g. 'next boot'.
>
> I think the only valid solution for permanent and drv_init params is to
> have 'next' and 'current' values reported by driver on read. Or maybe 
> go just with  'set' != 'get' then 'pending' as discussed above ?
>

The driver reporting 'current' and 'next' makes the most sense to me. 
'pending' would just be implied then. The 'set' != 'get' then 'pending' 
approach would not work on my multi host CX7 system, where rebooting the 
hosts individually does not trigger a fw reset.

To be clear, are we willing to go forward with treating swp_l4_csum_mode 
like other permanent params in nv_param.c in this series, and then defer 
the 'pending' solution to another series?

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jakub Kicinski]

On Mon, 10 Nov 2025 19:34:40 -0800 Saeed Mahameed wrote:
> On 10 Nov 15:01, Jakub Kicinski wrote:
> >On Fri, 7 Nov 2025 22:14:45 -0800 Saeed Mahameed wrote:  
> >> Plug in the err, NL_SET_ERR_MSG_FMT_MOD(.., .., err);
> >> other locations as well.  
> >
> >Incorrect. extack should basically be passed to perror()
> >IOW user space will add strerror(errno) after, anyway.
> >Adding the errno inside the string is pointless and ugly.  
> 
> ernno set by stack. err set by driver. we can't assume err will propagate
> to errno, this is up to the stack.
> 
> And not at all ugly, very useful debug hint to the user, unless you
> guarantee err == errno.

Not propagating errno should be fixed, if that happens.
We need clear expectations to avoid the messages being all over 
the place. Historically we haven't included err because formatting
was not an option. So I think we should continue this way.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jakub Kicinski]

[stripping some of the bouncy CCs.]

On Tue, 11 Nov 2025 15:39:03 +0100 Jiri Pirko wrote:
> Tue, Nov 11, 2025 at 04:26:34AM +0100, saeed@kernel.org wrote:
> >On 10 Nov 15:46, Jakub Kicinski wrote:  
> >> On Sun, 9 Nov 2025 11:46:37 +0100 Jiri Pirko wrote:  
> >> > >So, I checked a couple of flows internally, and it seems this allows
> >> > >some flexibility in the FW to decide later on which mode to pick,
> >> > >based on other parameters, which practically means
> >> > >"user has no preference on this param". Driver can only find out
> >> > >after boot, when it reads the runtime capabilities, but still
> >> > >this is a bug, by the time the driver reads this (in devlink), the
> >> > >default value should've already been determined by FW, so FW must
> >> > >return the actual runtime value. Which can only be one of the following  
> >> > 
> >> > I don't think it is correct to expose the "default" as a value.
> >> > 
> >> > On read, user should see the configured value, either "full_csum" or
> >> > "l4_only". Reporting "default" to the user does not make any sense.
> >> > On write, user should pass either "full_csum" or "l4_only". Why we would
> >> > ever want to pass "default"?  
> >> 
> >> FWIW I agree that this feels a bit odd. Should the default be a flag
> >> attr? On get flag being present means the value is the FW default (no
> >> override present). On set passing the flag means user wants to reset
> >> to FW default (remove override)?

Y'all did not respond to this part, should we assume that what 
I described is clear and makes sense? I think we should make that
part of the series, unlike the pending indication.

> >> > Regardless this patch, since this is param to be reflected on fw reboot
> >> > (permanent cmode), I think it would be nice to expose indication if
> >> > param value passed to user currently affects the fw, or if it is going
> >> > to be applied after fw reboot. Perhaps a simple bool attr would do?  
> >> 
> >> IIUC we're basically talking about user having no information that
> >> the update is pending? Could this be done by the core? Core can do
> >> a ->get prior to calling ->set and if the ->set succeeds and
> >> cmode != runtime record that the update is pending?
> >>   
> >
> >Could work if on GET driver reads 'current' value from FW, then it should
> >be simpler if GET != SET then 'pending', one problem though is if SET was
> >done by external tool or value wasn't applied after reboot, then we loose
> >that information, but do we care? I think we shouldn't.
> >  
> >> That feels very separate from the series tho, there are 3 permanent
> >> params in mlx5, already. Is there something that makes this one special?  
> 
> Agreed. That is why I wrote "regardless this patch". But I think the
> pending indication is definitelly nice to have.

Yes, I've been wondering why it's missing since the day devlink params
were added :)

> >In mlx5 they all have the same behavior, devlink sets 'next' value, devlink
> >reads 'next' value. The only special thing about the new param
> >is that it has a 'device_default' value and when you read that from 'next' it
> >will always show 'device_default' as the actual value is only
> >known at run time ,e.g. 'next boot'.
> >
> >I think the only valid solution for permanent and drv_init params is to
> >have 'next' and 'current' values reported by driver on read. Or maybe go just
> >with  'set' != 'get' then 'pending' as discussed above ?  
> 
> Hmm, is it possible to rebind the driver without fw going through
> next-boot phase? I'm wondering if it wouldn't be safer to have this
> pending flag set to be responsibility of the driver...

The downside is that drivers may either have bugs or not implement 
the new feature. So when there's no indication of pending change
the user will have no idea whether its because there's none or the
driver simply does not report both. 

My experience implementing the pending FW version in a couple of
products is that it takes a lot of "discussions" to get FW people 
to implement this sort of a thing right. mlx5 already has the right 
FW APIs so we should allow their full use. But I don't think the 
"what if user change the setting with fwctl", "what if user reloaded
the driver" corner cases should stop us from trying to get the core
to implement 99% of the cases right.

FTR I'm not aware of any Meta-internal products having permanent knobs.
I just don't think we can depend on the random people that submit
drivers these days to get this right. And devlink users will assume
that it's Linux that sucks if it doesn't work right, not vendor X.

Long story short I think we can add the reporting of both values via GET
but I'd definitely still like to see the core trying to do the tracking
automatically.

Re: [PATCH net-next v3 2/2] net/mlx5: implement swp_l4_csum_mode via devlink params

[Jiri Pirko]

Tue, Nov 11, 2025 at 04:48:57PM +0100, kuba@kernel.org wrote:
>[stripping some of the bouncy CCs.]
>
>On Tue, 11 Nov 2025 15:39:03 +0100 Jiri Pirko wrote:
>> Tue, Nov 11, 2025 at 04:26:34AM +0100, saeed@kernel.org wrote:
>> >On 10 Nov 15:46, Jakub Kicinski wrote:  
>> >> On Sun, 9 Nov 2025 11:46:37 +0100 Jiri Pirko wrote:  
>> >> > >So, I checked a couple of flows internally, and it seems this allows
>> >> > >some flexibility in the FW to decide later on which mode to pick,
>> >> > >based on other parameters, which practically means
>> >> > >"user has no preference on this param". Driver can only find out
>> >> > >after boot, when it reads the runtime capabilities, but still
>> >> > >this is a bug, by the time the driver reads this (in devlink), the
>> >> > >default value should've already been determined by FW, so FW must
>> >> > >return the actual runtime value. Which can only be one of the following  
>> >> > 
>> >> > I don't think it is correct to expose the "default" as a value.
>> >> > 
>> >> > On read, user should see the configured value, either "full_csum" or
>> >> > "l4_only". Reporting "default" to the user does not make any sense.
>> >> > On write, user should pass either "full_csum" or "l4_only". Why we would
>> >> > ever want to pass "default"?  
>> >> 
>> >> FWIW I agree that this feels a bit odd. Should the default be a flag
>> >> attr? On get flag being present means the value is the FW default (no
>> >> override present). On set passing the flag means user wants to reset
>> >> to FW default (remove override)?
>
>Y'all did not respond to this part, should we assume that what 
>I described is clear and makes sense? I think we should make that
>part of the series, unlike the pending indication.

I agree. The "default" flag sounds good to me.


>
>> >> > Regardless this patch, since this is param to be reflected on fw reboot
>> >> > (permanent cmode), I think it would be nice to expose indication if
>> >> > param value passed to user currently affects the fw, or if it is going
>> >> > to be applied after fw reboot. Perhaps a simple bool attr would do?  
>> >> 
>> >> IIUC we're basically talking about user having no information that
>> >> the update is pending? Could this be done by the core? Core can do
>> >> a ->get prior to calling ->set and if the ->set succeeds and
>> >> cmode != runtime record that the update is pending?
>> >>   
>> >
>> >Could work if on GET driver reads 'current' value from FW, then it should
>> >be simpler if GET != SET then 'pending', one problem though is if SET was
>> >done by external tool or value wasn't applied after reboot, then we loose
>> >that information, but do we care? I think we shouldn't.
>> >  
>> >> That feels very separate from the series tho, there are 3 permanent
>> >> params in mlx5, already. Is there something that makes this one special?  
>> 
>> Agreed. That is why I wrote "regardless this patch". But I think the
>> pending indication is definitelly nice to have.
>
>Yes, I've been wondering why it's missing since the day devlink params
>were added :)

Puzzles me. Nobody probably cared that much :/


>
>> >In mlx5 they all have the same behavior, devlink sets 'next' value, devlink
>> >reads 'next' value. The only special thing about the new param
>> >is that it has a 'device_default' value and when you read that from 'next' it
>> >will always show 'device_default' as the actual value is only
>> >known at run time ,e.g. 'next boot'.
>> >
>> >I think the only valid solution for permanent and drv_init params is to
>> >have 'next' and 'current' values reported by driver on read. Or maybe go just
>> >with  'set' != 'get' then 'pending' as discussed above ?  
>> 
>> Hmm, is it possible to rebind the driver without fw going through
>> next-boot phase? I'm wondering if it wouldn't be safer to have this
>> pending flag set to be responsibility of the driver...
>
>The downside is that drivers may either have bugs or not implement 
>the new feature. So when there's no indication of pending change
>the user will have no idea whether its because there's none or the
>driver simply does not report both. 
>
>My experience implementing the pending FW version in a couple of
>products is that it takes a lot of "discussions" to get FW people 
>to implement this sort of a thing right. mlx5 already has the right 
>FW APIs so we should allow their full use. But I don't think the 
>"what if user change the setting with fwctl", "what if user reloaded
>the driver" corner cases should stop us from trying to get the core
>to implement 99% of the cases right.
>
>FTR I'm not aware of any Meta-internal products having permanent knobs.
>I just don't think we can depend on the random people that submit
>drivers these days to get this right. And devlink users will assume
>that it's Linux that sucks if it doesn't work right, not vendor X.
>
>Long story short I think we can add the reporting of both values via GET
>but I'd definitely still like to see the core trying to do the tracking
>automatically.

I agree with tracking in core, allowing driver to opt-in with pending
flag value if it knows better overriding the core value.