On 7/28/17 7:51 AM, Jamal Hadi Salim wrote:
> On 17-07-25 10:41 AM, David Ahern wrote:
>> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote:
>>> In the most basic form, the user specifies the attribute policy as:
>>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data =
>>> &myvalidflags },
>>>
>>> where myvalidflags is the bit mask of the flags the kernel understands.
>>>
>>> If the user _does not_ provide myvalidflags then the attribute will
>>> also be rejected.
>>
>> No other netlink attribute has this requirement.
>
> This is the first one where we have to inspect content. We add things
> when we need them - as in this case.
Sure, the validation is required. My argument is that the validation
should be done where other attributes are validated -- inline with its
use. Nothing about this new bitfield says it must have a generic
validation code.
>
>> Users of the attributes
>> are the only ones that know if a value is valid or not (e.g, attribute
>> passing a device index) and those are always checked in line.
>
> It doesnt make sense that every user of the API has to repeat that
> validation code. Same principle as someone specifying that a type is
> u32 and have the nla validation check it. At some point we never had
> the u32 validation code. Then it was factored out because everyone
> repeats the same boilerplate code.
Every user of an attribute that uses a device index must verify the
device index is valid. The same code is repeated over and over.
Now you are suggesting to have 1 attribute whose content is validated by
generic infra and the rest are validated inline by the code using it. I
believe it is wrong and going to lead to problems.
