I'm trying to come up with a more acceptable patch to the problem discussed a few years
ago:
http://thread.gmane.org/gmane.linux.kernel.wireless.general/64582/focus=64626
The patch below appears to work as expected. In pktgen, you just have to set
the QoS to whatever value matches the queue you need.
This seem reasonable?
Thanks,
Ben
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index d445bb1..c770f19 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1544,6 +1544,18 @@ void ieee80211_xmit(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb,
}
}
+ /* This check needs to go in before the QoS header is set below. */
+ if (skb->priority > 7 ||
+ skb->queue_mapping != ieee802_1d_to_ac[skb->priority]) {
+ WARN_ONCE(1, "Invalid queue-mapping, priority: %i queue-mapping: %i. This is an expected warning
+ (int)(skb->priority), (int)(skb->queue_mapping));
+ /* Adjust queue-mapping to match what the wifi stack expects.
+ * pktgen will just have to set QoS bits accordingly instead
+ * of trying to set the queue_mapping directly.
+ */
+ skb_set_queue_mapping(skb, ieee80211_select_queue(sdata, skb));
+ }
+
ieee80211_set_qos_hdr(sdata, skb);
ieee80211_tx(sdata, skb, false, band);
}
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On Fri, May 31, 2013 at 11:51:45AM -0700, Ben Greear wrote:
> cfg80211_classify8021d uses it to determine the queue in if skb->priority is
> set to a special range (hard coded un-documented hack from hell, it appears).
>
> I didn't go looking to find out where those magic values might be set.
Actually, this hard coded hack from hell is documented :)
http://wireless.kernel.org/en/developers/Documentation/mac80211/queues?highlight=%28so_priority%29
--
Bob Copeland %% http://www.bobcopeland.com
On 05/30/2013 01:02 PM, Felix Fietkau wrote:
> On 2013-05-30 8:45 PM, Ben Greear wrote:
>> I'm trying to come up with a more acceptable patch to the problem discussed a few years
>> ago:
>>
>> http://thread.gmane.org/gmane.linux.kernel.wireless.general/64582/focus=64626
>>
>> The patch below appears to work as expected. In pktgen, you just have to set
>> the QoS to whatever value matches the queue you need.
>>
>> This seem reasonable?
> Why do you adjust the queue mapping instead of the skb priority? In that
> other thread you mentioned that pktgen should be able to control the
> queue, yet here you're taking queue control away from it for tx on mac80211.
I was having trouble getting it to work, but I was trying it at
a different place in the code when I was hacking on it...
Do you think I'd just need to adjust skb->priority in the
same spot in the code, or is there more to it?
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On Fri, 2013-05-31 at 11:51 -0700, Ben Greear wrote:
> > What I'm saying though is that I don't see where skb->priority is even
> > _used_ in the wifi stack. I can see it getting set, but not used.
>
> ieee80211_downgrade_queue
> wme_downgrade_ac // sort of
> ieee80211_select_queue_80211 // sort of...seems twiddling skb->priority is more of a by-product here.
>
> cfg80211_classify8021d
None of this actually matters, it's all within the select_queue() call
so doesn't need to store it in the skb.
I found it though -- the only thing that ever looks at it is
ieee80211_set_qos_hdr() to set the QoS header TID, and presumably that's
what ath9k complains about (hwsim is happy to just push packets.)
johannes
On 05/31/2013 09:14 PM, Johannes Berg wrote:
> On Fri, 2013-05-31 at 11:51 -0700, Ben Greear wrote:
>
>>> What I'm saying though is that I don't see where skb->priority is even
>>> _used_ in the wifi stack. I can see it getting set, but not used.
>>
>> ieee80211_downgrade_queue
>> wme_downgrade_ac // sort of
>> ieee80211_select_queue_80211 // sort of...seems twiddling skb->priority is more of a by-product here.
>>
>> cfg80211_classify8021d
>
> None of this actually matters, it's all within the select_queue() call
> so doesn't need to store it in the skb.
>
> I found it though -- the only thing that ever looks at it is
> ieee80211_set_qos_hdr() to set the QoS header TID, and presumably that's
> what ath9k complains about (hwsim is happy to just push packets.)
Sorry to chime in. The brcmfmac uses cfg80211_classify8021d() as well
(only when skb->priority equals zero) and puts the return value in
skb->priority. But also there it is not needed as a few lines below it
uses it to determine the WWM-AC fifo.
Regards,
Arend
On 2013-05-30 10:45 PM, Ben Greear wrote:
> On 05/30/2013 01:02 PM, Felix Fietkau wrote:
>> On 2013-05-30 8:45 PM, Ben Greear wrote:
>>> I'm trying to come up with a more acceptable patch to the problem discussed a few years
>>> ago:
>>>
>>> http://thread.gmane.org/gmane.linux.kernel.wireless.general/64582/focus=64626
>>>
>>> The patch below appears to work as expected. In pktgen, you just have to set
>>> the QoS to whatever value matches the queue you need.
>>>
>>> This seem reasonable?
>> Why do you adjust the queue mapping instead of the skb priority? In that
>> other thread you mentioned that pktgen should be able to control the
>> queue, yet here you're taking queue control away from it for tx on mac80211.
>
> I was having trouble getting it to work, but I was trying it at
> a different place in the code when I was hacking on it...
>
> Do you think I'd just need to adjust skb->priority in the
> same spot in the code, or is there more to it?
I think that could work.
- Felix
On 05/31/2013 12:56 AM, Felix Fietkau wrote:
> On 2013-05-30 10:45 PM, Ben Greear wrote:
>> On 05/30/2013 01:02 PM, Felix Fietkau wrote:
>>> On 2013-05-30 8:45 PM, Ben Greear wrote:
>>>> I'm trying to come up with a more acceptable patch to the problem discussed a few years
>>>> ago:
>>>>
>>>> http://thread.gmane.org/gmane.linux.kernel.wireless.general/64582/focus=64626
>>>>
>>>> The patch below appears to work as expected. In pktgen, you just have to set
>>>> the QoS to whatever value matches the queue you need.
>>>>
>>>> This seem reasonable?
>>> Why do you adjust the queue mapping instead of the skb priority? In that
>>> other thread you mentioned that pktgen should be able to control the
>>> queue, yet here you're taking queue control away from it for tx on mac80211.
>>
>> I was having trouble getting it to work, but I was trying it at
>> a different place in the code when I was hacking on it...
>>
>> Do you think I'd just need to adjust skb->priority in the
>> same spot in the code, or is there more to it?
> I think that could work.
[added netdev to CC list]
Before I go work on this, does anyone have any preference over
whether pktgen packets should obey the QoS or the xmit-queue?
Currently, the kernel is effectively broken, and has been for years,
so probably no one has ever used pktgen with wifi devices.
As it turns out, mapping QoS appears to work best for my own needs,
but I can also try using the skb->priority as Felix suggests.
If no one cares, then I'd prefer to stick with the QoS method I posted
recently, and ignore the xmit-queue that pktgen might select.
Thanks,
Ben
>
> - Felix
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On 05/31/2013 10:37 AM, Johannes Berg wrote:
> On Fri, 2013-05-31 at 10:21 -0700, Ben Greear wrote:
>
>> Before I go work on this, does anyone have any preference over
>> whether pktgen packets should obey the QoS or the xmit-queue?
>
> That's the core of the problem is that select_queue has the side effect
> of setting skb->priority in mac80211, no? Maybe that side effect needs
> to be removed?
I think it might be more that the wifi stacks have some specific
assumptions about how skb->priority maps to queues and QoS. If
they get out of sync, then the TID mappings and so forth get
confused.
I actually don't know exactly why pktgen shows this problem,
but it must be because it does direct calls to the hard_start_xmit
method of the netdev when (most?) other paths do dev_queue_xmit
or similar.
> Actually that makes it seem like something else should be doing packet
> classification, not mac80211 in select_queue()?
>
> Where is skb->priority actually really used in mac80211? I don't see
> much?
There's a bit more in net/wireless/util.c, at least (cfg80211_classify8021d, for instance).
The mac80211/wme.c uses it. Some of this is called from the drivers
(line 1916 or so of ath9k/xmit.c).
It's all a bit convoluted in my opinion, but there may well
be good reasons for it.
I think the network stack in general is not going to want
to bother with mapping QoS to xmit queues, so probably that
has to remain in the wifi stacks somewhere...
Thanks,
Ben
>
> johannes
>
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com
On Fri, 2013-05-31 at 11:00 -0700, Ben Greear wrote:
> I think it might be more that the wifi stacks have some specific
> assumptions about how skb->priority maps to queues and QoS. If
> they get out of sync, then the TID mappings and so forth get
> confused.
What I'm saying though is that I don't see where skb->priority is even
_used_ in the wifi stack. I can see it getting set, but not used.
> I actually don't know exactly why pktgen shows this problem,
> but it must be because it does direct calls to the hard_start_xmit
> method of the netdev when (most?) other paths do dev_queue_xmit
> or similar.
Other paths go through select_queue(), obviously.
> > Actually that makes it seem like something else should be doing packet
> > classification, not mac80211 in select_queue()?
> >
> > Where is skb->priority actually really used in mac80211? I don't see
> > much?
>
> There's a bit more in net/wireless/util.c, at least (cfg80211_classify8021d, for instance).
But that's pretty much all assignments.
> The mac80211/wme.c uses it. Some of this is called from the drivers
> (line 1916 or so of ath9k/xmit.c).
What's called there? I don't see any reason for that to use
skb->priority?
> It's all a bit convoluted in my opinion, but there may well
> be good reasons for it.
More likely legacy.
johannes
On Fri, 2013-05-31 at 10:21 -0700, Ben Greear wrote:
> Before I go work on this, does anyone have any preference over
> whether pktgen packets should obey the QoS or the xmit-queue?
That's the core of the problem is that select_queue has the side effect
of setting skb->priority in mac80211, no? Maybe that side effect needs
to be removed?
Actually that makes it seem like something else should be doing packet
classification, not mac80211 in select_queue()?
Where is skb->priority actually really used in mac80211? I don't see
much?
johannes
On 2013-05-30 8:45 PM, Ben Greear wrote:
> I'm trying to come up with a more acceptable patch to the problem discussed a few years
> ago:
>
> http://thread.gmane.org/gmane.linux.kernel.wireless.general/64582/focus=64626
>
> The patch below appears to work as expected. In pktgen, you just have to set
> the QoS to whatever value matches the queue you need.
>
> This seem reasonable?
Why do you adjust the queue mapping instead of the skb priority? In that
other thread you mentioned that pktgen should be able to control the
queue, yet here you're taking queue control away from it for tx on mac80211.
- Felix
On 05/31/2013 11:41 AM, Johannes Berg wrote:
> On Fri, 2013-05-31 at 11:00 -0700, Ben Greear wrote:
>
>> I think it might be more that the wifi stacks have some specific
>> assumptions about how skb->priority maps to queues and QoS. If
>> they get out of sync, then the TID mappings and so forth get
>> confused.
>
> What I'm saying though is that I don't see where skb->priority is even
> _used_ in the wifi stack. I can see it getting set, but not used.
ieee80211_downgrade_queue
wme_downgrade_ac // sort of
ieee80211_select_queue_80211 // sort of...seems twiddling skb->priority is more of a by-product here.
cfg80211_classify8021d
>>> Actually that makes it seem like something else should be doing packet
>>> classification, not mac80211 in select_queue()?
>>>
>>> Where is skb->priority actually really used in mac80211? I don't see
>>> much?
>>
>> There's a bit more in net/wireless/util.c, at least (cfg80211_classify8021d, for instance).
>
> But that's pretty much all assignments.
cfg80211_classify8021d uses it to determine the queue in if skb->priority is
set to a special range (hard coded un-documented hack from hell, it appears).
I didn't go looking to find out where those magic values might be set.
>> The mac80211/wme.c uses it. Some of this is called from the drivers
>> (line 1916 or so of ath9k/xmit.c).
>
> What's called there? I don't see any reason for that to use
> skb->priority?
Ok, I was confused about that..but that *is* the code that pukes if you
have have mis-matched queues like you get with pktgen in upstream kernels...
Thanks,
Ben
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com