[Repost without HTML; sorry about that]
On Sun, Feb 03, 2019 at 08:26:23AM +0000, Paul Blakey wrote:
>
>
> On 01/02/2019 15:23, Marcelo Leitner wrote:
> > On Tue, Jan 29, 2019 at 10:02:01AM +0200, Paul Blakey wrote:
> > ...
> >> diff --git a/include/uapi/linux/tc_act/tc_ct.h
> >> b/include/uapi/linux/tc_act/tc_ct.h
> >> new file mode 100644
> >> index 0000000..6dbd771
> >> --- /dev/null
> >> +++ b/include/uapi/linux/tc_act/tc_ct.h
> >> @@ -0,0 +1,29 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> >> +#ifndef __UAPI_TC_CT_H
> >> +#define __UAPI_TC_CT_H
> >> +
> >> +#include <linux/types.h>
> >> +#include <linux/pkt_cls.h>
> >> +
> >> +#define TCA_ACT_CT 18
> >> +
> >> +struct tc_ct {
> >> + tc_gen;
> >> + __u16 zone;
> >> + __u32 labels[4];
> >> + __u32 labels_mask[4];
> >> + __u32 mark;
> >> + __u32 mark_mask;
> >> + bool commit;
> >
> > This is one of the points that our implementations differs. You used a
> > struct and wrapped it into TCA_CT_PARMS attribute, while I broke it up
> > into several attributes.
> >
> > cls_flower and act_bpf, for example, doesn't use structs, but others
> > do.
> >
> > Both have pros and cons and I imagine this topic probably was already
> > discussed but I'm not aware of a recommendation. Do we have one?
>
> I guess flower uses a netlink attribute per key attribute because
> a lot of time, most of them won't be used, and you would send less.
> we can have ct, ct + snat, ct + dnat, zone and mark.... a lot of this
> won't be used sometimes.
>
> Also you can't add nested attributes to the struct easily.
>
> Also netlink attributes can be tested for existence, while a struct
> would need a special non valid value, or another field to specify which
> fields are used.
>
> both are hard to test if a requested attribute was ignored, besides
> checking the netlink echo or dumping the action back. if for example a
> older kernel module and newer userspace uses a attribute above
> enum TCA_CT_MAX (struct attributes also don't have max len, in nla_parse).
>
>
> All in all, I think mostly netlink attributes would be better.
+1
I believe that Flower uses more attributes because its regarded as being
more flexible and that benefit outweighs the extra cost - f.e. the netlink
messages would tend to be a bit larger if a struct was used.
>
> >
> >> +};
> >> +
> >> +enum {
> >> + TCA_CT_UNSPEC,
> >> + TCA_CT_PARMS,
> >> + TCA_CT_TM,
> >> + TCA_CT_PAD,
> >> + __TCA_CT_MAX
> >> +};
> >> +#define TCA_CT_MAX (__TCA_CT_MAX - 1)
> >> +
> >> +#endif /* __UAPI_TC_CT_H */
> > ...
> >
