On 17-07-28 11:13 AM, David Ahern wrote:

On 7/28/17 9:04 AM, Jamal Hadi Salim wrote:

quoted

Kernel side checking for device ifindex must know what a device
ifindex means.
That doesnt disqualify that the generic code checks that it
is of the same size as a signed 32b, etc. That is generic
stuff that can be factored out.

In this case:
Checking for whether bits selected are in the allowed range
that the kernel understands, that the bit value are set in
the right bit position, that the bits set in the correct bit
value position are also selected in the transaction.
That is generic code (which the content validation does).

Create a helper function then. It's the validation of attribute content
in 2 places that I object to. 1 attribute is validated in 1 place
(generic infra for this bitfield attribute), the others are validated in
line. Asymmetric validations is not a good design.

What is not generic in what is being validated though?
Content validation is when you say "This bit means the sky is blue".
You have to know the meaning of the bit.
The generic validation is opaque; "this bit is not supposed to be here".
I would argue that "this is bit is not supposed to be here unless
this other bit is present" is had to generalize but would be infra
as well (why i provided the ops for it, but removed it so we could
move on).

cheers,
jamal