Date:	Tue, 7 Feb 2012 20:33:49 +0000
From:	Dave Taht <dave.taht@...il.com>
To:	Simon Wunderlich <simon.wunderlich@...03.tu-chemnitz.de>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Pablo Neira Ayuso <pablo@...filter.org>,
	Patrick McHardy <kaber@...sh.net>,
	Jamal Hadi Salim <hadi@...erus.ca>,
	Johannes Berg <johannes@...solutions.net>,
	"John W. Linville" <linville@...driver.com>,
	Marek Lindner <lindner_marek@...oo.de>,
	Sven Eckelmann <sven@...fation.org>,
	netfilter-devel@...r.kernel.org, netfilter@...r.kernel.org,
	coreteam@...filter.org, linux-wireless@...r.kernel.org
Subject: Re: [PATCH] skbuff: Add new tc classify variable

On Tue, Feb 7, 2012 at 7:57 PM, Simon Wunderlich <
simon.wunderlich@...03.tu-chemnitz.de> wrote:

> On Tue, Feb 07, 2012 at 11:05:22AM -0800, Stephen Hemminger wrote:
> > On Tue,  7 Feb 2012 19:39:08 +0100
> > Simon Wunderlich <simon.wunderlich@...03.tu-chemnitz.de> wrote:
> >
> > I don't understand why this is better, we already have mark to do this.
> > Your method saves adding a tc filter to map mark to classid, but that i=
s
> hardly
> > a huge burden.
>
> Unfortunately, it is. We have previously built our trees by setting marks
> with iptables
> and matching the masks with tc and the u32 matcher, but we got a rather
> big performance
> impact as soon as the number of users grow. The target platform are WiFi
> access points.
> By using the proposed patch, the performance stays nearly constant at a
> growing number
> of users.
>

To put things in more general terms, the overspecialized behavior and use
of magic numbers in the wireless stack

... from net/wireless/util.c/*

Given a data frame determine the 802.1p/1d tag to use. */
unsigned int cfg80211_classify8021d(struct sk_buff *skb)
{
        unsigned int dscp;

        /* skb->priority values from 256->263 are magic values to
         * directly indicate a specific 802.1d priority.  This is used
         * to allow 802.1d priority to be passed directly in from VLAN
         * tags, etc.
         */
        if (skb->priority >=3D 256 && skb->priority <=3D 263)
                return skb->priority - 256;

        switch (skb->protocol) {
        case htons(ETH_P_IP):
                dscp =3D ipv4_get_dsfield(ip_hdr(skb)) & 0xfc;
                break;
        case htons(ETH_P_IPV6):
                dscp =3D ipv6_get_dsfield(ipv6_hdr(skb)) & 0xfc;
                break;
        default:
                return 0;
        }

        return dscp >> 5;
}


and the related select queue function calling this
which maps the 4 hardware queues wireless has, onto 4 mq subqdiscs...

overrides most anything you might want to do with iptables doing
classification directly, especially if you want to put things
into different hw queues.

and furthermore makes using other qdiscs than mq, such as the htb rate
limiter, problematic, as well as sch_mq has no ability to put in a top
level filter, anyway because stuff ends up in the 1:1,1:2,1:3,1:4
automagically...

Reducing the namespace to 'marks' vs classes is ok for some uses, I guess,
but how to handle the hw queues and the select_queue problem?

And while I'm at the problems that wireless has, having one (or 4) queues
per actual end-point rather than 4 hardware queues would make it possible
to address the bufferbloat problem with wireless and sanely calculating
aggregation values....

My thought on that was to be tossing a station identifier into the cb, but
that seems to alsobe a cross layer violation.

I like the idea of separating "priority" from "classification" and from
"hw queues" but a unifying metaphor and space in the skb seems
to be THE problem.




--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists