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?
