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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <eea67d27-f09a-df22-090e-f6f121bc55af@mellanox.com>
Date:   Sun, 3 Feb 2019 08:26:23 +0000
From:   Paul Blakey <paulb@...lanox.com>
To:     Marcelo Leitner <mleitner@...hat.com>
CC:     Paul Blakey <paulb@...lanox.com>, Guy Shattah <sguy@...lanox.com>,
        Aaron Conole <aconole@...hat.com>,
        John Hurley <john.hurley@...ronome.com>,
        Simon Horman <simon.horman@...ronome.com>,
        Justin Pettit <jpettit@....org>,
        Gregory Rose <gvrose8192@...il.com>,
        Eelco Chaudron <echaudro@...hat.com>,
        Flavio Leitner <fbl@...hat.com>,
        Florian Westphal <fwestpha@...hat.com>,
        Jiri Pirko <jiri@...nulli.us>, Rashid Khan <rkhan@...hat.com>,
        Sushil Kulkarni <sukulkar@...hat.com>,
        Andy Gospodarek <andrew.gospodarek@...adcom.com>,
        Roi Dayan <roid@...lanox.com>,
        Yossi Kuperman <yossiku@...lanox.com>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Rony Efraim <ronye@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH net-next 1/6 v2] net/sched: Introduce act_ct



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.

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