netdev - Re: [RFC PATCH 3/7] net: sched: add cls

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 02 Feb 2016 08:42:46 -0800
From:	John Fastabend <john.fastabend@...il.com>
To:	Or Gerlitz <gerlitz.or@...il.com>
CC:	Anjali Singhai Jain <anjali.singhai@...el.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	Jamal Hadi Salim <jhs@...atatu.com>, ast@...com,
	"Skidmore, Donald C" <donald.c.skidmore@...el.com>,
	horms@...ge.net.au, Linux Netdev List <netdev@...r.kernel.org>,
	Thomas Graf <tgraf@...g.ch>,
	David Miller <davem@...emloft.net>,
	Jiri Pirko <jiri@...lanox.com>
Subject: Re: [RFC PATCH 3/7] net: sched: add cls_u32 offload hooks for netdevs

On 16-02-02 08:25 AM, Or Gerlitz wrote:
> On Mon, Feb 1, 2016 at 3:50 AM, John Fastabend <john.fastabend@...il.com> wrote:
>> This patch allows netdev drivers to consume cls_u32 offloads via
>> the ndo_setup_tc ndo op.
>>
>> This works aligns with how network drivers have been doing qdisc
>> offloads for mqprio.
> 
> [...]
> 
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -779,17 +779,21 @@ static inline bool netdev_phys_item_id_same(struct netdev_phys_item_id *a,
>>  typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>                                        struct sk_buff *skb);
>>
>> -/* This structure holds attributes of qdisc and classifiers
>> +/* These structures hold the attributes of qdisc and classifiers
>>   * that are being passed to the netdevice through the setup_tc op.
>>   */
>>  enum {
>>         TC_SETUP_MQPRIO,
>> +       TC_SETUP_CLSU32,
>>  };
>>
>> +struct tc_cls_u32_offload;
>> +
>>  struct tc_to_netdev {
>>         unsigned int type;
>>         union {
>>                 u8 tc;
>> +               struct tc_cls_u32_offload *cls_u32;
>>         };
>>  };
> 
> So under this approach we're making the HW driver u32 aware. Do we
> really want to go there?
> 

Yes, I'm not convinced writing the universal language X to arbitrary
hardware is worth the complexity/cost at the moment. I already started
writing this universal block of code and it gets a bit complex to do
it right. Anyways none of this is exposed via UAPI so it can be
conslidated reworked as needed. Also I'm not to keen on going from
tc/netfilter/etc to language X (hw IR) to hardware when the block of
code to jump from u32 or flower to hardware is so simple. I added
flower support to the driver with about 100lines of code fwiw I'll
send the patch out later today, sure I skipped populating all the
fields by breaking out of some case statements but not that many.

I don't mind opening up some helper functions if you like my backend
structures. But anyways most the hard work is programming the hardware
and hoping someone did silicon validation anyways imo.

> The flow-dissector + actions structure way of describing matching and
> actions maybe had some
> drawbacks but it's not affiliated with a specific networking component
> (here TC/U32). When we look
> fwd do we expect everything (netfilter offloads for example) to be
> expressed in u32 terms?

I'm a bit tired of speculating about what_ifs when we see the netfilter
offload code lets take a look at consolidating. For now I have code that
_works_.

> 
> Or.
>