Hello David,
On Tue, Feb 07, 2012 at 01:58:41PM -0500, David Miller wrote:
> From: Simon Wunderlich <[email protected]>
> Date: Tue, 7 Feb 2012 19:39:08 +0100
>
> > The linux traffic control mechanism has different ways to select the
> > correct class of a qdisc. A common way to do this is to use tc filters
> > that are directly attached to a qdisc. Another approach is to use the
> > iptables classify module. The latter one can reduce the amount of work
> > necessary to process a packet when iptables is already involved in the
> > packet classification.
>
> Do not bloat up sk_buff any more. Add this, and the other existing
> tc_* members to the qdisc SKB control block instead.
>
Thanks for your feedback!
I guess you mean skb->cb, but this is also used within mac80211 for various things
(quoting include/net/mac80211.h):
* struct ieee80211_tx_info - skb transmit information
*
* This structure is placed in skb->cb for three uses:
* (1) mac80211 TX control - mac80211 tells the driver what to do
* (2) driver internal use (if applicable)
* (3) TX status information - driver tells mac80211 what happened
We could give it a try, but we most probably run into conflicts again.
I've messed up the CCs in my initial mail (and just resent it) - sorry about that.
Maybe the mac80211 guys have a suggestion as well :)
Cheers,
Simon
On Tue, Feb 7, 2012 at 7:57 PM, Simon Wunderlich <
[email protected]> 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 <[email protected]> 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
Le mardi 07 février 2012 à 21:16 +0100, Simon Wunderlich a écrit :
> Hello David,
>
> On Tue, Feb 07, 2012 at 01:58:41PM -0500, David Miller wrote:
> > From: Simon Wunderlich <[email protected]>
> > Date: Tue, 7 Feb 2012 19:39:08 +0100
> >
> > > The linux traffic control mechanism has different ways to select the
> > > correct class of a qdisc. A common way to do this is to use tc filters
> > > that are directly attached to a qdisc. Another approach is to use the
> > > iptables classify module. The latter one can reduce the amount of work
> > > necessary to process a packet when iptables is already involved in the
> > > packet classification.
> >
> > Do not bloat up sk_buff any more. Add this, and the other existing
> > tc_* members to the qdisc SKB control block instead.
> >
>
> Thanks for your feedback!
>
> I guess you mean skb->cb, but this is also used within mac80211 for various things
> (quoting include/net/mac80211.h):
>
> * struct ieee80211_tx_info - skb transmit information
> *
> * This structure is placed in skb->cb for three uses:
> * (1) mac80211 TX control - mac80211 tells the driver what to do
> * (2) driver internal use (if applicable)
> * (3) TX status information - driver tells mac80211 what happened
>
> We could give it a try, but we most probably run into conflicts again.
>
> I've messed up the CCs in my initial mail (and just resent it) - sorry about that.
> Maybe the mac80211 guys have a suggestion as well :)
>
I really dont understand what the conflict might be.
As long as skb is in Qdisc layer, Qdisc owns skb->cb[] (a part of it
actually, see current discussions about IB on netdev)
As soon as packet is given to device (mac80211), cb[] can be reused.
Or are you saying tc_class also might be needed by mac80211 in the
future ?
skb->cb is the right scratch pad to use.
The problem appears to be the wireless stack abusing some of these
fields (prio for one). And we do have marks in place already which
are 32 bit and global.
cheers,
jamal
On Wed, 2012-02-08 at 10:21 +0100, Eric Dumazet wrote:
> Le mardi 07 février 2012 à 21:16 +0100, Simon Wunderlich a écrit :
>
> I really dont understand what the conflict might be.
>
> As long as skb is in Qdisc layer, Qdisc owns skb->cb[] (a part of it
> actually, see current discussions about IB on netdev)
>
> As soon as packet is given to device (mac80211), cb[] can be reused.
>
> Or are you saying tc_class also might be needed by mac80211 in the
> future ?
>
>
>