Re: regression: tethering fails in 3.5 with iwlwifi

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

Hi,

> >             56138f5 iwlwifi: dont pull too much payload in skb head
> >             3edaf3e mac80211: manage AP netdev carrier state
> 
> The second patch (AP carrier state) actually exposed a connman issue
> which I fixed and submitted a connman patch:
> http://lists.connman.net/pipermail/connman/2012-September/011232.html
> 
> However, Eric's patch still causes tethering problems to me.


Let me recap a bit. Artem is using connman, which sets up the wifi
interface as part of a bridge. It runs wpa_supplicant to create an AP
(only AP and mesh mode interfaces can be bridged anyway).


Eric, you said:

> I would say some part of the stack assumes a header (I dont know wich
> one ?) is pulled in skb->head/data, and thats a bug.
> 
> Always use pskb_may_pull(skb, XXX) to make sure you can access XXX
> bytes in skb->data

I thought we'd figure out which part of the stack assumes a header, so I
asked Artem to test a one-line patch which adds "skb_linearize()" before
"netif_receive_skb()" in mac80211. This makes it work, but I'm not sure
where after that the bad assumption might be.

johannes

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

OK, but which netif_receive_skb(), as I count 5 occurrences in
net/mac80211/rx.c ?

Instead of skb_linearize() calls

you could try several values of XXX in

pskb_may_pull(skb, XXX)

So that you make sure XXX bytes are available in skb->head, and not
the whole frame.

One you know the limit for XXX, it might give a clue where a
pskb_may_pull(skb, XXX) is missing.

On Thu, Sep 20, 2012 at 2:35 PM, Johannes Berg
<johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> Hi,
>
>> >             56138f5 iwlwifi: dont pull too much payload in skb head
>> >             3edaf3e mac80211: manage AP netdev carrier state
>>
>> The second patch (AP carrier state) actually exposed a connman issue
>> which I fixed and submitted a connman patch:
>> http://lists.connman.net/pipermail/connman/2012-September/011232.html
>>
>> However, Eric's patch still causes tethering problems to me.
>
>
> Let me recap a bit. Artem is using connman, which sets up the wifi
> interface as part of a bridge. It runs wpa_supplicant to create an AP
> (only AP and mesh mode interfaces can be bridged anyway).
>
>
> Eric, you said:
>
>> I would say some part of the stack assumes a header (I dont know wich
>> one ?) is pulled in skb->head/data, and thats a bug.
>>
>> Always use pskb_may_pull(skb, XXX) to make sure you can access XXX
>> bytes in skb->data
>
> I thought we'd figure out which part of the stack assumes a header, so I
> asked Artem to test a one-line patch which adds "skb_linearize()" before
> "netif_receive_skb()" in mac80211. This makes it work, but I'm not sure
> where after that the bad assumption might be.
>
> johannes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

I guess you only need to make sure 14 bytes of ethernet header are
available before eth_type_trans(skb, dev);

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 61c621e..ffe5f84 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1795,9 +1795,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)

                if (skb) {
                        /* deliver to local stack */
-                       skb->protocol = eth_type_trans(skb, dev);
-                       memset(skb->cb, 0, sizeof(skb->cb));
-                       netif_receive_skb(skb);
+                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
+                               skb->protocol = eth_type_trans(skb, dev);
+                               memset(skb->cb, 0, sizeof(skb->cb));
+                               netif_receive_skb(skb);
+                       } else {
+                               kfree_skb(skb);
+                       }
                }
        }


On Thu, Sep 20, 2012 at 2:40 PM, Eric Dumazet <edumazet@google.com> wrote:
> OK, but which netif_receive_skb(), as I count 5 occurrences in
> net/mac80211/rx.c ?
>
> Instead of skb_linearize() calls
>
> you could try several values of XXX in
>
> pskb_may_pull(skb, XXX)
>
> So that you make sure XXX bytes are available in skb->head, and not
> the whole frame.
>
> One you know the limit for XXX, it might give a clue where a
> pskb_may_pull(skb, XXX) is missing.
>
> On Thu, Sep 20, 2012 at 2:35 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> Hi,
>>
>>> >             56138f5 iwlwifi: dont pull too much payload in skb head
>>> >             3edaf3e mac80211: manage AP netdev carrier state
>>>
>>> The second patch (AP carrier state) actually exposed a connman issue
>>> which I fixed and submitted a connman patch:
>>> http://lists.connman.net/pipermail/connman/2012-September/011232.html
>>>
>>> However, Eric's patch still causes tethering problems to me.
>>
>>
>> Let me recap a bit. Artem is using connman, which sets up the wifi
>> interface as part of a bridge. It runs wpa_supplicant to create an AP
>> (only AP and mesh mode interfaces can be bridged anyway).
>>
>>
>> Eric, you said:
>>
>>> I would say some part of the stack assumes a header (I dont know wich
>>> one ?) is pulled in skb->head/data, and thats a bug.
>>>
>>> Always use pskb_may_pull(skb, XXX) to make sure you can access XXX
>>> bytes in skb->data
>>
>> I thought we'd figure out which part of the stack assumes a header, so I
>> asked Artem to test a one-line patch which adds "skb_linearize()" before
>> "netif_receive_skb()" in mac80211. This makes it work, but I'm not sure
>> where after that the bad assumption might be.
>>
>> johannes
>>

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

Or its the lines with CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ?

What arch is it ?


On Thu, Sep 20, 2012 at 2:45 PM, Eric Dumazet <edumazet@google.com> wrote:
> I guess you only need to make sure 14 bytes of ethernet header are
> available before eth_type_trans(skb, dev);
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 61c621e..ffe5f84 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1795,9 +1795,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>
>                 if (skb) {
>                         /* deliver to local stack */
> -                       skb->protocol = eth_type_trans(skb, dev);
> -                       memset(skb->cb, 0, sizeof(skb->cb));
> -                       netif_receive_skb(skb);
> +                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
> +                               skb->protocol = eth_type_trans(skb, dev);
> +                               memset(skb->cb, 0, sizeof(skb->cb));
> +                               netif_receive_skb(skb);
> +                       } else {
> +                               kfree_skb(skb);
> +                       }
>                 }
>         }
>
>
> On Thu, Sep 20, 2012 at 2:40 PM, Eric Dumazet <edumazet@google.com> wrote:
>> OK, but which netif_receive_skb(), as I count 5 occurrences in
>> net/mac80211/rx.c ?
>>
>> Instead of skb_linearize() calls
>>
>> you could try several values of XXX in
>>
>> pskb_may_pull(skb, XXX)
>>
>> So that you make sure XXX bytes are available in skb->head, and not
>> the whole frame.
>>
>> One you know the limit for XXX, it might give a clue where a
>> pskb_may_pull(skb, XXX) is missing.
>>
>> On Thu, Sep 20, 2012 at 2:35 PM, Johannes Berg
>> <johannes@sipsolutions.net> wrote:
>>> Hi,
>>>
>>>> >             56138f5 iwlwifi: dont pull too much payload in skb head
>>>> >             3edaf3e mac80211: manage AP netdev carrier state
>>>>
>>>> The second patch (AP carrier state) actually exposed a connman issue
>>>> which I fixed and submitted a connman patch:
>>>> http://lists.connman.net/pipermail/connman/2012-September/011232.html
>>>>
>>>> However, Eric's patch still causes tethering problems to me.
>>>
>>>
>>> Let me recap a bit. Artem is using connman, which sets up the wifi
>>> interface as part of a bridge. It runs wpa_supplicant to create an AP
>>> (only AP and mesh mode interfaces can be bridged anyway).
>>>
>>>
>>> Eric, you said:
>>>
>>>> I would say some part of the stack assumes a header (I dont know wich
>>>> one ?) is pulled in skb->head/data, and thats a bug.
>>>>
>>>> Always use pskb_may_pull(skb, XXX) to make sure you can access XXX
>>>> bytes in skb->data
>>>
>>> I thought we'd figure out which part of the stack assumes a header, so I
>>> asked Artem to test a one-line patch which adds "skb_linearize()" before
>>> "netif_receive_skb()" in mac80211. This makes it work, but I'm not sure
>>> where after that the bad assumption might be.
>>>
>>> johannes
>>>

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Thu, 2012-09-20 at 14:46 +0200, Eric Dumazet wrote:
> Or its the lines with CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS ?
> 
> What arch is it ?

x86

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Thu, 2012-09-20 at 14:45 +0200, Eric Dumazet wrote:
> I guess you only need to make sure 14 bytes of ethernet header are
> available before eth_type_trans(skb, dev);
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 61c621e..ffe5f84 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1795,9 +1795,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
> 
>                 if (skb) {
>                         /* deliver to local stack */
> -                       skb->protocol = eth_type_trans(skb, dev);
> -                       memset(skb->cb, 0, sizeof(skb->cb));
> -                       netif_receive_skb(skb);
> +                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
> +                               skb->protocol = eth_type_trans(skb, dev);
> +                               memset(skb->cb, 0, sizeof(skb->cb));
> +                               netif_receive_skb(skb);
> +                       } else {
> +                               kfree_skb(skb);
> +                       }
>                 }
>         }

Yeah I was looking at the same code just now. However, we had actually
inserted the skb_linearize() *after* eth_type_trans(), so I'm confused.
Maybe it still works, more or less by accident?

johannes

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

Or its a buggy protocol ?

IP/UDP/TCP definitely works, but maybe another protocol assumes its
header is in skb->head


On Thu, Sep 20, 2012 at 2:47 PM, Johannes Berg
<johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Thu, 2012-09-20 at 14:45 +0200, Eric Dumazet wrote:
>> I guess you only need to make sure 14 bytes of ethernet header are
>> available before eth_type_trans(skb, dev);
>>
>> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
>> index 61c621e..ffe5f84 100644
>> --- a/net/mac80211/rx.c
>> +++ b/net/mac80211/rx.c
>> @@ -1795,9 +1795,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>>
>>                 if (skb) {
>>                         /* deliver to local stack */
>> -                       skb->protocol = eth_type_trans(skb, dev);
>> -                       memset(skb->cb, 0, sizeof(skb->cb));
>> -                       netif_receive_skb(skb);
>> +                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
>> +                               skb->protocol = eth_type_trans(skb, dev);
>> +                               memset(skb->cb, 0, sizeof(skb->cb));
>> +                               netif_receive_skb(skb);
>> +                       } else {
>> +                               kfree_skb(skb);
>> +                       }
>>                 }
>>         }
>
> Yeah I was looking at the same code just now. However, we had actually
> inserted the skb_linearize() *after* eth_type_trans(), so I'm confused.
> Maybe it still works, more or less by accident?
>
> johannes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Thu, 2012-09-20 at 14:48 +0200, Eric Dumazet wrote:
> Or its a buggy protocol ?
> 
> IP/UDP/TCP definitely works, but maybe another protocol assumes its
> header is in skb->head

It could be, I think it failed either on EAPOL (which is just userspace
registering a protocol socket) or DHCP (same?)

johannes

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

So its using a RAW socket ?

On Thu, Sep 20, 2012 at 2:50 PM, Johannes Berg
<johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> On Thu, 2012-09-20 at 14:48 +0200, Eric Dumazet wrote:
>> Or its a buggy protocol ?
>>
>> IP/UDP/TCP definitely works, but maybe another protocol assumes its
>> header is in skb->head
>
> It could be, I think it failed either on EAPOL (which is just userspace
> registering a protocol socket) or DHCP (same?)
>
> johannes
>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Thu, 2012-09-20 at 14:51 +0200, Eric Dumazet wrote:
> So its using a RAW socket ?

Yes:

socket(PF_PACKET, SOCK_RAW, htons(ETH_P_EAPOL))

johannes

> On Thu, Sep 20, 2012 at 2:50 PM, Johannes Berg
> <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org> wrote:
> > On Thu, 2012-09-20 at 14:48 +0200, Eric Dumazet wrote:
> >> Or its a buggy protocol ?
> >>
> >> IP/UDP/TCP definitely works, but maybe another protocol assumes its
> >> header is in skb->head
> >
> > It could be, I think it failed either on EAPOL (which is just userspace
> > registering a protocol socket) or DHCP (same?)
> >
> > johannes
> >
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Thu, 2012-09-20 at 14:47 +0200, Johannes Berg wrote:

> >                 if (skb) {
> >                         /* deliver to local stack */
> > -                       skb->protocol = eth_type_trans(skb, dev);
> > -                       memset(skb->cb, 0, sizeof(skb->cb));
> > -                       netif_receive_skb(skb);
> > +                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
> > +                               skb->protocol = eth_type_trans(skb, dev);
> > +                               memset(skb->cb, 0, sizeof(skb->cb));
> > +                               netif_receive_skb(skb);
> > +                       } else {
> > +                               kfree_skb(skb);
> > +                       }
> >                 }
> >         }
> 
> Yeah I was looking at the same code just now. However, we had actually
> inserted the skb_linearize() *after* eth_type_trans(), so I'm confused.

Ok actually, by the time we get here the ethernet header must be in the
skb head because we construct it from the 802.11 and llc/snap header.

johannes

Re: regression: tethering fails in 3.5 with iwlwifi

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

On Thu, 2012-09-20 at 14:45 +0200, Eric Dumazet wrote:
> I guess you only need to make sure 14 bytes of ethernet header are
> available before eth_type_trans(skb, dev);
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 61c621e..ffe5f84 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1795,9 +1795,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
> 
>                 if (skb) {
>                         /* deliver to local stack */
> -                       skb->protocol = eth_type_trans(skb, dev);
> -                       memset(skb->cb, 0, sizeof(skb->cb));
> -                       netif_receive_skb(skb);
> +                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
> +                               skb->protocol = eth_type_trans(skb, dev);
> +                               memset(skb->cb, 0, sizeof(skb->cb));
> +                               netif_receive_skb(skb);
> +                       } else {
> +                               kfree_skb(skb);
> +                       }
>                 }
>         }

Does not help, this one does:

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 61c621e..6888586 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1797,6 +1797,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 			/* deliver to local stack */
 			skb->protocol = eth_type_trans(skb, dev);
 			memset(skb->cb, 0, sizeof(skb->cb));
+			skb_linearize(skb);
 			netif_receive_skb(skb);
 		}

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

Try to pull 40 bytes : Thats OK for tcp performance, because 40 bytes
is the minimum size of IP+TCP headers

pskb_may_pull(skb, 40);

(instead of your  skb_linearize(skb);)


On Thu, Sep 20, 2012 at 2:58 PM, Artem Bityutskiy
<artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Thu, 2012-09-20 at 14:45 +0200, Eric Dumazet wrote:
>> I guess you only need to make sure 14 bytes of ethernet header are
>> available before eth_type_trans(skb, dev);
>>
>> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
>> index 61c621e..ffe5f84 100644
>> --- a/net/mac80211/rx.c
>> +++ b/net/mac80211/rx.c
>> @@ -1795,9 +1795,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>>
>>                 if (skb) {
>>                         /* deliver to local stack */
>> -                       skb->protocol = eth_type_trans(skb, dev);
>> -                       memset(skb->cb, 0, sizeof(skb->cb));
>> -                       netif_receive_skb(skb);
>> +                       if (pskb_may_pull(skb, sizeof(struct ethhdr))) {
>> +                               skb->protocol = eth_type_trans(skb, dev);
>> +                               memset(skb->cb, 0, sizeof(skb->cb));
>> +                               netif_receive_skb(skb);
>> +                       } else {
>> +                               kfree_skb(skb);
>> +                       }
>>                 }
>>         }
>
> Does not help, this one does:
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 61c621e..6888586 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1797,6 +1797,7 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>                         /* deliver to local stack */
>                         skb->protocol = eth_type_trans(skb, dev);
>                         memset(skb->cb, 0, sizeof(skb->cb));
> +                       skb_linearize(skb);
>                         netif_receive_skb(skb);
>                 }
>
> --
> Best Regards,
> Artem Bityutskiy
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

On Thu, 2012-09-20 at 15:04 +0200, Eric Dumazet wrote:
> Try to pull 40 bytes : Thats OK for tcp performance, because 40 bytes
> is the minimum size of IP+TCP headers
> 
> pskb_may_pull(skb, 40);
> 
> (instead of your  skb_linearize(skb);)

Oh well, I just realize all these mails were on netdev.

Sorry for the noise.


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Artem Bityutskiy]

[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]

On Thu, 2012-09-20 at 15:04 +0200, Eric Dumazet wrote:
> Try to pull 40 bytes : Thats OK for tcp performance, because 40 bytes
> is the minimum size of IP+TCP headers
> 
> pskb_may_pull(skb, 40);

OK, I've tried almost this (see below) and it solves my issue:

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 965e6ec..7f079d0 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1798,9 +1798,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
 
                if (skb) {
                        /* deliver to local stack */
-                       skb->protocol = eth_type_trans(skb, dev);
-                       memset(skb->cb, 0, sizeof(skb->cb));
-                       netif_receive_skb(skb);
+                       if (pskb_may_pull(skb, 40)) {
+                               skb->protocol = eth_type_trans(skb, dev);
+                               memset(skb->cb, 0, sizeof(skb->cb));
+                               netif_receive_skb(skb);
+                       } else {
+                               kfree_skb(skb);
+                       }
                }
        }

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

On Thu, 2012-09-20 at 16:22 +0300, Artem Bityutskiy wrote:

> 
> OK, I've tried almost this (see below) and it solves my issue:
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 965e6ec..7f079d0 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1798,9 +1798,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>  
>                 if (skb) {
>                         /* deliver to local stack */
> -                       skb->protocol = eth_type_trans(skb, dev);
> -                       memset(skb->cb, 0, sizeof(skb->cb));
> -                       netif_receive_skb(skb);
> +                       if (pskb_may_pull(skb, 40)) {
> +                               skb->protocol = eth_type_trans(skb, dev);
> +                               memset(skb->cb, 0, sizeof(skb->cb));
> +                               netif_receive_skb(skb);
> +                       } else {
> +                               kfree_skb(skb);
> +                       }
>                 }
>         }
> 

OK but you cant do that, or small frames will be dropped.

Anyway its a hack, we should find the buggy layer.

You could use dropwatch (drop_monitor) to check where frame is dropped.

modprobe drop_monitor
dropwatch -l kas

Re: regression: tethering fails in 3.5 with iwlwifi

[Eric Dumazet]

On Thu, 2012-09-20 at 16:22 +0300, Artem Bityutskiy wrote:
> On Thu, 2012-09-20 at 15:04 +0200, Eric Dumazet wrote:
> > Try to pull 40 bytes : Thats OK for tcp performance, because 40 bytes
> > is the minimum size of IP+TCP headers
> > 
> > pskb_may_pull(skb, 40);
> 
> OK, I've tried almost this (see below) and it solves my issue:
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 965e6ec..7f079d0 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1798,9 +1798,13 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
>  
>                 if (skb) {
>                         /* deliver to local stack */
> -                       skb->protocol = eth_type_trans(skb, dev);
> -                       memset(skb->cb, 0, sizeof(skb->cb));
> -                       netif_receive_skb(skb);
> +                       if (pskb_may_pull(skb, 40)) {
> +                               skb->protocol = eth_type_trans(skb, dev);
> +                               memset(skb->cb, 0, sizeof(skb->cb));
> +                               netif_receive_skb(skb);
> +                       } else {
> +                               kfree_skb(skb);
> +                       }
>                 }
>         }
> 

Please remove this hack and try the following bugfix in raw handler

icmp_filter() should not modify skb, or else its caller should not
assume ip_hdr() is unchanged.

 net/ipv4/raw.c |   29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index f242578..3fa8c96 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -128,25 +128,30 @@ found:
 }
 
 /*
- *	0 - deliver
- *	1 - block
+ *	false - deliver
+ *	true - block
  */
-static __inline__ int icmp_filter(struct sock *sk, struct sk_buff *skb)
+static bool icmp_filter(struct sock *sk, const struct sk_buff *skb)
 {
-	int type;
-
-	if (!pskb_may_pull(skb, sizeof(struct icmphdr)))
-		return 1;
-
-	type = icmp_hdr(skb)->type;
-	if (type < 32) {
+	__u8 _type;
+	const __u8 *type;
+
+	type = skb_header_pointer(skb,
+				  skb_transport_offset(skb) +
+				  offsetof(struct icmphdr, type),
+				  sizeof(_type),
+				  &_type);
+	if (!type)
+		return true;
+
+	if (*type < 32) {
 		__u32 data = raw_sk(sk)->filter.data;
 
-		return ((1 << type) & data) != 0;
+		return ((1U << *type) & data) != 0;
 	}
 
 	/* Do not block unknown ICMP types */
-	return 0;
+	return false;
 }
 
 /* IP input processing comes here for RAW socket delivery.

Re: regression: tethering fails in 3.5 with iwlwifi

[David Miller]

From: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Thu, 20 Sep 2012 16:25:49 +0200

> Please remove this hack and try the following bugfix in raw handler
> 
> icmp_filter() should not modify skb, or else its caller should not
> assume ip_hdr() is unchanged.

Right, good catch.

Please submit this fix formally Eric, thanks a lot.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Thu, 2012-09-20 at 14:40 +0200, Eric Dumazet wrote:
> OK, but which netif_receive_skb(), as I count 5 occurrences in
> net/mac80211/rx.c ?

The one you modified in the other email :-)

> Instead of skb_linearize() calls
> 
> you could try several values of XXX in
> 
> pskb_may_pull(skb, XXX)
> 
> So that you make sure XXX bytes are available in skb->head, and not
> the whole frame.
> 
> One you know the limit for XXX, it might give a clue where a
> pskb_may_pull(skb, XXX) is missing.

Good idea.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Johannes Berg]

On Mon, 2012-09-17 at 17:41 +0300, Artem Bityutskiy wrote:

> OK, finally I got it. After 3 days of hardcore intelligent bisecting
>         I've found out that tethering in 3.5 works for me if I revert
> these 2
>         patches:
>         
>             56138f5 iwlwifi: dont pull too much payload in skb head

I got back to this for a customer running 3.5, and after many failed
attempts realized that you have to have iptables for this problem to
actually happen. I reverse-bisected that the *fix* is
6caab7b0544e83e6c160b5e80f5a4a7dd69545c7, in 3.7.

Is there still any stable kernel 3.5/3.6 (or possibly before, though for
iwlwifi before doesn't matter) that this should be applied to?

johannes

Re: regression: tethering fails in 3.5 with iwlwifi

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Thu, 21 Mar 2013 20:34:11 +0100

> On Mon, 2012-09-17 at 17:41 +0300, Artem Bityutskiy wrote:
> 
>> OK, finally I got it. After 3 days of hardcore intelligent bisecting
>>         I've found out that tethering in 3.5 works for me if I revert
>> these 2
>>         patches:
>>         
>>             56138f5 iwlwifi: dont pull too much payload in skb head
> 
> I got back to this for a customer running 3.5, and after many failed
> attempts realized that you have to have iptables for this problem to
> actually happen. I reverse-bisected that the *fix* is
> 6caab7b0544e83e6c160b5e80f5a4a7dd69545c7, in 3.7.
> 
> Is there still any stable kernel 3.5/3.6 (or possibly before, though for
> iwlwifi before doesn't matter) that this should be applied to?

I've checked all of 3.0.x, 3.2.x, 3.4.x, and 3.8.x   They all have this
patch applied.

Those are the official stable kernels, everything else is outside of my
realm.

Re: regression: tethering fails in 3.5 with iwlwifi

[Artem Bityutskiy]

On Thu, 2013-03-21 at 20:34 +0100, Johannes Berg wrote:
> On Mon, 2012-09-17 at 17:41 +0300, Artem Bityutskiy wrote:
> 
> > OK, finally I got it. After 3 days of hardcore intelligent bisecting
> >         I've found out that tethering in 3.5 works for me if I revert
> > these 2
> >         patches:
> >         
> >             56138f5 iwlwifi: dont pull too much payload in skb head
> 
> I got back to this for a customer running 3.5, and after many failed
> attempts realized that you have to have iptables for this problem to
> actually happen. I reverse-bisected that the *fix* is
> 6caab7b0544e83e6c160b5e80f5a4a7dd69545c7, in 3.7.
> 
> Is there still any stable kernel 3.5/3.6 (or possibly before, though for
> iwlwifi before doesn't matter) that this should be applied to?

Hi Johannes,

thanks for remembering about this. We've switched to the 3.8 kernel
recently, and we are pulling stable kernels in regularly. Now we are
at 3.8.3.

-- 
Best Regards,
Artem Bityutskiy

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: regression: tethering fails in 3.5 with iwlwifi

[Luis Henriques]

On Thu, Mar 21, 2013 at 08:34:11PM +0100, Johannes Berg wrote:
> On Mon, 2012-09-17 at 17:41 +0300, Artem Bityutskiy wrote:
> 
> > OK, finally I got it. After 3 days of hardcore intelligent bisecting
> >         I've found out that tethering in 3.5 works for me if I revert
> > these 2
> >         patches:
> >         
> >             56138f5 iwlwifi: dont pull too much payload in skb head
> 
> I got back to this for a customer running 3.5, and after many failed
> attempts realized that you have to have iptables for this problem to
> actually happen. I reverse-bisected that the *fix* is
> 6caab7b0544e83e6c160b5e80f5a4a7dd69545c7, in 3.7.
> 
> Is there still any stable kernel 3.5/3.6 (or possibly before, though for
> iwlwifi before doesn't matter) that this should be applied to?

The 3.5.y.z extended stable tree already contains this commit as well.

Cheers,
--
Luis