Problem with queuing vlan tagged packets after migration from 3.16.0 to 4.9.0

Problem with queuing vlan tagged packets after migration from 3.16.0 to 4.9.0

[Bartek Kois]

Hi,
I`ve got problem while queuing with HFSC vlan tagged packets after 
migrating my tc scripts from Debian 8.2 (3.16.0-4-amd64) to Debian 9.5 
(4.9.0-6-amd64). tc filters added to eth1 do not classify correctly src 
and dst ip addresses anymore if they are encapsulated with vlan tag 
which wasn`t a problem previously. It works fine if I run them without 
vlan tagging oraz if the root device is a vlan (eg. tc filter add dev 
eth1.20). Could you please help me find out what has changed in kernel 
between those two versions and what is walkaround for that problem?

Example of my classification filters:

tc filter add dev eth1 parent 1:0 prio 4 protocol ip u32
tc filter add dev eth1 parent 1:0 prio 4 handle ${NETWORK_GROUP_HEX}: 
protocol ip u32 divisor 256
tc filter add dev eth1 protocol ip parent 1:0 prio 4 u32 ht 800:: match 
ip dst ${NETWORK_ADDRESS}/24 hashkey mask 0x000000ff at 16 link 
${NETWORK_GROUP_HEX}:
tc filter add dev eth1 parent 1:0 protocol ip prio 4 u32 ht 
${NETWORK_GROUP_HEX}:0x${ADDR_Q4_HEX} match ip dst $ADDR classid 
1:${MARK_NORMAL}


Best regards
Bartek Kois

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

[Cong Wang]

Hello,

On Sat, Dec 29, 2018 at 11:54 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>
>
> Hi,
> I`ve got problem while queuing with HFSC vlan tagged packets after
> migrating my tc scripts from Debian 8.2 (3.16.0-4-amd64) to Debian 9.5
> (4.9.0-6-amd64). tc filters added to eth1 do not classify correctly src
> and dst ip addresses anymore if they are encapsulated with vlan tag
> which wasn`t a problem previously. It works fine if I run them without
> vlan tagging oraz if the root device is a vlan (eg. tc filter add dev
> eth1.20). Could you please help me find out what has changed in kernel
> between those two versions and what is walkaround for that problem?

Does this problem still exist on some latest kernel? 4.9 is still too old
for upstream to be interesting. :(

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

[Bartek Kois]

Hi
I haven`t tested any newer kernels cause I thought that something 
related to packet classification has been changed permanently and I have 
to figure out what. Which one should I test?

Best regards
Bartek Kois

W dniu 30.12.2018 o 19:53, Cong Wang pisze:
> Hello,
>
> On Sat, Dec 29, 2018 at 11:54 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>>
>> Hi,
>> I`ve got problem while queuing with HFSC vlan tagged packets after
>> migrating my tc scripts from Debian 8.2 (3.16.0-4-amd64) to Debian 9.5
>> (4.9.0-6-amd64). tc filters added to eth1 do not classify correctly src
>> and dst ip addresses anymore if they are encapsulated with vlan tag
>> which wasn`t a problem previously. It works fine if I run them without
>> vlan tagging oraz if the root device is a vlan (eg. tc filter add dev
>> eth1.20). Could you please help me find out what has changed in kernel
>> between those two versions and what is walkaround for that problem?
> Does this problem still exist on some latest kernel? 4.9 is still too old
> for upstream to be interesting. :(

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

[Bartek Kois]

Hi,
I tested 4.20 and the problem remains (it is not possible to classify 
tagged packets if the root filter is on physical interface).

Best regards
Bartek Kois

W dniu 30.12.2018 o 22:14, Bartek Kois pisze:
> Hi
> I haven`t tested any newer kernels cause I thought that something 
> related to packet classification has been changed permanently and I 
> have to figure out what. Which one should I test?
>
> Best regards
> Bartek Kois
>
> W dniu 30.12.2018 o 19:53, Cong Wang pisze:
>> Hello,
>>
>> On Sat, Dec 29, 2018 at 11:54 AM Bartek Kois <bartek.kois@gmail.com> 
>> wrote:
>>>
>>> Hi,
>>> I`ve got problem while queuing with HFSC vlan tagged packets after
>>> migrating my tc scripts from Debian 8.2 (3.16.0-4-amd64) to Debian 9.5
>>> (4.9.0-6-amd64). tc filters added to eth1 do not classify correctly src
>>> and dst ip addresses anymore if they are encapsulated with vlan tag
>>> which wasn`t a problem previously. It works fine if I run them without
>>> vlan tagging oraz if the root device is a vlan (eg. tc filter add dev
>>> eth1.20). Could you please help me find out what has changed in kernel
>>> between those two versions and what is walkaround for that problem?
>> Does this problem still exist on some latest kernel? 4.9 is still too 
>> old
>> for upstream to be interesting. :(

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

[Cong Wang]

On Mon, Dec 31, 2018 at 10:13 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>
> Hi,
> I tested 4.20 and the problem remains (it is not possible to classify
> tagged packets if the root filter is on physical interface).

Hmm, I guess it is because the offset used by u32 filter is no
longer accurate when vlan tag is inserted into mac header.

On egress side, skb->data points to the mac header, so the offset
of IP header is different when vlan tag is involved.

Did this really work before? I don't follow vlan changes, it seems
it has been already like this for a long time.

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

[Bartek Kois]

Hi
Yes it did work since I remember (like around 2.4.x) and it changed 
since I moved from Debian 8 to 9. I would appreciate fixing that in the 
future beacuse it is essential for queueing traffic on the routers, but 
the question is why these filters don`t work in that case:

     tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match 
u32 0x0a000c08 0xffffffff at 20 classid 1:2001      # for 10.0.12.8 ip 
address
     tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match 
u32 0x0a000c09 0xffffffff at 20 classid 1:2002      # for 10.0.12.9 ip 
address
     tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match 
u32 0x0a000c10 0xffffffff at 20 classid 1:2003      # for 10.0.12.10 ip 
address

I`ve changed "at 16" which works without vlan tags to "at 20" to take 
vlan tag into account.

Best regards
Bartek Kois

W dniu 01.01.2019 o 20:33, Cong Wang pisze:
> On Mon, Dec 31, 2018 at 10:13 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>> Hi,
>> I tested 4.20 and the problem remains (it is not possible to classify
>> tagged packets if the root filter is on physical interface).
> Hmm, I guess it is because the offset used by u32 filter is no
> longer accurate when vlan tag is inserted into mac header.
>
> On egress side, skb->data points to the mac header, so the offset
> of IP header is different when vlan tag is involved.
>
> Did this really work before? I don't follow vlan changes, it seems
> it has been already like this for a long time.

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

[Cong Wang]

On Tue, Jan 1, 2019 at 11:46 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>
> Hi
> Yes it did work since I remember (like around 2.4.x) and it changed
> since I moved from Debian 8 to 9. I would appreciate fixing that in the
> future beacuse it is essential for queueing traffic on the routers, but
> the question is why these filters don`t work in that case:
>
>      tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
> u32 0x0a000c08 0xffffffff at 20 classid 1:2001      # for 10.0.12.8 ip
> address
>      tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
> u32 0x0a000c09 0xffffffff at 20 classid 1:2002      # for 10.0.12.9 ip
> address
>      tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
> u32 0x0a000c10 0xffffffff at 20 classid 1:2003      # for 10.0.12.10 ip
> address
>
> I`ve changed "at 16" which works without vlan tags to "at 20" to take
> vlan tag into account.

Yeah, this confirms my speculation.

The problem is essentially a design flaw of u32 filter, the IP header
and TCP header offsets are never fixed, for example VLAN tagging and
IP options. What's more, it is not easy for user-space to learn the offset
for different packets as it requires to parse into each packets.

I don't know whether we can fix this either, VLAN call path probably
already makes assumptions on the current skb->data position, if
we "fix" it for u32, it would probably break other things.

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

[Bartek Kois]

Hi
1. What exactly caused this change in the kernel?
2. I don`t understand why adding VLAN tag, which is just 4  additional 
bytes to the passing packet make it impossible to classify.
3. This whole thing makes the QoS under Linux routers hard to configure 
in scenarios with more than one VLAN which is pretty much every slightly 
bigger router nowadays especially if we use IFB and hashing filters. Is 
there any walkaround for that problem?

Best regards
Bartek Kois

W dniu 03.01.2019 o 04:30, Cong Wang pisze:
> On Tue, Jan 1, 2019 at 11:46 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>> Hi
>> Yes it did work since I remember (like around 2.4.x) and it changed
>> since I moved from Debian 8 to 9. I would appreciate fixing that in the
>> future beacuse it is essential for queueing traffic on the routers, but
>> the question is why these filters don`t work in that case:
>>
>>       tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
>> u32 0x0a000c08 0xffffffff at 20 classid 1:2001      # for 10.0.12.8 ip
>> address
>>       tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
>> u32 0x0a000c09 0xffffffff at 20 classid 1:2002      # for 10.0.12.9 ip
>> address
>>       tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
>> u32 0x0a000c10 0xffffffff at 20 classid 1:2003      # for 10.0.12.10 ip
>> address
>>
>> I`ve changed "at 16" which works without vlan tags to "at 20" to take
>> vlan tag into account.
> Yeah, this confirms my speculation.
>
> The problem is essentially a design flaw of u32 filter, the IP header
> and TCP header offsets are never fixed, for example VLAN tagging and
> IP options. What's more, it is not easy for user-space to learn the offset
> for different packets as it requires to parse into each packets.
>
> I don't know whether we can fix this either, VLAN call path probably
> already makes assumptions on the current skb->data position, if
> we "fix" it for u32, it would probably break other things.

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

[Cong Wang]

On Thu, Jan 3, 2019 at 7:25 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>
> Hi
> 1. What exactly caused this change in the kernel?


I don't follow VLAN changes, I guess it must be some change which
inserts the VLAN tag before this ->ndo_start_xmit().


> 2. I don`t understand why adding VLAN tag, which is just 4  additional
> bytes to the passing packet make it impossible to classify.

It is possible, you just have to specify the offset manually, as the
iproute2 can't detect the offset of IP header in this case. So it is just
inconvenient.


> 3. This whole thing makes the QoS under Linux routers hard to configure
> in scenarios with more than one VLAN which is pretty much every slightly
> bigger router nowadays especially if we use IFB and hashing filters. Is
> there any walkaround for that problem?
>

Just move these filters from physical device to the VLAN device instead?


Thanks.

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

[Bartek Kois]

Hi

W dniu 03.01.2019 o 21:44, Cong Wang pisze:
> On Thu, Jan 3, 2019 at 7:25 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>> Hi
>> 1. What exactly caused this change in the kernel?
>
> I don't follow VLAN changes, I guess it must be some change which
> inserts the VLAN tag before this ->ndo_start_xmit().
>
>
>> 2. I don`t understand why adding VLAN tag, which is just 4  additional
>> bytes to the passing packet make it impossible to classify.
> It is possible, you just have to specify the offset manually, as the
> iproute2 can't detect the offset of IP header in this case. So it is just
> inconvenient.
>
>> 3. This whole thing makes the QoS under Linux routers hard to configure
>> in scenarios with more than one VLAN which is pretty much every slightly
>> bigger router nowadays especially if we use IFB and hashing filters. Is
>> there any walkaround for that problem?
>>
> Just move these filters from physical device to the VLAN device instead?
>

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.

Best reagrds
Bartek Kois

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

[Cong Wang]

(Cc'ing Jamal)

On Fri, Jan 4, 2019 at 10:11 AM Bartek Kois <bartek.kois@gmail.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.

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.
However, for transport header, we still can't do anything here, as
we can't predict whether an IP packet contains IP options.

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.

Thanks.

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

[Jamal Hadi Salim]

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@gmail.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

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

[Simon Horman]

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@gmail.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.

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

[Jamal Hadi Salim]

On 2019-01-10 8:45 a.m., Simon Horman wrote:
> On Sun, Jan 06, 2019 at 09:44:30AM -0500, Jamal Hadi Salim wrote:

>> 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).
> 

Referring more to a large number of filters(N) in particular:
Flower will perform at O(N) to find a match (not sure
you can make it perform better than O(N)) but u32
can made to perform at _much better_ than O(logN);
i havent looked closely at Bartek's arrangement but cursory inspection
indicates he is using such an arrangement for performance.

>> 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.
> 

It will be a new feature in the sense the user will have to specify
something like (adding "mark" for clarify):

tc filter add .... protocol 802.1q .. u32 \
match u32 0 0 \
mark 15 \
vlanid 1234
action vlan pop reclassify

As opposed to what used to work before which would
have matched at packet offset.

If there is agreement (lots of bikeshedding last time)
i or someone else can whip a patch for this.

cheers,
jamal

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

[Cong Wang]

On Sat, Jan 12, 2019 at 4:12 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> It will be a new feature in the sense the user will have to specify
> something like (adding "mark" for clarify):
>
> tc filter add .... protocol 802.1q .. u32 \
> match u32 0 0 \
> mark 15 \
> vlanid 1234
> action vlan pop reclassify

This is clearly a new feature, however, it only solves one part of
the problem, the length of IPv4 header is still various and there
is no way to predict it.

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

[Jamal Hadi Salim]

On 2019-01-13 1:22 p.m., Cong Wang wrote:
> On Sat, Jan 12, 2019 at 4:12 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>>
>> It will be a new feature in the sense the user will have to specify
>> something like (adding "mark" for clarify):
>>
>> tc filter add .... protocol 802.1q .. u32 \
>> match u32 0 0 \
>> mark 15 \
>> vlanid 1234
>> action vlan pop reclassify
> 
> This is clearly a new feature, however, it only solves one part of
> the problem, the length of IPv4 header is still various and there
> is no way to predict it.


As i mentioned in my earlier email, that feature should work.
What is broken?

cheers,
jamal

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

[Cong Wang]

On Tue, Jan 15, 2019 at 7:09 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On 2019-01-13 1:22 p.m., Cong Wang wrote:
> > On Sat, Jan 12, 2019 at 4:12 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >>
> >> It will be a new feature in the sense the user will have to specify
> >> something like (adding "mark" for clarify):
> >>
> >> tc filter add .... protocol 802.1q .. u32 \
> >> match u32 0 0 \
> >> mark 15 \
> >> vlanid 1234
> >> action vlan pop reclassify
> >
> > This is clearly a new feature, however, it only solves one part of
> > the problem, the length of IPv4 header is still various and there
> > is no way to predict it.
>
>
> As i mentioned in my earlier email, that feature should work.
> What is broken?

I never say it is broken. I am saying you only fix vlan case by
introducing "vlanid XXX" and leave IPv4 option cases unfixed.
As I keep saying, this is a kinda design flaw of u32 probably
can't be workarounded.

Thanks.

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

[Jamal Hadi Salim]

On 2019-01-15 1:19 p.m., Cong Wang wrote:

> 
> I never say it is broken. I am saying you only fix vlan case by
> introducing "vlanid XXX" and leave IPv4 option cases unfixed.


Let me see if we can get clarity.

IP options or TCP options typically work. You teach u32 what
to use to compute the next header. Are they now broken?

Maybe you mean that if one was to look for ip options in a
vlan tagged packet they wont find it? If yes, I would account
that to the brokenness of the vlans that we have now. But
once the vlan is popped the rule looking for protocol
ip should work. Is that broken now?

Note: Typically you will look for ip options
when you match an ip packet not when you match a vlan id etc.

> As I keep saying, this is a kinda design flaw of u32 probably
> can't be workarounded.

It is still useful to be able to match vlan ids using u32.

cheers,
jamal

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

[Simon Horman]

On Sat, Jan 12, 2019 at 07:12:34AM -0500, Jamal Hadi Salim wrote:
> On 2019-01-10 8:45 a.m., Simon Horman wrote:
> > On Sun, Jan 06, 2019 at 09:44:30AM -0500, Jamal Hadi Salim wrote:
> 
> > > 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).
> > 
> 
> Referring more to a large number of filters(N) in particular:
> Flower will perform at O(N) to find a match (not sure
> you can make it perform better than O(N)) but u32
> can made to perform at _much better_ than O(logN);
> i havent looked closely at Bartek's arrangement but cursory inspection
> indicates he is using such an arrangement for performance.

For all filters are at the same priority with the same mask
Flower will use a hash lookup which I expect that would be better than
O(N). That was the only point I wanted to make.

> 
> > > 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.
> > 
> 
> It will be a new feature in the sense the user will have to specify
> something like (adding "mark" for clarify):
> 
> tc filter add .... protocol 802.1q .. u32 \
> match u32 0 0 \
> mark 15 \
> vlanid 1234
> action vlan pop reclassify
> 
> As opposed to what used to work before which would
> have matched at packet offset.

So the old feature is permanently broken?

> If there is agreement (lots of bikeshedding last time)
> i or someone else can whip a patch for this.

Backwards compatibility aside I have no objection.

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

[Jamal Hadi Salim]

On 2019-01-14 3:12 a.m., Simon Horman wrote:
> On Sat, Jan 12, 2019 at 07:12:34AM -0500, Jamal Hadi Salim wrote:
>> On 2019-01-10 8:45 a.m., Simon Horman wrote:
>>> On Sun, Jan 06, 2019 at 09:44:30AM -0500, Jamal Hadi Salim wrote:
>>

>>
>> As opposed to what used to work before which would
>> have matched at packet offset.
> 
> So the old feature is permanently broken?

Yes.

> 
>> If there is agreement (lots of bikeshedding last time)
>> i or someone else can whip a patch for this.
> 
> Backwards compatibility aside I have no objection.

Backwards compatibility should go back as far as
when it was broken. It wont work prio to that because
there was no vlan metadata on the skb.

cheers,
jamal

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

[Eric W. Biederman]

Jamal Hadi Salim <jhs@mojatatu.com> writes:

> 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@gmail.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.

Definitely more than 4. I was trying to sort pieces of this out maybe 6
years ago.  For a while vlan tags on software devices worked one way and
vlan tags on actual hardware work the way vlan tags work for everything
now.

Since every option involved breaking something we standardized on the
way hardware accelleration handled vlan tags.  Which caused some hiccups
on pure software devices.  That ``fix'' may have gone in after 3.16 and
be why Bartek is seeing issues.  Especially if his router had a very
lame ethernet device that did not support any hardware accelleration.

> 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)

Yes, and like you suggest down thread that will now require
a magic directive to read.  The upside is if you don't care about
the vlan tag your offsets are likely to be more constant now.

Eric

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

[Anton Danilov]

Hi.
There is the workaround - classify the packets with iptables+ipset -
it's enough fast and more friendly.

On Fri, 4 Jan 2019 at 00:21, Bartek Kois <bartek.kois@gmail.com> wrote:
>
> Hi
> 1. What exactly caused this change in the kernel?
> 2. I don`t understand why adding VLAN tag, which is just 4  additional
> bytes to the passing packet make it impossible to classify.
> 3. This whole thing makes the QoS under Linux routers hard to configure
> in scenarios with more than one VLAN which is pretty much every slightly
> bigger router nowadays especially if we use IFB and hashing filters. Is
> there any walkaround for that problem?
>
> Best regards
> Bartek Kois
>
> W dniu 03.01.2019 o 04:30, Cong Wang pisze:
> > On Tue, Jan 1, 2019 at 11:46 AM Bartek Kois <bartek.kois@gmail.com> wrote:
> >> Hi
> >> Yes it did work since I remember (like around 2.4.x) and it changed
> >> since I moved from Debian 8 to 9. I would appreciate fixing that in the
> >> future beacuse it is essential for queueing traffic on the routers, but
> >> the question is why these filters don`t work in that case:
> >>
> >>       tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
> >> u32 0x0a000c08 0xffffffff at 20 classid 1:2001      # for 10.0.12.8 ip
> >> address
> >>       tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
> >> u32 0x0a000c09 0xffffffff at 20 classid 1:2002      # for 10.0.12.9 ip
> >> address
> >>       tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
> >> u32 0x0a000c10 0xffffffff at 20 classid 1:2003      # for 10.0.12.10 ip
> >> address
> >>
> >> I`ve changed "at 16" which works without vlan tags to "at 20" to take
> >> vlan tag into account.
> > Yeah, this confirms my speculation.
> >
> > The problem is essentially a design flaw of u32 filter, the IP header
> > and TCP header offsets are never fixed, for example VLAN tagging and
> > IP options. What's more, it is not easy for user-space to learn the offset
> > for different packets as it requires to parse into each packets.
> >
> > I don't know whether we can fix this either, VLAN call path probably
> > already makes assumptions on the current skb->data position, if
> > we "fix" it for u32, it would probably break other things.



-- 
Anton.

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

[Bartek Kois]

Hi

Is this equally fast as hashing tables?

Best regards
Bartek Kois
W dniu 03.01.2019 o 22:49, Anton Danilov pisze:
> Hi.
> There is the workaround - classify the packets with iptables+ipset -
> it's enough fast and more friendly.
>
> On Fri, 4 Jan 2019 at 00:21, Bartek Kois <bartek.kois@gmail.com> wrote:
>> Hi
>> 1. What exactly caused this change in the kernel?
>> 2. I don`t understand why adding VLAN tag, which is just 4  additional
>> bytes to the passing packet make it impossible to classify.
>> 3. This whole thing makes the QoS under Linux routers hard to configure
>> in scenarios with more than one VLAN which is pretty much every slightly
>> bigger router nowadays especially if we use IFB and hashing filters. Is
>> there any walkaround for that problem?
>>
>> Best regards
>> Bartek Kois
>>
>> W dniu 03.01.2019 o 04:30, Cong Wang pisze:
>>> On Tue, Jan 1, 2019 at 11:46 AM Bartek Kois <bartek.kois@gmail.com> wrote:
>>>> Hi
>>>> Yes it did work since I remember (like around 2.4.x) and it changed
>>>> since I moved from Debian 8 to 9. I would appreciate fixing that in the
>>>> future beacuse it is essential for queueing traffic on the routers, but
>>>> the question is why these filters don`t work in that case:
>>>>
>>>>        tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
>>>> u32 0x0a000c08 0xffffffff at 20 classid 1:2001      # for 10.0.12.8 ip
>>>> address
>>>>        tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
>>>> u32 0x0a000c09 0xffffffff at 20 classid 1:2002      # for 10.0.12.9 ip
>>>> address
>>>>        tc filter add dev $LAN_ETH parent 1:0 protocol ip prio 4 u32 match
>>>> u32 0x0a000c10 0xffffffff at 20 classid 1:2003      # for 10.0.12.10 ip
>>>> address
>>>>
>>>> I`ve changed "at 16" which works without vlan tags to "at 20" to take
>>>> vlan tag into account.
>>> Yeah, this confirms my speculation.
>>>
>>> The problem is essentially a design flaw of u32 filter, the IP header
>>> and TCP header offsets are never fixed, for example VLAN tagging and
>>> IP options. What's more, it is not easy for user-space to learn the offset
>>> for different packets as it requires to parse into each packets.
>>>
>>> I don't know whether we can fix this either, VLAN call path probably
>>> already makes assumptions on the current skb->data position, if
>>> we "fix" it for u32, it would probably break other things.
>
>

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

[Jakub Kicinski]

On Sat, 29 Dec 2018 13:52:23 +0100, Bartek Kois wrote:
> Hi,
> I`ve got problem while queuing with HFSC vlan tagged packets after 
> migrating my tc scripts from Debian 8.2 (3.16.0-4-amd64) to Debian 9.5 
> (4.9.0-6-amd64). tc filters added to eth1 do not classify correctly src 
> and dst ip addresses anymore if they are encapsulated with vlan tag 
> which wasn`t a problem previously. It works fine if I run them without 
> vlan tagging oraz if the root device is a vlan (eg. tc filter add dev 
> eth1.20). Could you please help me find out what has changed in kernel 
> between those two versions and what is walkaround for that problem?

Could this be related to your device driver not stripping VLAN tags by
default any more?  Just a short in the dark..  Try:

$ ethtool -k lo | grep vlan

on working vs broken setup.  What is your HW/device driver?

> Example of my classification filters:
> 
> tc filter add dev eth1 parent 1:0 prio 4 protocol ip u32
> tc filter add dev eth1 parent 1:0 prio 4 handle ${NETWORK_GROUP_HEX}: 
> protocol ip u32 divisor 256
> tc filter add dev eth1 protocol ip parent 1:0 prio 4 u32 ht 800:: match 
> ip dst ${NETWORK_ADDRESS}/24 hashkey mask 0x000000ff at 16 link 
> ${NETWORK_GROUP_HEX}:
> tc filter add dev eth1 parent 1:0 protocol ip prio 4 u32 ht 
> ${NETWORK_GROUP_HEX}:0x${ADDR_Q4_HEX} match ip dst $ADDR classid 
> 1:${MARK_NORMAL}
> 
> 
> Best regards
> Bartek Kois

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

[Bartek Kois]

Witam
Working setup (driver e1000e):
# ethtool -k eth1 | grep vlan
rx-vlan-offload: on
tx-vlan-offload: on
rx-vlan-filter: on [fixed]
vlan-challenged: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]


Broken setup (driver e1000e):
# ethtool -k eth1 | grep vlan
rx-vlan-offload: on
tx-vlan-offload: on
rx-vlan-filter: on [fixed]
vlan-challenged: off [fixed]
tx-vlan-stag-hw-insert: off [fixed]
rx-vlan-stag-hw-parse: off [fixed]
rx-vlan-stag-filter: off [fixed]

The same happens in case of ixgbe driver (tested on different 
machine).I`ve been using this for several years and all of the sudden it 
stops working properly. I`ve tried to use u32 classifier with value and 
mask to match ip address on 16 or 20 byte (in case it contains 
additional 4 bytes of vlan tag) to check if it will work with no luck. 
Browsing the internet I found this notation: "protocol 802.1q", but it 
doesn`t work on my system.

Pozdrawiam
Bartek Kois

W dniu 31.12.2018 o 22:47, Jakub Kicinski pisze:
> On Sat, 29 Dec 2018 13:52:23 +0100, Bartek Kois wrote:
>> Hi,
>> I`ve got problem while queuing with HFSC vlan tagged packets after
>> migrating my tc scripts from Debian 8.2 (3.16.0-4-amd64) to Debian 9.5
>> (4.9.0-6-amd64). tc filters added to eth1 do not classify correctly src
>> and dst ip addresses anymore if they are encapsulated with vlan tag
>> which wasn`t a problem previously. It works fine if I run them without
>> vlan tagging oraz if the root device is a vlan (eg. tc filter add dev
>> eth1.20). Could you please help me find out what has changed in kernel
>> between those two versions and what is walkaround for that problem?
> Could this be related to your device driver not stripping VLAN tags by
> default any more?  Just a short in the dark..  Try:
>
> $ ethtool -k lo | grep vlan
>
> on working vs broken setup.  What is your HW/device driver?
>
>> Example of my classification filters:
>>
>> tc filter add dev eth1 parent 1:0 prio 4 protocol ip u32
>> tc filter add dev eth1 parent 1:0 prio 4 handle ${NETWORK_GROUP_HEX}:
>> protocol ip u32 divisor 256
>> tc filter add dev eth1 protocol ip parent 1:0 prio 4 u32 ht 800:: match
>> ip dst ${NETWORK_ADDRESS}/24 hashkey mask 0x000000ff at 16 link
>> ${NETWORK_GROUP_HEX}:
>> tc filter add dev eth1 parent 1:0 protocol ip prio 4 u32 ht
>> ${NETWORK_GROUP_HEX}:0x${ADDR_Q4_HEX} match ip dst $ADDR classid
>> 1:${MARK_NORMAL}
>>
>>
>> Best regards
>> Bartek Kois