* Re: [PATCH] pktgen: fix crash at module unload
From: Eric Dumazet @ 2012-05-09 23:29 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: David Miller, netdev
In-Reply-To: <874nrpyrp1.fsf@xmission.com>
On Wed, 2012-05-09 at 14:23 -0700, Eric W. Biederman wrote:
> That seems reasonable.
>
> Would it be easier to call unregister_netdevice_notifer before shutting
> down the threads, so you don't have the weird cases to deal with during
> shutdown?
>
> It looks like pg_cleanup doesn't take the pktgen_thread_lock, so
> I suspect that there are still races.
>
It was 'safe' because pktgen doesnt yet support cpu hotplug.
Given pktgen nature, I am not sure we should care, granted it doesnt
crash when a poor guy like me want to use it once in a while...
list was modified only at module load.
[PATCH v2] pktgen: fix crash at module unload
commit 7d3d43dab4e9 (net: In unregister_netdevice_notifier unregister
the netdevices.) makes pktgen crashing at module unload.
[ 296.820578] BUG: spinlock bad magic on CPU#6, rmmod/3267
[ 296.820719] lock: ffff880310c38000, .magic: ffff8803, .owner: <none>/-1, .owner_cpu: -1
[ 296.820943] Pid: 3267, comm: rmmod Not tainted 3.4.0-rc5+ #254
[ 296.821079] Call Trace:
[ 296.821211] [<ffffffff8168a715>] spin_dump+0x8a/0x8f
[ 296.821345] [<ffffffff8168a73b>] spin_bug+0x21/0x26
[ 296.821507] [<ffffffff812b4741>] do_raw_spin_lock+0x131/0x140
[ 296.821648] [<ffffffff8169188e>] _raw_spin_lock+0x1e/0x20
[ 296.821786] [<ffffffffa00cc0fd>] __pktgen_NN_threads+0x4d/0x140 [pktgen]
[ 296.821928] [<ffffffffa00ccf8d>] pktgen_device_event+0x10d/0x1e0 [pktgen]
[ 296.822073] [<ffffffff8154ed4f>] unregister_netdevice_notifier+0x7f/0x100
[ 296.822216] [<ffffffffa00d2a0b>] pg_cleanup+0x48/0x73 [pktgen]
[ 296.822357] [<ffffffff8109528e>] sys_delete_module+0x17e/0x2a0
[ 296.822502] [<ffffffff81699652>] system_call_fastpath+0x16/0x1b
Hold the pktgen_thread_lock while splicing pktgen_threads, and test
pktgen_exiting in pktgen_device_event() to make unload faster.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
---
net/core/pktgen.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index ffb5d38..b644505 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -1931,7 +1931,7 @@ static int pktgen_device_event(struct notifier_block *unused,
{
struct net_device *dev = ptr;
- if (!net_eq(dev_net(dev), &init_net))
+ if (!net_eq(dev_net(dev), &init_net) || pktgen_exiting)
return NOTIFY_DONE;
/* It is OK that we do not hold the group lock right now,
@@ -3755,12 +3755,18 @@ static void __exit pg_cleanup(void)
{
struct pktgen_thread *t;
struct list_head *q, *n;
+ struct list_head list;
/* Stop all interfaces & threads */
pktgen_exiting = true;
- list_for_each_safe(q, n, &pktgen_threads) {
+ mutex_lock(&pktgen_thread_lock);
+ list_splice(&list, &pktgen_threads);
+ mutex_unlock(&pktgen_thread_lock);
+
+ list_for_each_safe(q, n, &list) {
t = list_entry(q, struct pktgen_thread, th_list);
+ list_del(&t->th_list);
kthread_stop(t->tsk);
kfree(t);
}
^ permalink raw reply related
* Re: Netlink for kernel<->user space communication?
From: Arvid Brodin @ 2012-05-09 23:32 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20120507153307.551b29ed@nehalam.linuxnetplumber.net>
On 2012-05-08 00:33, Stephen Hemminger wrote:
> On Mon, 7 May 2012 18:43:23 +0000
> Arvid Brodin <Arvid.Brodin@xdin.com> wrote:
>
>> On Tue, 24 Apr 2012 16:57:55 -0700
>> Stephen Hemminger <shemminger@xxxxxxxxxx> wrote:
>>> On Tue, 24 Apr 2012 23:52:34 +0000
>>> Arvid Brodin <Arvid.Brodin@xxxxxxxx> wrote:
>>>
>>>> Hi.
>>>>
>>>> I'm writing a kernel driver for the HSR protocol, a standard for high availability
>>>> networks. I want to send messages from the kernel to user space about broken network
>>>> links. I also want user space to be able to ask the kernel about its view of the status of
>>>> nodes on the network.
>>>>
>>>> Netlink seems like a good tool for this. (Is it?)
>>>
>>> Yes.
>>>
>>>> But do I use raw netlink? (Described here: http://www.linuxjournal.com/article/7356 - but
>>>> this seems a bit out of date, the kernel API description differs from today's kernel
>>>> implementation.)
>>>
>>> No. Your driver probably looks like a device so you should be
>>> using rtnetlink messages.
>>
>> I'm already using rtnetlink messages to add and remove my device, which works fine (see
>> e.g. http://www.spinics.net/lists/netdev/msg192817.html - although I didn't think it
>> meaningful to include the iproute2 patch here, until the kernel part is ready).
>>
>> The protocol specifies transmission of "supervision frames" every 2 seconds, e.g. to check
>> link integrity. Every such frame should be received from two directions in the ring - if
>> only one is received, then there is a link problem.
>
> Why not just manipulate the carrier or operational state (see Documentation/networking/operstate)
> and use the existing notification on link changes. If you don't get heartbeat then change
> the state of the device to indicate lower device is down with set_operstate(), the necessary
> link everts propgate back as netlink events.
With HSR, all nodes in the network ring can detect a link problem anywhere in the ring. So
I need a way to communicate link problems that does not concern the host's devices at all,
but rather the state of the network as a whole. A typical message might say "Frames from
node 01:23:45:67:89:AB is only received over Slave Interface 1!" This indicates a problem
since all frames should be received over both slave interfaces. The broken link can be
anywhere between this node and the indicated node. If user space is aware of the network
topology, it can figure out exactly where the damage is by looking at which nodes' frames
are received over which slave interface.
(Thanks for the operstates info though, I hadn't discovered IF_OPER_LOWERLAYERDOWN! I will
use it to indicate a local slave is down.)
>> I'd like to notify user space about every such occurence. Is there a rtnetlink message
>> type that fits this? The stuff in rtnetlink.h seems to be mostly concerned with specific
>> user space commands (there is something called RTNLGRP_NOTIFY but I couldn't find any
>> instances of it being used in the kernel, nor any documentation).
>>
>
> I am trying to steer you to use existing API's because then existing programs and
> infrastructure can deal with the new device type.
I really appreciate that! I want to use existing API's as far as possible. That's why I
keep sending you all these questions. :)
--
Arvid Brodin
XDIN AB | Jan Stenbecks Torg 17 | SE-164 40 Kista | Sweden | xdin.com
^ permalink raw reply
* Re: [PATCH 3/3] usbnet: fix skb traversing races during unlink(v1)
From: Ming Lei @ 2012-05-09 23:47 UTC (permalink / raw)
To: David Miller
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
huajun.li.lee-Re5JQEeQqe8AvxtiuMwx3w, oneukum-l3A5Bk7waGM,
stable-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <20120503090450.41704527@tom-ThinkPad-T410>
Hi David,
On Thu, May 3, 2012 at 9:04 AM, Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From a87ff961f0a5d50223bd084dfac4fe5ce84f3913 Mon Sep 17 00:00:00 2001
> From: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Thu, 26 Apr 2012 11:33:46 +0800
> Subject: [PATCH] usbnet: fix skb traversing races during unlink(v2)
>
> Commit 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d(net/usbnet: avoid
> recursive locking in usbnet_stop()) fixes the recursive locking
> problem by releasing the skb queue lock before unlink, but may
> cause skb traversing races:
> - after URB is unlinked and the queue lock is released,
> the refered skb and skb->next may be moved to done queue,
> even be released
> - in skb_queue_walk_safe, the next skb is still obtained
> by next pointer of the last skb
> - so maybe trigger oops or other problems
>
> This patch extends the usage of entry->state to describe 'start_unlink'
> state, so always holding the queue(rx/tx) lock to change the state if
> the referd skb is in rx or tx queue because we need to know if the
> refered urb has been started unlinking in unlink_urbs.
>
> The other part of this patch is based on Huajun's patch:
> always traverse from head of the tx/rx queue to get skb which is
> to be unlinked but not been started unlinking.
>
> Signed-off-by: Huajun Li <huajun.li.lee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> Cc: stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
> ---
> v2:
> - if the Rx URB has been marked as being unlink, just not resubmit it
> in complete handler, so usb_block_urb/usb_unblock_urb can be avoided,
Considered that this one(v2) doesn't depend on usb tree any more and looks
no one objects it, could you apply this one on your tree?
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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: [PATCH] net: update the usage of CHECKSUM_UNNECESSARY
From: Ben Hutchings @ 2012-05-10 0:05 UTC (permalink / raw)
To: Michael S. Tsirkin, David Miller; +Cc: Yi Zou, netdev, devel, jeffrey.t.kirsher
In-Reply-To: <20120509060308.GA13522@redhat.com>
On Wed, 2012-05-09 at 09:03 +0300, Michael S. Tsirkin wrote:
> On Tue, May 08, 2012 at 07:20:58PM +0100, Ben Hutchings wrote:
> > On Tue, 2012-05-08 at 20:48 +0300, Michael S. Tsirkin wrote:
> > > On Mon, Mar 19, 2012 at 02:12:41PM -0700, Yi Zou wrote:
> > > > As suggested by Ben, this adds the clarification on the usage of
> > > > CHECKSUM_UNNECESSARY on the outgoing patch. Also add the usage
> > > > description of NETIF_F_FCOE_CRC and CHECKSUM_UNNECESSARY
> > > > for the kernel FCoE protocol driver.
> > > >
> > > > This is a follow-up to the following:
> > > > http://patchwork.ozlabs.org/patch/147315/
> > > >
> > > > Signed-off-by: Yi Zou <yi.zou@intel.com>
> > > > Cc: Ben Hutchings <bhutchings@solarflare.com>
> > > > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > > Cc: www.Open-FCoE.org <devel@open-fcoe.org>
> > > > ---
> > > >
> > > > include/linux/skbuff.h | 7 +++++++
> > > > 1 files changed, 7 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > > > index 8dc8257..a2b9953 100644
> > > > --- a/include/linux/skbuff.h
> > > > +++ b/include/linux/skbuff.h
> > > > @@ -94,6 +94,13 @@
> > > > * about CHECKSUM_UNNECESSARY. 8)
> > > > * NETIF_F_IPV6_CSUM about as dumb as the last one but does IPv6 instead.
> > > > *
> > > > + * UNNECESSARY: device will do per protocol specific csum. Protocol drivers
> > > > + * that do not want net to perform the checksum calculation should use
> > > > + * this flag in their outgoing skbs.
> > > > + * NETIF_F_FCOE_CRC this indicates the device can do FCoE FC CRC
> > > > + * offload. Correspondingly, the FCoE protocol driver
> > > > + * stack should use CHECKSUM_UNNECESSARY.
> > > > + *
> > > > * Any questions? No questions, good. --ANK
> > > > */
> > > >
> > >
> > > So just to make sure I understand, you never get
> > > UNNECESSARY packets on tx unless you declared NETIF_F_FCOE_CRC?
> > >
> > > Maybe the comment says this somehow but could not figure it out.
> >
> > That's what should happen now. In future CHECKSUM_UNNECESSARY could be
> > used on output by other protocols which don't use TCP/IP-style
> > checksums, but always dependent on the output device supporting the
> > relevant offload feature.
> >
> > Ben.
>
> Isn't there another case: a device passes UNNECESSARY in on rx,
> and the skb is forwarded to another device?
> For example it is handled this way by tun, giving a nice
> performance boost for VMs, see 10a8d94a95742bb15b4e617ee9884bb4381362be
Hmm... I thought UNNECESSARY was supposed to be replaced by NONE on
output, but I don't see where that would happen. Which would mean we
had an undocumented case here, and now we have ambiguity. :-(
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH] ehea: fix losing of NEQ events when one event occurred early
From: Thadeu Lima de Souza Cascardo @ 2012-05-10 0:10 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Thadeu Lima de Souza Cascardo
The NEQ interrupt is only triggered when there was no previous pending
interrupt. If we request irq handling after an interrupt has occurred,
we will never get an interrupt until we call H_RESET_EVENTS.
Events seem to be cleared when we first register the NEQ. So, when we
requested irq handling right after registering it, a possible race with
an interrupt was much less likely. Now, there is a chance we may lose
this race and never get any events.
The fix here is to poll and acknowledge any events that might have
happened right after registering the irq handler.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ehea/ehea_main.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index c9069a2..f088e59 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -3335,6 +3335,9 @@ static int __devinit ehea_probe_adapter(struct platform_device *dev,
goto out_shutdown_ports;
}
+ /* Handle any events that might be pending. */
+ tasklet_hi_schedule(&adapter->neq_tasklet);
+
ret = 0;
goto out;
--
1.7.4.4
^ permalink raw reply related
* Re: [NET_NEXT RFC PATCH 3/3] ixgbe: added reg_ops file to debugfs
From: Ben Hutchings @ 2012-05-10 0:18 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Catherine Sullivan, netdev
In-Reply-To: <20120509161449.43704ea7@nehalam.linuxnetplumber.net>
On Wed, 2012-05-09 at 16:14 -0700, Stephen Hemminger wrote:
> On Wed, 09 May 2012 16:09:50 -0700
> Catherine Sullivan <catherine.sullivan@intel.com> wrote:
>
> > Added the reg_ops file to debugfs with commands to read and write
> > a register to give users the ability to read and write individual
> > registers on the fly.
> >
> > Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
> > ---
> >
> > drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 112 ++++++++++++++++++++++
> > 1 files changed, 112 insertions(+), 0 deletions(-)
> >
>
> Aren't these registers already in ethtool?
ethtool register access is read-only and would be very heavy-weight for
interactive debugging. Not sure this is the best interface to
read/write registers, but it's certainly not redundant.
> You are also allowing write without any security checking.
The file permissions are set to 0600 so it's root-only.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] netlink: connector: implement cn_netlink_reply
From: Ben Hutchings @ 2012-05-10 0:20 UTC (permalink / raw)
To: Alban Crequy
Cc: Evgeniy Polyakov, netdev, Vincent Sanders,
Javier Martinez Canillas, Rodrigo Moya
In-Reply-To: <1336574243-12063-1-git-send-email-alban.crequy@collabora.co.uk>
On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote:
> In a connector callback, it was not possible to reply to a message only to a
> sender. This patch implements cn_netlink_reply(). It uses the connector socket
> to send an unicast netlink message back to the sender.
[...]
We try to avoid adding functions with no users. You'll need to submit
the code that's intended to use this as well.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] ehea: fix losing of NEQ events when one event occurred early
From: David Miller @ 2012-05-10 0:29 UTC (permalink / raw)
To: cascardo; +Cc: netdev
In-Reply-To: <1336608618-31497-1-git-send-email-cascardo@linux.vnet.ibm.com>
From: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
Date: Wed, 9 May 2012 21:10:18 -0300
> + /* Handle any events that might be pending. */
> + tasklet_hi_schedule(&adapter->neq_tasklet);
> +
>
> ret = 0;
These extra empty lines are completely unnecessary, please remove
them.
^ permalink raw reply
* Re: [PATCH 08/14] batman-adv: ignore protocol packets if the interface did not enable this protocol
From: David Miller @ 2012-05-10 0:41 UTC (permalink / raw)
To: ordex-GaUfNO9RBHfsrOwW+9ziJQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
lindner_marek-LWAfsSFWpa4
In-Reply-To: <1336561976-16088-9-git-send-email-ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
From: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
Date: Wed, 9 May 2012 13:12:50 +0200
> + /* did we receive a B.A.T.M.A.N. IV OGM packet on an interface
> + * that does not have B.A.T.M.A.N. IV enabled ? */
Needs to be:
/* did we receive a B.A.T.M.A.N. IV OGM packet on an interface
* that does not have B.A.T.M.A.N. IV enabled ?
*/
^ permalink raw reply
* Re: [PATCH 11/14] batman-adv: Adding hard_iface specific sysfs wrapper macros for UINT
From: David Miller @ 2012-05-10 0:43 UTC (permalink / raw)
To: ordex-GaUfNO9RBHfsrOwW+9ziJQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r,
lindner_marek-LWAfsSFWpa4
In-Reply-To: <1336561976-16088-12-git-send-email-ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
From: Antonio Quartulli <ordex-GaUfNO9RBHfsrOwW+9ziJQ@public.gmane.org>
Date: Wed, 9 May 2012 13:12:53 +0200
> + /**
> + * Mark the forwarded packet when it is not coming from our best
> + * next hop. We still need to forward the packet for our neighbor
> + * link quality detection to work in case the packet originated
> + * from a single hop neighbor. Otherwise we can simply drop the
> + * ogm.
> + */
Start comments with "/*" not "/**", and properly indent these lines.
^ permalink raw reply
* Re: [PATCH 09/14] batman-adv: refactoring API: find generalized name for bat_ogm_update_mac callback
From: David Miller @ 2012-05-10 0:42 UTC (permalink / raw)
To: ordex; +Cc: netdev, b.a.t.m.a.n, lindner_marek
In-Reply-To: <1336561976-16088-10-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <ordex@autistici.org>
Date: Wed, 9 May 2012 13:12:51 +0200
> + /* (re-)init mac addresses of the protocol information
> + * belonging to this hard-interface */
Needs to be:
/* (re-)init mac addresses of the protocol information
* belonging to this hard-interface
*/
^ permalink raw reply
* Re: [PATCH 07/14] batman-adv: split neigh_new function into generic and batman iv specific parts
From: David Miller @ 2012-05-10 0:41 UTC (permalink / raw)
To: ordex; +Cc: netdev, b.a.t.m.a.n, lindner_marek
In-Reply-To: <1336561976-16088-8-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <ordex@autistici.org>
Date: Wed, 9 May 2012 13:12:49 +0200
> From: Marek Lindner <lindner_marek@yahoo.de>
>
> Signed-off-by: Marek Lindner <lindner_marek@yahoo.de>
> Acked-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
> Signed-off-by: Antonio Quartulli <ordex@autistici.org>
The namespace pollution of the batman-adv code needs to improve,
and I'm putting my foot down starting with this change.
If you have a static function which is therefore private to a
source file, name it whatever you want.
But once it gets exported out of that file, you have to give it
an appropriate name. Probably with a "batman_adv_" prefix or
similar.
Otherwise, for one thing, if this code is statically built into
the tree it can conflict with symbol names elsewhere and break
the build.
^ permalink raw reply
* Re: [PATCH] netlink: connector: implement cn_netlink_reply
From: Evgeniy Polyakov @ 2012-05-10 0:45 UTC (permalink / raw)
To: Ben Hutchings
Cc: Alban Crequy, netdev, Vincent Sanders, Javier Martinez Canillas,
Rodrigo Moya
In-Reply-To: <1336609248.2499.20.camel@bwh-desktop.uk.solarflarecom.com>
On Thu, May 10, 2012 at 01:20:48AM +0100, Ben Hutchings (bhutchings@solarflare.com) wrote:
> On Wed, 2012-05-09 at 15:37 +0100, Alban Crequy wrote:
> > In a connector callback, it was not possible to reply to a message only to a
> > sender. This patch implements cn_netlink_reply(). It uses the connector socket
> > to send an unicast netlink message back to the sender.
> [...]
>
> We try to avoid adding functions with no users. You'll need to submit
> the code that's intended to use this as well.
I have no objection against this patch, but as correctly stated it is
useless without users. Alban, what is the code you want this
functionality to be used in? Do you plan to submit it? Can you submit
this change in the patch with your code?
--
Evgeniy Polyakov
^ permalink raw reply
* FW: PROBLEM: VLAN LRO issue
From: Steven Liu (劉人豪) @ 2012-05-10 1:01 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hi Sir,
I found the issue about VLAN LRO feature and below is bug description and patch for your reference, thanks.
[1.] One line summary of the problem:
VLAN LRO issue
[2.] Full description of the problem/report:
LRO implementation cannot handle VLAN tagged packets correctly.
It uses correct packet length when creating new LRO session, but uses wrong packet length when putting following packets into exist LRO session.
[3.] Keywords (i.e., modules, networking, kernel):
Networking
[4.] Kernel version (from /proc/version):
Linux-3.4-rc3 and below
[5.] Output of Oops.. message (if applicable) with symbolic information
resolved (see Documentation/oops-tracing.txt) System is still working, but cannot speed up VLAN tagged traffic.
[X.] Other notes, patches, fixes, workarounds:
We have to apply below patch to fix this issue.
--- linux-3.4-rc3/net/ipv4/inet_lro.orig 2012-05-08 20:09:44.810366089 +0800
+++ linux-3.4-rc3/net/ipv4/inet_lro.c 2012-05-08 20:09:33.331679419 +0800
@@ -353,7 +353,7 @@ static int __lro_proc_skb(struct net_lro
if (lro_desc->tcp_next_seq != ntohl(tcph->seq))
goto out2;
- if (lro_tcp_ip_check(iph, tcph, skb->len, lro_desc))
+ if (lro_tcp_ip_check(iph, tcph, skb->len - vlan_hdr_len, lro_desc))
goto out2;
lro_add_packet(lro_desc, skb, iph, tcph);
Regards,
Steven
************* Email Confidentiality Notice ********************
The information contained in this e-mail message (including any
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be
conveyed only to the designated recipient(s). Any use, dissemination,
distribution, printing, retaining or copying of this e-mail (including its
attachments) by unintended recipient(s) is strictly prohibited and may
be unlawful. If you are not an intended recipient of this e-mail, or believe
that you have received this e-mail in error, please notify the sender
immediately (by replying to this e-mail), delete any and all copies of
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
^ permalink raw reply
* Re: [PATCH 00/13] net: Add and use ether_addr_equal
From: David Miller @ 2012-05-10 1:21 UTC (permalink / raw)
To: joe
Cc: coreteam, netdev, bridge, linux-wireless, linux-kernel,
linux-bluetooth, netfilter, netfilter-devel
In-Reply-To: <cover.1336538937.git.joe@perches.com>
From: Joe Perches <joe@perches.com>
Date: Tue, 8 May 2012 21:56:44 -0700
> Add a boolean function to test 2 ethernet addresses for equality
> Convert compare_ether_addr uses to ether_addr_equal
This series looks great, I'll apply all of it.
Thanks Joe.
That case you didn't convert in mac80211 is probably the
bug Johannes was talking about which started this whole
discussion.
^ permalink raw reply
* Re: [PATCH V3 Resend 09/12] net/stmmac: Remove conditional compilation of clk code
From: David Miller @ 2012-05-10 1:22 UTC (permalink / raw)
To: viresh.kumar
Cc: andrew, mturquette, sshtylyov, netdev, spear-devel, linux-kernel,
w.sang, viresh.linux, peppe.cavallaro, linux, akpm, jgarzik,
linux-arm-kernel, LW
In-Reply-To: <8e66b900fe5ee10c8db93d2f12c912e3dfb0708f.1336448639.git.viresh.kumar@st.com>
From: Viresh Kumar <viresh.kumar@st.com>
Date: Tue, 8 May 2012 09:22:36 +0530
> With addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h,
> there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif
> macros.
>
> This also fixes error paths of probe(), as a goto is required in this patch.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH v4 2/2] macvtap: restore vlan header on user read
From: David Miller @ 2012-05-10 1:24 UTC (permalink / raw)
To: mst; +Cc: basil.gor, ebiederm, netdev
In-Reply-To: <20120508192806.GB28536@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Tue, 8 May 2012 22:28:06 +0300
> On Fri, May 04, 2012 at 12:55:24PM +0400, Basil Gor wrote:
>> Ethernet vlan header is not on the packet and kept in the skb->vlan_tci
>> when it comes from lower dev. This patch inserts vlan header in user
>> buffer during skb copy on user read.
>>
>> Signed-off-by: Basil Gor <basil.gor@gmail.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> Eric, ack?
Eric please review these two patches.
^ permalink raw reply
* Re: FW: PROBLEM: VLAN LRO issue
From: Eric Dumazet @ 2012-05-10 1:25 UTC (permalink / raw)
To: Steven Liu (劉人豪); +Cc: netdev@vger.kernel.org
In-Reply-To: <599E416D1C5C994980C45977191D726906AB2FE86E@MTKMBS03.mediatek.inc>
On Thu, 2012-05-10 at 09:01 +0800, Steven Liu (劉人豪) wrote:
> Hi Sir,
>
> I found the issue about VLAN LRO feature and below is bug description and patch for your reference, thanks.
>
> [1.] One line summary of the problem:
> VLAN LRO issue
>
> [2.] Full description of the problem/report:
> LRO implementation cannot handle VLAN tagged packets correctly.
> It uses correct packet length when creating new LRO session, but uses wrong packet length when putting following packets into exist LRO session.
>
> [3.] Keywords (i.e., modules, networking, kernel):
> Networking
>
> [4.] Kernel version (from /proc/version):
> Linux-3.4-rc3 and below
>
> [5.] Output of Oops.. message (if applicable) with symbolic information
> resolved (see Documentation/oops-tracing.txt) System is still working, but cannot speed up VLAN tagged traffic.
>
> [X.] Other notes, patches, fixes, workarounds:
> We have to apply below patch to fix this issue.
>
> --- linux-3.4-rc3/net/ipv4/inet_lro.orig 2012-05-08 20:09:44.810366089 +0800
> +++ linux-3.4-rc3/net/ipv4/inet_lro.c 2012-05-08 20:09:33.331679419 +0800
> @@ -353,7 +353,7 @@ static int __lro_proc_skb(struct net_lro
> if (lro_desc->tcp_next_seq != ntohl(tcph->seq))
> goto out2;
>
> - if (lro_tcp_ip_check(iph, tcph, skb->len, lro_desc))
> + if (lro_tcp_ip_check(iph, tcph, skb->len - vlan_hdr_len, lro_desc))
> goto out2;
>
> lro_add_packet(lro_desc, skb, iph, tcph);
>
Hi
This looks like a good fix, could you please submit an official patch ?
Documentation/SubmittingPatches
^ permalink raw reply
* Re: [PATCH 00/13] net: Add and use ether_addr_equal
From: Joe Perches @ 2012-05-10 1:48 UTC (permalink / raw)
To: David Miller, Julia Lawall
Cc: coreteam, netdev, bridge, linux-wireless, linux-kernel,
linux-bluetooth, netfilter, netfilter-devel
In-Reply-To: <20120509.212110.437114897180819434.davem@davemloft.net>
On Wed, 2012-05-09 at 21:21 -0400, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Tue, 8 May 2012 21:56:44 -0700
>
> > Add a boolean function to test 2 ethernet addresses for equality
> > Convert compare_ether_addr uses to ether_addr_equal
>
> This series looks great, I'll apply all of it.
coccinelle is a nifty, better sed, tool. Thanks Julia et al.
> That case you didn't convert in mac80211 is probably the
> bug Johannes was talking about which started this whole
> discussion.
Looks like it.
Do you want a similar patch/patches for drivers/net?
If I break it out by nominal driver/maintainer,
it'll be a highish number of low density patches.
$ git grep -w compare_ether_addr drivers/net | wc -l
59
$ git grep -w -l compare_ether_addr drivers/net | wc -l
31
Maybe just a single patch?
Also the compare_ether_addr_64bits function is
still used a couple dozen times. Maybe another
patch for those? ether_addr_equal_64bits?
^ permalink raw reply
* Re: [PATCH 00/13] net: Add and use ether_addr_equal
From: David Miller @ 2012-05-10 1:53 UTC (permalink / raw)
To: joe
Cc: coreteam, linux-bluetooth, netdev, bridge, linux-wireless,
linux-kernel, julia.lawall, netfilter, netfilter-devel
In-Reply-To: <1336614484.1905.19.camel@joe2Laptop>
From: Joe Perches <joe@perches.com>
Date: Wed, 09 May 2012 18:48:04 -0700
> Maybe just a single patch?
Yes, I think high density on this one, it's extremely
mechanical.
> Also the compare_ether_addr_64bits function is
> still used a couple dozen times. Maybe another
> patch for those? ether_addr_equal_64bits?
Use your best judgment.
^ permalink raw reply
* pull request: sfc 2012-05-09
From: Ben Hutchings @ 2012-05-10 2:06 UTC (permalink / raw)
To: David Miller; +Cc: linux-net-drivers, netdev
The following changes since commit 477206a018f902895bfcd069dd820bfe94c187b1:
r8169: fix unsigned int wraparound with TSO (2012-05-08 19:34:10 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc.git sfc-3.4
(commit 3132d2827d92c2ee47fdf4dbec75bba0a2f291cb)
This fixes a regression introduced since Linux 3.3 which will result in
a crash on load in certain configurations.
Ben.
Ben Hutchings (1):
sfc: Fix division by zero when using one RX channel and no SR-IOV
drivers/net/ethernet/sfc/efx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH net] sfc: Fix division by zero when using one RX channel and no SR-IOV
From: Ben Hutchings @ 2012-05-10 2:08 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1336615595.2499.24.camel@bwh-desktop.uk.solarflarecom.com>
If RSS is disabled on the PF (efx->n_rx_channels == 1) we try to set
up the indirection table so that VFs can use it, setting
efx->rss_spread = efx_vf_size(efx). But if SR-IOV was disabled at
compile time, this evaluates to 0 and we end up dividing by zero when
initialising the table.
I considered changing the fallback definition of efx_vf_size() to
return 1, but its value is really meaningless if we are not going to
enable VFs. Therefore add a condition of efx_sriov_wanted(efx) in
efx_probe_interrupts().
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 3cbfbff..4a00053 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1349,7 +1349,7 @@ static int efx_probe_interrupts(struct efx_nic *efx)
}
/* RSS might be usable on VFs even if it is disabled on the PF */
- efx->rss_spread = (efx->n_rx_channels > 1 ?
+ efx->rss_spread = ((efx->n_rx_channels > 1 || !efx_sriov_wanted(efx)) ?
efx->n_rx_channels : efx_vf_size(efx));
return 0;
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* pull request: sfc-next 2012-05-09
From: Ben Hutchings @ 2012-05-10 2:26 UTC (permalink / raw)
To: David Miller; +Cc: linux-net-drivers, netdev
[-- Attachment #1: Type: text/plain, Size: 1997 bytes --]
The following changes since commit 2e7d21c54adbab6d10481eddc685328f89bb6389:
e1000e: Fix merge conflict (net->net-next) (2012-05-09 12:06:39 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem
(commit ba62b2a8608ca52234fc8bea27bfebbdc4f98c2e)
1. A fix for recovery from some hardware failure modes, by Stuart Hodgson.
2. Reduction of RX jitter, by David Riddoch.
3. ethtool interface and implementation for reading plug-in modules, mostly by
Stuart Hodgson.
4. A minor fix to PCI device cleanup.
Ben.
Ben Hutchings (3):
sfc: Fix missing cleanup in failure path of efx_pci_probe()
ethtool: Split ethtool_get_eeprom() to allow for additional EEPROM accessors
sfc: Implement module EEPROM access for SFE4002 and SFN4112F
David Riddoch (2):
sfc: Fill RX rings completely full, rather than to 95% full
sfc: By default refill RX rings as soon as space for a batch
Stuart Hodgson (3):
sfc: Do not attempt to flush queues if DMA is disabled
ethtool: Extend the ethtool API to obtain plugin module eeprom data
sfc: Added support for new ethtool APIs for obtaining module eeprom
drivers/net/ethernet/sfc/efx.c | 36 +++++++++------
drivers/net/ethernet/sfc/ethtool.c | 35 +++++++++++++++
drivers/net/ethernet/sfc/mcdi_phy.c | 76 +++++++++++++++++++++++++++++++++
drivers/net/ethernet/sfc/net_driver.h | 8 ++-
drivers/net/ethernet/sfc/qt202x_phy.c | 33 ++++++++++++++
drivers/net/ethernet/sfc/rx.c | 31 ++++++-------
include/linux/ethtool.h | 33 ++++++++++++++
net/core/ethtool.c | 71 +++++++++++++++++++++++++++---
8 files changed, 282 insertions(+), 41 deletions(-)
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* [PATCH net-next 1/8] sfc: Do not attempt to flush queues if DMA is disabled
From: Ben Hutchings @ 2012-05-10 2:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1336616799.2499.32.camel@bwh-desktop.uk.solarflarecom.com>
efx_nic_fatal_interrupt() disables DMA before scheduling a reset.
After this, we need not and *cannot* flush queues.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 33 +++++++++++++++++++--------------
1 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 3cbfbff..5cc58a3 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -656,25 +656,30 @@ static void efx_stop_datapath(struct efx_nic *efx)
struct efx_channel *channel;
struct efx_tx_queue *tx_queue;
struct efx_rx_queue *rx_queue;
+ struct pci_dev *dev = efx->pci_dev;
int rc;
EFX_ASSERT_RESET_SERIALISED(efx);
BUG_ON(efx->port_enabled);
- rc = efx_nic_flush_queues(efx);
- if (rc && EFX_WORKAROUND_7803(efx)) {
- /* Schedule a reset to recover from the flush failure. The
- * descriptor caches reference memory we're about to free,
- * but falcon_reconfigure_mac_wrapper() won't reconnect
- * the MACs because of the pending reset. */
- netif_err(efx, drv, efx->net_dev,
- "Resetting to recover from flush failure\n");
- efx_schedule_reset(efx, RESET_TYPE_ALL);
- } else if (rc) {
- netif_err(efx, drv, efx->net_dev, "failed to flush queues\n");
- } else {
- netif_dbg(efx, drv, efx->net_dev,
- "successfully flushed all queues\n");
+ /* Only perform flush if dma is enabled */
+ if (dev->is_busmaster) {
+ rc = efx_nic_flush_queues(efx);
+
+ if (rc && EFX_WORKAROUND_7803(efx)) {
+ /* Schedule a reset to recover from the flush failure. The
+ * descriptor caches reference memory we're about to free,
+ * but falcon_reconfigure_mac_wrapper() won't reconnect
+ * the MACs because of the pending reset. */
+ netif_err(efx, drv, efx->net_dev,
+ "Resetting to recover from flush failure\n");
+ efx_schedule_reset(efx, RESET_TYPE_ALL);
+ } else if (rc) {
+ netif_err(efx, drv, efx->net_dev, "failed to flush queues\n");
+ } else {
+ netif_dbg(efx, drv, efx->net_dev,
+ "successfully flushed all queues\n");
+ }
}
efx_for_each_channel(channel, efx) {
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCH net-next 2/8] sfc: Fix missing cleanup in failure path of efx_pci_probe()
From: Ben Hutchings @ 2012-05-10 2:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1336616799.2499.32.camel@bwh-desktop.uk.solarflarecom.com>
We need to clear the private data pointer in the PCI device.
Also reorder cleanup in efx_pci_remove() for symmetry.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 5cc58a3..8253d21 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2497,8 +2497,8 @@ static void efx_pci_remove(struct pci_dev *pci_dev)
efx_fini_io(efx);
netif_dbg(efx, drv, efx->net_dev, "shutdown successful\n");
- pci_set_drvdata(pci_dev, NULL);
efx_fini_struct(efx);
+ pci_set_drvdata(pci_dev, NULL);
free_netdev(efx->net_dev);
};
@@ -2700,6 +2700,7 @@ static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
fail2:
efx_fini_struct(efx);
fail1:
+ pci_set_drvdata(pci_dev, NULL);
WARN_ON(rc > 0);
netif_dbg(efx, drv, efx->net_dev, "initialisation failed. rc=%d\n", rc);
free_netdev(net_dev);
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ 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