netdev - Re: [net-next PATCH 7/7] net: ixgbe: add support for tc

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:	Wed, 3 Feb 2016 11:02:45 -0800
From:	"Fastabend, John R" <john.fastabend@...il.com>
To:	Jamal Hadi Salim <jhs@...atatu.com>, "Amir Vadai\"" <amir@...ai.me>
Cc:	ogerlitz@...lanox.com, jiri@...nulli.us,
	jeffrey.t.kirsher@...el.com, netdev@...r.kernel.org,
	davem@...emloft.net
Subject: Re: [net-next PATCH 7/7] net: ixgbe: add support for tc_u32 offload

On 2/3/2016 4:46 AM, Jamal Hadi Salim wrote:
[...]

>>>>
>>>
>>> What are you doing w.r.t priorities? Are the filters processed by the
>>> order of the priorities?
>>>

In the same order as software processes filters. I tried to faithfully
translate the u32 classify and  u32_change loops into hardware. If I
missed some case its a bug and I'll fix it. My reading and experiments
of u32 indicate the ht:bucket:node build a handle and this is how we
order rules.

When I test this I create a veth pair and create the same rule set on
the veth pair as my hardware netdev. Then inject a skb into both
the veth and hardware netdev if you get different results its a bug.

>>
>> The rules are put in order by the handles which is populated in
>> my command above such that 'ht 1: order 3' gives handle 1::3 and
>> 'ht 800: order 1' gives 800::1. Take a look at this block in cls_u32
>>
>>          if (err == 0) {
>>                  struct tc_u_knode __rcu **ins;
>>                  struct tc_u_knode *pins;
>>
>>                  ins = &ht->ht[TC_U32_HASH(handle)];
>>                  for (pins = rtnl_dereference(*ins); pins;
>>                       ins = &pins->next, pins = rtnl_dereference(*ins))
>>                          if (TC_U32_NODE(handle) <
>> TC_U32_NODE(pins->handle))
>>                                  break;
>>
>>                  RCU_INIT_POINTER(n->next, pins);
>>                  rcu_assign_pointer(*ins, n);
>>                  u32_replace_hw_knode(tp, n);
>>                  *arg = (unsigned long)n;
>>                  return 0;
>>
>>
>> If you leave ht and order off the tc cli I believe 'tc' just
>> picks some semi-arbitrary ones for you. I've been in the habit
>> of always specifying them even for software filters.
>>
> 
> The default table id is essentially 0x800. Default bucket is 0.
> "order" essentially is the filter id. And given you can link tables
> (Nice work John!); essentially the ht:bucket:nodeid is an "address" to
> a specific filter on a specific table and when makes sense a specific
> hash bucket. Some other way to look at it is as a way to construct
> a mapping to a TCAM key.

Sure as long as your mapping works/looks like the software only case
it doesn't matter how you do the hardware mapping and my guess is this
is going to be highly device specific. Even with the handful of devices
I have it looks different depending on the device.

Note if you don't do the table links there really is no safe
way to get into the headers beyond the IP header because you need the
nexthdr stuff. Also just to stick this out there I have a patch to let
cls_u32 start processing at the ethernet header instead of the
transport header similar to how bpf works. This negative offset business
doesn't really work as best I can tell.

> What John is doing is essentially taking the nodeid and trying to use
> it as a priority. In otherwise the abstraction is reduced to a linked
> list in which the ordering is how the list is traversed.
> It may work in this case, but i am for being able to explicitly specify
> priorities.

But that doesn't exist in software today. If users want explicit order
today they build the ht, nodeid out correctly just use that. If you
are working on a TCAM just use the nodeid that should be equivalent to
priority.

And  although the ixgbe supports only a single table the mapping on more
complex devices will take it onto multiple tables if that optimizes
things.

Note I need to do a couple fixes on my existing code one to abort when
given a bad ifindex and two to hard abort when a hashtable is given
a higher order rule that I can't support. Both are fairly small tweaks
to the ixgbe code not to the infrastructure.

> 
> cheers,
> jamal
>