Since I removed the master netdev, we've been
keeping internal queues only, and even before
that we never told the networking stack above
the virtual interfaces about congestion. This
means that packets are queued in mac80211 and
the upper layers never know, possibly leading
to memory exhaustion and other problems.
This patch makes all interfaces multiqueue and
uses ndo_select_queue to put the packets into
queues per AC. Additionally, when the driver
stops a queue, we now stop all corresponding
queues for the virtual interfaces as well.
The injection case will use VO by default for
non-data frames, and BE for data frames, but
downgrade any data frames according to ACM. It
needs to be fleshed out in the future to allow
chosing the queue/AC in radiotap.
Reported-by: Lennert Buytenhek <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
Cc: [email protected] [2.6.32]
---
I know it's late, and large, but still would be good to have in .33
since the issue is fairly serious.
net/mac80211/iface.c | 39 +++++++++++++++++++-
net/mac80211/rx.c | 4 +-
net/mac80211/tx.c | 5 ++
net/mac80211/util.c | 12 ++++++
net/mac80211/wme.c | 96 +++++++++++++++++++++++++++++++++++++--------------
net/mac80211/wme.h | 8 +++-
6 files changed, 132 insertions(+), 32 deletions(-)
--- wireless-testing.orig/net/mac80211/iface.c 2010-01-05 16:06:21.000000000 +0100
+++ wireless-testing/net/mac80211/iface.c 2010-01-05 16:26:55.000000000 +0100
@@ -15,12 +15,14 @@
#include <linux/netdevice.h>
#include <linux/rtnetlink.h>
#include <net/mac80211.h>
+#include <net/ieee80211_radiotap.h>
#include "ieee80211_i.h"
#include "sta_info.h"
#include "debugfs_netdev.h"
#include "mesh.h"
#include "led.h"
#include "driver-ops.h"
+#include "wme.h"
/**
* DOC: Interface list locking
@@ -657,6 +659,12 @@ static void ieee80211_teardown_sdata(str
WARN_ON(flushed);
}
+static u16 ieee80211_netdev_select_queue(struct net_device *dev,
+ struct sk_buff *skb)
+{
+ return ieee80211_select_queue(IEEE80211_DEV_TO_SUB_IF(dev), skb);
+}
+
static const struct net_device_ops ieee80211_dataif_ops = {
.ndo_open = ieee80211_open,
.ndo_stop = ieee80211_stop,
@@ -665,8 +673,34 @@ static const struct net_device_ops ieee8
.ndo_set_multicast_list = ieee80211_set_multicast_list,
.ndo_change_mtu = ieee80211_change_mtu,
.ndo_set_mac_address = ieee80211_change_mac,
+ .ndo_select_queue = ieee80211_netdev_select_queue,
};
+static u16 ieee80211_monitor_select_queue(struct net_device *dev,
+ struct sk_buff *skb)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
+ struct ieee80211_hdr *hdr;
+ struct ieee80211_radiotap_header *rtap = (void *)skb->data;
+
+ if (local->hw.queues < 4)
+ return 0;
+
+ if (skb->len < 4 ||
+ skb->len < rtap->it_len + 2 /* frame control */)
+ return 0; /* doesn't matter, frame will be dropped */
+
+ hdr = (void *)((u8 *)skb->data + rtap->it_len);
+
+ if (!ieee80211_is_data(hdr->frame_control)) {
+ skb->priority = 7;
+ return ieee802_1d_to_ac[skb->priority];
+ }
+
+ return ieee80211_downgrade_queue(local, skb);
+}
+
static const struct net_device_ops ieee80211_monitorif_ops = {
.ndo_open = ieee80211_open,
.ndo_stop = ieee80211_stop,
@@ -675,6 +709,7 @@ static const struct net_device_ops ieee8
.ndo_set_multicast_list = ieee80211_set_multicast_list,
.ndo_change_mtu = ieee80211_change_mtu,
.ndo_set_mac_address = eth_mac_addr,
+ .ndo_select_queue = ieee80211_monitor_select_queue,
};
static void ieee80211_if_setup(struct net_device *dev)
@@ -781,8 +816,8 @@ int ieee80211_if_add(struct ieee80211_lo
ASSERT_RTNL();
- ndev = alloc_netdev(sizeof(*sdata) + local->hw.vif_data_size,
- name, ieee80211_if_setup);
+ ndev = alloc_netdev_mq(sizeof(*sdata) + local->hw.vif_data_size,
+ name, ieee80211_if_setup, local->hw.queues);
if (!ndev)
return -ENOMEM;
dev_net_set(ndev, wiphy_net(local->hw.wiphy));
--- wireless-testing.orig/net/mac80211/tx.c 2010-01-05 16:06:22.000000000 +0100
+++ wireless-testing/net/mac80211/tx.c 2010-01-05 16:06:51.000000000 +0100
@@ -1511,7 +1511,7 @@ static void ieee80211_xmit(struct ieee80
return;
}
- ieee80211_select_queue(local, skb);
+ ieee80211_set_qos_hdr(local, skb);
ieee80211_tx(sdata, skb, false);
rcu_read_unlock();
}
@@ -2289,6 +2289,9 @@ void ieee80211_tx_skb(struct ieee80211_s
skb_set_network_header(skb, 0);
skb_set_transport_header(skb, 0);
+ /* send all internal mgmt frames on VO */
+ skb_set_queue_mapping(skb, 0);
+
/*
* The other path calling ieee80211_xmit is from the tasklet,
* and while we can handle concurrent transmissions locking
--- wireless-testing.orig/net/mac80211/wme.c 2010-01-05 16:06:21.000000000 +0100
+++ wireless-testing/net/mac80211/wme.c 2010-01-05 16:15:15.000000000 +0100
@@ -44,22 +44,69 @@ static int wme_downgrade_ac(struct sk_bu
}
-/* Indicate which queue to use. */
-static u16 classify80211(struct ieee80211_local *local, struct sk_buff *skb)
+/* Indicate which queue to use. */
+u16 ieee80211_select_queue(struct ieee80211_sub_if_data *sdata,
+ struct sk_buff *skb)
{
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta = NULL;
+ u32 sta_flags = 0;
+ const u8 *ra = NULL;
+ bool qos = false;
- if (!ieee80211_is_data(hdr->frame_control)) {
- /* management frames go on AC_VO queue, but are sent
- * without QoS control fields */
- return 0;
+ if (local->hw.queues < 4 || skb->len < 6) {
+ skb->priority = 0; /* required for correct WPA/11i MIC */
+ return min_t(u16, local->hw.queues - 1,
+ ieee802_1d_to_ac[skb->priority]);
+ }
+
+ rcu_read_lock();
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP_VLAN:
+ rcu_read_lock();
+ sta = rcu_dereference(sdata->u.vlan.sta);
+ if (sta)
+ sta_flags = get_sta_flags(sta);
+ rcu_read_unlock();
+ if (sta)
+ break;
+ case NL80211_IFTYPE_AP:
+ ra = skb->data;
+ break;
+ case NL80211_IFTYPE_WDS:
+ ra = sdata->u.wds.remote_addr;
+ break;
+#ifdef CONFIG_MAC80211_MESH
+ case NL80211_IFTYPE_MESH_POINT:
+ /*
+ * XXX: This is clearly broken ... but already was before,
+ * because ieee80211_fill_mesh_addresses() would clear A1
+ * except for multicast addresses.
+ */
+ break;
+#endif
+ case NL80211_IFTYPE_STATION:
+ ra = sdata->u.mgd.bssid;
+ break;
+ case NL80211_IFTYPE_ADHOC:
+ ra = skb->data;
+ break;
+ default:
+ break;
}
- if (0 /* injected */) {
- /* use AC from radiotap */
+ if (!sta && ra && !is_multicast_ether_addr(ra)) {
+ sta = sta_info_get(sdata, ra);
+ if (sta)
+ sta_flags = get_sta_flags(sta);
}
- if (!ieee80211_is_data_qos(hdr->frame_control)) {
+ if (sta_flags & WLAN_STA_WME)
+ qos = true;
+
+ rcu_read_unlock();
+
+ if (!qos) {
skb->priority = 0; /* required for correct WPA/11i MIC */
return ieee802_1d_to_ac[skb->priority];
}
@@ -68,6 +115,12 @@ static u16 classify80211(struct ieee8021
* data frame has */
skb->priority = cfg80211_classify8021d(skb);
+ return ieee80211_downgrade_queue(local, skb);
+}
+
+u16 ieee80211_downgrade_queue(struct ieee80211_local *local,
+ struct sk_buff *skb)
+{
/* in case we are a client verify acm is not set for this ac */
while (unlikely(local->wmm_acm & BIT(skb->priority))) {
if (wme_downgrade_ac(skb)) {
@@ -85,24 +138,17 @@ static u16 classify80211(struct ieee8021
return ieee802_1d_to_ac[skb->priority];
}
-void ieee80211_select_queue(struct ieee80211_local *local, struct sk_buff *skb)
+void ieee80211_set_qos_hdr(struct ieee80211_local *local, struct sk_buff *skb)
{
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- u16 queue;
- u8 tid;
-
- queue = classify80211(local, skb);
- if (unlikely(queue >= local->hw.queues))
- queue = local->hw.queues - 1;
-
- /*
- * Now we know the 1d priority, fill in the QoS header if
- * there is one (and we haven't done this before).
- */
+ struct ieee80211_hdr *hdr = (void *)skb->data;
+
+ /* Fill in the QoS header if there is one. */
if (ieee80211_is_data_qos(hdr->frame_control)) {
u8 *p = ieee80211_get_qos_ctl(hdr);
- u8 ack_policy = 0;
+ u8 ack_policy = 0, tid;
+
tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
+
if (unlikely(local->wifi_wme_noack_test))
ack_policy |= QOS_CONTROL_ACK_POLICY_NOACK <<
QOS_CONTROL_ACK_POLICY_SHIFT;
@@ -110,6 +156,4 @@ void ieee80211_select_queue(struct ieee8
*p++ = ack_policy | tid;
*p = 0;
}
-
- skb_set_queue_mapping(skb, queue);
}
--- wireless-testing.orig/net/mac80211/wme.h 2010-01-05 16:06:21.000000000 +0100
+++ wireless-testing/net/mac80211/wme.h 2010-01-05 16:15:26.000000000 +0100
@@ -20,7 +20,11 @@
extern const int ieee802_1d_to_ac[8];
-void ieee80211_select_queue(struct ieee80211_local *local,
- struct sk_buff *skb);
+u16 ieee80211_select_queue(struct ieee80211_sub_if_data *sdata,
+ struct sk_buff *skb);
+void ieee80211_set_qos_hdr(struct ieee80211_local *local, struct sk_buff *skb);
+u16 ieee80211_downgrade_queue(struct ieee80211_local *local,
+ struct sk_buff *skb);
+
#endif /* _WME_H */
--- wireless-testing.orig/net/mac80211/rx.c 2010-01-05 16:06:21.000000000 +0100
+++ wireless-testing/net/mac80211/rx.c 2010-01-05 16:06:51.000000000 +0100
@@ -1665,7 +1665,9 @@ ieee80211_rx_h_mesh_fwding(struct ieee80
memset(info, 0, sizeof(*info));
info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
info->control.vif = &rx->sdata->vif;
- ieee80211_select_queue(local, fwd_skb);
+ skb_set_queue_mapping(skb,
+ ieee80211_select_queue(rx->sdata, fwd_skb));
+ ieee80211_set_qos_hdr(local, skb);
if (is_multicast_ether_addr(fwd_hdr->addr1))
IEEE80211_IFSTA_MESH_CTR_INC(&sdata->u.mesh,
fwded_mcast);
--- wireless-testing.orig/net/mac80211/util.c 2010-01-05 16:06:21.000000000 +0100
+++ wireless-testing/net/mac80211/util.c 2010-01-05 16:06:51.000000000 +0100
@@ -269,6 +269,7 @@ static void __ieee80211_wake_queue(struc
enum queue_stop_reason reason)
{
struct ieee80211_local *local = hw_to_local(hw);
+ struct ieee80211_sub_if_data *sdata;
if (WARN_ON(queue >= hw->queues))
return;
@@ -281,6 +282,11 @@ static void __ieee80211_wake_queue(struc
if (!skb_queue_empty(&local->pending[queue]))
tasklet_schedule(&local->tx_pending_tasklet);
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(sdata, &local->interfaces, list)
+ netif_tx_wake_queue(netdev_get_tx_queue(sdata->dev, queue));
+ rcu_read_unlock();
}
void ieee80211_wake_queue_by_reason(struct ieee80211_hw *hw, int queue,
@@ -305,11 +311,17 @@ static void __ieee80211_stop_queue(struc
enum queue_stop_reason reason)
{
struct ieee80211_local *local = hw_to_local(hw);
+ struct ieee80211_sub_if_data *sdata;
if (WARN_ON(queue >= hw->queues))
return;
__set_bit(reason, &local->queue_stop_reasons[queue]);
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(sdata, &local->interfaces, list)
+ netif_tx_stop_queue(netdev_get_tx_queue(sdata->dev, queue));
+ rcu_read_unlock();
}
void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
On Tue, Jan 05, 2010 at 06:00:58PM +0100, Johannes Berg wrote:
> Since I removed the master netdev, we've been
> keeping internal queues only, and even before
> that we never told the networking stack above
> the virtual interfaces about congestion. This
> means that packets are queued in mac80211 and
> the upper layers never know, possibly leading
> to memory exhaustion and other problems.
>
> This patch makes all interfaces multiqueue and
> uses ndo_select_queue to put the packets into
> queues per AC. Additionally, when the driver
> stops a queue, we now stop all corresponding
> queues for the virtual interfaces as well.
>
> The injection case will use VO by default for
> non-data frames, and BE for data frames, but
> downgrade any data frames according to ACM. It
> needs to be fleshed out in the future to allow
> chosing the queue/AC in radiotap.
>
> Reported-by: Lennert Buytenhek <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> Cc: [email protected] [2.6.32]
Tested-by: Lennert Buytenhek <[email protected]>
(I haven't run into the issue that Larry described so far.)
On 2010-01-05 6:16 PM, John W. Linville wrote:
> On Tue, Jan 05, 2010 at 06:00:58PM +0100, Johannes Berg wrote:
>> Since I removed the master netdev, we've been
>> keeping internal queues only, and even before
>> that we never told the networking stack above
>> the virtual interfaces about congestion. This
>> means that packets are queued in mac80211 and
>> the upper layers never know, possibly leading
>> to memory exhaustion and other problems.
>>
>> This patch makes all interfaces multiqueue and
>> uses ndo_select_queue to put the packets into
>> queues per AC. Additionally, when the driver
>> stops a queue, we now stop all corresponding
>> queues for the virtual interfaces as well.
>>
>> The injection case will use VO by default for
>> non-data frames, and BE for data frames, but
>> downgrade any data frames according to ACM. It
>> needs to be fleshed out in the future to allow
>> chosing the queue/AC in radiotap.
>>
>> Reported-by: Lennert Buytenhek <[email protected]>
>> Signed-off-by: Johannes Berg <[email protected]>
>> Cc: [email protected] [2.6.32]
>> ---
>> I know it's late, and large, but still would be good to have in .33
>> since the issue is fairly serious.
>
> Obviously I'd like to see some testing. Lennert, does this patch
> resolve the issues you raised?
I tested this patch on embedded hardware, and it makes a huge difference
there. It fixes some serious TCP throughput drops and latency spikes
that I observed with ath9k earlier.
I think this should be merged ASAP.
- Felix
On Tue, Jan 05, 2010 at 12:16:03PM -0500, John W. Linville wrote:
> > Since I removed the master netdev, we've been
> > keeping internal queues only, and even before
> > that we never told the networking stack above
> > the virtual interfaces about congestion. This
> > means that packets are queued in mac80211 and
> > the upper layers never know, possibly leading
> > to memory exhaustion and other problems.
> >
> > This patch makes all interfaces multiqueue and
> > uses ndo_select_queue to put the packets into
> > queues per AC. Additionally, when the driver
> > stops a queue, we now stop all corresponding
> > queues for the virtual interfaces as well.
> >
> > The injection case will use VO by default for
> > non-data frames, and BE for data frames, but
> > downgrade any data frames according to ACM. It
> > needs to be fleshed out in the future to allow
> > chosing the queue/AC in radiotap.
> >
> > Reported-by: Lennert Buytenhek <[email protected]>
> > Signed-off-by: Johannes Berg <[email protected]>
> > Cc: [email protected] [2.6.32]
> > ---
> > I know it's late, and large, but still would be good to have in .33
> > since the issue is fairly serious.
>
> Obviously I'd like to see some testing. Lennert, does this patch
> resolve the issues you raised?
I got an earlier version of this patch from Johannes and tested that,
and that seemed to work as intended. I'll re-try with this version
of the patch, and I'll ask to have some QA done on it as well.
(FWIW, I think that the approach taken by this patch is the right way
to go.)
On 01/06/2010 05:05 AM, Johannes Berg wrote:
> On Tue, 2010-01-05 at 23:30 -0600, Larry Finger wrote:
>
>> Reauthentication fails with a pop-up to reenter the WPA2 secret as I'm
>> using NetworkManager. I can only reconnect if b43 (and mac80211) are
>> unloaded and reloaded. I pulled the patch and confirmed that proper
>> behavior is returned.
>
> Ouch.
>
> I can't seem to reproduce this with hwsim, but it never touches the
> queues.
>
> I think it may be because I forgot about
> 53623f1a09c7a7d23b74f0f7d93dba0ebde1006b ...
>
> The patch below is certainly needed, but I'm not entirely sure it'll fix
> your issue, can you give it a try please?
>
> johannes
>
This patch seems to fix the problem. I've been running for 5 hours with it, and
no interruption.
Thanks,
Larry
On 2010-01-06 10:51 PM, Johannes Berg wrote:
> On Wed, 2010-01-06 at 23:26 +0200, Thomas Backlund wrote:
>
>> With this patch applied to 2.6.33-rc3 kernel crashes at boot...
>>
>> I have a iwl4965 connecting to a wpa2-psk encrypted network, arch is
>> x86_64, distro Mandriva Linux and the kernel has preempt enabled.
>>
>> Am I only missing some patches, or did I hit a real bug ?
>
> It'd help if you said what crashes and how since nobody else seems to be
> having that particular problem with this patch.
I have an idea.
The patch adds the following chunk of code:
> + if (!sta && ra && !is_multicast_ether_addr(ra)) {
> + sta = sta_info_get(sdata, ra);
> + if (sta)
> + sta_flags = get_sta_flags(sta);
In wireless-testing, sta_info_get takes sdata as first argument, in
2.6.33-rc3 it expects a pointer to local. This should have emitted a
compiler warning... I saw the same thing when applying the patch to an
older compat-wireless snapshot.
- Felix
On Thu, Jan 7, 2010 at 6:01 AM, Lennert Buytenhek
<[email protected]> wrote:
> Commit 'mac80211: fix skb buffering issue' added an ->ndo_select_queue()
> for monitor interfaces which can end up dereferencing ieee802_1d_to_ac[]
> beyond the end of the array for injected data packets (as skb->priority
> isn't guaranteed to be zero or within [0:7]), which then triggers the
> WARN_ON in net/core/dev.c:dev_cap_txqueue(). Fix this by always setting
> the priority to zero on injected data frames.
>
> Signed-off-by: Lennert Buytenhek <[email protected]>
The other patches are for stable as well, so this should be for stable too?
The stable wireless whore, Luis
On Tue, Jan 05, 2010 at 08:07:52PM +0100, Felix Fietkau wrote:
> >> Since I removed the master netdev, we've been
> >> keeping internal queues only, and even before
> >> that we never told the networking stack above
> >> the virtual interfaces about congestion. This
> >> means that packets are queued in mac80211 and
> >> the upper layers never know, possibly leading
> >> to memory exhaustion and other problems.
> >>
> >> This patch makes all interfaces multiqueue and
> >> uses ndo_select_queue to put the packets into
> >> queues per AC. Additionally, when the driver
> >> stops a queue, we now stop all corresponding
> >> queues for the virtual interfaces as well.
> >>
> >> The injection case will use VO by default for
> >> non-data frames, and BE for data frames, but
> >> downgrade any data frames according to ACM. It
> >> needs to be fleshed out in the future to allow
> >> chosing the queue/AC in radiotap.
> >>
> >> Reported-by: Lennert Buytenhek <[email protected]>
> >> Signed-off-by: Johannes Berg <[email protected]>
> >> Cc: [email protected] [2.6.32]
> >> ---
> >> I know it's late, and large, but still would be good to have in .33
> >> since the issue is fairly serious.
> >
> > Obviously I'd like to see some testing. Lennert, does this patch
> > resolve the issues you raised?
>
> I tested this patch on embedded hardware, and it makes a huge
> difference there. It fixes some serious TCP throughput drops and
> latency spikes that I observed with ath9k earlier.
Good to hear. While you're testing anyway, could you check whether
shortening the wlan0 tx queue length to, say, 100 entries (ifconfig
wlan0 txqueuelen 100) has any further effect on latency, or on CPU
usage?
On Thu, Jan 07, 2010 at 07:45:14AM -0800, Luis R. Rodriguez wrote:
> > Commit 'mac80211: fix skb buffering issue' added an ->ndo_select_queue()
> > for monitor interfaces which can end up dereferencing ieee802_1d_to_ac[]
> > beyond the end of the array for injected data packets (as skb->priority
> > isn't guaranteed to be zero or within [0:7]), which then triggers the
> > WARN_ON in net/core/dev.c:dev_cap_txqueue(). ?Fix this by always setting
> > the priority to zero on injected data frames.
> >
> > Signed-off-by: Lennert Buytenhek <[email protected]>
>
> The other patches are for stable as well, so this should be for stable too?
Yep. Actually, it should probably just be folded into Johannes' patch
as it is queued for stable, I think.
On 07.01.2010 00:39, Thomas Backlund wrote:
> On 07.01.2010 00:03, Felix Fietkau wrote:
>> On 2010-01-06 10:51 PM, Johannes Berg wrote:
>>> On Wed, 2010-01-06 at 23:26 +0200, Thomas Backlund wrote:
>>>
>>>> With this patch applied to 2.6.33-rc3 kernel crashes at boot...
>>>>
>>>> I have a iwl4965 connecting to a wpa2-psk encrypted network, arch is
>>>> x86_64, distro Mandriva Linux and the kernel has preempt enabled.
>>>>
>>>> Am I only missing some patches, or did I hit a real bug ?
>>>
>>> It'd help if you said what crashes and how since nobody else seems to be
>>> having that particular problem with this patch.
>> I have an idea.
>> The patch adds the following chunk of code:
>>> + if (!sta&& ra&& !is_multicast_ether_addr(ra)) {
>>> + sta = sta_info_get(sdata, ra);
>>> + if (sta)
>>> + sta_flags = get_sta_flags(sta);
>> In wireless-testing, sta_info_get takes sdata as first argument, in
>> 2.6.33-rc3 it expects a pointer to local. This should have emitted a
>> compiler warning... I saw the same thing when applying the patch to an
>> older compat-wireless snapshot.
>>
>
> Oh so it did (unfortunately the kernel build does not honour LC_ALL=C
> anymore, so the warning is in swedish):
>
>> net/mac80211/wme.c: I funktion "ieee80211_select_queue":
>> net/mac80211/wme.c:99: varning: skickar argument 1 till "sta_info_get"
>> fr?n inkompatibel pekartyp
>> net/mac80211/sta_info.h:408: anm: "struct ieee80211_local *"
>> f?rv?ntades men argumentet har typ "struct ieee80211_sub_if_data *"
>
As I cant get it from any log, and dont have a camera available right
now, I just wrote some lines to paper wich confirms the breakage in the
above build error...
The trace shows:
RIP: sta_info_get
ieee80211_select_queue
ieee80211_netdev_select_queue
...
Now I assume this will work ok when wireless-testing gets merged into
main, but as this one is also tagged for stable 2.6.32, there is some
more fixes needed to be queued for stable...
--
Thomas
The according patch in wireless-2.6 [1] has in net/mac80211/iface.c:
.ndo_set_mac_address = eth_mac_addr
...
.ndo_set_mac_address = eth_mac_addr
In the above mentionned, it is:
.ndo_set_mac_address = ieee80211_change_mac,
...
.ndo_set_mac_address = eth_mac_addr,
I guess in Johannes' patch this should be in both cases the same
(ieee80211_change_mac).
NOTE:
AFAICS, "mac80211: fix-up build breakage in 2.6.33" [2] is required in addition.
- Sedat -
[1] http://git.kernel.org/?p=linux/kernel/git/linville/wireless-2.6.git;a=blobdiff;f=net/mac80211/iface.c;h=c261cdb359ebbff176245430761512dc743b084c;hp=80c16f6e2af67e2798da4c417d69ce94dae86b71;hb=cf0277e714a0db302a8f80e1b85fd61c32cf00b3;hpb=301a8234ea81938f0f083ae4e274d9c9296f3c86
[2] http://git.kernel.org/?p=linux/kernel/git/linville/wireless-2.6.git;a=commit;h=debde9ea24d5512400456b1b64df361e422f078d
On Thu, Jan 7, 2010 at 5:10 AM, Lennert Buytenhek
<[email protected]> wrote:
> On Wed, Jan 06, 2010 at 11:31:10PM +0100, Sedat Dilek wrote:
>
>> Why are there two different ".ndo_set_mac_address" in [1]?
>>
>> static const struct net_device_ops ieee80211_dataif_ops = {
>> ...
>> .ndo_set_mac_address = ieee80211_change_mac,
>> + .ndo_select_queue = ieee80211_netdev_select_queue,
>> };
>>
>> static const struct net_device_ops ieee80211_monitorif_ops = {
>> ...
>> .ndo_set_mac_address = eth_mac_addr,
>> + .ndo_select_queue = ieee80211_monitor_select_queue,
>> };
>
> Because a different queue selection algorithm is needed for
> monitor (ieee80211_monitor_select_queue) and non-monitor
> (ieee80211_netdev_select_queue) interfaces. The commit message
> mentions this.
>
Commit 'mac80211: fix skb buffering issue' added an ->ndo_select_queue()
for monitor interfaces which can end up dereferencing ieee802_1d_to_ac[]
beyond the end of the array for injected data packets (as skb->priority
isn't guaranteed to be zero or within [0:7]), which then triggers the
WARN_ON in net/core/dev.c:dev_cap_txqueue(). Fix this by always setting
the priority to zero on injected data frames.
Signed-off-by: Lennert Buytenhek <[email protected]>
--
We have seen some crashing with crypto enabled with the multiqueue
patch due to this issue, so some fix along these lines is fairly
important.
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index c20dddd..2f34662 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -699,6 +699,7 @@ static u16 ieee80211_monitor_select_queue(struct net_device *dev,
return ieee802_1d_to_ac[skb->priority];
}
+ skb->priority = 0;
return ieee80211_downgrade_queue(local, skb);
}
---
Why are there two different ".ndo_set_mac_address" in [1]?
static const struct net_device_ops ieee80211_dataif_ops = {
...
.ndo_set_mac_address = ieee80211_change_mac,
+ .ndo_select_queue = ieee80211_netdev_select_queue,
};
static const struct net_device_ops ieee80211_monitorif_ops = {
...
.ndo_set_mac_address = eth_mac_addr,
+ .ndo_select_queue = ieee80211_monitor_select_queue,
};
Your patch needs following fix to be applicable against 2.6.33-rc3:
--- linux-2.6.33-rc3.orig/net/mac80211/iface.c
+++ linux-2.6.33-rc3/net/mac80211/iface.c
@@ -651,7 +651,7 @@
.ndo_start_xmit = ieee80211_subif_start_xmit,
.ndo_set_multicast_list = ieee80211_set_multicast_list,
.ndo_change_mtu = ieee80211_change_mtu,
- .ndo_set_mac_address = eth_mac_addr,
+ .ndo_set_mac_address = ieee80211_change_mac,
};
static const struct net_device_ops ieee80211_monitorif_ops = {
-EOF-
Kind Regards,
- Sedat -
[1] http://patchwork.kernel.org/patch/71058/
On Tue, Jan 5, 2010 at 6:00 PM, Johannes Berg <[email protected]> wrote:
> Since I removed the master netdev, we've been
> keeping internal queues only, and even before
> that we never told the networking stack above
> the virtual interfaces about congestion. This
> means that packets are queued in mac80211 and
> the upper layers never know, possibly leading
> to memory exhaustion and other problems.
>
> This patch makes all interfaces multiqueue and
> uses ndo_select_queue to put the packets into
> queues per AC. Additionally, when the driver
> stops a queue, we now stop all corresponding
> queues for the virtual interfaces as well.
>
> The injection case will use VO by default for
> non-data frames, and BE for data frames, but
> downgrade any data frames according to ACM. It
> needs to be fleshed out in the future to allow
> chosing the queue/AC in radiotap.
>
> Reported-by: Lennert Buytenhek <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> Cc: [email protected] [2.6.32]
> ---
> I know it's late, and large, but still would be good to have in .33
> since the issue is fairly serious.
>
> net/mac80211/iface.c | 39 +++++++++++++++++++-
> net/mac80211/rx.c | 4 +-
> net/mac80211/tx.c | 5 ++
> net/mac80211/util.c | 12 ++++++
> net/mac80211/wme.c | 96 +++++++++++++++++++++++++++++++++++++--------------
> net/mac80211/wme.h | 8 +++-
> 6 files changed, 132 insertions(+), 32 deletions(-)
>
> --- wireless-testing.orig/net/mac80211/iface.c 2010-01-05 16:06:21.000000000 +0100
> +++ wireless-testing/net/mac80211/iface.c 2010-01-05 16:26:55.000000000 +0100
> @@ -15,12 +15,14 @@
> #include <linux/netdevice.h>
> #include <linux/rtnetlink.h>
> #include <net/mac80211.h>
> +#include <net/ieee80211_radiotap.h>
> #include "ieee80211_i.h"
> #include "sta_info.h"
> #include "debugfs_netdev.h"
> #include "mesh.h"
> #include "led.h"
> #include "driver-ops.h"
> +#include "wme.h"
>
> /**
> * DOC: Interface list locking
> @@ -657,6 +659,12 @@ static void ieee80211_teardown_sdata(str
> WARN_ON(flushed);
> }
>
> +static u16 ieee80211_netdev_select_queue(struct net_device *dev,
> + struct sk_buff *skb)
> +{
> + return ieee80211_select_queue(IEEE80211_DEV_TO_SUB_IF(dev), skb);
> +}
> +
> static const struct net_device_ops ieee80211_dataif_ops = {
> .ndo_open = ieee80211_open,
> .ndo_stop = ieee80211_stop,
> @@ -665,8 +673,34 @@ static const struct net_device_ops ieee8
> .ndo_set_multicast_list = ieee80211_set_multicast_list,
> .ndo_change_mtu = ieee80211_change_mtu,
> .ndo_set_mac_address = ieee80211_change_mac,
> + .ndo_select_queue = ieee80211_netdev_select_queue,
> };
>
> +static u16 ieee80211_monitor_select_queue(struct net_device *dev,
> + struct sk_buff *skb)
> +{
> + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + struct ieee80211_local *local = sdata->local;
> + struct ieee80211_hdr *hdr;
> + struct ieee80211_radiotap_header *rtap = (void *)skb->data;
> +
> + if (local->hw.queues < 4)
> + return 0;
> +
> + if (skb->len < 4 ||
> + skb->len < rtap->it_len + 2 /* frame control */)
> + return 0; /* doesn't matter, frame will be dropped */
> +
> + hdr = (void *)((u8 *)skb->data + rtap->it_len);
> +
> + if (!ieee80211_is_data(hdr->frame_control)) {
> + skb->priority = 7;
> + return ieee802_1d_to_ac[skb->priority];
> + }
> +
> + return ieee80211_downgrade_queue(local, skb);
> +}
> +
> static const struct net_device_ops ieee80211_monitorif_ops = {
> .ndo_open = ieee80211_open,
> .ndo_stop = ieee80211_stop,
> @@ -675,6 +709,7 @@ static const struct net_device_ops ieee8
> .ndo_set_multicast_list = ieee80211_set_multicast_list,
> .ndo_change_mtu = ieee80211_change_mtu,
> .ndo_set_mac_address = eth_mac_addr,
> + .ndo_select_queue = ieee80211_monitor_select_queue,
> };
>
> static void ieee80211_if_setup(struct net_device *dev)
> @@ -781,8 +816,8 @@ int ieee80211_if_add(struct ieee80211_lo
>
> ASSERT_RTNL();
>
> - ndev = alloc_netdev(sizeof(*sdata) + local->hw.vif_data_size,
> - name, ieee80211_if_setup);
> + ndev = alloc_netdev_mq(sizeof(*sdata) + local->hw.vif_data_size,
> + name, ieee80211_if_setup, local->hw.queues);
> if (!ndev)
> return -ENOMEM;
> dev_net_set(ndev, wiphy_net(local->hw.wiphy));
> --- wireless-testing.orig/net/mac80211/tx.c 2010-01-05 16:06:22.000000000 +0100
> +++ wireless-testing/net/mac80211/tx.c 2010-01-05 16:06:51.000000000 +0100
> @@ -1511,7 +1511,7 @@ static void ieee80211_xmit(struct ieee80
> return;
> }
>
> - ieee80211_select_queue(local, skb);
> + ieee80211_set_qos_hdr(local, skb);
> ieee80211_tx(sdata, skb, false);
> rcu_read_unlock();
> }
> @@ -2289,6 +2289,9 @@ void ieee80211_tx_skb(struct ieee80211_s
> skb_set_network_header(skb, 0);
> skb_set_transport_header(skb, 0);
>
> + /* send all internal mgmt frames on VO */
> + skb_set_queue_mapping(skb, 0);
> +
> /*
> * The other path calling ieee80211_xmit is from the tasklet,
> * and while we can handle concurrent transmissions locking
> --- wireless-testing.orig/net/mac80211/wme.c 2010-01-05 16:06:21.000000000 +0100
> +++ wireless-testing/net/mac80211/wme.c 2010-01-05 16:15:15.000000000 +0100
> @@ -44,22 +44,69 @@ static int wme_downgrade_ac(struct sk_bu
> }
>
>
> -/* Indicate which queue to use. */
> -static u16 classify80211(struct ieee80211_local *local, struct sk_buff *skb)
> +/* Indicate which queue to use. */
> +u16 ieee80211_select_queue(struct ieee80211_sub_if_data *sdata,
> + struct sk_buff *skb)
> {
> - struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> + struct ieee80211_local *local = sdata->local;
> + struct sta_info *sta = NULL;
> + u32 sta_flags = 0;
> + const u8 *ra = NULL;
> + bool qos = false;
>
> - if (!ieee80211_is_data(hdr->frame_control)) {
> - /* management frames go on AC_VO queue, but are sent
> - * without QoS control fields */
> - return 0;
> + if (local->hw.queues < 4 || skb->len < 6) {
> + skb->priority = 0; /* required for correct WPA/11i MIC */
> + return min_t(u16, local->hw.queues - 1,
> + ieee802_1d_to_ac[skb->priority]);
> + }
> +
> + rcu_read_lock();
> + switch (sdata->vif.type) {
> + case NL80211_IFTYPE_AP_VLAN:
> + rcu_read_lock();
> + sta = rcu_dereference(sdata->u.vlan.sta);
> + if (sta)
> + sta_flags = get_sta_flags(sta);
> + rcu_read_unlock();
> + if (sta)
> + break;
> + case NL80211_IFTYPE_AP:
> + ra = skb->data;
> + break;
> + case NL80211_IFTYPE_WDS:
> + ra = sdata->u.wds.remote_addr;
> + break;
> +#ifdef CONFIG_MAC80211_MESH
> + case NL80211_IFTYPE_MESH_POINT:
> + /*
> + * XXX: This is clearly broken ... but already was before,
> + * because ieee80211_fill_mesh_addresses() would clear A1
> + * except for multicast addresses.
> + */
> + break;
> +#endif
> + case NL80211_IFTYPE_STATION:
> + ra = sdata->u.mgd.bssid;
> + break;
> + case NL80211_IFTYPE_ADHOC:
> + ra = skb->data;
> + break;
> + default:
> + break;
> }
>
> - if (0 /* injected */) {
> - /* use AC from radiotap */
> + if (!sta && ra && !is_multicast_ether_addr(ra)) {
> + sta = sta_info_get(sdata, ra);
> + if (sta)
> + sta_flags = get_sta_flags(sta);
> }
>
> - if (!ieee80211_is_data_qos(hdr->frame_control)) {
> + if (sta_flags & WLAN_STA_WME)
> + qos = true;
> +
> + rcu_read_unlock();
> +
> + if (!qos) {
> skb->priority = 0; /* required for correct WPA/11i MIC */
> return ieee802_1d_to_ac[skb->priority];
> }
> @@ -68,6 +115,12 @@ static u16 classify80211(struct ieee8021
> * data frame has */
> skb->priority = cfg80211_classify8021d(skb);
>
> + return ieee80211_downgrade_queue(local, skb);
> +}
> +
> +u16 ieee80211_downgrade_queue(struct ieee80211_local *local,
> + struct sk_buff *skb)
> +{
> /* in case we are a client verify acm is not set for this ac */
> while (unlikely(local->wmm_acm & BIT(skb->priority))) {
> if (wme_downgrade_ac(skb)) {
> @@ -85,24 +138,17 @@ static u16 classify80211(struct ieee8021
> return ieee802_1d_to_ac[skb->priority];
> }
>
> -void ieee80211_select_queue(struct ieee80211_local *local, struct sk_buff *skb)
> +void ieee80211_set_qos_hdr(struct ieee80211_local *local, struct sk_buff *skb)
> {
> - struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> - u16 queue;
> - u8 tid;
> -
> - queue = classify80211(local, skb);
> - if (unlikely(queue >= local->hw.queues))
> - queue = local->hw.queues - 1;
> -
> - /*
> - * Now we know the 1d priority, fill in the QoS header if
> - * there is one (and we haven't done this before).
> - */
> + struct ieee80211_hdr *hdr = (void *)skb->data;
> +
> + /* Fill in the QoS header if there is one. */
> if (ieee80211_is_data_qos(hdr->frame_control)) {
> u8 *p = ieee80211_get_qos_ctl(hdr);
> - u8 ack_policy = 0;
> + u8 ack_policy = 0, tid;
> +
> tid = skb->priority & IEEE80211_QOS_CTL_TAG1D_MASK;
> +
> if (unlikely(local->wifi_wme_noack_test))
> ack_policy |= QOS_CONTROL_ACK_POLICY_NOACK <<
> QOS_CONTROL_ACK_POLICY_SHIFT;
> @@ -110,6 +156,4 @@ void ieee80211_select_queue(struct ieee8
> *p++ = ack_policy | tid;
> *p = 0;
> }
> -
> - skb_set_queue_mapping(skb, queue);
> }
> --- wireless-testing.orig/net/mac80211/wme.h 2010-01-05 16:06:21.000000000 +0100
> +++ wireless-testing/net/mac80211/wme.h 2010-01-05 16:15:26.000000000 +0100
> @@ -20,7 +20,11 @@
>
> extern const int ieee802_1d_to_ac[8];
>
> -void ieee80211_select_queue(struct ieee80211_local *local,
> - struct sk_buff *skb);
> +u16 ieee80211_select_queue(struct ieee80211_sub_if_data *sdata,
> + struct sk_buff *skb);
> +void ieee80211_set_qos_hdr(struct ieee80211_local *local, struct sk_buff *skb);
> +u16 ieee80211_downgrade_queue(struct ieee80211_local *local,
> + struct sk_buff *skb);
> +
>
> #endif /* _WME_H */
> --- wireless-testing.orig/net/mac80211/rx.c 2010-01-05 16:06:21.000000000 +0100
> +++ wireless-testing/net/mac80211/rx.c 2010-01-05 16:06:51.000000000 +0100
> @@ -1665,7 +1665,9 @@ ieee80211_rx_h_mesh_fwding(struct ieee80
> memset(info, 0, sizeof(*info));
> info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
> info->control.vif = &rx->sdata->vif;
> - ieee80211_select_queue(local, fwd_skb);
> + skb_set_queue_mapping(skb,
> + ieee80211_select_queue(rx->sdata, fwd_skb));
> + ieee80211_set_qos_hdr(local, skb);
> if (is_multicast_ether_addr(fwd_hdr->addr1))
> IEEE80211_IFSTA_MESH_CTR_INC(&sdata->u.mesh,
> fwded_mcast);
> --- wireless-testing.orig/net/mac80211/util.c 2010-01-05 16:06:21.000000000 +0100
> +++ wireless-testing/net/mac80211/util.c 2010-01-05 16:06:51.000000000 +0100
> @@ -269,6 +269,7 @@ static void __ieee80211_wake_queue(struc
> enum queue_stop_reason reason)
> {
> struct ieee80211_local *local = hw_to_local(hw);
> + struct ieee80211_sub_if_data *sdata;
>
> if (WARN_ON(queue >= hw->queues))
> return;
> @@ -281,6 +282,11 @@ static void __ieee80211_wake_queue(struc
>
> if (!skb_queue_empty(&local->pending[queue]))
> tasklet_schedule(&local->tx_pending_tasklet);
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(sdata, &local->interfaces, list)
> + netif_tx_wake_queue(netdev_get_tx_queue(sdata->dev, queue));
> + rcu_read_unlock();
> }
>
> void ieee80211_wake_queue_by_reason(struct ieee80211_hw *hw, int queue,
> @@ -305,11 +311,17 @@ static void __ieee80211_stop_queue(struc
> enum queue_stop_reason reason)
> {
> struct ieee80211_local *local = hw_to_local(hw);
> + struct ieee80211_sub_if_data *sdata;
>
> if (WARN_ON(queue >= hw->queues))
> return;
>
> __set_bit(reason, &local->queue_stop_reasons[queue]);
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(sdata, &local->interfaces, list)
> + netif_tx_stop_queue(netdev_get_tx_queue(sdata->dev, queue));
> + rcu_read_unlock();
> }
>
> void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 05.01.2010 19:00, Johannes Berg
wrote:<johannes-<johannes-<[email protected]>[email protected]>[email protected]>
> Since I removed the master netdev, we've been
> keeping internal queues only, and even before
> that we never told the networking stack above
> the virtual interfaces about congestion. This
> means that packets are queued in mac80211 and
> the upper layers never know, possibly leading
> to memory exhaustion and other problems.
>
> This patch makes all interfaces multiqueue and
> uses ndo_select_queue to put the packets into
> queues per AC. Additionally, when the driver
> stops a queue, we now stop all corresponding
> queues for the virtual interfaces as well.
>
> The injection case will use VO by default for
> non-data frames, and BE for data frames, but
> downgrade any data frames according to ACM. It
> needs to be fleshed out in the future to allow
> chosing the queue/AC in radiotap.
>
> Reported-by: Lennert Buytenhek<[email protected]>
> Signed-off-by: Johannes Berg<[email protected]>
> Cc: [email protected] [2.6.32]
> ---
> I know it's late, and large, but still would be good to have in .33
> since the issue is fairly serious.
>
> net/mac80211/iface.c | 39 +++++++++++++++++++-
> net/mac80211/rx.c | 4 +-
> net/mac80211/tx.c | 5 ++
> net/mac80211/util.c | 12 ++++++
> net/mac80211/wme.c | 96 +++++++++++++++++++++++++++++++++++++--------------
> net/mac80211/wme.h | 8 +++-
> 6 files changed, 132 insertions(+), 32 deletions(-)
>
With this patch applied to 2.6.33-rc3 kernel crashes at boot...
I have a iwl4965 connecting to a wpa2-psk encrypted network, arch is
x86_64, distro Mandriva Linux and the kernel has preempt enabled.
Am I only missing some patches, or did I hit a real bug ?
--
Thomas
On Wed, 2010-01-06 at 23:26 +0200, Thomas Backlund wrote:
> With this patch applied to 2.6.33-rc3 kernel crashes at boot...
>
> I have a iwl4965 connecting to a wpa2-psk encrypted network, arch is
> x86_64, distro Mandriva Linux and the kernel has preempt enabled.
>
> Am I only missing some patches, or did I hit a real bug ?
It'd help if you said what crashes and how since nobody else seems to be
having that particular problem with this patch.
johannes
On Wed, Jan 06, 2010 at 11:31:10PM +0100, Sedat Dilek wrote:
> Why are there two different ".ndo_set_mac_address" in [1]?
>
> static const struct net_device_ops ieee80211_dataif_ops = {
> ...
> .ndo_set_mac_address = ieee80211_change_mac,
> + .ndo_select_queue = ieee80211_netdev_select_queue,
> };
>
> static const struct net_device_ops ieee80211_monitorif_ops = {
> ...
> .ndo_set_mac_address = eth_mac_addr,
> + .ndo_select_queue = ieee80211_monitor_select_queue,
> };
Because a different queue selection algorithm is needed for
monitor (ieee80211_monitor_select_queue) and non-monitor
(ieee80211_netdev_select_queue) interfaces. The commit message
mentions this.
On Tue, Jan 05, 2010 at 06:00:58PM +0100, Johannes Berg wrote:
> Since I removed the master netdev, we've been
> keeping internal queues only, and even before
> that we never told the networking stack above
> the virtual interfaces about congestion. This
> means that packets are queued in mac80211 and
> the upper layers never know, possibly leading
> to memory exhaustion and other problems.
>
> This patch makes all interfaces multiqueue and
> uses ndo_select_queue to put the packets into
> queues per AC. Additionally, when the driver
> stops a queue, we now stop all corresponding
> queues for the virtual interfaces as well.
>
> The injection case will use VO by default for
> non-data frames, and BE for data frames, but
> downgrade any data frames according to ACM. It
> needs to be fleshed out in the future to allow
> chosing the queue/AC in radiotap.
>
> Reported-by: Lennert Buytenhek <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> Cc: [email protected] [2.6.32]
> ---
> I know it's late, and large, but still would be good to have in .33
> since the issue is fairly serious.
Obviously I'd like to see some testing. Lennert, does this patch
resolve the issues you raised?
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On 07.01.2010 08:57, Johannes Berg wrote:
> On Thu, 2010-01-07 at 01:19 +0200, Thomas Backlund wrote:
>
>>>> net/mac80211/wme.c: I funktion "ieee80211_select_queue":
>>>> net/mac80211/wme.c:99: varning: skickar argument 1 till
>> "sta_info_get"
>>>> fr?n inkompatibel pekartyp
>>>> net/mac80211/sta_info.h:408: anm: "struct ieee80211_local *"
>>>> f?rv?ntades men argumentet har typ "struct ieee80211_sub_if_data *"
>>>
>>
>> As I cant get it from any log, and dont have a camera available right
>> now, I just wrote some lines to paper wich confirms the breakage in
>> the
>> above build error...
>>
>> The trace shows:
>>
>> RIP: sta_info_get
>
> Well as the message tells you, sta_info_get needs a local argument in
> your tree, rather than 'sdata', so you can just do 'sdata->local'
> there ...... It doesn't need any particular fixups, just a bit of merge
> resolving ...
>
Yeah,
that was it...
Now it works as intended!
Thanks!
--
Thomas
On Thu, 2010-01-07 at 01:19 +0200, Thomas Backlund wrote:
> >> net/mac80211/wme.c: I funktion "ieee80211_select_queue":
> >> net/mac80211/wme.c:99: varning: skickar argument 1 till
> "sta_info_get"
> >> fr?n inkompatibel pekartyp
> >> net/mac80211/sta_info.h:408: anm: "struct ieee80211_local *"
> >> f?rv?ntades men argumentet har typ "struct ieee80211_sub_if_data *"
> >
>
> As I cant get it from any log, and dont have a camera available right
> now, I just wrote some lines to paper wich confirms the breakage in
> the
> above build error...
>
> The trace shows:
>
> RIP: sta_info_get
Well as the message tells you, sta_info_get needs a local argument in
your tree, rather than 'sdata', so you can just do 'sdata->local'
there ...... It doesn't need any particular fixups, just a bit of merge
resolving ...
johannes
On 07.01.2010 00:03, Felix Fietkau wrote:
> On 2010-01-06 10:51 PM, Johannes Berg wrote:
>> On Wed, 2010-01-06 at 23:26 +0200, Thomas Backlund wrote:
>>
>>> With this patch applied to 2.6.33-rc3 kernel crashes at boot...
>>>
>>> I have a iwl4965 connecting to a wpa2-psk encrypted network, arch is
>>> x86_64, distro Mandriva Linux and the kernel has preempt enabled.
>>>
>>> Am I only missing some patches, or did I hit a real bug ?
>>
>> It'd help if you said what crashes and how since nobody else seems to be
>> having that particular problem with this patch.
> I have an idea.
> The patch adds the following chunk of code:
>> + if (!sta&& ra&& !is_multicast_ether_addr(ra)) {
>> + sta = sta_info_get(sdata, ra);
>> + if (sta)
>> + sta_flags = get_sta_flags(sta);
> In wireless-testing, sta_info_get takes sdata as first argument, in
> 2.6.33-rc3 it expects a pointer to local. This should have emitted a
> compiler warning... I saw the same thing when applying the patch to an
> older compat-wireless snapshot.
>
Oh so it did (unfortunately the kernel build does not honour LC_ALL=C
anymore, so the warning is in swedish):
> net/mac80211/wme.c: I funktion "ieee80211_select_queue":
> net/mac80211/wme.c:99: varning: skickar argument 1 till "sta_info_get" fr?n inkompatibel pekartyp
> net/mac80211/sta_info.h:408: anm: "struct ieee80211_local *" f?rv?ntades men argumentet har typ "struct ieee80211_sub_if_data *"
--
Thomas
On 01/05/2010 11:00 AM, Johannes Berg wrote:
> Since I removed the master netdev, we've been
> keeping internal queues only, and even before
--snip--
> Reported-by: Lennert Buytenhek <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> Cc: [email protected] [2.6.32]
> ---
> I know it's late, and large, but still would be good to have in .33
> since the issue is fairly serious.
I'm not sure this is ready for prime time. After I installed it, my b43 was
never able to reconnect after any kind of disconnect. The log is uninformative:
===========================================
b43-phy0 debug: Wireless interface started
b43-phy0 debug: Adding Interface type 2
wlan0: direct probe to 00:14:bf:85:49:fa (try 1)
wlan0: direct probe responded
wlan0: authenticate with 00:14:bf:85:49:fa (try 1)
wlan0: authenticated
wlan0: associate with 00:14:bf:85:49:fa (try 1)
wlan0: RX AssocResp from 00:14:bf:85:49:fa (capab=0x411 status=0 aid=1)
wlan0: associated
b43-phy0 debug: Using hardware based encryption for keyidx: 0, mac:
00:14:bf:85:49:fa
b43-phy0 debug: Disabling hardware based encryption for keyidx: 0, mac:
00:14:bf:85:49:fa
wlan0: deauthenticating from 00:14:bf:85:49:fa by local choice (reason=3)
===========================================
Reauthentication fails with a pop-up to reenter the WPA2 secret as I'm using
NetworkManager. I can only reconnect if b43 (and mac80211) are unloaded and
reloaded. I pulled the patch and confirmed that proper behavior is returned.
Larry
On Tue, Jan 05, 2010 at 12:16:03PM -0500, John W. Linville wrote:
> > Since I removed the master netdev, we've been
> > keeping internal queues only, and even before
> > that we never told the networking stack above
> > the virtual interfaces about congestion. This
> > means that packets are queued in mac80211 and
> > the upper layers never know, possibly leading
> > to memory exhaustion and other problems.
> >
> > This patch makes all interfaces multiqueue and
> > uses ndo_select_queue to put the packets into
> > queues per AC. Additionally, when the driver
> > stops a queue, we now stop all corresponding
> > queues for the virtual interfaces as well.
> >
> > The injection case will use VO by default for
> > non-data frames, and BE for data frames, but
> > downgrade any data frames according to ACM. It
> > needs to be fleshed out in the future to allow
> > chosing the queue/AC in radiotap.
> >
> > Reported-by: Lennert Buytenhek <[email protected]>
> > Signed-off-by: Johannes Berg <[email protected]>
> > Cc: [email protected] [2.6.32]
> > ---
> > I know it's late, and large, but still would be good to have in .33
> > since the issue is fairly serious.
>
> Obviously I'd like to see some testing. Lennert, does this
> patch resolve the issues you raised?
I had some QA done on 2.6.32 + this version of the patch (slightly
tweaked to make it apply to .32), and it solves the OOM issues we were
seeing, throughput looks good, the 8% more cpu that 2.6.32 uses
relative to 2.6.31 in the same forwarding test seems to be at least
partially taken care of by this patch, and there haven't been any
stability issues so far.
On Tue, 2010-01-05 at 23:30 -0600, Larry Finger wrote:
> Reauthentication fails with a pop-up to reenter the WPA2 secret as I'm
> using NetworkManager. I can only reconnect if b43 (and mac80211) are
> unloaded and reloaded. I pulled the patch and confirmed that proper
> behavior is returned.
Ouch.
I can't seem to reproduce this with hwsim, but it never touches the
queues.
I think it may be because I forgot about
53623f1a09c7a7d23b74f0f7d93dba0ebde1006b ...
The patch below is certainly needed, but I'm not entirely sure it'll fix
your issue, can you give it a try please?
johannes
--- wireless-testing.orig/net/mac80211/iface.c 2010-01-06 12:00:03.000000000 +0100
+++ wireless-testing/net/mac80211/iface.c 2010-01-06 12:02:17.000000000 +0100
@@ -328,7 +328,7 @@ static int ieee80211_open(struct net_dev
if (sdata->vif.type == NL80211_IFTYPE_STATION)
ieee80211_queue_work(&local->hw, &sdata->u.mgd.work);
- netif_start_queue(dev);
+ netif_tx_start_all_queues(dev);
return 0;
err_del_interface:
@@ -356,7 +356,7 @@ static int ieee80211_stop(struct net_dev
/*
* Stop TX on this interface first.
*/
- netif_stop_queue(dev);
+ netif_tx_stop_all_queues(dev);
/*
* Purge work for this interface.
--- wireless-testing.orig/net/mac80211/mlme.c 2010-01-06 12:00:03.000000000 +0100
+++ wireless-testing/net/mac80211/mlme.c 2010-01-06 12:02:17.000000000 +0100
@@ -731,7 +731,7 @@ static void ieee80211_set_associated(str
ieee80211_recalc_smps(local, sdata);
mutex_unlock(&local->iflist_mtx);
- netif_start_queue(sdata->dev);
+ netif_tx_start_all_queues(sdata->dev);
netif_carrier_on(sdata->dev);
}
@@ -767,7 +767,7 @@ static void ieee80211_set_disassoc(struc
* time -- we don't want the scan code to enable queues.
*/
- netif_stop_queue(sdata->dev);
+ netif_tx_stop_all_queues(sdata->dev);
netif_carrier_off(sdata->dev);
rcu_read_lock();
--- wireless-testing.orig/net/mac80211/offchannel.c 2010-01-06 12:02:37.000000000 +0100
+++ wireless-testing/net/mac80211/offchannel.c 2010-01-06 12:03:02.000000000 +0100
@@ -113,7 +113,7 @@ void ieee80211_offchannel_stop_beaconing
*/
if (sdata->vif.type != NL80211_IFTYPE_STATION &&
sdata->vif.type != NL80211_IFTYPE_MONITOR)
- netif_stop_queue(sdata->dev);
+ netif_tx_stop_all_queues(sdata->dev);
}
mutex_unlock(&local->iflist_mtx);
}
@@ -131,7 +131,7 @@ void ieee80211_offchannel_stop_station(s
continue;
if (sdata->vif.type == NL80211_IFTYPE_STATION) {
- netif_stop_queue(sdata->dev);
+ netif_tx_stop_all_queues(sdata->dev);
if (sdata->u.mgd.associated)
ieee80211_offchannel_ps_enable(sdata);
}
@@ -153,7 +153,7 @@ void ieee80211_offchannel_return(struct
if (sdata->vif.type == NL80211_IFTYPE_STATION) {
if (sdata->u.mgd.associated)
ieee80211_offchannel_ps_disable(sdata);
- netif_wake_queue(sdata->dev);
+ netif_tx_wake_all_queues(sdata->dev);
}
/* re-enable beaconing */