Re: [RFC PATCH net-next 1/6 v2] net/sched: Introduce act_ct

[Marcelo Leitner <mleitner@redhat.com>]

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?

> +};
> +
> +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 */
...