Sat, Nov 08, 2025 at 07:14:45AM +0100, [email protected] 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 <[email protected]> >> --- >> >> 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 [...]
