Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit

On 06/20/2018 07:53 AM, Jamal Hadi Salim wrote:

On 19/06/18 08:39 AM, Michel Machado wrote:

quoted

 Notice that, different from skbmod, there's no field parm->flags in 
skbedit. Skbedit infers the flags in d->flags from the presence of the 
parameters of each of its actions. But SKBEDIT_F_INHERITDSFIELD has no 
parameter and adding field parm->flags breaks backward compatibility 
with user space as pointed out by Marcelo Ricardo Leitner. Our 
solution was to add TCA_SKBEDIT_FLAGS, so SKBEDIT_F_INHERITDSFIELD and 
future flag-only actions can be added.

Ok, that makes sense - thanks. I am not so sure about using
64 bits (32 bits would have been fine to match the size of
the kernel flags), but other than that LGTM.

    The choice for the u64 is meant to keep the interface between kernel 
and user space the same for hopefully a longer time than it would be 
with a u32. Changing from u32 to u64 in the kernel, when the need 
arrives, won't impact applications. This interface choice was motivated 
by the backward compatibility issue mentioned above.

    Thank you for the review, Jamal.

[ ]'s
Michel Machado