Date:   Thu, 10 Jan 2019 14:45:55 +0100
From:   Simon Horman <simon.horman@...ronome.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Bartek Kois <bartek.kois@...il.com>,
        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 Sun, Jan 06, 2019 at 09:44:30AM -0500, Jamal Hadi Salim wrote:
> 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.

FWIW, flower's lookup is hash-based so I would expect it to do better
than O(n). 

> 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

That sounds more like fixing a long standing regression than a new feature
to me. I'm not sure that I understand what it would be pushing.

Powered by blists - more mailing lists