2008-09-26 09:01:34

by Luis R. Rodriguez

[permalink] [raw]
Subject: [PATCH] mac80211: Fix oops on wme.c where mdev is assumed to be a wiphy

After Johannes' patch which set mdev as non wiphy
there were some remaining bits of code which required
changing the assumption that mdev was a wiphy. We
change two callers to get to struct ieee80211_local
the non-wiphy and actual mdev way.

Signed-off-by: Luis R. Rodriguez <[email protected]>
---

Tested with ath5k on v2.6.27-rc7-1972-gfb8961a.

net/mac80211/wme.c | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
index 6748ded..7d114bd 100644
--- a/net/mac80211/wme.c
+++ b/net/mac80211/wme.c
@@ -75,8 +75,16 @@ static int wme_downgrade_ac(struct sk_buff *skb)
/* Indicate which queue to use. */
static u16 classify80211(struct sk_buff *skb, struct net_device *dev)
{
- struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_local *local;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct ieee80211_master_priv *mpriv;
+
+ /* this is not for wiphys, just for mdev */
+ BUG_ON (dev->ieee80211_ptr);
+
+ mpriv = netdev_priv(dev);
+ local = mpriv->local;
+

if (!ieee80211_is_data(hdr->frame_control)) {
/* management frames go on AC_VO queue, but are sent
@@ -114,12 +122,19 @@ static u16 classify80211(struct sk_buff *skb, struct net_device *dev)
u16 ieee80211_select_queue(struct net_device *dev, struct sk_buff *skb)
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_local *local;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+ struct ieee80211_master_priv *mpriv;
struct sta_info *sta;
u16 queue;
u8 tid;

+ /* this is not for wiphys, just for mdev */
+ BUG_ON (dev->ieee80211_ptr);
+
+ mpriv = netdev_priv(dev);
+ local = mpriv->local;
+
queue = classify80211(skb, dev);
if (unlikely(queue >= local->hw.queues))
queue = local->hw.queues - 1;
--
1.5.6.rc2.15.g457bb.dirty



2008-09-26 11:29:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Fix oops on wme.c where mdev is assumed to be a wiphy

On Fri, 2008-09-26 at 12:51 +0200, Johannes Berg wrote:
> On Fri, 2008-09-26 at 02:01 -0700, Luis R. Rodriguez wrote:
> > After Johannes' patch which set mdev as non wiphy
> > there were some remaining bits of code which required
> > changing the assumption that mdev was a wiphy. We
> > change two callers to get to struct ieee80211_local
> > the non-wiphy and actual mdev way.
> >
> > Signed-off-by: Luis R. Rodriguez <[email protected]>
>
> Acked-by: Johannes Berg <[email protected]>
>
> I _really_ wonder why I haven't run into this. Odd. Thanks for fixing.

Damn. Now I know. I forgot to add these changes to the patch in quilt!
And I have another one too.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-26 10:52:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: Fix oops on wme.c where mdev is assumed to be a wiphy

On Fri, 2008-09-26 at 02:01 -0700, Luis R. Rodriguez wrote:
> After Johannes' patch which set mdev as non wiphy
> there were some remaining bits of code which required
> changing the assumption that mdev was a wiphy. We
> change two callers to get to struct ieee80211_local
> the non-wiphy and actual mdev way.
>
> Signed-off-by: Luis R. Rodriguez <[email protected]>

Acked-by: Johannes Berg <[email protected]>

I _really_ wonder why I haven't run into this. Odd. Thanks for fixing.

> ---
>
> Tested with ath5k on v2.6.27-rc7-1972-gfb8961a.
>
> net/mac80211/wme.c | 19 +++++++++++++++++--
> 1 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/net/mac80211/wme.c b/net/mac80211/wme.c
> index 6748ded..7d114bd 100644
> --- a/net/mac80211/wme.c
> +++ b/net/mac80211/wme.c
> @@ -75,8 +75,16 @@ static int wme_downgrade_ac(struct sk_buff *skb)
> /* Indicate which queue to use. */
> static u16 classify80211(struct sk_buff *skb, struct net_device *dev)
> {
> - struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> + struct ieee80211_local *local;
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> + struct ieee80211_master_priv *mpriv;
> +
> + /* this is not for wiphys, just for mdev */
> + BUG_ON (dev->ieee80211_ptr);
> +
> + mpriv = netdev_priv(dev);
> + local = mpriv->local;
> +
>
> if (!ieee80211_is_data(hdr->frame_control)) {
> /* management frames go on AC_VO queue, but are sent
> @@ -114,12 +122,19 @@ static u16 classify80211(struct sk_buff *skb, struct net_device *dev)
> u16 ieee80211_select_queue(struct net_device *dev, struct sk_buff *skb)
> {
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> - struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> + struct ieee80211_local *local;
> struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
> + struct ieee80211_master_priv *mpriv;
> struct sta_info *sta;
> u16 queue;
> u8 tid;
>
> + /* this is not for wiphys, just for mdev */
> + BUG_ON (dev->ieee80211_ptr);
> +
> + mpriv = netdev_priv(dev);
> + local = mpriv->local;
> +
> queue = classify80211(skb, dev);
> if (unlikely(queue >= local->hw.queues))
> queue = local->hw.queues - 1;


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-26 17:37:20

by Andrey Yurovsky

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fixups for "make master iface not wireless"

Is it possible that further fixups are needed? I cannot add
interfaces (for example, via iw) because doing so causes
ieee80211_change_iface() to return -ENODEV when the master interface
is looked up by __dev_get_by_index()

-Andrey

2008-09-26 11:35:00

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] mac80211: fixups for "make master iface not wireless"

In "mac80211: make master iface not wireless" I accidentally
forgot to include these changes ... leading to the expected
BUG_ON errors.

Signed-off-by: Johannes Berg <[email protected]>
---
The wme.c change is pretty much identical to Luis's fix,
the iface.c change is not strictly required, it's just an
additional cleanup.

Sorry I goofed this, it seems I simply forgot to 'quilt edit' the two
files in question, this is the diff I had in my tree after quilt pop -a.

net/mac80211/iface.c | 9 ++++-----
net/mac80211/wme.c | 8 ++++----
2 files changed, 8 insertions(+), 9 deletions(-)

--- everything.orig/net/mac80211/iface.c 2008-09-26 13:31:28.000000000 +0200
+++ everything/net/mac80211/iface.c 2008-09-26 13:31:32.000000000 +0200
@@ -58,8 +58,9 @@ static inline int identical_mac_addr_all

static int ieee80211_open(struct net_device *dev)
{
- struct ieee80211_sub_if_data *sdata, *nsdata;
- struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_sub_if_data *nsdata;
+ struct ieee80211_local *local = sdata->local;
struct sta_info *sta;
struct ieee80211_if_init_conf conf;
u32 changed = 0;
@@ -67,8 +68,6 @@ static int ieee80211_open(struct net_dev
bool need_hw_reconfig = 0;
u8 null_addr[ETH_ALEN] = {0};

- sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-
/* fail early if user set an invalid address */
if (compare_ether_addr(dev->dev_addr, null_addr) &&
!is_valid_ether_addr(dev->dev_addr))
@@ -512,8 +511,8 @@ static int ieee80211_stop(struct net_dev

static void ieee80211_set_multicast_list(struct net_device *dev)
{
- struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
int allmulti, promisc, sdata_allmulti, sdata_promisc;

allmulti = !!(dev->flags & IFF_ALLMULTI);
--- everything.orig/net/mac80211/wme.c 2008-09-26 13:31:28.000000000 +0200
+++ everything/net/mac80211/wme.c 2008-09-26 13:31:32.000000000 +0200
@@ -73,9 +73,8 @@ static int wme_downgrade_ac(struct sk_bu


/* Indicate which queue to use. */
-static u16 classify80211(struct sk_buff *skb, struct net_device *dev)
+static u16 classify80211(struct ieee80211_local *local, struct sk_buff *skb)
{
- struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;

if (!ieee80211_is_data(hdr->frame_control)) {
@@ -113,14 +112,15 @@ static u16 classify80211(struct sk_buff

u16 ieee80211_select_queue(struct net_device *dev, struct sk_buff *skb)
{
+ struct ieee80211_master_priv *mpriv = netdev_priv(dev);
+ struct ieee80211_local *local = mpriv->local;
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
- struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct sta_info *sta;
u16 queue;
u8 tid;

- queue = classify80211(skb, dev);
+ queue = classify80211(local, skb);
if (unlikely(queue >= local->hw.queues))
queue = local->hw.queues - 1;




2008-09-26 19:16:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fixups for "make master iface not wireless"

On Fri, 2008-09-26 at 10:40 -0700, Andrey Yurovsky wrote:
> Ah, I see now that I can add interface on the non-master intefaces
> (ex: wlan0), so that's probably the intention. Thanks,

Indeed, you can add them as "siblings" (to other interfaces) or to the
parent "phy" device with iw:

dev <devname> interface add <name> type <type> [mesh_id <meshid>]
phy <phyname> interface add <name> type <type> [mesh_id <meshid>]

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-26 17:40:24

by Andrey Yurovsky

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fixups for "make master iface not wireless"

Ah, I see now that I can add interface on the non-master intefaces
(ex: wlan0), so that's probably the intention. Thanks,

-Andrey

2008-09-26 12:57:52

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fixups for "make master iface not wireless"

Johannes Berg wrote:
> In "mac80211: make master iface not wireless" I accidentally
> forgot to include these changes ... leading to the expected
> BUG_ON errors.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> The wme.c change is pretty much identical to Luis's fix,
> the iface.c change is not strictly required, it's just an
> additional cleanup.
>
> Sorry I goofed this, it seems I simply forgot to 'quilt edit' the two
> files in question, this is the diff I had in my tree after quilt pop -a.

This one fixes it for me. As both Luis's and Johannes's fixes were
available this morning, I only tested this one.

Larry