* [PATCH net] ipv6: tcp: fix race in IPV6_2292PKTOPTIONS
From: Eric Dumazet @ 2015-01-21 11:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
IPv6 TCP sockets store in np->pktoptions skbs, and use skb_set_owner_r()
to charge the skb to socket.
It means that destructor must be called while socket is locked.
Therefore, we cannot use skb_get() or atomic_inc(&skb->users)
to protect ourselves : kfree_skb() might race with other users
manipulating sk->sk_forward_alloc
Fix this race by holding socket lock for the duration of
ip6_datagram_recv_ctl()
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Since this bug is very old, feel free to apply on net-next ;)
net/ipv6/ipv6_sockglue.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index 66980d8d98d1f5b3ef7a50dc33cb9b617f25604d..8d766d9100cba408525faf5818b7b0c6b6bc543c 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -996,13 +996,9 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
lock_sock(sk);
skb = np->pktoptions;
if (skb)
- atomic_inc(&skb->users);
- release_sock(sk);
-
- if (skb) {
ip6_datagram_recv_ctl(sk, &msg, skb);
- kfree_skb(skb);
- } else {
+ release_sock(sk);
+ if (!skb) {
if (np->rxopt.bits.rxinfo) {
struct in6_pktinfo src_info;
src_info.ipi6_ifindex = np->mcast_oif ? np->mcast_oif :
^ permalink raw reply related
* [PATCH v2] net: wireless: atmel: Remove open-coded and wrong strcasecmp
From: Rasmus Villemoes @ 2015-01-21 11:52 UTC (permalink / raw)
To: Simon Kelley, Kalle Valo
Cc: Sergei Shtylyov, Rasmus Villemoes, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1421414907-26764-1-git-send-email-linux@rasmusvillemoes.dk>
The kernel's string library does in fact have strcasecmp, at least
since ded220bd8f08 ("[STRING]: Move strcasecmp/strncasecmp to
lib/string.c"). Moreover, this open-coded version is in fact wrong: If
the strings only differ in their last character, a and b have already
been incremented to point to the terminating NUL bytes, so they would
wrongly be treated as equal.
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
v2: Address Sergei's comments to the commit message.
drivers/net/wireless/atmel.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/net/wireless/atmel.c b/drivers/net/wireless/atmel.c
index 9183f1cf89a7..55db9f03eb2a 100644
--- a/drivers/net/wireless/atmel.c
+++ b/drivers/net/wireless/atmel.c
@@ -45,7 +45,6 @@
#include <linux/ptrace.h>
#include <linux/slab.h>
#include <linux/string.h>
-#include <linux/ctype.h>
#include <linux/timer.h>
#include <asm/byteorder.h>
#include <asm/io.h>
@@ -2699,16 +2698,7 @@ static int atmel_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
domain[REGDOMAINSZ] = 0;
rc = -EINVAL;
for (i = 0; i < ARRAY_SIZE(channel_table); i++) {
- /* strcasecmp doesn't exist in the library */
- char *a = channel_table[i].name;
- char *b = domain;
- while (*a) {
- char c1 = *a++;
- char c2 = *b++;
- if (tolower(c1) != tolower(c2))
- break;
- }
- if (!*a && !*b) {
+ if (!strcasecmp(channel_table[i].name, domain)) {
priv->config_reg_domain = channel_table[i].reg_domain;
rc = 0;
}
--
2.1.3
^ permalink raw reply related
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Wolfgang Grandegger @ 2015-01-21 11:53 UTC (permalink / raw)
To: Andri Yngvason
Cc: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150121103319.14511.57709@shannon>
On Wed, 21 Jan 2015 10:33:19 +0000, Andri Yngvason
<andri.yngvason@marel.com> wrote:
> Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>
>> Replace most of the can interface's state and error counters
>> handling with the new can-dev can_change_state() mechanism.
>>
>> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>> ---
>> drivers/net/can/usb/kvaser_usb.c | 114
>> +++++++++++++++++++--------------------
>> 1 file changed, 55 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/net/can/usb/kvaser_usb.c
>> b/drivers/net/can/usb/kvaser_usb.c
>> index 971c5f9..0386d3f 100644
>> --- a/drivers/net/can/usb/kvaser_usb.c
>> +++ b/drivers/net/can/usb/kvaser_usb.c
...
>
> Looks good.
Would be nice to see some "candump" traces as well. Ahmed, could you
please generate such traces doing:
1. Execute in a session:
# candump -t d -e any,0:0,#FFFFFFFF
2. Execute in another session:
# cangen -g 10 -D i can0
3. Disconnect the CAN cable
4. After a while reconnect the CAN cable
5. Stop candump and save the trace.
and to test bus-off:
1. Execute in a session:
# candump -t d -e any,0:0,#FFFFFFFF
2. Execute in another session:
# cangen -g 10 -D i can0
3. Short-circuit the CAN low and high wires of the CAN cable
4. After a while remove the short-circuit.
5. Stop candump and save the trace.
Thanks,
Wolfgang.
^ permalink raw reply
* [PATCH net-next v2] rhashtable: rhashtable_remove() must unlink in both tbl and future_tbl
From: Thomas Graf @ 2015-01-21 11:54 UTC (permalink / raw)
To: davem, Sergei Shtylyov
Cc: Ying Xue, richard.alpe@ericsson.com >> Richard Alpe, Netdev,
tipc-discussion
In-Reply-To: <54BEA1F9.1050908@cogentembedded.com>
As removals can occur during resizes, entries may be referred to from
both tbl and future_tbl when the removal is requested. Therefore
rhashtable_remove() must unlink the entry in both tables if this is
the case. The existing code did search both tables but stopped when it
hit the first match.
Failing to unlink in both tables resulted in use after free.
Fixes: 97defe1ecf86 ("rhashtable: Per bucket locks & deferred expansion/shrinking")
Reported-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
v2: 12 digit commit reference, commit msg clarification as pointed
out by Sergei.
lib/rhashtable.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 84a78e3..bc2d0d8 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -585,6 +585,7 @@ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj)
struct rhash_head *he;
spinlock_t *lock;
unsigned int hash;
+ bool ret = false;
rcu_read_lock();
tbl = rht_dereference_rcu(ht->tbl, ht);
@@ -602,17 +603,16 @@ restart:
}
rcu_assign_pointer(*pprev, obj->next);
- atomic_dec(&ht->nelems);
-
- spin_unlock_bh(lock);
-
- rhashtable_wakeup_worker(ht);
-
- rcu_read_unlock();
- return true;
+ ret = true;
+ break;
}
+ /* The entry may be linked in either 'tbl', 'future_tbl', or both.
+ * 'future_tbl' only exists for a short period of time during
+ * resizing. Thus traversing both is fine and the added cost is
+ * very rare.
+ */
if (tbl != rht_dereference_rcu(ht->future_tbl, ht)) {
spin_unlock_bh(lock);
@@ -625,9 +625,15 @@ restart:
}
spin_unlock_bh(lock);
+
+ if (ret) {
+ atomic_dec(&ht->nelems);
+ rhashtable_wakeup_worker(ht);
+ }
+
rcu_read_unlock();
- return false;
+ return ret;
}
EXPORT_SYMBOL_GPL(rhashtable_remove);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] rhashtable: rhashtable_remove() must unlink in both tbl and future_tbl
From: Thomas Graf @ 2015-01-21 11:56 UTC (permalink / raw)
To: Ying Xue
Cc: davem, richard.alpe@ericsson.com >> Richard Alpe, Netdev,
tipc-discussion
In-Reply-To: <54BF65C2.9010404@windriver.com>
On 01/21/15 at 04:39pm, Ying Xue wrote:
> Your below changes are reasonable for me. But after I applied the patch,
> my reported two issues still happened :( And failure logs are completely
> same before.
Correct. The patch fixes the panic that you reported as well:
[ 17.033416] BUG: unable to handle kernel paging request at
ffffffffffffff77
[ 17.034009] IP: [<ffffffffa0019b8c>] jhash+0xec/0x160 [tipc]
[...]
[ 17.034009] Call Trace:
[ 17.034009] [<ffffffff813b38e9>] head_hashfn+0x29/0x50
[ 17.034009] [<ffffffff813b475b>] rhashtable_expand+0x37b/0x680
[ 17.034009] [<ffffffff813b4b38>] rht_deferred_worker+0xd8/0xe0
[ 17.034009] [<ffffffff81074d10>] process_one_work+0x200/0x800
[ 17.034009] [<ffffffff81074c71>] ? process_one_work+0x161/0x800
[ 17.034009] [<ffffffff81074c71>] ? process_one_work+0x161/0x800
[ 17.034009] [<ffffffff8107533c>] process_scheduled_works+0x2c/0x40
[ 17.034009] [<ffffffff810758d3>] worker_thread+0x253/0x4b0
[ 17.034009] [<ffffffff81075680>] ? rescuer_thread+0x330/0x330
[ 17.034009] [<ffffffff8107bb6f>] kthread+0xef/0x110
[ 17.034009] [<ffffffff8107ba80>] ? flush_kthread_work+0x170/0x170
[ 17.034009] [<ffffffff81a44a2c>] ret_from_fork+0x7c/0xb0
[ 17.034009] [<ffffffff8107ba80>] ? flush_kthread_work+0x170/0x170
^ permalink raw reply
* RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: David Laight @ 2015-01-21 11:56 UTC (permalink / raw)
To: 'Hiroshi Shimamoto', Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, linux-kernel@vger.kernel.org, Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E08FF6@BPXM14GP.gisp.nec.co.jp>
From: Hiroshi Shimamoto
> My concern is what is the real issue that VF multicast promiscuous mode can cause.
> I think there is the 4k entries to filter multicast address, and the current ixgbe/ixgbevf
> can turn all bits on from VM. That is almost same as enabling multicast promiscuous mode.
> I mean that we can receive all multicast addresses by an onerous operation in untrusted VM.
> I think we should clarify what is real security issue in this context.
If you are worried about passing un-enabled multicasts to users then
what about doing a software hash of received multicasts and checking
against an actual list of multicasts enabled for that hash entry.
Under normal conditions there is likely to be only a single address to check.
It may (or may not) be best to use the same hash as any hashing hardware
filter uses.
David
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-21 11:59 UTC (permalink / raw)
To: Thomas Graf; +Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121113751.GT20315@casper.infradead.org>
On 21.01, Thomas Graf wrote:
> On 01/21/15 at 10:40am, Patrick McHardy wrote:
> > On the danger of repeating myself, every (converted) user requires
> > that we at least keep the existing semantics since it is exposed to
> > userspace. My opinion is that NLM_F_DUMP_INTR is fine if userspace
> > indicates support, but without that, we have to take care of that
> > in the kernel.
>
> Absolutely agreed. I think this is an excellent low cost path for
> future users where dumps are rare.
>
> > An automatic restart handles this well. Userspace always had to
> > expect duplicates.
>
> Maybe I don't understand the restart yet. How do you restart if the
> dump was already started and the user has read part of the dump?
You use the mutex to prevent concurrent resizes and dumps, for dumps
that have left the kernel you simply reset the iterator state after
a resize operation.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Thomas Graf @ 2015-01-21 12:07 UTC (permalink / raw)
To: Patrick McHardy
Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121115924.GW3012@acer.localdomain>
On 01/21/15 at 11:59am, Patrick McHardy wrote:
> On 21.01, Thomas Graf wrote:
> > On 01/21/15 at 10:40am, Patrick McHardy wrote:
> > > An automatic restart handles this well. Userspace always had to
> > > expect duplicates.
> >
> > Maybe I don't understand the restart yet. How do you restart if the
> > dump was already started and the user has read part of the dump?
>
> You use the mutex to prevent concurrent resizes and dumps, for dumps
> that have left the kernel you simply reset the iterator state after
> a resize operation.
I see, so we possibly see a lot of duplicates but we will never miss
any entries.
So in summary we do:
2) We allow rhashtable users to opt in to Herbert's consistent walker
relying on the use of an additional bit.
2) If user supports NLM_F_DUMP_INTR we block out resizes during
message construction and set NLM_F_DUMP_INTR if a resize/rehash
interrupted us while the user read.
3) If (1) and (2) are not available we do (2) but restart the dump
on interruption thus causing duplicates but never missing any
entries.
I think this offers good flexibility for new users and compatibility
for existing users.
^ permalink raw reply
* Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
From: Patrick McHardy @ 2015-01-21 12:09 UTC (permalink / raw)
To: Thomas Graf; +Cc: Herbert Xu, davem, paulmck, ying.xue, netdev, netfilter-devel
In-Reply-To: <20150121120727.GC12570@casper.infradead.org>
On 21.01, Thomas Graf wrote:
> On 01/21/15 at 11:59am, Patrick McHardy wrote:
> > On 21.01, Thomas Graf wrote:
> > > On 01/21/15 at 10:40am, Patrick McHardy wrote:
> > > > An automatic restart handles this well. Userspace always had to
> > > > expect duplicates.
> > >
> > > Maybe I don't understand the restart yet. How do you restart if the
> > > dump was already started and the user has read part of the dump?
> >
> > You use the mutex to prevent concurrent resizes and dumps, for dumps
> > that have left the kernel you simply reset the iterator state after
> > a resize operation.
>
> I see, so we possibly see a lot of duplicates but we will never miss
> any entries.
>
> So in summary we do:
>
> 2) We allow rhashtable users to opt in to Herbert's consistent walker
> relying on the use of an additional bit.
>
> 2) If user supports NLM_F_DUMP_INTR we block out resizes during
> message construction and set NLM_F_DUMP_INTR if a resize/rehash
> interrupted us while the user read.
>
> 3) If (1) and (2) are not available we do (2) but restart the dump
> on interruption thus causing duplicates but never missing any
> entries.
>
> I think this offers good flexibility for new users and compatibility
> for existing users.
Yep, sounds good to me.
^ permalink raw reply
* Re: [PATCH 2/3] netlink: Mark dumps as inconsistent which have been interrupted by a resize
From: Thomas Graf @ 2015-01-21 12:17 UTC (permalink / raw)
To: Ying Xue; +Cc: davem, kaber, herbert, paulmck, netdev, netfilter-devel
In-Reply-To: <54BF5FC4.4070808@windriver.com>
On 01/21/15 at 04:13pm, Ying Xue wrote:
> On 01/20/2015 09:20 PM, Thomas Graf wrote:
> > A deferred resize of nl_table causes the offsets that Netlink diag keeps
> > to become inaccurate. Mark the dump as inconsistent and have user space
> > request a new dump.
> >
> > Signed-off-by: Thomas Graf <tgraf@suug.ch>
> > ---
> > net/netlink/af_netlink.c | 10 ++++++++++
> > net/netlink/af_netlink.h | 1 +
> > net/netlink/diag.c | 1 +
> > 3 files changed, 12 insertions(+)
> >
> > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> > index 7a94185..e214557 100644
> > --- a/net/netlink/af_netlink.c
> > +++ b/net/netlink/af_netlink.c
> > @@ -91,6 +91,9 @@ static inline int netlink_is_kernel(struct sock *sk)
> > struct netlink_table *nl_table;
> > EXPORT_SYMBOL_GPL(nl_table);
> >
> > +atomic_t nl_table_seq;
>
> It sounds like the atomic variable is not initialized.
Thanks for the review. We also need to avoid hitting 0 when we overflow
on a seq increment. The netfilter code is doing this correctly but several
other users are suffering from this as well.
I'll address this in v2 together with the other discussed changes.
^ permalink raw reply
* Re: [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Hiroshi Shimamoto @ 2015-01-21 12:18 UTC (permalink / raw)
To: David Laight, Skidmore, Donald C, Bjørn Mork
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, Hayato Momma, linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD01D6@AcuExch.aculab.com>
> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
>
> From: Hiroshi Shimamoto
> > My concern is what is the real issue that VF multicast promiscuous mode can cause.
> > I think there is the 4k entries to filter multicast address, and the current ixgbe/ixgbevf
> > can turn all bits on from VM. That is almost same as enabling multicast promiscuous mode.
> > I mean that we can receive all multicast addresses by an onerous operation in untrusted VM.
> > I think we should clarify what is real security issue in this context.
>
> If you are worried about passing un-enabled multicasts to users then
> what about doing a software hash of received multicasts and checking
> against an actual list of multicasts enabled for that hash entry.
> Under normal conditions there is likely to be only a single address to check.
>
> It may (or may not) be best to use the same hash as any hashing hardware
> filter uses.
thanks for the comment. But I don't think that is the point.
I guess, introducing VF multicast promiscuous mode seems to add new privilege
to peek every multicast packet in VM and that doesn't look good.
On the other hand, I think that there has been the same privilege in the current
ixgbe/ixgbevf implementation already. Or I'm reading the code wrongly.
I'd like to clarify what is the issue of allowing to receive all multicast packets.
thanks,
Hiroshi
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH v5 4/5] can: kvaser_usb: Retry the first bulk transfer on -ETIMEDOUT
From: Sergei Shtylyov @ 2015-01-21 12:24 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Andri Yngvason, Linux-CAN, netdev, LKML
In-Reply-To: <20150120214834.GD16828@linux>
Hello.
On 1/21/2015 12:48 AM, Ahmed S. Darwish wrote:
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> On some x86 laptops, plugging a Kvaser device again after an
> unplug makes the firmware always ignore the very first command.
> For such a case, provide some room for retries instead of
> completly exiting the driver init code.
Completely.
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 640b0eb..068e76c 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
[...]
> @@ -1632,7 +1632,15 @@ static int kvaser_usb_probe(struct usb_interface *intf,
>
> usb_set_intfdata(intf, dev);
>
> - err = kvaser_usb_get_software_info(dev);
> + /* On some x86 laptops, plugging a Kvaser device again after
> + * an unplug makes the firmware always ignore the very first
> + * command. For such a case, provide some room for retries
> + * instead of completly exiting the driver.
Completely.
[...]
WBR, Sergei
^ permalink raw reply
* RE: BW regression after "tcp: refine TSO autosizing"
From: David Laight @ 2015-01-21 12:26 UTC (permalink / raw)
To: 'Rick Jones', Eric Dumazet, Dave Taht
Cc: Eyal Perry, Yuchung Cheng, Neal Cardwell, Eyal Perry, Or Gerlitz,
Linux Netdev List, Amir Vadai, Yevgeny Petrilin, Saeed Mahameed,
Ido Shamay, Amir Ancel
In-Reply-To: <54BEA91D.1050001@hp.com>
From: Of Rick Jones
> >> Are you saying that at long last, delayed acks as we knew them are
> >> dead, dead, dead?
> >
> > Sorry, I can not parse what you are saying.
> >
> > In case you missed it, it has nothing to do with delayed ACK but GRO on
> > receiver.
>
> Dave - assuming I've interpreted Eric's comments correctly, I believe
> the answer to your question is No. Your desire for a world brimming
> with ack-every-other purity has not been fulfilled :)
>
> However, the engineers formerly at Mentat are probably pleased that a
> functional near-equivalent to their ACK avoidance heuristic has ended-up
> being implemented and tacitly accepted, albeit by the back door :)
I must recheck something I discovered a while back with more recent kernels.
There has been a bad interaction between 'slow start' and 'delayed acks'
when nagle is disabled on 0 RTT local links with uni-directional traffic.
'Slow start' would refuse to send more than 4 messages until it received
an ack (rather than 4 mss of data).
The receiving system wouldn't send an ack until the timer expired
(or several mss of data were received) by which time the sender could have
a lot of data queued.
Due to the 0 RTT and bursty nature of the data 'slow start' happened
all the time.
David
^ permalink raw reply
* [PATCH 2/2] net/fsl: Replace spin_event_timeout() with arch independent in xgmac_mdio
From: shh.xie @ 2015-01-21 11:08 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
spin_event_timeout() is PPC dependent, use an arch independent
equivalent instead.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
drivers/net/ethernet/freescale/xgmac_mdio.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index ab9a6bf..3a83bc2 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -52,12 +52,16 @@ struct tgec_mdio_controller {
static int xgmac_wait_until_free(struct device *dev,
struct tgec_mdio_controller __iomem *regs)
{
- uint32_t status;
+ unsigned int timeout;
/* Wait till the bus is free */
- status = spin_event_timeout(
- !((ioread32be(®s->mdio_stat)) & MDIO_STAT_BSY), TIMEOUT, 0);
- if (!status) {
+ timeout = TIMEOUT;
+ while ((ioread32be(®s->mdio_stat) & MDIO_STAT_BSY) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout) {
dev_err(dev, "timeout waiting for bus to be free\n");
return -ETIMEDOUT;
}
@@ -71,12 +75,16 @@ static int xgmac_wait_until_free(struct device *dev,
static int xgmac_wait_until_done(struct device *dev,
struct tgec_mdio_controller __iomem *regs)
{
- uint32_t status;
+ unsigned int timeout;
/* Wait till the MDIO write is complete */
- status = spin_event_timeout(
- !((ioread32be(®s->mdio_data)) & MDIO_DATA_BSY), TIMEOUT, 0);
- if (!status) {
+ timeout = TIMEOUT;
+ while ((ioread32be(®s->mdio_data) & MDIO_DATA_BSY) && timeout) {
+ cpu_relax();
+ timeout--;
+ }
+
+ if (!timeout) {
dev_err(dev, "timeout waiting for operation to complete\n");
return -ETIMEDOUT;
}
--
1.8.4.1
^ permalink raw reply related
* [PATCH 1/2] net/fsl: drop in_be32() & out_be32() in xgmac_mdio
From: shh.xie @ 2015-01-21 11:07 UTC (permalink / raw)
To: netdev, davem; +Cc: Shaohui Xie
From: Shaohui Xie <Shaohui.Xie@freescale.com>
Use ioread32be() & iowrite32be() instead.
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
drivers/net/ethernet/freescale/xgmac_mdio.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
index 3a76e23..ab9a6bf 100644
--- a/drivers/net/ethernet/freescale/xgmac_mdio.c
+++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
@@ -56,7 +56,7 @@ static int xgmac_wait_until_free(struct device *dev,
/* Wait till the bus is free */
status = spin_event_timeout(
- !((in_be32(®s->mdio_stat)) & MDIO_STAT_BSY), TIMEOUT, 0);
+ !((ioread32be(®s->mdio_stat)) & MDIO_STAT_BSY), TIMEOUT, 0);
if (!status) {
dev_err(dev, "timeout waiting for bus to be free\n");
return -ETIMEDOUT;
@@ -75,7 +75,7 @@ static int xgmac_wait_until_done(struct device *dev,
/* Wait till the MDIO write is complete */
status = spin_event_timeout(
- !((in_be32(®s->mdio_data)) & MDIO_DATA_BSY), TIMEOUT, 0);
+ !((ioread32be(®s->mdio_data)) & MDIO_DATA_BSY), TIMEOUT, 0);
if (!status) {
dev_err(dev, "timeout waiting for operation to complete\n");
return -ETIMEDOUT;
@@ -96,7 +96,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
u32 mdio_ctl, mdio_stat;
int ret;
- mdio_stat = in_be32(®s->mdio_stat);
+ mdio_stat = ioread32be(®s->mdio_stat);
if (regnum & MII_ADDR_C45) {
/* Clause 45 (ie 10G) */
dev_addr = (regnum >> 16) & 0x1f;
@@ -107,7 +107,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
mdio_stat &= ~MDIO_STAT_ENC;
}
- out_be32(®s->mdio_stat, mdio_stat);
+ iowrite32be(mdio_stat, ®s->mdio_stat);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -115,11 +115,11 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
/* Set the port and dev addr */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- out_be32(®s->mdio_ctl, mdio_ctl);
+ iowrite32be(mdio_ctl, ®s->mdio_ctl);
/* Set the register address */
if (regnum & MII_ADDR_C45) {
- out_be32(®s->mdio_addr, regnum & 0xffff);
+ iowrite32be(regnum & 0xffff, ®s->mdio_addr);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -127,7 +127,7 @@ static int xgmac_mdio_write(struct mii_bus *bus, int phy_id, int regnum, u16 val
}
/* Write the value to the register */
- out_be32(®s->mdio_data, MDIO_DATA(value));
+ iowrite32be(MDIO_DATA(value), ®s->mdio_data);
ret = xgmac_wait_until_done(&bus->dev, regs);
if (ret)
@@ -150,7 +150,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
uint16_t value;
int ret;
- mdio_stat = in_be32(®s->mdio_stat);
+ mdio_stat = ioread32be(®s->mdio_stat);
if (regnum & MII_ADDR_C45) {
dev_addr = (regnum >> 16) & 0x1f;
mdio_stat |= MDIO_STAT_ENC;
@@ -159,7 +159,7 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
mdio_stat &= ~MDIO_STAT_ENC;
}
- out_be32(®s->mdio_stat, mdio_stat);
+ iowrite32be(mdio_stat, ®s->mdio_stat);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -167,11 +167,11 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
/* Set the Port and Device Addrs */
mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
- out_be32(®s->mdio_ctl, mdio_ctl);
+ iowrite32be(mdio_ctl, ®s->mdio_ctl);
/* Set the register address */
if (regnum & MII_ADDR_C45) {
- out_be32(®s->mdio_addr, regnum & 0xffff);
+ iowrite32be(regnum & 0xffff, ®s->mdio_addr);
ret = xgmac_wait_until_free(&bus->dev, regs);
if (ret)
@@ -179,21 +179,21 @@ static int xgmac_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
}
/* Initiate the read */
- out_be32(®s->mdio_ctl, mdio_ctl | MDIO_CTL_READ);
+ iowrite32be(mdio_ctl | MDIO_CTL_READ, ®s->mdio_ctl);
ret = xgmac_wait_until_done(&bus->dev, regs);
if (ret)
return ret;
/* Return all Fs if nothing was there */
- if (in_be32(®s->mdio_stat) & MDIO_STAT_RD_ER) {
+ if (ioread32be(®s->mdio_stat) & MDIO_STAT_RD_ER) {
dev_err(&bus->dev,
"Error while reading PHY%d reg at %d.%hhu\n",
phy_id, dev_addr, regnum);
return 0xffff;
}
- value = in_be32(®s->mdio_data) & 0xffff;
+ value = ioread32be(®s->mdio_data) & 0xffff;
dev_dbg(&bus->dev, "read %04x\n", value);
return value;
--
1.8.4.1
^ permalink raw reply related
* [PATCH net 0/2] s390/qeth patches for net
From: Ursula Braun @ 2015-01-21 12:39 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, ursula.braun, ubraun
From: Ursula Braun <ursula.braun@de.ibm.com>
Hi Dave,
here are two s390/qeth patches built for net.
One patch is quite large, but we would like to fix the locking warning
seen in recent kernels as soon as possible. But if you want me to submit
these patches for net-next, I will do.
shortlog:
Eugene Crosser (1)
qeth: clean up error handling
Thomas-Mich Richter (1)
qeth: Fix locking warning during qeth device setup
Thanks,
Ursula
^ permalink raw reply
* [PATCH net 1/2] qeth: clean up error handling
From: Ursula Braun @ 2015-01-21 12:39 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, ursula.braun, ubraun, Eugene Crosser
In-Reply-To: <1421843950-58220-1-git-send-email-ubraun@linux.vnet.ibm.com>
From: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
In the functions that are registering and unregistering MAC
addresses in the qeth-handled hardware, remove callback functions
that are unnesessary, as only the return code is analyzed.
Translate hardware response codes to semi-standard 'errno'-like
codes for readability.
Add kernel-doc description to the internal API function
qeth_send_control_data().
Signed-off-by: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Reviewed-by: Thomas-Mich Richter <tmricht@linux.vnet.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 36 +++++++-
drivers/s390/net/qeth_l2_main.c | 187 +++++++++++++++++---------------------
2 files changed, 117 insertions(+), 106 deletions(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index f407e37..e2fbfff 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -2021,10 +2021,36 @@ void qeth_prepare_control_data(struct qeth_card *card, int len,
}
EXPORT_SYMBOL_GPL(qeth_prepare_control_data);
+/**
+ * qeth_send_control_data() - send control command to the card
+ * @card: qeth_card structure pointer
+ * @len: size of the command buffer
+ * @iob: qeth_cmd_buffer pointer
+ * @reply_cb: callback function pointer
+ * @cb_card: pointer to the qeth_card structure
+ * @cb_reply: pointer to the qeth_reply structure
+ * @cb_cmd: pointer to the original iob for non-IPA
+ * commands, or to the qeth_ipa_cmd structure
+ * for the IPA commands.
+ * @reply_param: private pointer passed to the callback
+ *
+ * Returns the value of the `return_code' field of the response
+ * block returned from the hardware, or other error indication.
+ * Value of zero indicates successful execution of the command.
+ *
+ * Callback function gets called one or more times, with cb_cmd
+ * pointing to the response returned by the hardware. Callback
+ * function must return non-zero if more reply blocks are expected,
+ * and zero if the last or only reply block is received. Callback
+ * function can get the value of the reply_param pointer from the
+ * field 'param' of the structure qeth_reply.
+ */
+
int qeth_send_control_data(struct qeth_card *card, int len,
struct qeth_cmd_buffer *iob,
- int (*reply_cb)(struct qeth_card *, struct qeth_reply *,
- unsigned long),
+ int (*reply_cb)(struct qeth_card *cb_card,
+ struct qeth_reply *cb_reply,
+ unsigned long cb_cmd),
void *reply_param)
{
int rc;
@@ -2932,6 +2958,12 @@ void qeth_prepare_ipa_cmd(struct qeth_card *card, struct qeth_cmd_buffer *iob,
}
EXPORT_SYMBOL_GPL(qeth_prepare_ipa_cmd);
+/**
+ * qeth_send_ipa_cmd() - send an IPA command
+ *
+ * See qeth_send_control_data() for explanation of the arguments.
+ */
+
int qeth_send_ipa_cmd(struct qeth_card *card, struct qeth_cmd_buffer *iob,
int (*reply_cb)(struct qeth_card *, struct qeth_reply*,
unsigned long),
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index d02cd1a..b7b9d5c 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -27,10 +27,7 @@ static int qeth_l2_set_offline(struct ccwgroup_device *);
static int qeth_l2_stop(struct net_device *);
static int qeth_l2_send_delmac(struct qeth_card *, __u8 *);
static int qeth_l2_send_setdelmac(struct qeth_card *, __u8 *,
- enum qeth_ipa_cmds,
- int (*reply_cb) (struct qeth_card *,
- struct qeth_reply*,
- unsigned long));
+ enum qeth_ipa_cmds);
static void qeth_l2_set_multicast_list(struct net_device *);
static int qeth_l2_recover(void *);
static void qeth_bridgeport_query_support(struct qeth_card *card);
@@ -130,56 +127,68 @@ static struct net_device *qeth_l2_netdev_by_devno(unsigned char *read_dev_no)
return ndev;
}
-static int qeth_l2_send_setgroupmac_cb(struct qeth_card *card,
- struct qeth_reply *reply,
- unsigned long data)
+static int qeth_setdel_makerc(struct qeth_card *card, int retcode)
{
- struct qeth_ipa_cmd *cmd;
- __u8 *mac;
+ int rc;
- QETH_CARD_TEXT(card, 2, "L2Sgmacb");
- cmd = (struct qeth_ipa_cmd *) data;
- mac = &cmd->data.setdelmac.mac[0];
- /* MAC already registered, needed in couple/uncouple case */
- if (cmd->hdr.return_code == IPA_RC_L2_DUP_MAC) {
- QETH_DBF_MESSAGE(2, "Group MAC %pM already existing on %s \n",
- mac, QETH_CARD_IFNAME(card));
- cmd->hdr.return_code = 0;
+ if (retcode)
+ QETH_CARD_TEXT_(card, 2, "err%d", retcode);
+ switch (retcode) {
+ case IPA_RC_SUCCESS:
+ rc = 0;
+ break;
+ case IPA_RC_L2_UNSUPPORTED_CMD:
+ rc = -ENOSYS;
+ break;
+ case IPA_RC_L2_ADDR_TABLE_FULL:
+ rc = -ENOSPC;
+ break;
+ case IPA_RC_L2_DUP_MAC:
+ case IPA_RC_L2_DUP_LAYER3_MAC:
+ rc = -EEXIST;
+ break;
+ case IPA_RC_L2_MAC_NOT_AUTH_BY_HYP:
+ case IPA_RC_L2_MAC_NOT_AUTH_BY_ADP:
+ rc = -EPERM;
+ break;
+ case IPA_RC_L2_MAC_NOT_FOUND:
+ rc = -ENOENT;
+ break;
+ default:
+ rc = -EIO;
+ break;
}
- if (cmd->hdr.return_code)
- QETH_DBF_MESSAGE(2, "Could not set group MAC %pM on %s: %x\n",
- mac, QETH_CARD_IFNAME(card), cmd->hdr.return_code);
- return 0;
+ return rc;
}
static int qeth_l2_send_setgroupmac(struct qeth_card *card, __u8 *mac)
{
- QETH_CARD_TEXT(card, 2, "L2Sgmac");
- return qeth_l2_send_setdelmac(card, mac, IPA_CMD_SETGMAC,
- qeth_l2_send_setgroupmac_cb);
-}
-
-static int qeth_l2_send_delgroupmac_cb(struct qeth_card *card,
- struct qeth_reply *reply,
- unsigned long data)
-{
- struct qeth_ipa_cmd *cmd;
- __u8 *mac;
+ int rc;
- QETH_CARD_TEXT(card, 2, "L2Dgmacb");
- cmd = (struct qeth_ipa_cmd *) data;
- mac = &cmd->data.setdelmac.mac[0];
- if (cmd->hdr.return_code)
- QETH_DBF_MESSAGE(2, "Could not delete group MAC %pM on %s: %x\n",
- mac, QETH_CARD_IFNAME(card), cmd->hdr.return_code);
- return 0;
+ QETH_CARD_TEXT(card, 2, "L2Sgmac");
+ rc = qeth_setdel_makerc(card, qeth_l2_send_setdelmac(card, mac,
+ IPA_CMD_SETGMAC));
+ if (rc == -EEXIST)
+ QETH_DBF_MESSAGE(2, "Group MAC %pM already existing on %s\n",
+ mac, QETH_CARD_IFNAME(card));
+ else if (rc)
+ QETH_DBF_MESSAGE(2, "Could not set group MAC %pM on %s: %d\n",
+ mac, QETH_CARD_IFNAME(card), rc);
+ return rc;
}
static int qeth_l2_send_delgroupmac(struct qeth_card *card, __u8 *mac)
{
+ int rc;
+
QETH_CARD_TEXT(card, 2, "L2Dgmac");
- return qeth_l2_send_setdelmac(card, mac, IPA_CMD_DELGMAC,
- qeth_l2_send_delgroupmac_cb);
+ rc = qeth_setdel_makerc(card, qeth_l2_send_setdelmac(card, mac,
+ IPA_CMD_DELGMAC));
+ if (rc)
+ QETH_DBF_MESSAGE(2,
+ "Could not delete group MAC %pM on %s: %d\n",
+ mac, QETH_CARD_IFNAME(card), rc);
+ return rc;
}
static void qeth_l2_add_mc(struct qeth_card *card, __u8 *mac, int vmac)
@@ -197,10 +206,11 @@ static void qeth_l2_add_mc(struct qeth_card *card, __u8 *mac, int vmac)
mc->is_vmac = vmac;
if (vmac) {
- rc = qeth_l2_send_setdelmac(card, mac, IPA_CMD_SETVMAC,
- NULL);
+ rc = qeth_setdel_makerc(card,
+ qeth_l2_send_setdelmac(card, mac, IPA_CMD_SETVMAC));
} else {
- rc = qeth_l2_send_setgroupmac(card, mac);
+ rc = qeth_setdel_makerc(card,
+ qeth_l2_send_setgroupmac(card, mac));
}
if (!rc)
@@ -218,7 +228,7 @@ static void qeth_l2_del_all_mc(struct qeth_card *card, int del)
if (del) {
if (mc->is_vmac)
qeth_l2_send_setdelmac(card, mc->mc_addr,
- IPA_CMD_DELVMAC, NULL);
+ IPA_CMD_DELVMAC);
else
qeth_l2_send_delgroupmac(card, mc->mc_addr);
}
@@ -539,10 +549,7 @@ out:
}
static int qeth_l2_send_setdelmac(struct qeth_card *card, __u8 *mac,
- enum qeth_ipa_cmds ipacmd,
- int (*reply_cb) (struct qeth_card *,
- struct qeth_reply*,
- unsigned long))
+ enum qeth_ipa_cmds ipacmd)
{
struct qeth_ipa_cmd *cmd;
struct qeth_cmd_buffer *iob;
@@ -552,78 +559,50 @@ static int qeth_l2_send_setdelmac(struct qeth_card *card, __u8 *mac,
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setdelmac.mac_length = OSA_ADDR_LEN;
memcpy(&cmd->data.setdelmac.mac, mac, OSA_ADDR_LEN);
- return qeth_send_ipa_cmd(card, iob, reply_cb, NULL);
+ return qeth_send_ipa_cmd(card, iob, NULL, NULL);
}
-static int qeth_l2_send_setmac_cb(struct qeth_card *card,
- struct qeth_reply *reply,
- unsigned long data)
+static int qeth_l2_send_setmac(struct qeth_card *card, __u8 *mac)
{
- struct qeth_ipa_cmd *cmd;
+ int rc;
- QETH_CARD_TEXT(card, 2, "L2Smaccb");
- cmd = (struct qeth_ipa_cmd *) data;
- if (cmd->hdr.return_code) {
- QETH_CARD_TEXT_(card, 2, "L2er%x", cmd->hdr.return_code);
+ QETH_CARD_TEXT(card, 2, "L2Setmac");
+ rc = qeth_setdel_makerc(card, qeth_l2_send_setdelmac(card, mac,
+ IPA_CMD_SETVMAC));
+ if (rc == 0) {
+ card->info.mac_bits |= QETH_LAYER2_MAC_REGISTERED;
+ memcpy(card->dev->dev_addr, mac, OSA_ADDR_LEN);
+ dev_info(&card->gdev->dev,
+ "MAC address %pM successfully registered on device %s\n",
+ card->dev->dev_addr, card->dev->name);
+ } else {
card->info.mac_bits &= ~QETH_LAYER2_MAC_REGISTERED;
- switch (cmd->hdr.return_code) {
- case IPA_RC_L2_DUP_MAC:
- case IPA_RC_L2_DUP_LAYER3_MAC:
+ switch (rc) {
+ case -EEXIST:
dev_warn(&card->gdev->dev,
- "MAC address %pM already exists\n",
- cmd->data.setdelmac.mac);
+ "MAC address %pM already exists\n", mac);
break;
- case IPA_RC_L2_MAC_NOT_AUTH_BY_HYP:
- case IPA_RC_L2_MAC_NOT_AUTH_BY_ADP:
+ case -EPERM:
dev_warn(&card->gdev->dev,
- "MAC address %pM is not authorized\n",
- cmd->data.setdelmac.mac);
- break;
- default:
+ "MAC address %pM is not authorized\n", mac);
break;
}
- } else {
- card->info.mac_bits |= QETH_LAYER2_MAC_REGISTERED;
- memcpy(card->dev->dev_addr, cmd->data.setdelmac.mac,
- OSA_ADDR_LEN);
- dev_info(&card->gdev->dev,
- "MAC address %pM successfully registered on device %s\n",
- card->dev->dev_addr, card->dev->name);
}
- return 0;
-}
-
-static int qeth_l2_send_setmac(struct qeth_card *card, __u8 *mac)
-{
- QETH_CARD_TEXT(card, 2, "L2Setmac");
- return qeth_l2_send_setdelmac(card, mac, IPA_CMD_SETVMAC,
- qeth_l2_send_setmac_cb);
-}
-
-static int qeth_l2_send_delmac_cb(struct qeth_card *card,
- struct qeth_reply *reply,
- unsigned long data)
-{
- struct qeth_ipa_cmd *cmd;
-
- QETH_CARD_TEXT(card, 2, "L2Dmaccb");
- cmd = (struct qeth_ipa_cmd *) data;
- if (cmd->hdr.return_code) {
- QETH_CARD_TEXT_(card, 2, "err%d", cmd->hdr.return_code);
- return 0;
- }
- card->info.mac_bits &= ~QETH_LAYER2_MAC_REGISTERED;
-
- return 0;
+ return rc;
}
static int qeth_l2_send_delmac(struct qeth_card *card, __u8 *mac)
{
+ int rc;
+
QETH_CARD_TEXT(card, 2, "L2Delmac");
if (!(card->info.mac_bits & QETH_LAYER2_MAC_REGISTERED))
return 0;
- return qeth_l2_send_setdelmac(card, mac, IPA_CMD_DELVMAC,
- qeth_l2_send_delmac_cb);
+ rc = qeth_setdel_makerc(card, qeth_l2_send_setdelmac(card, mac,
+ IPA_CMD_DELVMAC));
+ if (rc == 0)
+ card->info.mac_bits &= ~QETH_LAYER2_MAC_REGISTERED;
+ return rc;
}
static int qeth_l2_request_initial_mac(struct qeth_card *card)
@@ -687,7 +666,7 @@ static int qeth_l2_set_mac_address(struct net_device *dev, void *p)
return -ERESTARTSYS;
}
rc = qeth_l2_send_delmac(card, &card->dev->dev_addr[0]);
- if (!rc || (rc == IPA_RC_L2_MAC_NOT_FOUND))
+ if (!rc || (rc == -ENOENT))
rc = qeth_l2_send_setmac(card, addr->sa_data);
return rc ? -EINVAL : 0;
}
--
2.1.4
^ permalink raw reply related
* [PATCH net 2/2] 390/qeth: Fix locking warning during qeth device setup
From: Ursula Braun @ 2015-01-21 12:39 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, ursula.braun, ubraun, Thomas Richter
In-Reply-To: <1421843950-58220-1-git-send-email-ubraun@linux.vnet.ibm.com>
From: Thomas Richter <tmricht@linux.vnet.ibm.com>
Do not wait for channel command buffers in IPA commands.
The potential wait could be done while holding a spin lock and causes
in recent kernels such a bug if kernel lock debugging is enabled:
kernel: BUG: sleeping function called from invalid context at drivers/s390/net/qeth_core_main.c:
794
kernel: in_atomic(): 1, irqs_disabled(): 0, pid: 2031, name: NetworkManager
kernel: 2 locks held by NetworkManager/2031:
kernel: #0: (rtnl_mutex){+.+.+.}, at: [<00000000006e0d7a>] rtnetlink_rcv+0x32/0x50
kernel: #1: (_xmit_ETHER){+.....}, at: [<00000000006cfe90>] dev_set_rx_mode+0x30/0x50
kernel: CPU: 0 PID: 2031 Comm: NetworkManager Not tainted 3.18.0-rc5-next-20141124 #1
kernel: 00000000275fb1f0 00000000275fb280 0000000000000002 0000000000000000
00000000275fb320 00000000275fb298 00000000275fb298 00000000007e326a
0000000000000000 000000000099ce2c 00000000009b4988 000000000000000b
00000000275fb2e0 00000000275fb280 0000000000000000 0000000000000000
0000000000000000 00000000001129c8 00000000275fb280 00000000275fb2e0
kernel: Call Trace:
kernel: ([<00000000001128b0>] show_trace+0xf8/0x158)
kernel: [<000000000011297a>] show_stack+0x6a/0xe8
kernel: [<00000000007e995a>] dump_stack+0x82/0xb0
kernel: [<000000000017d668>] ___might_sleep+0x170/0x228
kernel: [<000003ff80026f0e>] qeth_wait_for_buffer+0x36/0xd0 [qeth]
kernel: [<000003ff80026fe2>] qeth_get_ipacmd_buffer+0x3a/0xc0 [qeth]
kernel: [<000003ff80105078>] qeth_l3_send_setdelmc+0x58/0xf8 [qeth_l3]
kernel: [<000003ff8010b1fe>] qeth_l3_set_ip_addr_list+0x2c6/0x848 [qeth_l3]
kernel: [<000003ff8010bbb4>] qeth_l3_set_multicast_list+0x434/0xc48 [qeth_l3]
kernel: [<00000000006cfe9a>] dev_set_rx_mode+0x3a/0x50
kernel: [<00000000006cff90>] __dev_open+0xe0/0x140
kernel: [<00000000006d02a0>] __dev_change_flags+0xa0/0x178
kernel: [<00000000006d03a8>] dev_change_flags+0x30/0x70
kernel: [<00000000006e14ee>] do_setlink+0x346/0x9a0
...
The device driver has plenty of command buffers available
per channel for channel command communication.
In the extremely rare case when there is no command buffer
available, return a NULL pointer and issue a warning
in the kernel log. The caller handles the case when
a NULL pointer is encountered and returns an error.
In the case the wait for command buffer is possible
(because no lock is held as in the OSN case), still wait
until a channel command buffer is available.
Signed-off-by: Thomas Richter <tmricht@linux.vnet.ibm.com>
Signed-off-by: Ursula Braun <ursula.braun@de.ibm.com>
Reviewed-by: Eugene Crosser <Eugene.Crosser@ru.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 81 ++++++++++++++++++++++++++++++++-------
drivers/s390/net/qeth_l2_main.c | 35 ++++++++++++-----
drivers/s390/net/qeth_l3_main.c | 50 ++++++++++++++++++------
3 files changed, 132 insertions(+), 34 deletions(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index e2fbfff..642c77c 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -1784,6 +1784,8 @@ static int qeth_idx_activate_get_answer(struct qeth_channel *channel,
QETH_DBF_TEXT(SETUP, 2, "idxanswr");
card = CARD_FROM_CDEV(channel->ccwdev);
iob = qeth_get_buffer(channel);
+ if (!iob)
+ return -ENOMEM;
iob->callback = idx_reply_cb;
memcpy(&channel->ccw, READ_CCW, sizeof(struct ccw1));
channel->ccw.count = QETH_BUFSIZE;
@@ -1834,6 +1836,8 @@ static int qeth_idx_activate_channel(struct qeth_channel *channel,
QETH_DBF_TEXT(SETUP, 2, "idxactch");
iob = qeth_get_buffer(channel);
+ if (!iob)
+ return -ENOMEM;
iob->callback = idx_reply_cb;
memcpy(&channel->ccw, WRITE_CCW, sizeof(struct ccw1));
channel->ccw.count = IDX_ACTIVATE_SIZE;
@@ -2940,9 +2944,16 @@ struct qeth_cmd_buffer *qeth_get_ipacmd_buffer(struct qeth_card *card,
struct qeth_cmd_buffer *iob;
struct qeth_ipa_cmd *cmd;
- iob = qeth_wait_for_buffer(&card->write);
- cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
- qeth_fill_ipacmd_header(card, cmd, ipacmd, prot);
+ iob = qeth_get_buffer(&card->write);
+ if (iob) {
+ cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
+ qeth_fill_ipacmd_header(card, cmd, ipacmd, prot);
+ } else {
+ dev_warn(&card->gdev->dev,
+ "The qeth driver ran out of channel command buffers\n");
+ QETH_DBF_MESSAGE(1, "%s The qeth driver ran out of channel command buffers",
+ dev_name(&card->gdev->dev));
+ }
return iob;
}
@@ -3000,6 +3011,8 @@ int qeth_send_startlan(struct qeth_card *card)
QETH_DBF_TEXT(SETUP, 2, "strtlan");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_STARTLAN, 0);
+ if (!iob)
+ return -ENOMEM;
rc = qeth_send_ipa_cmd(card, iob, NULL, NULL);
return rc;
}
@@ -3045,11 +3058,13 @@ static struct qeth_cmd_buffer *qeth_get_adapter_cmd(struct qeth_card *card,
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SETADAPTERPARMS,
QETH_PROT_IPV4);
- cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
- cmd->data.setadapterparms.hdr.cmdlength = cmdlen;
- cmd->data.setadapterparms.hdr.command_code = command;
- cmd->data.setadapterparms.hdr.used_total = 1;
- cmd->data.setadapterparms.hdr.seq_no = 1;
+ if (iob) {
+ cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
+ cmd->data.setadapterparms.hdr.cmdlength = cmdlen;
+ cmd->data.setadapterparms.hdr.command_code = command;
+ cmd->data.setadapterparms.hdr.used_total = 1;
+ cmd->data.setadapterparms.hdr.seq_no = 1;
+ }
return iob;
}
@@ -3062,6 +3077,8 @@ int qeth_query_setadapterparms(struct qeth_card *card)
QETH_CARD_TEXT(card, 3, "queryadp");
iob = qeth_get_adapter_cmd(card, IPA_SETADP_QUERY_COMMANDS_SUPPORTED,
sizeof(struct qeth_ipacmd_setadpparms));
+ if (!iob)
+ return -ENOMEM;
rc = qeth_send_ipa_cmd(card, iob, qeth_query_setadapterparms_cb, NULL);
return rc;
}
@@ -3112,6 +3129,8 @@ int qeth_query_ipassists(struct qeth_card *card, enum qeth_prot_versions prot)
QETH_DBF_TEXT_(SETUP, 2, "qipassi%i", prot);
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_QIPASSIST, prot);
+ if (!iob)
+ return -ENOMEM;
rc = qeth_send_ipa_cmd(card, iob, qeth_query_ipassists_cb, NULL);
return rc;
}
@@ -3151,6 +3170,8 @@ int qeth_query_switch_attributes(struct qeth_card *card,
return -ENOMEDIUM;
iob = qeth_get_adapter_cmd(card, IPA_SETADP_QUERY_SWITCH_ATTRIBUTES,
sizeof(struct qeth_ipacmd_setadpparms_hdr));
+ if (!iob)
+ return -ENOMEM;
return qeth_send_ipa_cmd(card, iob,
qeth_query_switch_attributes_cb, sw_info);
}
@@ -3178,6 +3199,8 @@ static int qeth_query_setdiagass(struct qeth_card *card)
QETH_DBF_TEXT(SETUP, 2, "qdiagass");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SET_DIAG_ASS, 0);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.diagass.subcmd_len = 16;
cmd->data.diagass.subcmd = QETH_DIAGS_CMD_QUERY;
@@ -3229,6 +3252,8 @@ int qeth_hw_trap(struct qeth_card *card, enum qeth_diags_trap_action action)
QETH_DBF_TEXT(SETUP, 2, "diagtrap");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SET_DIAG_ASS, 0);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.diagass.subcmd_len = 80;
cmd->data.diagass.subcmd = QETH_DIAGS_CMD_TRAP;
@@ -4194,6 +4219,8 @@ void qeth_setadp_promisc_mode(struct qeth_card *card)
iob = qeth_get_adapter_cmd(card, IPA_SETADP_SET_PROMISC_MODE,
sizeof(struct qeth_ipacmd_setadpparms));
+ if (!iob)
+ return;
cmd = (struct qeth_ipa_cmd *)(iob->data + IPA_PDU_HEADER_SIZE);
cmd->data.setadapterparms.data.mode = mode;
qeth_send_ipa_cmd(card, iob, qeth_setadp_promisc_mode_cb, NULL);
@@ -4264,6 +4291,8 @@ int qeth_setadpparms_change_macaddr(struct qeth_card *card)
iob = qeth_get_adapter_cmd(card, IPA_SETADP_ALTER_MAC_ADDRESS,
sizeof(struct qeth_ipacmd_setadpparms));
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setadapterparms.data.change_addr.cmd = CHANGE_ADDR_READ_MAC;
cmd->data.setadapterparms.data.change_addr.addr_size = OSA_ADDR_LEN;
@@ -4377,6 +4406,8 @@ static int qeth_setadpparms_set_access_ctrl(struct qeth_card *card,
iob = qeth_get_adapter_cmd(card, IPA_SETADP_SET_ACCESS_CONTROL,
sizeof(struct qeth_ipacmd_setadpparms_hdr) +
sizeof(struct qeth_set_access_ctrl));
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
access_ctrl_req = &cmd->data.setadapterparms.data.set_access_ctrl;
access_ctrl_req->subcmd_code = isolation;
@@ -4620,6 +4651,10 @@ int qeth_snmp_command(struct qeth_card *card, char __user *udata)
iob = qeth_get_adapter_cmd(card, IPA_SETADP_SET_SNMP_CONTROL,
QETH_SNMP_SETADP_CMDLENGTH + req_len);
+ if (!iob) {
+ rc = -ENOMEM;
+ goto out;
+ }
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
memcpy(&cmd->data.setadapterparms.data.snmp, &ureq->cmd, req_len);
rc = qeth_send_ipa_snmp_cmd(card, iob, QETH_SETADP_BASE_LEN + req_len,
@@ -4631,7 +4666,7 @@ int qeth_snmp_command(struct qeth_card *card, char __user *udata)
if (copy_to_user(udata, qinfo.udata, qinfo.udata_len))
rc = -EFAULT;
}
-
+out:
kfree(ureq);
kfree(qinfo.udata);
return rc;
@@ -4702,6 +4737,10 @@ int qeth_query_oat_command(struct qeth_card *card, char __user *udata)
iob = qeth_get_adapter_cmd(card, IPA_SETADP_QUERY_OAT,
sizeof(struct qeth_ipacmd_setadpparms_hdr) +
sizeof(struct qeth_query_oat));
+ if (!iob) {
+ rc = -ENOMEM;
+ goto out_free;
+ }
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
oat_req = &cmd->data.setadapterparms.data.query_oat;
oat_req->subcmd_code = oat_data.command;
@@ -4767,6 +4806,8 @@ static int qeth_query_card_info(struct qeth_card *card,
return -EOPNOTSUPP;
iob = qeth_get_adapter_cmd(card, IPA_SETADP_QUERY_CARD_INFO,
sizeof(struct qeth_ipacmd_setadpparms_hdr));
+ if (!iob)
+ return -ENOMEM;
return qeth_send_ipa_cmd(card, iob, qeth_query_card_info_cb,
(void *)carrier_info);
}
@@ -5092,11 +5133,23 @@ retriable:
card->options.adp.supported_funcs = 0;
card->options.sbp.supported_funcs = 0;
card->info.diagass_support = 0;
- qeth_query_ipassists(card, QETH_PROT_IPV4);
- if (qeth_is_supported(card, IPA_SETADAPTERPARMS))
- qeth_query_setadapterparms(card);
- if (qeth_adp_supported(card, IPA_SETADP_SET_DIAG_ASSIST))
- qeth_query_setdiagass(card);
+ rc = qeth_query_ipassists(card, QETH_PROT_IPV4);
+ if (rc == -ENOMEM)
+ goto out;
+ if (qeth_is_supported(card, IPA_SETADAPTERPARMS)) {
+ rc = qeth_query_setadapterparms(card);
+ if (rc < 0) {
+ QETH_DBF_TEXT_(SETUP, 2, "6err%d", rc);
+ goto out;
+ }
+ }
+ if (qeth_adp_supported(card, IPA_SETADP_SET_DIAG_ASSIST)) {
+ rc = qeth_query_setdiagass(card);
+ if (rc < 0) {
+ QETH_DBF_TEXT_(SETUP, 2, "7err%d", rc);
+ goto out;
+ }
+ }
return 0;
out:
dev_warn(&card->gdev->dev, "The qeth device driver failed to recover "
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index b7b9d5c..ce87ae7 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -132,7 +132,7 @@ static int qeth_setdel_makerc(struct qeth_card *card, int retcode)
int rc;
if (retcode)
- QETH_CARD_TEXT_(card, 2, "err%d", retcode);
+ QETH_CARD_TEXT_(card, 2, "err%04x", retcode);
switch (retcode) {
case IPA_RC_SUCCESS:
rc = 0;
@@ -154,6 +154,9 @@ static int qeth_setdel_makerc(struct qeth_card *card, int retcode)
case IPA_RC_L2_MAC_NOT_FOUND:
rc = -ENOENT;
break;
+ case -ENOMEM:
+ rc = -ENOMEM;
+ break;
default:
rc = -EIO;
break;
@@ -301,6 +304,8 @@ static int qeth_l2_send_setdelvlan(struct qeth_card *card, __u16 i,
QETH_CARD_TEXT_(card, 4, "L2sdv%x", ipacmd);
iob = qeth_get_ipacmd_buffer(card, ipacmd, QETH_PROT_IPV4);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setdelvlan.vlan_id = i;
return qeth_send_ipa_cmd(card, iob,
@@ -323,6 +328,7 @@ static int qeth_l2_vlan_rx_add_vid(struct net_device *dev,
{
struct qeth_card *card = dev->ml_priv;
struct qeth_vlan_vid *id;
+ int rc;
QETH_CARD_TEXT_(card, 4, "aid:%d", vid);
if (!vid)
@@ -338,7 +344,11 @@ static int qeth_l2_vlan_rx_add_vid(struct net_device *dev,
id = kmalloc(sizeof(struct qeth_vlan_vid), GFP_ATOMIC);
if (id) {
id->vid = vid;
- qeth_l2_send_setdelvlan(card, vid, IPA_CMD_SETVLAN);
+ rc = qeth_l2_send_setdelvlan(card, vid, IPA_CMD_SETVLAN);
+ if (rc) {
+ kfree(id);
+ return rc;
+ }
spin_lock_bh(&card->vlanlock);
list_add_tail(&id->list, &card->vid_list);
spin_unlock_bh(&card->vlanlock);
@@ -353,6 +363,7 @@ static int qeth_l2_vlan_rx_kill_vid(struct net_device *dev,
{
struct qeth_vlan_vid *id, *tmpid = NULL;
struct qeth_card *card = dev->ml_priv;
+ int rc = 0;
QETH_CARD_TEXT_(card, 4, "kid:%d", vid);
if (card->info.type == QETH_CARD_TYPE_OSM) {
@@ -373,11 +384,11 @@ static int qeth_l2_vlan_rx_kill_vid(struct net_device *dev,
}
spin_unlock_bh(&card->vlanlock);
if (tmpid) {
- qeth_l2_send_setdelvlan(card, vid, IPA_CMD_DELVLAN);
+ rc = qeth_l2_send_setdelvlan(card, vid, IPA_CMD_DELVLAN);
kfree(tmpid);
}
qeth_l2_set_multicast_list(card->dev);
- return 0;
+ return rc;
}
static int qeth_l2_stop_card(struct qeth_card *card, int recovery_mode)
@@ -556,6 +567,8 @@ static int qeth_l2_send_setdelmac(struct qeth_card *card, __u8 *mac,
QETH_CARD_TEXT(card, 2, "L2sdmac");
iob = qeth_get_ipacmd_buffer(card, ipacmd, QETH_PROT_IPV4);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setdelmac.mac_length = OSA_ADDR_LEN;
memcpy(&cmd->data.setdelmac.mac, mac, OSA_ADDR_LEN);
@@ -630,7 +643,7 @@ static int qeth_l2_request_initial_mac(struct qeth_card *card)
if (rc) {
QETH_DBF_MESSAGE(2, "couldn't get MAC address on "
"device %s: x%x\n", CARD_BUS_ID(card), rc);
- QETH_DBF_TEXT_(SETUP, 2, "1err%d", rc);
+ QETH_DBF_TEXT_(SETUP, 2, "1err%04x", rc);
return rc;
}
QETH_DBF_HEX(SETUP, 2, card->dev->dev_addr, OSA_ADDR_LEN);
@@ -975,7 +988,7 @@ static int __qeth_l2_set_online(struct ccwgroup_device *gdev, int recovery_mode)
recover_flag = card->state;
rc = qeth_core_hardsetup_card(card);
if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
+ QETH_DBF_TEXT_(SETUP, 2, "2err%04x", rc);
rc = -ENODEV;
goto out_remove;
}
@@ -1709,6 +1722,8 @@ static void qeth_bridgeport_query_support(struct qeth_card *card)
QETH_CARD_TEXT(card, 2, "brqsuppo");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SETBRIDGEPORT, 0);
+ if (!iob)
+ return;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.sbp.hdr.cmdlength =
sizeof(struct qeth_ipacmd_sbp_hdr) +
@@ -1784,6 +1799,8 @@ int qeth_bridgeport_query_ports(struct qeth_card *card,
if (!(card->options.sbp.supported_funcs & IPA_SBP_QUERY_BRIDGE_PORTS))
return -EOPNOTSUPP;
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SETBRIDGEPORT, 0);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.sbp.hdr.cmdlength =
sizeof(struct qeth_ipacmd_sbp_hdr);
@@ -1796,9 +1813,7 @@ int qeth_bridgeport_query_ports(struct qeth_card *card,
if (rc)
return rc;
rc = qeth_bridgeport_makerc(card, &cbctl, IPA_SBP_QUERY_BRIDGE_PORTS);
- if (rc)
- return rc;
- return 0;
+ return rc;
}
EXPORT_SYMBOL_GPL(qeth_bridgeport_query_ports);
@@ -1852,6 +1867,8 @@ int qeth_bridgeport_setrole(struct qeth_card *card, enum qeth_sbp_roles role)
if (!(card->options.sbp.supported_funcs & setcmd))
return -EOPNOTSUPP;
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SETBRIDGEPORT, 0);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.sbp.hdr.cmdlength = cmdlength;
cmd->data.sbp.hdr.command_code = setcmd;
diff --git a/drivers/s390/net/qeth_l3_main.c b/drivers/s390/net/qeth_l3_main.c
index 625227ad..e2a0ee8 100644
--- a/drivers/s390/net/qeth_l3_main.c
+++ b/drivers/s390/net/qeth_l3_main.c
@@ -549,6 +549,8 @@ static int qeth_l3_send_setdelmc(struct qeth_card *card,
QETH_CARD_TEXT(card, 4, "setdelmc");
iob = qeth_get_ipacmd_buffer(card, ipacmd, addr->proto);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
memcpy(&cmd->data.setdelipm.mac, addr->mac, OSA_ADDR_LEN);
if (addr->proto == QETH_PROT_IPV6)
@@ -588,6 +590,8 @@ static int qeth_l3_send_setdelip(struct qeth_card *card,
QETH_CARD_TEXT_(card, 4, "flags%02X", flags);
iob = qeth_get_ipacmd_buffer(card, ipacmd, addr->proto);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
if (addr->proto == QETH_PROT_IPV6) {
memcpy(cmd->data.setdelip6.ip_addr, &addr->u.a6.addr,
@@ -616,6 +620,8 @@ static int qeth_l3_send_setrouting(struct qeth_card *card,
QETH_CARD_TEXT(card, 4, "setroutg");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SETRTG, prot);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setrtg.type = (type);
rc = qeth_send_ipa_cmd(card, iob, NULL, NULL);
@@ -1049,12 +1055,14 @@ static struct qeth_cmd_buffer *qeth_l3_get_setassparms_cmd(
QETH_CARD_TEXT(card, 4, "getasscm");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SETASSPARMS, prot);
- cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
- cmd->data.setassparms.hdr.assist_no = ipa_func;
- cmd->data.setassparms.hdr.length = 8 + len;
- cmd->data.setassparms.hdr.command_code = cmd_code;
- cmd->data.setassparms.hdr.return_code = 0;
- cmd->data.setassparms.hdr.seq_no = 0;
+ if (iob) {
+ cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
+ cmd->data.setassparms.hdr.assist_no = ipa_func;
+ cmd->data.setassparms.hdr.length = 8 + len;
+ cmd->data.setassparms.hdr.command_code = cmd_code;
+ cmd->data.setassparms.hdr.return_code = 0;
+ cmd->data.setassparms.hdr.seq_no = 0;
+ }
return iob;
}
@@ -1090,6 +1098,8 @@ static int qeth_l3_send_simple_setassparms_ipv6(struct qeth_card *card,
QETH_CARD_TEXT(card, 4, "simassp6");
iob = qeth_l3_get_setassparms_cmd(card, ipa_func, cmd_code,
0, QETH_PROT_IPV6);
+ if (!iob)
+ return -ENOMEM;
rc = qeth_l3_send_setassparms(card, iob, 0, 0,
qeth_l3_default_setassparms_cb, NULL);
return rc;
@@ -1108,6 +1118,8 @@ static int qeth_l3_send_simple_setassparms(struct qeth_card *card,
length = sizeof(__u32);
iob = qeth_l3_get_setassparms_cmd(card, ipa_func, cmd_code,
length, QETH_PROT_IPV4);
+ if (!iob)
+ return -ENOMEM;
rc = qeth_l3_send_setassparms(card, iob, length, data,
qeth_l3_default_setassparms_cb, NULL);
return rc;
@@ -1494,6 +1506,8 @@ static int qeth_l3_iqd_read_initial_mac(struct qeth_card *card)
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_CREATE_ADDR,
QETH_PROT_IPV6);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
*((__u16 *) &cmd->data.create_destroy_addr.unique_id[6]) =
card->info.unique_id;
@@ -1537,6 +1551,8 @@ static int qeth_l3_get_unique_id(struct qeth_card *card)
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_CREATE_ADDR,
QETH_PROT_IPV6);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
*((__u16 *) &cmd->data.create_destroy_addr.unique_id[6]) =
card->info.unique_id;
@@ -1611,6 +1627,8 @@ qeth_diags_trace(struct qeth_card *card, enum qeth_diags_trace_cmds diags_cmd)
QETH_DBF_TEXT(SETUP, 2, "diagtrac");
iob = qeth_get_ipacmd_buffer(card, IPA_CMD_SET_DIAG_ASS, 0);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.diagass.subcmd_len = 16;
cmd->data.diagass.subcmd = QETH_DIAGS_CMD_TRACE;
@@ -2442,6 +2460,8 @@ static int qeth_l3_query_arp_cache_info(struct qeth_card *card,
IPA_CMD_ASS_ARP_QUERY_INFO,
sizeof(struct qeth_arp_query_data) - sizeof(char),
prot);
+ if (!iob)
+ return -ENOMEM;
cmd = (struct qeth_ipa_cmd *)(iob->data+IPA_PDU_HEADER_SIZE);
cmd->data.setassparms.data.query_arp.request_bits = 0x000F;
cmd->data.setassparms.data.query_arp.reply_bits = 0;
@@ -2535,6 +2555,8 @@ static int qeth_l3_arp_add_entry(struct qeth_card *card,
IPA_CMD_ASS_ARP_ADD_ENTRY,
sizeof(struct qeth_arp_cache_entry),
QETH_PROT_IPV4);
+ if (!iob)
+ return -ENOMEM;
rc = qeth_l3_send_setassparms(card, iob,
sizeof(struct qeth_arp_cache_entry),
(unsigned long) entry,
@@ -2574,6 +2596,8 @@ static int qeth_l3_arp_remove_entry(struct qeth_card *card,
IPA_CMD_ASS_ARP_REMOVE_ENTRY,
12,
QETH_PROT_IPV4);
+ if (!iob)
+ return -ENOMEM;
rc = qeth_l3_send_setassparms(card, iob,
12, (unsigned long)buf,
qeth_l3_default_setassparms_cb, NULL);
@@ -3262,6 +3286,8 @@ static const struct net_device_ops qeth_l3_osa_netdev_ops = {
static int qeth_l3_setup_netdev(struct qeth_card *card)
{
+ int rc;
+
if (card->info.type == QETH_CARD_TYPE_OSD ||
card->info.type == QETH_CARD_TYPE_OSX) {
if ((card->info.link_type == QETH_LINK_TYPE_LANE_TR) ||
@@ -3293,7 +3319,9 @@ static int qeth_l3_setup_netdev(struct qeth_card *card)
return -ENODEV;
card->dev->flags |= IFF_NOARP;
card->dev->netdev_ops = &qeth_l3_netdev_ops;
- qeth_l3_iqd_read_initial_mac(card);
+ rc = qeth_l3_iqd_read_initial_mac(card);
+ if (rc)
+ return rc;
if (card->options.hsuid[0])
memcpy(card->dev->perm_addr, card->options.hsuid, 9);
} else
@@ -3360,7 +3388,7 @@ static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
recover_flag = card->state;
rc = qeth_core_hardsetup_card(card);
if (rc) {
- QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
+ QETH_DBF_TEXT_(SETUP, 2, "2err%04x", rc);
rc = -ENODEV;
goto out_remove;
}
@@ -3401,7 +3429,7 @@ static int __qeth_l3_set_online(struct ccwgroup_device *gdev, int recovery_mode)
contin:
rc = qeth_l3_setadapter_parms(card);
if (rc)
- QETH_DBF_TEXT_(SETUP, 2, "2err%d", rc);
+ QETH_DBF_TEXT_(SETUP, 2, "2err%04x", rc);
if (!card->options.sniffer) {
rc = qeth_l3_start_ipassists(card);
if (rc) {
@@ -3410,10 +3438,10 @@ contin:
}
rc = qeth_l3_setrouting_v4(card);
if (rc)
- QETH_DBF_TEXT_(SETUP, 2, "4err%d", rc);
+ QETH_DBF_TEXT_(SETUP, 2, "4err%04x", rc);
rc = qeth_l3_setrouting_v6(card);
if (rc)
- QETH_DBF_TEXT_(SETUP, 2, "5err%d", rc);
+ QETH_DBF_TEXT_(SETUP, 2, "5err%04x", rc);
}
netif_tx_disable(card->dev);
--
2.1.4
^ permalink raw reply related
* [PATCH net v2] ipv4: try to cache dst_entries which would cause a redirect
From: Hannes Frederic Sowa @ 2015-01-21 12:41 UTC (permalink / raw)
To: netdev; +Cc: Julian Anastasov, Marcelo Leitner, Florian Westphal
Not caching dst_entries which cause redirects could be exploited by hosts
on the same subnet, causing a severe DoS attack. This effect aggravated
since commit f88649721268999 ("ipv4: fix dst race in sk_dst_get()").
Lookups causing redirects will be allocated with DST_NOCACHE set which
will force dst_release to free them via RCU. Unfortunately waiting for
RCU grace period just takes too long, we can end up with >1M dst_entries
waiting to be released and the system will run OOM. rcuos threads cannot
catch up under high softirq load.
Attaching the flag to emit a redirect later on to the specific skb allows
us to cache those dst_entries thus reducing the pressure on allocation
and deallocation.
This issue was discovered by Marcelo Leitner.
Cc: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Marcelo Leitner <mleitner@redhat.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
v2:
Julian noticed that v1 did omit the redirect flag in rtnetlink queries,
fixed. Thanks!
include/net/ip.h | 11 ++++++-----
net/ipv4/ip_forward.c | 3 ++-
net/ipv4/route.c | 8 ++++----
3 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 0e5a0ba..14211ea 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -39,11 +39,12 @@ struct inet_skb_parm {
struct ip_options opt; /* Compiled IP options */
unsigned char flags;
-#define IPSKB_FORWARDED 1
-#define IPSKB_XFRM_TUNNEL_SIZE 2
-#define IPSKB_XFRM_TRANSFORMED 4
-#define IPSKB_FRAG_COMPLETE 8
-#define IPSKB_REROUTED 16
+#define IPSKB_FORWARDED BIT(0)
+#define IPSKB_XFRM_TUNNEL_SIZE BIT(1)
+#define IPSKB_XFRM_TRANSFORMED BIT(2)
+#define IPSKB_FRAG_COMPLETE BIT(3)
+#define IPSKB_REROUTED BIT(4)
+#define IPSKB_DOREDIRECT BIT(5)
u16 frag_max_size;
};
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index 3a83ce5..787b3c2 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -129,7 +129,8 @@ int ip_forward(struct sk_buff *skb)
* We now generate an ICMP HOST REDIRECT giving the route
* we calculated.
*/
- if (rt->rt_flags&RTCF_DOREDIRECT && !opt->srr && !skb_sec_path(skb))
+ if (IPCB(skb)->flags & IPSKB_DOREDIRECT && !opt->srr &&
+ !skb_sec_path(skb))
ip_rt_send_redirect(skb);
skb->priority = rt_tos2priority(iph->tos);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 2000110..868c829 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1568,10 +1568,8 @@ static int __mkroute_input(struct sk_buff *skb,
do_cache = res->fi && !itag;
if (out_dev == in_dev && err && IN_DEV_TX_REDIRECTS(out_dev) &&
(IN_DEV_SHARED_MEDIA(out_dev) ||
- inet_addr_onlink(out_dev, saddr, FIB_RES_GW(*res)))) {
- flags |= RTCF_DOREDIRECT;
- do_cache = false;
- }
+ inet_addr_onlink(out_dev, saddr, FIB_RES_GW(*res))))
+ IPCB(skb)->flags |= IPSKB_DOREDIRECT;
if (skb->protocol != htons(ETH_P_IP)) {
/* Not IP (i.e. ARP). Do not create route, if it is
@@ -2316,6 +2314,8 @@ static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
r->rtm_flags = (rt->rt_flags & ~0xFFFF) | RTM_F_CLONED;
if (rt->rt_flags & RTCF_NOTIFY)
r->rtm_flags |= RTM_F_NOTIFY;
+ if (IPCB(skb)->flags & IPSKB_DOREDIRECT)
+ r->rtm_flags |= RTCF_DOREDIRECT;
if (nla_put_be32(skb, RTA_DST, dst))
goto nla_put_failure;
--
2.1.0
^ permalink raw reply related
* [PATCH net 0/2] net: marvell: Fix highmem support on non-TSO path
From: Ezequiel Garcia @ 2015-01-21 12:54 UTC (permalink / raw)
To: netdev, Russell King, David Miller; +Cc: B38611, fabio.estevam, Ezequiel Garcia
These two commits are fixes to the issue reported by Russell King on
mv643xx_eth. Namely, the introduction of a regression by commit 69ad0dd7af22
which removed the support for highmem skb fragments. The guilty commit
introduced the assumption of fragment's payload being located in lowmem pages.
A similar pattern can be found in the original mvneta driver (in fact, the
regression was introduced by copy-pasting the mvneta code).
These fixes are for the non-TSO egress path in mvneta and mv643xx_eth drivers.
The TSO path needs a more intrusive change, as the TSO API needs to be fixed
(e.g. to make it work in skb fragments, instead of pointers to data).
Russell, as I'm still unable to reproduce this, do you think you can
give it a spin over there?
Ezequiel Garcia (2):
net: mvneta: Fix highmem support in the non-TSO egress path
net: mv643xx_eth: Fix highmem support in non-TSO egress path
drivers/net/ethernet/marvell/mv643xx_eth.c | 26 ++++++++++++++------
drivers/net/ethernet/marvell/mvneta.c | 39 ++++++++++++++++++------------
2 files changed, 43 insertions(+), 22 deletions(-)
--
2.2.1
^ permalink raw reply
* [PATCH 1/2] net: mvneta: Fix highmem support in the non-TSO egress path
From: Ezequiel Garcia @ 2015-01-21 12:54 UTC (permalink / raw)
To: netdev, Russell King, David Miller; +Cc: B38611, fabio.estevam, Ezequiel Garcia
In-Reply-To: <1421844850-30886-1-git-send-email-ezequiel.garcia@free-electrons.com>
The current implementation is broken and does not support
a skb fragment being in a highmem page. By using page_address()
to get the address of a fragment's page, we are assuming a
lowmem page. However, such assumption is incorrect and proper
highmem support is required instead.
This commit fixes this by using the skb_frag_dma_map() helper,
which takes care of mapping the skb fragment properly.
Fixes: c5aff18204da ("net: mvneta: driver for Marvell Armada 370/XP network unit")
Reported-by: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/net/ethernet/marvell/mvneta.c | 39 +++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 96208f1..adec923 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1296,10 +1296,22 @@ static void mvneta_txq_bufs_free(struct mvneta_port *pp,
mvneta_txq_inc_get(txq);
- if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr))
- dma_unmap_single(pp->dev->dev.parent,
- tx_desc->buf_phys_addr,
- tx_desc->data_size, DMA_TO_DEVICE);
+ if (!IS_TSO_HEADER(txq, tx_desc->buf_phys_addr)) {
+
+ /* The first descriptor is either a TSO header or
+ * the linear part of the skb.
+ */
+ if (tx_desc->command & MVNETA_TXD_F_DESC)
+ dma_unmap_single(pp->dev->dev.parent,
+ tx_desc->buf_phys_addr,
+ tx_desc->data_size,
+ DMA_TO_DEVICE);
+ else
+ dma_unmap_page(pp->dev->dev.parent,
+ tx_desc->buf_phys_addr,
+ tx_desc->data_size,
+ DMA_TO_DEVICE);
+ }
if (!skb)
continue;
dev_kfree_skb_any(skb);
@@ -1669,14 +1681,11 @@ static int mvneta_tx_frag_process(struct mvneta_port *pp, struct sk_buff *skb,
for (i = 0; i < nr_frags; i++) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
- void *addr = page_address(frag->page.p) + frag->page_offset;
-
tx_desc = mvneta_txq_next_desc_get(txq);
- tx_desc->data_size = frag->size;
-
- tx_desc->buf_phys_addr =
- dma_map_single(pp->dev->dev.parent, addr,
- tx_desc->data_size, DMA_TO_DEVICE);
+ tx_desc->data_size = skb_frag_size(frag);
+ tx_desc->buf_phys_addr = skb_frag_dma_map(pp->dev->dev.parent,
+ frag, 0, tx_desc->data_size,
+ DMA_TO_DEVICE);
if (dma_mapping_error(pp->dev->dev.parent,
tx_desc->buf_phys_addr)) {
@@ -1704,10 +1713,10 @@ error:
*/
for (i = i - 1; i >= 0; i--) {
tx_desc = txq->descs + i;
- dma_unmap_single(pp->dev->dev.parent,
- tx_desc->buf_phys_addr,
- tx_desc->data_size,
- DMA_TO_DEVICE);
+ dma_unmap_page(pp->dev->dev.parent,
+ tx_desc->buf_phys_addr,
+ tx_desc->data_size,
+ DMA_TO_DEVICE);
mvneta_txq_desc_put(txq);
}
--
2.2.1
^ permalink raw reply related
* [PATCH 2/2] net: mv643xx_eth: Fix highmem support in non-TSO egress path
From: Ezequiel Garcia @ 2015-01-21 12:54 UTC (permalink / raw)
To: netdev, Russell King, David Miller; +Cc: B38611, fabio.estevam, Ezequiel Garcia
In-Reply-To: <1421844850-30886-1-git-send-email-ezequiel.garcia@free-electrons.com>
Commit 69ad0dd7af22b61d9e0e68e56b6290121618b0fb
Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
Date: Mon May 19 13:59:59 2014 -0300
net: mv643xx_eth: Use dma_map_single() to map the skb fragments
caused a nasty regression by removing the support for highmem skb
fragments. By using page_address() to get the address of a fragment's
page, we are assuming a lowmem page. However, such assumption is incorrect,
as fragments can be in highmem pages, resulting in very nasty issues.
This commit fixes this by using the skb_frag_dma_map() helper,
which takes care of mapping the skb fragment properly.
Fixes: 69ad0dd7af22 ("net: mv643xx_eth: Use dma_map_single() to map the skb fragments")
Reported-by: Russell King <linux@arm.linux.org.uk>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
---
drivers/net/ethernet/marvell/mv643xx_eth.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
index a62fc38..0c77f0e 100644
--- a/drivers/net/ethernet/marvell/mv643xx_eth.c
+++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
@@ -879,10 +879,8 @@ static void txq_submit_frag_skb(struct tx_queue *txq, struct sk_buff *skb)
skb_frag_t *this_frag;
int tx_index;
struct tx_desc *desc;
- void *addr;
this_frag = &skb_shinfo(skb)->frags[frag];
- addr = page_address(this_frag->page.p) + this_frag->page_offset;
tx_index = txq->tx_curr_desc++;
if (txq->tx_curr_desc == txq->tx_ring_size)
txq->tx_curr_desc = 0;
@@ -902,8 +900,9 @@ static void txq_submit_frag_skb(struct tx_queue *txq, struct sk_buff *skb)
desc->l4i_chk = 0;
desc->byte_cnt = skb_frag_size(this_frag);
- desc->buf_ptr = dma_map_single(mp->dev->dev.parent, addr,
- desc->byte_cnt, DMA_TO_DEVICE);
+ desc->buf_ptr = skb_frag_dma_map(mp->dev->dev.parent,
+ this_frag, 0, desc->byte_cnt,
+ DMA_TO_DEVICE);
}
}
@@ -1065,9 +1064,22 @@ static int txq_reclaim(struct tx_queue *txq, int budget, int force)
reclaimed++;
txq->tx_desc_count--;
- if (!IS_TSO_HEADER(txq, desc->buf_ptr))
- dma_unmap_single(mp->dev->dev.parent, desc->buf_ptr,
- desc->byte_cnt, DMA_TO_DEVICE);
+ if (!IS_TSO_HEADER(txq, desc->buf_ptr)) {
+
+ /* The first descriptor is either a TSO header or
+ * the linear part of the skb.
+ */
+ if (desc->cmd_sts & TX_FIRST_DESC)
+ dma_unmap_single(mp->dev->dev.parent,
+ desc->buf_ptr,
+ desc->byte_cnt,
+ DMA_TO_DEVICE);
+ else
+ dma_unmap_page(mp->dev->dev.parent,
+ desc->buf_ptr,
+ desc->byte_cnt,
+ DMA_TO_DEVICE);
+ }
if (cmd_sts & TX_ENABLE_INTERRUPT) {
struct sk_buff *skb = __skb_dequeue(&txq->tx_skb);
--
2.2.1
^ permalink raw reply related
* iproute2 and kernel clock resolutions
From: Ignacy Gawedzki @ 2015-01-21 13:22 UTC (permalink / raw)
To: netdev; +Cc: Patrick McHardy, Stephen Hemminger
Hi,
While reading tc/tc_core.c, I'm having a hard time making sense of what
tc_core_init() is actually supposed to do.
It looks just as if clock_res is not filled with the correct value from
/proc/net/psched, taking into account the "compatibility hack" that follows.
AFAICT from the historical kernel Git repository, the third value in
/proc/net/psched has always been hardcoded as 1000000, so it seems that the
condition clock_res == 1000000000 is always false. Shouldn't clock_res be
filled with the fourth value instead of the third, by any chance?
Could also anyone explain to me what this hack is actually about?
Thanks,
Ignacy
--
Ignacy Gawędzki
R&D Engineer
Green Communications
^ permalink raw reply
* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Olivier Sobrie @ 2015-01-21 13:55 UTC (permalink / raw)
To: Alan Stern; +Cc: Jan Dumon, Greg Kroah-Hartman, linux-kernel, linux-usb, netdev
In-Reply-To: <Pine.LNX.4.44L0.1501201022360.1150-100000@iolanthe.rowland.org>
Hello Alan,
On Tue, Jan 20, 2015 at 10:26:30AM -0500, Alan Stern wrote:
> On Tue, 20 Jan 2015, Olivier Sobrie wrote:
>
> > When usb_queue_reset() is called it schedules a work in view of
> > resetting the usb interface. When the reset work is running, it
> > can be scheduled again (e.g. by the usb disconnect method of
> > the driver).
> >
> > Consider that the reset work is queued again while the reset work
> > is running and that this work leads to a forced unbinding of the
> > usb interface (e.g. because a driver is bound to the interface
> > and has no pre/post_reset methods - see usb_reset_device()).
> > In such condition, usb_unbind_interface() gets called and this
> > function calls usb_cancel_queued_reset() which does nothing
> > because the flag "reset_running" is set to 1. The second reset
> > work that has been scheduled is therefore not cancelled.
> > Later, the usb_reset_device() tries to rebind the interface.
> > If it fails, then the usb interface context which contain the
> > reset work struct is freed and it most likely crash when the
> > second reset work tries to be run.
>
> There was an earlier patch posted for testing (no results yet)
> affecting this same region of code:
>
> http://marc.info/?l=linux-usb&m=142064533924019&w=2
>
> It should fix the problem described here, because (among other things)
> it adds usb_get/put_intf calls to the delayed-reset routines.
I tested your patch. It also fixes the problem I observed.
You can drop mine.
For your info:
My test consists in powering down a usb hso modem while one of its
serial port is opened. It leads to two URB failures, each urb callback
queues a reset.
Without your fix (or without the one I sent), a crash happens after
less than ~20 power up/down sequences. With your fix, after more than
1000 power up/down I don't see any crash.
Thanks,
--
Olivier
^ permalink raw reply
* [3.16.y-ckt stable] Patch "xen-netfront: Fix handling packets on compound pages with skb_linearize" has been added to staging queue
From: Luis Henriques @ 2015-01-21 14:02 UTC (permalink / raw)
To: Zoltan Kiss
Cc: Wei Liu, Ian Campbell, Paul Durrant, netdev, linux-kernel,
xen-devel, David S. Miller, Stefan Bader, Luis Henriques,
kernel-team
This is a note to let you know that I have just added a patch titled
xen-netfront: Fix handling packets on compound pages with skb_linearize
to the linux-3.16.y-queue branch of the 3.16.y-ckt extended stable tree
which can be found at:
http://kernel.ubuntu.com/git?p=ubuntu/linux.git;a=shortlog;h=refs/heads/linux-3.16.y-queue
This patch is scheduled to be released in version 3.16.7-ckt5.
If you, or anyone else, feels it should not be added to this tree, please
reply to this email.
For more information about the 3.16.y-ckt tree, see
https://wiki.ubuntu.com/Kernel/Dev/ExtendedStable
Thanks.
-Luis
------
>From de2d635708a0a0a8e3b38e25e1cbf81a06f6fc27 Mon Sep 17 00:00:00 2001
From: Zoltan Kiss <zoltan.kiss@citrix.com>
Date: Mon, 11 Aug 2014 18:32:23 +0100
Subject: xen-netfront: Fix handling packets on compound pages with
skb_linearize
commit 97a6d1bb2b658ac85ed88205ccd1ab809899884d upstream.
There is a long known problem with the netfront/netback interface: if the guest
tries to send a packet which constitues more than MAX_SKB_FRAGS + 1 ring slots,
it gets dropped. The reason is that netback maps these slots to a frag in the
frags array, which is limited by size. Having so many slots can occur since
compound pages were introduced, as the ring protocol slice them up into
individual (non-compound) page aligned slots. The theoretical worst case
scenario looks like this (note, skbs are limited to 64 Kb here):
linear buffer: at most PAGE_SIZE - 17 * 2 bytes, overlapping page boundary,
using 2 slots
first 15 frags: 1 + PAGE_SIZE + 1 bytes long, first and last bytes are at the
end and the beginning of a page, therefore they use 3 * 15 = 45 slots
last 2 frags: 1 + 1 bytes, overlapping page boundary, 2 * 2 = 4 slots
Although I don't think this 51 slots skb can really happen, we need a solution
which can deal with every scenario. In real life there is only a few slots
overdue, but usually it causes the TCP stream to be blocked, as the retry will
most likely have the same buffer layout.
This patch solves this problem by linearizing the packet. This is not the
fastest way, and it can fail much easier as it tries to allocate a big linear
area for the whole packet, but probably easier by an order of magnitude than
anything else. Probably this code path is not touched very frequently anyway.
Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xenproject.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Cc: Stefan Bader <stefan.bader@canonical.com>
Signed-off-by: Luis Henriques <luis.henriques@canonical.com>
---
drivers/net/xen-netfront.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 7a4cd11e6a07..3e0019503440 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -623,9 +623,10 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
slots = DIV_ROUND_UP(offset + len, PAGE_SIZE) +
xennet_count_skb_frag_slots(skb);
if (unlikely(slots > MAX_SKB_FRAGS + 1)) {
- net_alert_ratelimited(
- "xennet: skb rides the rocket: %d slots\n", slots);
- goto drop;
+ net_dbg_ratelimited("xennet: skb rides the rocket: %d slots, %d bytes\n",
+ slots, skb->len);
+ if (skb_linearize(skb))
+ goto drop;
}
spin_lock_irqsave(&queue->tx_lock, flags);
--
2.1.4
^ permalink raw reply related
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