On 06/20/2018 07:53 AM, Jamal Hadi Salim wrote:
On 19/06/18 08:39 AM, Michel Machado wrote:
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
