netdev - Re: Problem with queuing vlan tagged packets after migration from 3.16.0 to 4.9.0

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]

Message-ID: <0ac07009-2add-a35c-d5b4-0b3f3b699d49@mojatatu.com>
Date:   Sun, 6 Jan 2019 09:44:30 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Cong Wang <xiyou.wangcong@...il.com>,
        Bartek Kois <bartek.kois@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: Problem with queuing vlan tagged packets after migration from
 3.16.0 to 4.9.0

On 2019-01-05 12:03 a.m., Cong Wang wrote:
> (Cc'ing Jamal)
> 
> On Fri, Jan 4, 2019 at 10:11 AM Bartek Kois <bartek.kois@...il.com> wrote:
>>
>> Basically my current scenario looks like this:
>> - router with eth0 as WAN and eth1 as LAN with 10-20 vlans,
>> - around 1000-2000 ip addresses in differnets subnets behind router (on
>> the LAN side),
>> - QoS made with tc + ifb (for upload queuing) + hasing filters (for
>> performance reasons)
>>
>> Moving this to two queuing trees (one on vlan and one on ifbx) per each
>> vlan makes this really hard to configure, but not impossible as long as
>> I can redirect single VLAN to ifb (don`t know if that is possible).
>> Anton suggested to use iptables+ipset but I don`t think that would be a
>> good idea to do that in scenario with so many queues.
> 
> Yeah, understood.
> 

Classifying vlans via u32 has been broken for some time (4 years or
more?). Some change made on general vlan handling (I think patch was
from Jiri). So someone like Bartek would be in for a suprise
if hasnt upgraded since.

vlan info is part of the skb metadata now and not visible
in the packet data. To extract it youd have to use the inlines
in include/linux/if_vlan.h(skb_vlan_tag_get() and friends)

> Perhaps we should just export this offset via a u32 filter dump,
> so that user-space could at least know the offset of IP header.

I didnt follow this part.

> However, for transport header, we still can't do anything here, as
> we can't predict whether an IP packet contains IP options.
>

u32 does allow you to add rules that interpret transport header
offsets from ip header (hashkey semantics for example etc)

> On the other hand, you can try other filters, for example, flower
> filter should work well with VLAN too, although it is probably not as
> fast as u32.

If Bartek's goal is performance - flower at a host level would be
the wrong thing to use.

How about the following untested example using two rules:
The first one matches the vlan with basic and allows
lookup to continue to lower priority u32 rule.
The second low prio rule would match the proper IP header.

#match vlan id 561
sudo $TC filter add dev $DEV parent ffff: protocol 802.1Q prio 1 \
   basic match 'meta(vlan mask 0xfff eq 0x0231)' \
   classid 1:1 \
   action continue
sudo $TC filter add dev $DEV parent ffff: protocol 802.1Q prio 1 \
   u32 match ip src 10.0.0.0/24 blah blah

You can use flower instead of basic but such one offs basic would
be more effective.
Bartek, if you say you have 20 vlans: worst case scenario
here is you are going to do 20 lookups (with basic) before
hitting the efficient u32 lookup alg and of course best case
is 1 lookup.

We can add vlan as a metadata extension to u32 - just
like skb->mark is. It will help to remove the need for basic
filter but may be pushing it

cheers,
jamal