Tue, Nov 11, 2025 at 04:26:34AM +0100, [email protected] 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... >
