Tc filtering: broken 802_3 classifier?

Tc filtering: broken 802_3 classifier?

[Waskiewicz Jr, Peter P]

I've been trying to use tc filtering to filter on ethertype, among other
things in the MAC layer.  I'm running into multiple issues, and want to
put this out there in case I'm using the filters wrong, or if there
really is a bug in the filter code (I've stared at most of it today, and
my head hurts).

Here's the scenario.  I am running on a recent 2.6.23 GIT tree, and am
using sch_prio with no multiqueue turned on in the qdisc.  The network
interface in question is e1000 (no multiqueue).

# tc qdisc add dev eth0 root handle 1: prio

Now to see the flowid's packet counts incrementing, I add explicit
classful qdiscs to the leaves:

# tc qdisc add dev eth0 parent 1:1 handle 10: pfifo
# tc qdisc add dev eth0 parent 1:2 handle 20: pfifo
# tc qdisc add dev eth0 parent 1:3 handle 30: pfifo

Now packet counts can be seen with:

# tc -s qdisc ls dev eth0

I can add a filter for IP for ssh, and it works as intended:

# tc filter add dev eth0 protocol ip parent 1: prio 1 u32 match ip dport
22 0xffff flowid 1:3

This will shove ssh traffic into the 3rd pfifo queue, where by default
it will flow into flowid 1:1.  This is good.

Now I add a filter for ethernet (802.3), and things aren't as happy:

# tc filter add dev eth0 protocol 802_3 parent 1: prio 2 u32 match u32
<first 4 bytes of dst mac address> 0xffffffff at 0 match u32 <last 2
bytes of dst mac address> 0xffff0000 at 4 flowid 1:1

This should match the destination MAC address of outgoing packets, and
put it into flowid 1:1.  For pings, using the normal priomap, they go
into 1:2, so ping should be a good candidate for seeing if it goes into
1:1.  In this case, it does not filter into 1:1.

If I expand this into 8 flows on a multiqueue NIC, using sch_prio or
sch_rr, adding any 802_3 filter to the chain will cause any traffic that
hits the classifier (i.e. no other filters match first) to go into
flowid 1:1, regardless if it actually matches.  Remove the 802_3 filter
from the chain, and all filtering starts working again.

I'm trying to get state from the classifier code now when it's running,
but it's a really big mess of black magic.  I'm wondering if anyone is
also seeing this behavior, and if they've tried to fix it.  If not, I'll
continue to search for a solution, but I'm just polling the community to
see if this is a known issue, or if I'm doing something wrong.

Thanks,

-PJ Waskiewicz
Intel Corporation
peter.p.waskiewicz.jr@intel.com <mailto:peter.p.waskiewicz.jr@intel.com>

Re: Tc filtering: broken 802_3 classifier?

[Patrick McHardy]

Waskiewicz Jr, Peter P wrote:
> [...]
> Now I add a filter for ethernet (802.3), and things aren't as happy:
> 
> # tc filter add dev eth0 protocol 802_3 parent 1: prio 2 u32 match u32
> <first 4 bytes of dst mac address> 0xffffffff at 0 match u32 <last 2
> bytes of dst mac address> 0xffff0000 at 4 flowid 1:1
> 
> This should match the destination MAC address of outgoing packets, and
> put it into flowid 1:1.  For pings, using the normal priomap, they go
> into 1:2, so ping should be a good candidate for seeing if it goes into
> 1:1.  In this case, it does not filter into 1:1.


The protocol match is on skb->protocol, so it case of ethernet its
on the ethernet protocol, which is ETH_P_IP or "ip" for IPv4.

RE: Tc filtering: broken 802_3 classifier?

[Waskiewicz Jr, Peter P]

> The protocol match is on skb->protocol, so it case of 
> ethernet its on the ethernet protocol, which is ETH_P_IP or 
> "ip" for IPv4.

I see that in the code, but the reason I started worrying was when I
added the 802_3 classifier on 8 flows, it would shove all traffic into
flowid 1:1, no matter if it matched or not.

I'll keep investigating and see if I can narrow down what I'm seeing.

Thanks Patrick,
-PJ

Re: Tc filtering: broken 802_3 classifier?

[Patrick McHardy]

Waskiewicz Jr, Peter P wrote:
>>The protocol match is on skb->protocol, so it case of 
>>ethernet its on the ethernet protocol, which is ETH_P_IP or 
>>"ip" for IPv4.
> 
> 
> I see that in the code, but the reason I started worrying was when I
> added the 802_3 classifier on 8 flows, it would shove all traffic into
> flowid 1:1, no matter if it matched or not.
> 
> I'll keep investigating and see if I can narrow down what I'm seeing.


I'm not sure what you're expecting. skb->protocol is usually not set
to ETH_P_802_3, which is why the filter is not matching.

RE: Tc filtering: broken 802_3 classifier?

[Waskiewicz Jr, Peter P]

> Waskiewicz Jr, Peter P wrote:
> >>The protocol match is on skb->protocol, so it case of 
> ethernet its on 
> >>the ethernet protocol, which is ETH_P_IP or "ip" for IPv4.
> > 
> > 
> > I see that in the code, but the reason I started worrying was when I
> > added the 802_3 classifier on 8 flows, it would shove all 
> traffic into
> > flowid 1:1, no matter if it matched or not.
> > 
> > I'll keep investigating and see if I can narrow down what 
> I'm seeing.
> 
> 
> I'm not sure what you're expecting. skb->protocol is usually not set
> to ETH_P_802_3, which is why the filter is not matching.

I understand that.  I had two issues, which you cleared up one by
reminding me that the protocol matches on skb->protocol before it tries
to run the ->classify() routine.  The other issue I am seeing is with 8
bands, an 802_3 filter is affecting classification of IP traffic.  For
example, I have an 802_3 filter to look for dst MAC address, but an ssh
packet, which without any filters should go into flowid 1:3 on my
system, is getting pushed into flowid 1:1.  I remove the 802_3 filter,
and ssh traffic starts going back into 1:3.  No other filters on the
system.  That's the main issue I'm seeing, so I'll keep investigating to
see what's going on.

-PJ

Re: Tc filtering: broken 802_3 classifier?

[Patrick McHardy]

Waskiewicz Jr, Peter P wrote:
>>Waskiewicz Jr, Peter P wrote:
>>
>>I'm not sure what you're expecting. skb->protocol is usually not set
>>to ETH_P_802_3, which is why the filter is not matching.
> 
> 
> I understand that.  I had two issues, which you cleared up one by
> reminding me that the protocol matches on skb->protocol before it tries
> to run the ->classify() routine.  The other issue I am seeing is with 8
> bands, an 802_3 filter is affecting classification of IP traffic.  For
> example, I have an 802_3 filter to look for dst MAC address, but an ssh
> packet, which without any filters should go into flowid 1:3 on my
> system, is getting pushed into flowid 1:1.  I remove the 802_3 filter,
> and ssh traffic starts going back into 1:3.  No other filters on the
> system.  That's the main issue I'm seeing, so I'll keep investigating to
> see what's going on.


In case of prio, if your manually installed filters don't match, it will
fall back to the skb->priority based classification, which is based
on tos and is probably responsible for what you're seeing. Feel free to
investigate, but you could save us all some time by simply posting what
you're doing, what you're expecting and what is actually happening,
there's probably a good explanation.

RE: Tc filtering: broken 802_3 classifier?

[Waskiewicz Jr, Peter P]

> In case of prio, if your manually installed filters don't 
> match, it will fall back to the skb->priority based 
> classification, which is based on tos and is probably 
> responsible for what you're seeing. Feel free to investigate, 
> but you could save us all some time by simply posting what 
> you're doing, what you're expecting and what is actually 
> happening, there's probably a good explanation.

I thought I did that before, but I probably wasn't clear.  I'll try
again (and if I'm still not clear, please pop me in the head).  I am
aware that skb->priority is used if no filter matches, and that is
derived from tos (and gets set in ipsockglue).

This is my setup.  8 bands with prio, with a priomap that is nice and
simple:

# tc qdisc add dev eth0 root handle 1: prio bands 8 priomap 0 0 1 1 2 2
3 3 4 4 5 5 6 6 7 7

With this configuration, ICMP will default to flowid 1:1 (band 0), and
ssh will default to flowid 1:4 (band 3) based on TOS.  I add this filter
(802_3) and all traffic starts flowing into flowid 1:1 (including ssh),
even though it should never match:

# tc filter add dev eth0 protocol 802_3 parent 1: prio 2 u32 match u32
0x00000800 0x0000ffff at 12 flowid 1:6

As soon as I remove the filter:

# tc filter del dev eth0 protocol 802_3 prio 2

ssh flows back into flowid 1:4.  No filters of protocol ip were added,
only the 802.3 filter.

I hope this is more clear as to what I'm seeing.

Thanks,
-PJ

Re: Tc filtering: broken 802_3 classifier?

[Patrick McHardy]

Waskiewicz Jr, Peter P wrote:
>>In case of prio, if your manually installed filters don't 
>>match, it will fall back to the skb->priority based 
>>classification, which is based on tos and is probably 
>>responsible for what you're seeing. Feel free to investigate, 
>>but you could save us all some time by simply posting what 
>>you're doing, what you're expecting and what is actually 
>>happening, there's probably a good explanation.
> 
> 
> I thought I did that before, but I probably wasn't clear.  I'll try
> again (and if I'm still not clear, please pop me in the head).  I am
> aware that skb->priority is used if no filter matches, and that is
> derived from tos (and gets set in ipsockglue).
> 
> This is my setup.  8 bands with prio, with a priomap that is nice and
> simple:
> 
> # tc qdisc add dev eth0 root handle 1: prio bands 8 priomap 0 0 1 1 2 2
> 3 3 4 4 5 5 6 6 7 7
> 
> With this configuration, ICMP will default to flowid 1:1 (band 0), and
> ssh will default to flowid 1:4 (band 3) based on TOS.  I add this filter
> (802_3) and all traffic starts flowing into flowid 1:1 (including ssh),
> even though it should never match:
> 
> # tc filter add dev eth0 protocol 802_3 parent 1: prio 2 u32 match u32
> 0x00000800 0x0000ffff at 12 flowid 1:6
> 
> As soon as I remove the filter:
> 
> # tc filter del dev eth0 protocol 802_3 prio 2
> 
> ssh flows back into flowid 1:4.  No filters of protocol ip were added,
> only the 802.3 filter.
> 
> I hope this is more clear as to what I'm seeing.


It is .. now let me think about the good explanation, it doesn't
make sense at first :)

Re: Tc filtering: broken 802_3 classifier?

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]

Patrick McHardy wrote:
> Waskiewicz Jr, Peter P wrote:
> 
>>This is my setup.  8 bands with prio, with a priomap that is nice and
>>simple:
>>
>># tc qdisc add dev eth0 root handle 1: prio bands 8 priomap 0 0 1 1 2 2
>>3 3 4 4 5 5 6 6 7 7
>>
>>With this configuration, ICMP will default to flowid 1:1 (band 0), and
>>ssh will default to flowid 1:4 (band 3) based on TOS.  I add this filter
>>(802_3) and all traffic starts flowing into flowid 1:1 (including ssh),
>>even though it should never match:
>>
>># tc filter add dev eth0 protocol 802_3 parent 1: prio 2 u32 match u32
>>0x00000800 0x0000ffff at 12 flowid 1:6
>>
>>As soon as I remove the filter:
>>
>># tc filter del dev eth0 protocol 802_3 prio 2
>>
>>ssh flows back into flowid 1:4.  No filters of protocol ip were added,
>>only the 802.3 filter.
>>
>>I hope this is more clear as to what I'm seeing.
>
> 
> It is .. now let me think about the good explanation, it doesn't
> make sense at first :)


First of all - good catch :) This really is a bug, and one that
has existed for quite some time. Whats happening is that
tc_classify returns -1 because no filter matches, but this
is not caught in the switch statement and the !q->filter_list
condition is false. So band is set to the uninitialized value
of res.classid, and the band >= q->bands checks catches this
as invalid and uses 0. The sad thing is that this is one of
the typical constructs gcc falsely warns about for primitive
types, in this case it doesn't care. Anyway, what should
really be happening in this case is that skb->priority is
used, as without any filters.

This patch should fix it, but other qdiscs might need similar
fixes I believe. I'll look into that tommorrow.


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 911 bytes --]

diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 2d8c084..f37dd8c 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -38,22 +38,21 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 	struct prio_sched_data *q = qdisc_priv(sch);
 	u32 band = skb->priority;
 	struct tcf_result res;
+	int err;
 
 	*qerr = NET_XMIT_BYPASS;
 	if (TC_H_MAJ(skb->priority) != sch->handle) {
+		err = tc_classify(skb, q->filter_list, &res);
 #ifdef CONFIG_NET_CLS_ACT
-		switch (tc_classify(skb, q->filter_list, &res)) {
+		switch (err) {
 		case TC_ACT_STOLEN:
 		case TC_ACT_QUEUED:
 			*qerr = NET_XMIT_SUCCESS;
 		case TC_ACT_SHOT:
 			return NULL;
 		}
-
-		if (!q->filter_list ) {
-#else
-		if (!q->filter_list || tc_classify(skb, q->filter_list, &res)) {
 #endif
+		if (!q->filter_list || err < 0) {
 			if (TC_H_MAJ(band))
 				band = 0;
 			band = q->prio2band[band&TC_PRIO_MAX];

Re: Tc filtering: broken 802_3 classifier?

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 26 Jul 2007 01:58:54 +0200

> This patch should fix it, but other qdiscs might need similar
> fixes I believe. I'll look into that tommorrow.

Thanks for figuring this out Patrick, let me know when you
have a final version to apply.