2014-01-16 13:34:52

by Pontus Fuchs

[permalink] [raw]
Subject: [PATCH] nl80211: Reset split_start on error path when split dump is not used

When the netlink skb is exhausted split_start is left set. In the
subsequent retry, with a larger buffer, the dump is continued from the
failing point instead of from the beginning.

This was causing my rt28xx based USB dongle to now show up when
running "iw list" with an old iw version without split dump support.

Signed-off-by: Pontus Fuchs <[email protected]>
---
net/wireless/nl80211.c | 2 ++
1 file changed, 2 insertions(+)

Supersedes "nl80211: Don't touch split_start when split dump is not used"

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d0afd82..ba6d438 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1631,6 +1631,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
return genlmsg_end(msg, hdr);

nla_put_failure:
+ if (!state->split)
+ state->split_start = 0;
genlmsg_cancel(msg, hdr);
return -EMSGSIZE;
}
--
1.8.3.2



2014-01-16 14:00:01

by Pontus Fuchs

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

On 2014-01-16 14:53, Johannes Berg wrote:

>>> @@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
>>> !skb->len &&
>>> cb->min_dump_alloc < 4096) {
>>> cb->min_dump_alloc = 4096;
>>> + state->split_start = 0;
>>> rtnl_unlock();
>>> return 1;
>>> }
>>
>> Will that not cause problem if split scan is in use and one of the split
>> chunks needs to be resent due to EMSGSIZE?
>
> That's not supposed to happen, and the "!skb->len" check should catch
> it.
>
> johannes

Ok. Thanks. Lets go for that version then.

//Pontus



2014-01-16 13:41:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

On Thu, 2014-01-16 at 14:34 +0100, Pontus Fuchs wrote:
> When the netlink skb is exhausted split_start is left set. In the
> subsequent retry, with a larger buffer, the dump is continued from the
> failing point instead of from the beginning.

Hmm. Maybe it should be like this instead?

johannes

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 138dc3b..2e08af5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
!skb->len &&
cb->min_dump_alloc < 4096) {
cb->min_dump_alloc = 4096;
+ state->split_start = 0;
rtnl_unlock();
return 1;
}



2014-01-16 13:51:04

by Pontus Fuchs

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

On 2014-01-16 14:41, Johannes Berg wrote:
> On Thu, 2014-01-16 at 14:34 +0100, Pontus Fuchs wrote:
>> When the netlink skb is exhausted split_start is left set. In the
>> subsequent retry, with a larger buffer, the dump is continued from the
>> failing point instead of from the beginning.
>
> Hmm. Maybe it should be like this instead?
>
> johannes
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 138dc3b..2e08af5 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
> !skb->len &&
> cb->min_dump_alloc < 4096) {
> cb->min_dump_alloc = 4096;
> + state->split_start = 0;
> rtnl_unlock();
> return 1;
> }

Will that not cause problem if split scan is in use and one of the split
chunks needs to be resent due to EMSGSIZE?

//Pontus


2014-01-16 13:53:20

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

On Thu, 2014-01-16 at 14:50 +0100, Pontus Fuchs wrote:
> On 2014-01-16 14:41, Johannes Berg wrote:
> > On Thu, 2014-01-16 at 14:34 +0100, Pontus Fuchs wrote:
> >> When the netlink skb is exhausted split_start is left set. In the
> >> subsequent retry, with a larger buffer, the dump is continued from the
> >> failing point instead of from the beginning.
> >
> > Hmm. Maybe it should be like this instead?
> >
> > johannes
> >
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 138dc3b..2e08af5 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
> > !skb->len &&
> > cb->min_dump_alloc < 4096) {
> > cb->min_dump_alloc = 4096;
> > + state->split_start = 0;
> > rtnl_unlock();
> > return 1;
> > }
>
> Will that not cause problem if split scan is in use and one of the split
> chunks needs to be resent due to EMSGSIZE?

That's not supposed to happen, and the "!skb->len" check should catch
it.

johannes