* Re: [PATCH v2 0/9] net/macb: driver enhancement concerning GEM support, ring logic and cleanup
From: Nicolas Ferre @ 2012-09-20 13:03 UTC (permalink / raw)
To: David Miller
Cc: netdev, havard, bhutchings, linux-arm-kernel, plagnioj,
patrice.vilchez, linux-kernel
In-Reply-To: <20120919.135006.394934820049386022.davem@davemloft.net>
On 09/19/2012 07:50 PM, David Miller :
> From: Nicolas Ferre <nicolas.ferre@atmel.com>
> Date: Wed, 19 Sep 2012 13:55:13 +0200
>
>> This is an enhancement work that began several years ago. I try to catchup with
>> some performance improvement that has been implemented then by Havard.
>> The ring index logic and the TX error path modification are the biggest changes
>> but some cleanup/debugging have been added along the way.
>> The GEM revision will benefit from the Gigabit support.
>>
>> The series has been tested on several Atmel AT91 SoC with the two MACB/GEM
>> flavors.
>>
>> v2: - modify the tx error handling: now uses a workqueue
>> - information provided by ethtool -i were not accurate: removed
>
> Don't submit patches like this.
>
> When you put an RFC right in the middle of the series, it screws everything
> up.
>
> It means that I can't only apply the parts that are not RFC.
I will submit a v3 patch series when I am more confident about the patch
that I have tagged as RFC...
And as you noted last time that I have included a modified patch in a
series:
"Please, when you receive feedback on your patches, you need to
resubmit the whole patch series for review not just the patches where
changes were asked for."
==> I thought that it was a better idea to post the whole patch series
so that people could figure out the context. As the TX error path is
greatly modified, it could make senses.
Now, is it possible to review this series as it is or should I repost
patches? attached to the previous thread? RFC patch alone?
puzzled,
--
Nicolas Ferre
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Artem Bityutskiy @ 2012-09-20 12:58 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Johannes Berg, linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <CANn89iKStHAwVZtpgQMzEpByGYG2FKpjSQsJFT-9qQmjw+KJ8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- 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 --]
^ permalink raw reply related
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <CANn89iLudX29yU8ziNyN+8gN3b2L55L9m5p6ob_wnfZ3fp_TDg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Eric Dumazet @ 2012-09-20 12:51 UTC (permalink / raw)
To: Johannes Berg
Cc: artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <1348145450.4161.31.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: artem.bityutskiy, linux-wireless, netdev
In-Reply-To: <1348145269.4161.28.camel@jlt4.sipsolutions.net>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:50 UTC (permalink / raw)
To: Eric Dumazet
Cc: artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <CANn89iLnv2kWLd_hTpMXjMJ=r+hOvb2q2Oy6L-s5+6SzqxYvjA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:50 UTC (permalink / raw)
To: Eric Dumazet; +Cc: artem.bityutskiy, linux-wireless, netdev
In-Reply-To: <CANn89iLksBMJ=Ep+VcPSAOPzqst70hk8hR5=mGte2vSx=49zUA@mail.gmail.com>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Eric Dumazet @ 2012-09-20 12:48 UTC (permalink / raw)
To: Johannes Berg
Cc: artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <1348145269.4161.28.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:47 UTC (permalink / raw)
To: Eric Dumazet; +Cc: artem.bityutskiy, linux-wireless, netdev
In-Reply-To: <CANn89iKStHAwVZtpgQMzEpByGYG2FKpjSQsJFT-9qQmjw+KJ8g@mail.gmail.com>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <CANn89iJkXrZPB14KU8xA-Dn8_n=H3J4-R60BTZFV+STF9_ASdA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Eric Dumazet @ 2012-09-20 12:46 UTC (permalink / raw)
To: Johannes Berg; +Cc: artem.bityutskiy, linux-wireless, netdev
In-Reply-To: <CANn89iKStHAwVZtpgQMzEpByGYG2FKpjSQsJFT-9qQmjw+KJ8g@mail.gmail.com>
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
>>>
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Eric Dumazet @ 2012-09-20 12:45 UTC (permalink / raw)
To: Johannes Berg; +Cc: artem.bityutskiy, linux-wireless, netdev
In-Reply-To: <CANn89iJkXrZPB14KU8xA-Dn8_n=H3J4-R60BTZFV+STF9_ASdA@mail.gmail.com>
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
>>
^ permalink raw reply related
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Eric Dumazet @ 2012-09-20 12:40 UTC (permalink / raw)
To: Johannes Berg
Cc: artem.bityutskiy-VuQAYsv1563Yd54FQh9/CA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA, netdev
In-Reply-To: <1348144524.4161.26.camel-8Nb76shvtaUJvtFkdXX2HixXY32XiHfO@public.gmane.org>
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
^ permalink raw reply
* Re: regression: tethering fails in 3.5 with iwlwifi
From: Johannes Berg @ 2012-09-20 12:35 UTC (permalink / raw)
To: artem.bityutskiy; +Cc: Eric Dumazet, linux-wireless, netdev
In-Reply-To: <1348142775.2388.10.camel@sauron.fi.intel.com>
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
^ permalink raw reply
* Re: [PATCH v3] net-tcp: TCP/IP stack bypass for loopback connections
From: Eric Dumazet @ 2012-09-20 11:51 UTC (permalink / raw)
To: sclark46; +Cc: Bruce Curtis, David Miller, edumazet, netdev
In-Reply-To: <505AFDE9.4080602@earthlink.net>
On Thu, 2012-09-20 at 07:28 -0400, Stephen Clark wrote:
>
> Does this mean traffic on the loopback interface will not traverse
> netfilter?
>
Yes this was already mentioned.
Only the SYN / SYNACK messages will
All data will bypass IP stack, qdisc (if any), loopback driver, and
netfilter.
^ permalink raw reply
* Re: [PATCH v3] net-tcp: TCP/IP stack bypass for loopback connections
From: Stephen Clark @ 2012-09-20 11:28 UTC (permalink / raw)
To: Bruce Curtis; +Cc: Eric Dumazet, David Miller, edumazet, netdev
In-Reply-To: <CAEkNxbEJjAu3+3yDGPGSzzee-LY_797RdNbBgcC6=-aDHfEAJQ@mail.gmail.com>
On 09/19/2012 05:19 PM, Bruce Curtis wrote:
> On Wed, Sep 19, 2012 at 2:03 PM, Eric Dumazet<eric.dumazet@gmail.com> wrote:
>
>> On Wed, 2012-09-19 at 16:34 -0400, David Miller wrote:
>>
>>
>>> I have an idea on how to handle this.
>>>
>>> In drivers/net/loopback.c:loopback_tx(), skip the SKB orphan operation
>>> if there is a friend socket at skb->friend.
>>>
>>> When sending such friend SKBs out at connection startup, arrange it
>>> such that the skb->destructor will zap the skb->friend pointer to
>>> NULL.
>>>
>>> Also, in skb_orphan*(), if necessary, set skb->friend to NULL.
>>>
>>> skb->sk will hold a reference to the socket, and since skb->friend
>>> will be equal, this will make sure a pointer to an unreferenced
>>> socket does not escape.
>>>
>> I now am wondering if we still need skb->friend field.
>>
>> If skb->sk is not zeroed by a premature skb_orphan(), then
>>
>> skb->sk->sk_friend gives the friend ?
>>
>>
>>
Does this mean traffic on the loopback interface will not traverse
netfilter?
--
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety." (Ben Franklin)
"The course of history shows that as a government grows, liberty
decreases." (Thomas Jefferson)
^ permalink raw reply
* Re: [PATCH net-next] net: only run neigh_forced_gc() from one cpu
From: Lorenzo Colitti @ 2012-09-20 11:22 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, maze, therbert
In-Reply-To: <20120919.235102.1659819445753338481.davem@davemloft.net>
On Thu, Sep 20, 2012 at 12:51 PM, David Miller <davem@davemloft.net> wrote:
>> If this patch makes IPv6 performance better without affecting IPv4, it's a
>> good idea to apply it anyway, right? IPv6 dst entry garbage collection can
>> potentially cause serious performance issues on any server with a public
>> IPv6 address, and this patch substantially improves the situation.
>
> He's targetting net-next, and I've told him both in previous public
> discussions and in recent private communication that the correct fix
> is to make ipv6 routes use ref-count-less neighbour handling schemes
> like ipv4.
Fair enough. Removing the cache is a better solution - requiring a
separate cache entry for every address you want to send a packet to is
not suited to a world where every user has 2^64 addresses or more. But
if removing the route cache for IPv6 is a large amount of work that
nobody will sign up for, then fixing the symptoms might be better than
nothing.
The performance degradation could become an attack vector. Of course
the people that run IPv6 servers today can maintain their own patches,
but that's sort of suboptimal.
Is there something else that can be done other than moving to
non-refcounted neighbours?
^ permalink raw reply
* Re: Oops with latest (netfilter) nf-next tree, when unloading iptable_nat
From: Patrick McHardy @ 2012-09-20 10:31 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Jesper Dangaard Brouer, Florian Westphal, netfilter-devel, netdev,
yongjun_wei
In-Reply-To: <20120920100859.GB20828@1984>
>> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
>> index dcb2791..0f241be 100644
>> --- a/net/netfilter/nf_conntrack_core.c
>> +++ b/net/netfilter/nf_conntrack_core.c
>> @@ -1224,6 +1224,8 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
>> spin_lock_bh(&nf_conntrack_lock);
>> for (; *bucket < net->ct.htable_size; (*bucket)++) {
>> hlist_nulls_for_each_entry(h, n, &net->ct.hash[*bucket], hnnode) {
>> + if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
>> + continue;
>
> I think this will make the deletion of entries via `conntrack -F'
> slowier as we'll have to iterate over more entries (we won't delete
> entries for the reply tuple).
Slightly maybe, but I doubt it makes much of a difference.
> I think I prefer Florian's patch, it's fairly small and it does not
> change the current nf_ct_iterate behaviour or adding some
> nf_nat_iterate cleanup.
I don't think I've received it. Could you forward it to me please?
^ permalink raw reply
* Re: [PATCH v2] USB: remove dbg() usage in USB networking drivers
From: Greg Kroah-Hartman @ 2012-09-20 10:30 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-usb, linux-kernel
In-Reply-To: <1348135633.5604.5.camel@joe2Laptop>
On Thu, Sep 20, 2012 at 03:07:13AM -0700, Joe Perches wrote:
> On Wed, 2012-09-19 at 20:46 +0100, Greg Kroah-Hartman wrote:
> > The dbg() USB macro is so old, it predates me. The USB networking drivers are
> > the last hold-out using this macro, and we want to get rid of it, so replace
> > the usage of it with the proper netdev_dbg() or dev_dbg() (depending on the
> > context) calls.
>
> OK, one more bit of trivia
>
> > diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
> []
> > @@ -422,8 +419,9 @@ static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
> > if (!(skb->len & 0x01)) {
> > #ifdef DEBUG
> > struct net_device *net = dev->net;
> > - dbg("rx framesize %d range %d..%d mtu %d", skb->len,
> > - net->hard_header_len, dev->hard_mtu, net->mtu);
> > + netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
> > + skb->len, net->hard_header_len, dev->hard_mtu,
> > + net->mtu);
> > #endif
>
> maybe
> netdev_dbg(net, ...
>
> or remove the odd #ifdef DEBUG surround used to guard
> the otherwise unused net variable and use:
>
> netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
> skb->len, dev->net->hard_header_len, dev->hard_mtu,
> dev->net->mtu);
>
Yeah, that would be better.
Even better would be just to delete all of this debug crud from these
drivers. Almost all of the messages are there from when the developer
originally wrote the driver, trying to figure out what was going on.
>From what I have seen, in doing all of these cleanups, is that the need
for maybe a few debug lines that can be used if users have issues, but
the majority are just useless.
But, as I'm not the author or maintainer of these drivers, I'll be nice
and just leave them in, all I want to do is get rid of the old, foolish,
macros for debugging and use the proper dynamic debug code that works so
much better.
So I'll leave this change alone, and if someone wants to do the cleanup
better, the 3 liner above is fine with me to add later.
thanks,
greg k-h
^ permalink raw reply
* Re: Oops with latest (netfilter) nf-next tree, when unloading iptable_nat
From: Pablo Neira Ayuso @ 2012-09-20 10:08 UTC (permalink / raw)
To: Patrick McHardy
Cc: Jesper Dangaard Brouer, Florian Westphal, netfilter-devel, netdev,
yongjun_wei
In-Reply-To: <Pine.GSO.4.63.1209200855500.8409@stinky-local.trash.net>
On Thu, Sep 20, 2012 at 08:57:04AM +0200, Patrick McHardy wrote:
> On Wed, 19 Sep 2012, Jesper Dangaard Brouer wrote:
>
> >On Fri, 2012-09-14 at 15:15 +0200, Patrick McHardy wrote:
> >>On Fri, 14 Sep 2012, Pablo Neira Ayuso wrote:
> >>
> >[...cut...]
> >>>>Patrick, any other idea?
> >>>
> >[...cut...]
> >>>>
> >>>We can add nf_nat_iterate_cleanup that can iterate over the NAT
> >>>hashtable to replace current usage of nf_ct_iterate_cleanup.
> >>
> >>Lets just bail out when IPS_SRC_NAT_DONE is not set, that should also fix
> >>it. Could you try this patch please?
> >
> >On Fri, 2012-09-14 at 15:15 +0200, Patrick McHardy wrote:
> >diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> >>index 29d4452..8b5d220 100644
> >>--- a/net/netfilter/nf_nat_core.c
> >>+++ b/net/netfilter/nf_nat_core.c
> >>@@ -481,6 +481,8 @@ static int nf_nat_proto_clean(struct nf_conn *i,
> >void *data)
> >>
> >> if (!nat)
> >> return 0;
> >>+ if (!(i->status & IPS_SRC_NAT_DONE))
> >>+ return 0;
> >> if ((clean->l3proto && nf_ct_l3num(i) != clean->l3proto) ||
> >> (clean->l4proto && nf_ct_protonum(i) != clean->l4proto))
> >> return 0;
> >>
> >
> >No it does not work :-(
>
> Ok I think I understand the problem now, we're invoking the NAT cleanup
> callback twice with clean->hash = true, once for each direction of the
> conntrack.
>
> Does this patch fix the problem?
> commit 6c46a3bfb2776ca098565daf7e872a3283d14e0d
> Author: Patrick McHardy <kaber@trash.net>
> Date: Thu Sep 20 08:43:02 2012 +0200
>
> netfilter: nf_nat: fix oops when unloading protocol modules
>
> When unloading a protocol module nf_ct_iterate_cleanup() is used to
> remove all conntracks using the protocol from the bysource hash and
> clean their NAT sections. Since the conntrack isn't actually killed,
> the NAT callback is invoked twice, once for each direction, which
> causes an oops when trying to delete it from the bysource hash for
> the second time.
>
> The same oops can also happen when removing both an L3 and L4 protocol
> since the cleanup function doesn't check whether the conntrack has
> already been cleaned up.
>
> Pid: 4052, comm: modprobe Not tainted 3.6.0-rc3-test-nat-unload-fix+ #32 Red Hat KVM
> RIP: 0010:[<ffffffffa002c303>] [<ffffffffa002c303>] nf_nat_proto_clean+0x73/0xd0 [nf_nat]
> RSP: 0018:ffff88007808fe18 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff8800728550c0 RCX: ffff8800756288b0
> RDX: dead000000200200 RSI: ffff88007808fe88 RDI: ffffffffa002f208
> RBP: ffff88007808fe28 R08: ffff88007808e000 R09: 0000000000000000
> R10: dead000000200200 R11: dead000000100100 R12: ffffffff81c6dc00
> R13: ffff8800787582b8 R14: ffff880078758278 R15: ffff88007808fe88
> FS: 00007f515985d700(0000) GS:ffff88007cd00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 00007f515986a000 CR3: 000000007867a000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process modprobe (pid: 4052, threadinfo ffff88007808e000, task ffff8800756288b0)
> Stack:
> ffff88007808fe68 ffffffffa002c290 ffff88007808fe78 ffffffff815614e3
> ffffffff00000000 00000aeb00000246 ffff88007808fe68 ffffffff81c6dc00
> ffff88007808fe88 ffffffffa00358a0 0000000000000000 000000000040f5b0
> Call Trace:
> [<ffffffffa002c290>] ? nf_nat_net_exit+0x50/0x50 [nf_nat]
> [<ffffffff815614e3>] nf_ct_iterate_cleanup+0xc3/0x170
> [<ffffffffa002c55a>] nf_nat_l3proto_unregister+0x8a/0x100 [nf_nat]
> [<ffffffff812a0303>] ? compat_prepare_timeout+0x13/0xb0
> [<ffffffffa0035848>] nf_nat_l3proto_ipv4_exit+0x10/0x23 [nf_nat_ipv4]
> ...
>
> To fix this,
>
> - check whether the conntrack has already been cleaned up in
> nf_nat_proto_clean
>
> - change nf_ct_iterate_cleanup() to only invoke the callback function
> once for each conntrack (IP_CT_DIR_ORIGINAL).
>
> The second change doesn't affect other callers since when conntracks are
> actually killed, both directions are removed from the hash immediately
> and the callback is already only invoked once. If it is not killed, the
> second callback invocation will always return the same decision not to
> kill it.
>
> Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index dcb2791..0f241be 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1224,6 +1224,8 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
> spin_lock_bh(&nf_conntrack_lock);
> for (; *bucket < net->ct.htable_size; (*bucket)++) {
> hlist_nulls_for_each_entry(h, n, &net->ct.hash[*bucket], hnnode) {
> + if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
> + continue;
I think this will make the deletion of entries via `conntrack -F'
slowier as we'll have to iterate over more entries (we won't delete
entries for the reply tuple).
I think I prefer Florian's patch, it's fairly small and it does not
change the current nf_ct_iterate behaviour or adding some
nf_nat_iterate cleanup.
> ct = nf_ct_tuplehash_to_ctrack(h);
> if (iter(ct, data))
> goto found;
> diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
> index 1816ad3..65cf694 100644
> --- a/net/netfilter/nf_nat_core.c
> +++ b/net/netfilter/nf_nat_core.c
> @@ -481,6 +481,8 @@ static int nf_nat_proto_clean(struct nf_conn *i, void *data)
>
> if (!nat)
> return 0;
> + if (!(i->status & IPS_SRC_NAT_DONE))
> + return 0;
> if ((clean->l3proto && nf_ct_l3num(i) != clean->l3proto) ||
> (clean->l4proto && nf_ct_protonum(i) != clean->l4proto))
> return 0;
^ permalink raw reply
* [net-next 4/4 v2] ixgbevf: scheduling while atomic in reset hw path
From: Jeff Kirsher @ 2012-09-20 10:07 UTC (permalink / raw)
To: davem; +Cc: John Fastabend, netdev, gospo, sassmann, Eric Dumazet,
Jeff Kirsher
In-Reply-To: <1348135637-17857-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: John Fastabend <john.r.fastabend@intel.com>
In ixgbevf_reset_hw_vf() msleep is called while holding mbx_lock
resulting in a schedule while atomic bug with trace below.
This patch uses mdelay instead.
BUG: scheduling while atomic: ip/6539/0x00000002
2 locks held by ip/6539:
#0: (rtnl_mutex){+.+.+.}, at: [<ffffffff81419cc3>] rtnl_lock+0x17/0x19
#1: (&(&adapter->mbx_lock)->rlock){+.+...}, at: [<ffffffffa0030855>] ixgbevf_reset+0x30/0xc1 [ixgbevf]
Modules linked in: ixgbevf ixgbe mdio libfc scsi_transport_fc 8021q scsi_tgt garp stp llc cpufreq_ondemand acpi_cpufreq freq_table mperf ipv6 uinput igb coretemp hwmon crc32c_intel ioatdma i2c_i801 shpchp microcode lpc_ich mfd_core i2c_core joydev dca pcspkr serio_raw pata_acpi ata_generic usb_storage pata_jmicron
Pid: 6539, comm: ip Not tainted 3.6.0-rc3jk-net-next+ #104
Call Trace:
[<ffffffff81072202>] __schedule_bug+0x6a/0x79
[<ffffffff814bc7e0>] __schedule+0xa2/0x684
[<ffffffff8108f85f>] ? trace_hardirqs_off+0xd/0xf
[<ffffffff814bd0c0>] schedule+0x64/0x66
[<ffffffff814bb5e2>] schedule_timeout+0xa6/0xca
[<ffffffff810536b9>] ? lock_timer_base+0x52/0x52
[<ffffffff812629e0>] ? __udelay+0x15/0x17
[<ffffffff814bb624>] schedule_timeout_uninterruptible+0x1e/0x20
[<ffffffff810541c0>] msleep+0x1b/0x22
[<ffffffffa002e723>] ixgbevf_reset_hw_vf+0x90/0xe5 [ixgbevf]
[<ffffffffa0030860>] ixgbevf_reset+0x3b/0xc1 [ixgbevf]
[<ffffffffa0032fba>] ixgbevf_open+0x43/0x43e [ixgbevf]
[<ffffffff81409610>] ? dev_set_rx_mode+0x2e/0x33
[<ffffffff8140b0f1>] __dev_open+0xa0/0xe5
[<ffffffff814097ed>] __dev_change_flags+0xbe/0x142
[<ffffffff8140b01c>] dev_change_flags+0x21/0x56
[<ffffffff8141a843>] do_setlink+0x2e2/0x7f4
[<ffffffff81016e36>] ? native_sched_clock+0x37/0x39
[<ffffffff8141b0ac>] rtnl_newlink+0x277/0x4bb
[<ffffffff8141aee9>] ? rtnl_newlink+0xb4/0x4bb
[<ffffffff812217d1>] ? selinux_capable+0x32/0x3a
[<ffffffff8104fb17>] ? ns_capable+0x4f/0x67
[<ffffffff81419cc3>] ? rtnl_lock+0x17/0x19
[<ffffffff81419f28>] rtnetlink_rcv_msg+0x236/0x253
[<ffffffff81419cf2>] ? rtnetlink_rcv+0x2d/0x2d
[<ffffffff8142fd42>] netlink_rcv_skb+0x43/0x94
[<ffffffff81419ceb>] rtnetlink_rcv+0x26/0x2d
[<ffffffff8142faf1>] netlink_unicast+0xee/0x174
[<ffffffff81430327>] netlink_sendmsg+0x26a/0x288
[<ffffffff813fb04f>] ? rcu_read_unlock+0x56/0x67
[<ffffffff813f5e6d>] __sock_sendmsg_nosec+0x58/0x61
[<ffffffff813f81b7>] __sock_sendmsg+0x3d/0x48
[<ffffffff813f8339>] sock_sendmsg+0x6e/0x87
[<ffffffff81107c9f>] ? might_fault+0xa5/0xac
[<ffffffff81402a72>] ? copy_from_user+0x2a/0x2c
[<ffffffff81402e62>] ? verify_iovec+0x54/0xaa
[<ffffffff813f9834>] __sys_sendmsg+0x206/0x288
[<ffffffff810694fa>] ? up_read+0x23/0x3d
[<ffffffff811307e5>] ? fcheck_files+0xac/0xea
[<ffffffff8113095e>] ? fget_light+0x3a/0xb9
[<ffffffff813f9a2e>] sys_sendmsg+0x42/0x60
[<ffffffff814c5ba9>] system_call_fastpath+0x16/0x1b
CC: Eric Dumazet <edumazet@google.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
Tested-By: Robert Garrett <robertx.e.garrett@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/vf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 690801b..87b3f3b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -100,7 +100,7 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
msgbuf[0] = IXGBE_VF_RESET;
mbx->ops.write_posted(hw, msgbuf, 1);
- msleep(10);
+ mdelay(10);
/* set our "perm_addr" based on info provided by PF */
/* also set up the mc_filter_type which is piggy backed
--
1.7.11.4
^ permalink raw reply related
* [net-next 3/4] ixgbevf: Add support for VF API negotiation
From: Jeff Kirsher @ 2012-09-20 10:07 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1348135637-17857-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change makes it so that the VF can support the PF/VF API negotiation
protocol. Specifically in this case we are adding support for API 1.0
which will mean that the VF is capable of cleaning up buffers that span
multiple descriptors without triggering an error.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Sibai Li <sibai.li@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/defines.h | 1 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 23 ++++++++++++++
drivers/net/ethernet/intel/ixgbevf/mbx.h | 21 +++++++++++--
drivers/net/ethernet/intel/ixgbevf/vf.c | 37 +++++++++++++++++++++++
drivers/net/ethernet/intel/ixgbevf/vf.h | 3 ++
5 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/defines.h b/drivers/net/ethernet/intel/ixgbevf/defines.h
index 418af82..da17ccf 100644
--- a/drivers/net/ethernet/intel/ixgbevf/defines.h
+++ b/drivers/net/ethernet/intel/ixgbevf/defines.h
@@ -272,5 +272,6 @@ struct ixgbe_adv_tx_context_desc {
/* Error Codes */
#define IXGBE_ERR_INVALID_MAC_ADDR -1
#define IXGBE_ERR_RESET_FAILED -2
+#define IXGBE_ERR_INVALID_ARGUMENT -3
#endif /* _IXGBEVF_DEFINES_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index a5d9cc5..c5ffe1d 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1334,6 +1334,25 @@ static void ixgbevf_init_last_counter_stats(struct ixgbevf_adapter *adapter)
adapter->stats.base_vfmprc = adapter->stats.last_vfmprc;
}
+static void ixgbevf_negotiate_api(struct ixgbevf_adapter *adapter)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ int api[] = { ixgbe_mbox_api_10,
+ ixgbe_mbox_api_unknown };
+ int err = 0, idx = 0;
+
+ spin_lock(&adapter->mbx_lock);
+
+ while (api[idx] != ixgbe_mbox_api_unknown) {
+ err = ixgbevf_negotiate_api_version(hw, api[idx]);
+ if (!err)
+ break;
+ idx++;
+ }
+
+ spin_unlock(&adapter->mbx_lock);
+}
+
static void ixgbevf_up_complete(struct ixgbevf_adapter *adapter)
{
struct net_device *netdev = adapter->netdev;
@@ -1399,6 +1418,8 @@ void ixgbevf_up(struct ixgbevf_adapter *adapter)
{
struct ixgbe_hw *hw = &adapter->hw;
+ ixgbevf_negotiate_api(adapter);
+
ixgbevf_configure(adapter);
ixgbevf_up_complete(adapter);
@@ -2388,6 +2409,8 @@ static int ixgbevf_open(struct net_device *netdev)
}
}
+ ixgbevf_negotiate_api(adapter);
+
/* allocate transmit descriptors */
err = ixgbevf_setup_all_tx_resources(adapter);
if (err)
diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
index cf9131c..946ce86 100644
--- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
+++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
@@ -76,12 +76,29 @@
/* bits 23:16 are used for exra info for certain messages */
#define IXGBE_VT_MSGINFO_MASK (0xFF << IXGBE_VT_MSGINFO_SHIFT)
+/* definitions to support mailbox API version negotiation */
+
+/*
+ * each element denotes a version of the API; existing numbers may not
+ * change; any additions must go at the end
+ */
+enum ixgbe_pfvf_api_rev {
+ ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
+ ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
+ /* This value should always be last */
+ ixgbe_mbox_api_unknown, /* indicates that API version is not known */
+};
+
+/* mailbox API, legacy requests */
#define IXGBE_VF_RESET 0x01 /* VF requests reset */
#define IXGBE_VF_SET_MAC_ADDR 0x02 /* VF requests PF to set MAC addr */
#define IXGBE_VF_SET_MULTICAST 0x03 /* VF requests PF to set MC addr */
#define IXGBE_VF_SET_VLAN 0x04 /* VF requests PF to set VLAN */
-#define IXGBE_VF_SET_LPE 0x05 /* VF requests PF to set VMOLR.LPE */
-#define IXGBE_VF_SET_MACVLAN 0x06 /* VF requests PF for unicast filter */
+
+/* mailbox API, version 1.0 VF requests */
+#define IXGBE_VF_SET_LPE 0x05 /* VF requests PF to set VMOLR.LPE */
+#define IXGBE_VF_SET_MACVLAN 0x06 /* VF requests PF for unicast filter */
+#define IXGBE_VF_API_NEGOTIATE 0x08 /* negotiate API version */
/* length of permanent address message returned from PF */
#define IXGBE_VF_PERMADDR_MSG_LEN 4
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 3d555a1..690801b 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -79,6 +79,9 @@ static s32 ixgbevf_reset_hw_vf(struct ixgbe_hw *hw)
/* Call adapter stop to disable tx/rx and clear interrupts */
hw->mac.ops.stop_adapter(hw);
+ /* reset the api version */
+ hw->api_version = ixgbe_mbox_api_10;
+
IXGBE_WRITE_REG(hw, IXGBE_VFCTRL, IXGBE_CTRL_RST);
IXGBE_WRITE_FLUSH(hw);
@@ -433,6 +436,40 @@ void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size)
ixgbevf_write_msg_read_ack(hw, msgbuf, 2);
}
+/**
+ * ixgbevf_negotiate_api_version - Negotiate supported API version
+ * @hw: pointer to the HW structure
+ * @api: integer containing requested API version
+ **/
+int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api)
+{
+ int err;
+ u32 msg[3];
+
+ /* Negotiate the mailbox API version */
+ msg[0] = IXGBE_VF_API_NEGOTIATE;
+ msg[1] = api;
+ msg[2] = 0;
+ err = hw->mbx.ops.write_posted(hw, msg, 3);
+
+ if (!err)
+ err = hw->mbx.ops.read_posted(hw, msg, 3);
+
+ if (!err) {
+ msg[0] &= ~IXGBE_VT_MSGTYPE_CTS;
+
+ /* Store value and return 0 on success */
+ if (msg[0] == (IXGBE_VF_API_NEGOTIATE | IXGBE_VT_MSGTYPE_ACK)) {
+ hw->api_version = api;
+ return 0;
+ }
+
+ err = IXGBE_ERR_INVALID_ARGUMENT;
+ }
+
+ return err;
+}
+
static const struct ixgbe_mac_operations ixgbevf_mac_ops = {
.init_hw = ixgbevf_init_hw_vf,
.reset_hw = ixgbevf_reset_hw_vf,
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h
index 07fd876..47f11a5 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.h
@@ -137,6 +137,8 @@ struct ixgbe_hw {
u8 revision_id;
bool adapter_stopped;
+
+ int api_version;
};
struct ixgbevf_hw_stats {
@@ -171,5 +173,6 @@ struct ixgbevf_info {
};
void ixgbevf_rlpml_set_vf(struct ixgbe_hw *hw, u16 max_size);
+int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api);
#endif /* __IXGBE_VF_H__ */
--
1.7.11.4
^ permalink raw reply related
* [net-next 2/4] igb: Support to enable EEE on all eee_supported devices
From: Jeff Kirsher @ 2012-09-20 10:07 UTC (permalink / raw)
To: davem; +Cc: Akeem G. Abodunrin, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1348135637-17857-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: "Akeem G. Abodunrin" <akeem.g.abodunrin@intel.com>
Current implementation enables EEE on only i350 device. This patch enables
EEE on all eee_supported devices. Also, configured LPI clock to keep
running before EEE is enabled on i210 and i211 devices.
Signed-off-by: Akeem G. Abodunrin <akeem.g.abodunrin@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/e1000_82575.c | 17 +++++++++++++----
drivers/net/ethernet/intel/igb/e1000_defines.h | 3 ++-
drivers/net/ethernet/intel/igb/e1000_regs.h | 1 +
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_82575.c b/drivers/net/ethernet/intel/igb/e1000_82575.c
index ba994fb..ca4641e 100644
--- a/drivers/net/ethernet/intel/igb/e1000_82575.c
+++ b/drivers/net/ethernet/intel/igb/e1000_82575.c
@@ -2223,11 +2223,10 @@ out:
s32 igb_set_eee_i350(struct e1000_hw *hw)
{
s32 ret_val = 0;
- u32 ipcnfg, eeer, ctrl_ext;
+ u32 ipcnfg, eeer;
- ctrl_ext = rd32(E1000_CTRL_EXT);
- if ((hw->mac.type != e1000_i350) ||
- (ctrl_ext & E1000_CTRL_EXT_LINK_MODE_MASK))
+ if ((hw->mac.type < e1000_i350) ||
+ (hw->phy.media_type != e1000_media_type_copper))
goto out;
ipcnfg = rd32(E1000_IPCNFG);
eeer = rd32(E1000_EEER);
@@ -2240,6 +2239,14 @@ s32 igb_set_eee_i350(struct e1000_hw *hw)
E1000_EEER_RX_LPI_EN |
E1000_EEER_LPI_FC);
+ /* keep the LPI clock running before EEE is enabled */
+ if (hw->mac.type == e1000_i210 || hw->mac.type == e1000_i211) {
+ u32 eee_su;
+ eee_su = rd32(E1000_EEE_SU);
+ eee_su &= ~E1000_EEE_SU_LPI_CLK_STP;
+ wr32(E1000_EEE_SU, eee_su);
+ }
+
} else {
ipcnfg &= ~(E1000_IPCNFG_EEE_1G_AN |
E1000_IPCNFG_EEE_100M_AN);
@@ -2249,6 +2256,8 @@ s32 igb_set_eee_i350(struct e1000_hw *hw)
}
wr32(E1000_IPCNFG, ipcnfg);
wr32(E1000_EEER, eeer);
+ rd32(E1000_IPCNFG);
+ rd32(E1000_EEER);
out:
return ret_val;
diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index cae3070..de4b41e 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -857,8 +857,9 @@
#define E1000_IPCNFG_EEE_100M_AN 0x00000004 /* EEE Enable 100M AN */
#define E1000_EEER_TX_LPI_EN 0x00010000 /* EEE Tx LPI Enable */
#define E1000_EEER_RX_LPI_EN 0x00020000 /* EEE Rx LPI Enable */
-#define E1000_EEER_FRC_AN 0x10000000 /* Enable EEE in loopback */
+#define E1000_EEER_FRC_AN 0x10000000 /* Enable EEE in loopback */
#define E1000_EEER_LPI_FC 0x00040000 /* EEE Enable on FC */
+#define E1000_EEE_SU_LPI_CLK_STP 0X00800000 /* EEE LPI Clock Stop */
/* SerDes Control */
#define E1000_GEN_CTL_READY 0x80000000
diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h
index faec840..e5db485 100644
--- a/drivers/net/ethernet/intel/igb/e1000_regs.h
+++ b/drivers/net/ethernet/intel/igb/e1000_regs.h
@@ -349,6 +349,7 @@
/* Energy Efficient Ethernet "EEE" register */
#define E1000_IPCNFG 0x0E38 /* Internal PHY Configuration */
#define E1000_EEER 0x0E30 /* Energy Efficient Ethernet */
+#define E1000_EEE_SU 0X0E34 /* EEE Setup */
/* Thermal Sensor Register */
#define E1000_THSTAT 0x08110 /* Thermal Sensor Status */
--
1.7.11.4
^ permalink raw reply related
* [net-next 1/4] igb: Remove artificial restriction on RQDPC stat reading
From: Jeff Kirsher @ 2012-09-20 10:07 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1348135637-17857-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
For some reason the reading of the RQDPC register was being artificially
limited to 4K. Instead of limiting the value we should read the value and
add the full amount. Otherwise this can lead to a misleading number of
dropped packets when the actual value is in fact much higher.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 19d7666..246646b 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4681,11 +4681,13 @@ void igb_update_stats(struct igb_adapter *adapter,
bytes = 0;
packets = 0;
for (i = 0; i < adapter->num_rx_queues; i++) {
- u32 rqdpc_tmp = rd32(E1000_RQDPC(i)) & 0x0FFF;
+ u32 rqdpc = rd32(E1000_RQDPC(i));
struct igb_ring *ring = adapter->rx_ring[i];
- ring->rx_stats.drops += rqdpc_tmp;
- net_stats->rx_fifo_errors += rqdpc_tmp;
+ if (rqdpc) {
+ ring->rx_stats.drops += rqdpc;
+ net_stats->rx_fifo_errors += rqdpc;
+ }
do {
start = u64_stats_fetch_begin_bh(&ring->rx_syncp);
--
1.7.11.4
^ permalink raw reply related
* [net-next 0/4 v2][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2012-09-20 10:07 UTC (permalink / raw)
To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann
This series contains updates to igb and ixgbevf.
v2: updated patch description in 04 patch (ixgbevf: scheduling while
atomic in reset hw path)
The following are changes since commit aee77e4accbeb2c86b1d294cd84fec4a12dde3bd:
r8169: use unlimited DMA burst for TX
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Akeem G. Abodunrin (1):
igb: Support to enable EEE on all eee_supported devices
Alexander Duyck (2):
igb: Remove artificial restriction on RQDPC stat reading
ixgbevf: Add support for VF API negotiation
John Fastabend (1):
ixgbevf: scheduling while atomic in reset hw path
drivers/net/ethernet/intel/igb/e1000_82575.c | 17 +++++++---
drivers/net/ethernet/intel/igb/e1000_defines.h | 3 +-
drivers/net/ethernet/intel/igb/e1000_regs.h | 1 +
drivers/net/ethernet/intel/igb/igb_main.c | 8 +++--
drivers/net/ethernet/intel/ixgbevf/defines.h | 1 +
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 23 +++++++++++++
drivers/net/ethernet/intel/ixgbevf/mbx.h | 21 ++++++++++--
drivers/net/ethernet/intel/ixgbevf/vf.c | 39 ++++++++++++++++++++++-
drivers/net/ethernet/intel/ixgbevf/vf.h | 3 ++
9 files changed, 105 insertions(+), 11 deletions(-)
--
1.7.11.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox