Re: [RFC iproute2 0/6] devlink: add policy check for all attributes

[Jiri Pirko <jiri@nvidia.com>]

Mon, Aug 08, 2022 at 07:02:49PM CEST, jacob.e.keller@intel.com wrote:
>
>
>> -----Original Message-----
>> From: Jiri Pirko <jiri@nvidia.com>
>> Sent: Monday, August 08, 2022 3:32 AM
>> To: Keller, Jacob E <jacob.e.keller@intel.com>
>> Cc: netdev@vger.kernel.org; Jonathan Corbet <corbet@lwn.net>; David S. Miller
>> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski
>> <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David Ahern
>> <dsahern@kernel.org>; Stephen Hemminger <stephen@networkplumber.org>
>> Subject: Re: [RFC iproute2 0/6] devlink: add policy check for all attributes
>> 
>> Sat, Aug 06, 2022 at 01:41:49AM CEST, jacob.e.keller@intel.com wrote:
>> 
>> 
>> [...]
>> 
>> >This is intended to eventually go along with improvements to the policy
>> >reporting in devlink kernel code to report separate policy for each command.
>> 
>> Can you explain this a bit please?
>
>Currently devlink only reports a global policy which is the same for every command. This means that commands like DEVLINK_CMD_FLASH report the same attributes as valid as other commands like DEVLINK_CMD_INFO_GET. The policy (if the command is strict) would only require that attributes be one of the known attributes according to the general devlink policy.
>
>However, none of the commands accept or honor all the attributes. The netlink policy support allows each command to report an individual policy that would only include the attributes which the command uses and would honor the meaning of.
>
>Without per-command policy, there is no way for user space to tell when the kernel changed support for some attribute (such as the DEVLINK_ATTR_DRY_RUN I recently proposed). Yes, you can use maxattr to determine of the kernel knows about the attribute, but that doesn't guarantee that every command supports it.
>
>The per-command policy would report only the subset of attributes supported by the command. In strict validation, this would also make the kernel reject commands which tried to send other attributes. Ideally we would also have nested attribute policy, so each nested attribute would express the policy for the subset of attributes which are valid within that nest as well.
>
>By adding policy checking support to user space, we can make sure that at least iproute2 userspace won't accidentally send an unsupported attribute to a command, but that only works once the policy for the command in the kernel is updated to list the specific policy. Right now, this will effectively get the generic policy and indicate that all known attributes in the policy table are accepted.
>
>Note that this means strict validation on its own is not enough.  It doesn't really matter if a command is set to strict validation when the validation still allows every attribute in the general policy, regardless of whether the command currently does anything with the attribute. Any of the unsupported ones get silently ignored, with no warning to the user.
>
>Related to this, also think we should determine a plan for how to migrate devlink to strict validation, once the per-command policy is defined and implemented. However, I am not sure what the backwards-compatibility issues exist for switching.
>
>Hope this explains things,

It is, thanks.

Do you have patches for the per-cmd policy?


>Jake