* Re: [net-next-2.6 PATCH] e1000e: use static params to save stack space
From: David Miller @ 2010-04-13 9:59 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, jesse.brandeburg
In-Reply-To: <20100409205044.32158.23239.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 09 Apr 2010 13:51:09 -0700
> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
>
> used a modified checkstack to get the 56 number
> (normally checkstack wouldn't show this low a value)
>
> checkstack before:
> 0x0000012f e1000e_check_options [e1000e]: 272
>
> after:
> 0x0000012f e1000e_check_options [e1000e]: 56
>
>
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [PATCH] (net-2.6) stmmac update - Apr 2010
From: Giuseppe CAVALLARO @ 2010-04-13 10:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100413.015352.223941252.davem@davemloft.net>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi David,
On 04/13/2010 10:53 AM, David Miller wrote:
> From: Giuseppe CAVALLARO <peppe.cavallaro@st.com>
> Date: Fri, 9 Apr 2010 12:24:15 +0200
>
>> Hello,
>> this is another subset of patches to make the driver more generic.
>>
>> This patches splits the dma and core code for the mac 10/100 device
>> (as already done for the gmac) and reorganizes the descriptor
>> structures.
>> In the first version of the driver, the mac10/100 could only use
>> normal descriptors and the gmac could only use the enhanced ones.
>> This limit has been removed and this kind of information comes
>> from the platform.
>
> Please:
>
> 1) respin your patches against net-next-2.6
I'll do this soon.
> 2) Add numbers to your patch posting subject lines so there
> is no confusion about what order your patches should be
> applied.
Oops! you are right. Thanks!
>
> 3) Make sure the driver compiles successfully at each step
> of applying your patches, not just after all the changes
> are applied.
Sure! I test this always.
Many thanks.
Best Regards
Giuseppe
> Thanks.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJLxECpAAoJEAm9vY9TshdvuH8H/RbUTVa+o7qJjRyXgb34h5t4
KSwdcnWwY90d2i/3zLvWlwaSn4J5zrSWc6EdXRpU/1zBI+xcQ0wAFieqF8R0QOoE
qIKliOPCJ7NrsV30J4xhe2R0q7e423vmTdUfcNB1VXqBaZOQOPZmKjApRT240c6E
CwLfTANwAQ6t8pl6rlyww/nTEPuOVx0ETvAGEfHloWa3Sr5TvgXndfD2dnDU9cs1
wgg/6YzehTRioMM3Rf7KySuVMcENbCkyy3R4zSNvwJONZqVqMkTWDZ1XIpq3BvpV
r2AAjXdXtVB2Z6lroP2UVCrYLcm3LB0WUdzTqrj0zpzW9cm1DEOb2nhjcWVLNHY=
=h25h
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] xtables: make XT_ALIGN() usable in exported headers by exporting __ALIGN_KERNEL()
From: Patrick McHardy @ 2010-04-13 10:01 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: linux-kernel, netdev, shemminger, bhutchings, andreas, hadi,
hideaki
In-Reply-To: <4BC437C6.8020602@trash.net>
Patrick McHardy wrote:
> Alexey Dobriyan wrote:
>> XT_ALIGN() was rewritten through ALIGN() by commit 42107f5009da223daa800d6da6904d77297ae829
>> "netfilter: xtables: symmetric COMPAT_XT_ALIGN definition".
>> ALIGN() is not exported in userspace headers, which created compile problem for tc(8)
>> and will create problem for iptables(8).
>>
>> We can't export generic looking name ALIGN() but we can export less generic
>> __ALIGN_KERNEL() (suggested by Ben Hutchings).
>> Google knows nothing about __ALIGN_KERNEL().
>>
>> COMPAT_XT_ALIGN() changed for symmetry.
>
> Since there haven't been any objections, I've applied your patch.
This breaks compilation by removing __ALIGN_MASK(). Please fix this up.
^ permalink raw reply
* [PATCH - resubmit] Fix some #includes in CAN drivers (rebased for net-next-2.6)
From: Hans J. Koch @ 2010-04-13 10:03 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: Chris Elston, Sebastian Haas,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, Per Dalen, Oliver Hartkopp,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Wolfgang Grandegger,
Christian Pellegrin
In the current implementation, CAN drivers need to #include <linux/can.h>
_before_ they #include <linux/can/dev.h>, which is both ugly and
unnecessary.
Fix this by including <linux/can.h> in <linux/can/dev.h> and remove the
#include <linux/can.h> lines from drivers.
Signed-off-by: Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
drivers/net/can/at91_can.c | 1 -
drivers/net/can/bfin_can.c | 1 -
drivers/net/can/mcp251x.c | 1 -
drivers/net/can/mscan/mpc5xxx_can.c | 1 -
drivers/net/can/mscan/mscan.c | 1 -
drivers/net/can/sja1000/ems_pci.c | 1 -
drivers/net/can/sja1000/kvaser_pci.c | 1 -
drivers/net/can/sja1000/plx_pci.c | 1 -
drivers/net/can/sja1000/sja1000.c | 1 -
drivers/net/can/sja1000/sja1000_isa.c | 1 -
drivers/net/can/sja1000/sja1000_of_platform.c | 1 -
drivers/net/can/sja1000/sja1000_platform.c | 1 -
drivers/net/can/ti_hecc.c | 1 -
include/linux/can/dev.h | 1 +
14 files changed, 1 insertion(+), 13 deletions(-)
Index: net-next-2.6/include/linux/can/dev.h
===================================================================
--- net-next-2.6.orig/include/linux/can/dev.h 2010-04-13 11:27:39.000000000 +0200
+++ net-next-2.6/include/linux/can/dev.h 2010-04-13 11:31:57.000000000 +0200
@@ -14,6 +14,7 @@
#ifndef CAN_DEV_H
#define CAN_DEV_H
+#include <linux/can.h>
#include <linux/can/netlink.h>
#include <linux/can/error.h>
Index: net-next-2.6/drivers/net/can/at91_can.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/at91_can.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/at91_can.c 2010-04-13 11:31:57.000000000 +0200
@@ -35,7 +35,6 @@
#include <linux/string.h>
#include <linux/types.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/error.h>
Index: net-next-2.6/drivers/net/can/bfin_can.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/bfin_can.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/bfin_can.c 2010-04-13 11:31:57.000000000 +0200
@@ -18,7 +18,6 @@
#include <linux/skbuff.h>
#include <linux/platform_device.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/error.h>
Index: net-next-2.6/drivers/net/can/mcp251x.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/mcp251x.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/mcp251x.c 2010-04-13 11:31:57.000000000 +0200
@@ -58,7 +58,6 @@
*
*/
-#include <linux/can.h>
#include <linux/can/core.h>
#include <linux/can/dev.h>
#include <linux/can/platform/mcp251x.h>
Index: net-next-2.6/drivers/net/can/ti_hecc.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/ti_hecc.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/ti_hecc.c 2010-04-13 11:31:57.000000000 +0200
@@ -47,7 +47,6 @@
#include <linux/platform_device.h>
#include <linux/clk.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/error.h>
#include <linux/can/platform/ti_hecc.h>
Index: net-next-2.6/drivers/net/can/mscan/mpc5xxx_can.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/mscan/mpc5xxx_can.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/mscan/mpc5xxx_can.c 2010-04-13 11:31:57.000000000 +0200
@@ -25,7 +25,6 @@
#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/netdevice.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/of_platform.h>
#include <sysdev/fsl_soc.h>
Index: net-next-2.6/drivers/net/can/mscan/mscan.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/mscan/mscan.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/mscan/mscan.c 2010-04-13 11:31:57.000000000 +0200
@@ -28,7 +28,6 @@
#include <linux/if_arp.h>
#include <linux/if_ether.h>
#include <linux/list.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/error.h>
#include <linux/io.h>
Index: net-next-2.6/drivers/net/can/sja1000/ems_pci.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/ems_pci.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/ems_pci.c 2010-04-13 11:31:57.000000000 +0200
@@ -24,7 +24,6 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/pci.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/io.h>
Index: net-next-2.6/drivers/net/can/sja1000/kvaser_pci.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/kvaser_pci.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/kvaser_pci.c 2010-04-13 11:31:57.000000000 +0200
@@ -36,7 +36,6 @@
#include <linux/netdevice.h>
#include <linux/delay.h>
#include <linux/pci.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/io.h>
Index: net-next-2.6/drivers/net/can/sja1000/plx_pci.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/plx_pci.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/plx_pci.c 2010-04-13 11:31:57.000000000 +0200
@@ -27,7 +27,6 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/pci.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/io.h>
Index: net-next-2.6/drivers/net/can/sja1000/sja1000.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/sja1000.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/sja1000.c 2010-04-13 11:31:57.000000000 +0200
@@ -60,7 +60,6 @@
#include <linux/skbuff.h>
#include <linux/delay.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/error.h>
Index: net-next-2.6/drivers/net/can/sja1000/sja1000_isa.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/sja1000_isa.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/sja1000_isa.c 2010-04-13 11:31:57.000000000 +0200
@@ -23,7 +23,6 @@
#include <linux/delay.h>
#include <linux/irq.h>
#include <linux/io.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/platform/sja1000.h>
Index: net-next-2.6/drivers/net/can/sja1000/sja1000_of_platform.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/sja1000_of_platform.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/sja1000_of_platform.c 2010-04-13 11:31:57.000000000 +0200
@@ -38,7 +38,6 @@
#include <linux/interrupt.h>
#include <linux/netdevice.h>
#include <linux/delay.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/of_platform.h>
Index: net-next-2.6/drivers/net/can/sja1000/sja1000_platform.c
===================================================================
--- net-next-2.6.orig/drivers/net/can/sja1000/sja1000_platform.c 2010-04-13 11:27:33.000000000 +0200
+++ net-next-2.6/drivers/net/can/sja1000/sja1000_platform.c 2010-04-13 11:31:57.000000000 +0200
@@ -24,7 +24,6 @@
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/irq.h>
-#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/platform/sja1000.h>
#include <linux/io.h>
^ permalink raw reply
* Re: forcedeth driver hangs under heavy load
From: stephen mulcahy @ 2010-04-13 10:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Ben Hutchings, Ayaz Abdulla, 572201
In-Reply-To: <1271091581.16881.41.camel@edumazet-laptop>
Eric Dumazet wrote:
> OK it seems forcedeth has problem with checksums ?
>
> Try to change "ethtool -k eth0" settings ?
>
> ethtool -K eth0 tso off tx off
Yes, that makes an unresponsive system responsive again immediately, nice!
Should the driver default to disabling this until we problem is corrected?
-stephen
^ permalink raw reply
* [PATCH net-next-2.6] chelsio: Fix build warning.
From: David Miller @ 2010-04-13 10:08 UTC (permalink / raw)
To: netdev
GCC warns that:
drivers/net/chelsio/sge.c:463:11: warning: operation on 's->port' may be undefined
Better to eliminate the side effects in the calculation and
express what was intended here.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/chelsio/sge.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/chelsio/sge.c b/drivers/net/chelsio/sge.c
index 475304f..a8ffc1e 100644
--- a/drivers/net/chelsio/sge.c
+++ b/drivers/net/chelsio/sge.c
@@ -460,7 +460,7 @@ static struct sk_buff *sched_skb(struct sge *sge, struct sk_buff *skb,
again:
for (i = 0; i < MAX_NPORTS; i++) {
- s->port = ++s->port & (MAX_NPORTS - 1);
+ s->port = (s->port + 1) & (MAX_NPORTS - 1);
skbq = &s->p[s->port].skbq;
skb = skb_peek(skbq);
--
1.7.0.4
^ permalink raw reply related
* Re: [net-2.6 PATCH] igb: restrict WoL for 82576 ET2 Quad Port Server Adapter
From: David Miller @ 2010-04-13 10:09 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann, alexander.h.duyck
In-Reply-To: <20100409195029.30616.21221.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 09 Apr 2010 12:51:34 -0700
> From: Stefan Assmann <sassmann@redhat.com>
>
> Restrict Wake-on-LAN to first port on 82576 ET2 quad port NICs, as it is
> only supported there.
>
> Signed-off-by: Stefan Assmann <sassmann@redhat.com>
> Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: [net-2.6 PATCH] e1000e: stop cleaning when we reach tx_ring->next_to_use
From: David Miller @ 2010-04-13 10:09 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, terry.loftin, bruce.w.allan
In-Reply-To: <20100409202917.31293.79618.stgit@localhost.localdomain>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 09 Apr 2010 13:29:49 -0700
> From: Terry Loftin <terry.loftin@hp.com>
>
> Tx ring buffers after tx_ring->next_to_use are volatile and could
> change, possibly causing a crash. Stop cleaning when we hit
> tx_ring->next_to_use.
>
> Signed-off-by: Terry Loftin <terry.loftin@hp.com>
> Acked-by: Bruce Allan <bruce.w.allan@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply
* Re: Linux 2.6.34-rc3 + CAN build problem
From: David Miller @ 2010-04-13 10:09 UTC (permalink / raw)
To: eric.dumazet
Cc: nm127, socketcan, oliver.hartkopp, urs.thuermann, socketcan-core,
netdev, linux-kernel
In-Reply-To: <1270892851.2093.32.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 10 Apr 2010 11:47:31 +0200
> [PATCH] can: avoids a false warning
>
> At this point optlen == sizeof(sfilter) but some compilers are dumb.
>
> Reported-by: Németh Márton <nm127@freemail.h
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH kernel 2.6.34-rc3-git9] smc91c92_cs: define multicast_table as unsigned char
From: David Miller @ 2010-04-13 10:09 UTC (permalink / raw)
To: ken_kawasaki; +Cc: netdev
In-Reply-To: <20100411075014.d3e80847.ken_kawasaki@spring.nifty.jp>
From: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
Date: Sun, 11 Apr 2010 07:50:14 +0900
>
> smc91c92_cs:
> * define multicast_table as unsigned char
> * remove unnecessary "#ifndef final_version"
>
> Signed-off-by: Ken Kawasaki <ken_kawasaki@spring.nifty.jp>
It is clear there are not too many testers of multicast
with this driver :-)
Thanks for fixing this bug, patch applied.
^ permalink raw reply
* SO_REUSEADDR with UDP (again)
From: Michal Svoboda @ 2010-04-13 9:34 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1447 bytes --]
Hello,
(redirected here from LKML)
I found SO_REUSEADDR on UDP sockets to behave somewhat nasty. If you
create a UDP socket with that flag and bind it to a port, then anyone
doing the same later will "steal" your packets, ie.
1. process A binds to port 12345 with SO_REUSEADDR, packets to that port
go to process A
2. process B binds to port 12345 with SO_REUSEADDR, packets to that port
now go to process B
3. A dies, fires up again, packets go back to A
4. A dies, does not fire up, packets go to B, as if they were stacked
And this works even if A and B are owned by different users, thus anyone
can "steal" packets from anyone as long as they use SO_REUSEADDR.
However, in most programs that's the default.
Furthermore, one can lock-out a particular source from being "stolen" by
using connect() to that source, ie.
1. process A binds to port 12345 with SO_REUSEADDR, gets the packets
2. B does the same, gets the packets, but also connect()s to the source
of the packets
3. A can now restart or try to bind again, but does not get the packets
(from that source)
(I haven't tested the case if A also issues a connect() even if it does
not receive packets.)
All of this seems confusing to me, and the fact that users can steal
packets from each other seems like a mild security risk. I've found some
discussions about this from circa 2002, but the above cases were not
mentioned. So - a problem or not?
Michal Svoboda
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH v2] net: batch skb dequeueing from softnet input_pkt_queue
From: Eric Dumazet @ 2010-04-13 10:19 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <o2v412e6f7f1004130250rdbe17c5dr5c11c3cbbb2f6dbb@mail.gmail.com>
Le mardi 13 avril 2010 à 17:50 +0800, Changli Gao a écrit :
> On Tue, Apr 13, 2010 at 4:08 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> > Probably not necessary.
> >
> >> + volatile bool flush_processing_queue;
> >
> > Use of 'volatile' is strongly discouraged, I would say, forbidden.
> >
>
> volatile is used to avoid compiler optimization.
volatile might be used on special macros only, not to guard a variable.
volatile was pre SMP days. We need something better defined these days.
>
> > Its usually a sign of 'I dont exactly what memory ordering I need, so I
> > throw a volatile just in case'. We live in a world full of RCU, read ,
> > write, full barriers. And these apis are well documented.
> >
>
> There isn't memory accessing order problem.
>
Really ? If you persist using this volatile thing, then you'll have to
convince Mr Linus Torvald himself. Prepare your best guns, because in
fact you'll have to convince about one hundred people that know for sure
how volatile is weak.
> >> @@ -2803,6 +2808,7 @@ static void flush_backlog(void *arg)
> >> __skb_unlink(skb, &queue->input_pkt_queue);
> >> kfree_skb(skb);
> >> }
> >> + queue->flush_processing_queue = true;
> >
> > Probably not necessary
> >
>
> If flush_backlog() is called when there are still packets in
> processing_queue, there maybe some packets refer to the netdev gone,
> if we remove this line.
We dont need this "processing_queue". Once you remove it, there is no
extra work to perform.
>
> >> rps_unlock(queue);
> >> }
> >>
> >> @@ -3112,14 +3118,23 @@ static int process_backlog(struct napi_struct *napi, int quota)
> >> struct softnet_data *queue = &__get_cpu_var(softnet_data);
> >> unsigned long start_time = jiffies;
> >>
> >> + if (queue->flush_processing_queue) {
> >
> > Really... this is bloat IMHO
>
>
> Any better idea?
>
Yes, I have explained it to you.
> >
> >>
> >
> > I advise to keep it simple.
> >
> > My suggestion would be to limit this patch only to process_backlog().
> >
> > Really if you touch other areas, there is too much risk.
> >
> > Perform sort of skb_queue_splice_tail_init() into a local (stack) queue,
> > but the trick is to not touch input_pkt_queue.qlen, so that we dont slow
> > down enqueue_to_backlog().
> >
> > Process at most 'quota' skbs (or jiffies limit).
> >
> > relock queue.
> > input_pkt_queue.qlen -= number_of_handled_skbs;
> >
>
> Oh no, in order to let latter packets in as soon as possible, we have
> to update qlen immediately.
>
Absolutely not. You missed something apparently.
You pay the price at each packet enqueue, because you have to compute
the sum of two lengthes, and guess what, if you do this you have a cache
line miss in one of the operand. Your patch as is is suboptimal.
Remember : this batch mode should not change packet queueing at all,
only speed it because of less cache line misses.
^ permalink raw reply
* Re: [PATCH v2] net: batch skb dequeueing from softnet input_pkt_queue
From: Eric Dumazet @ 2010-04-13 10:23 UTC (permalink / raw)
To: Changli Gao; +Cc: David S. Miller, netdev
In-Reply-To: <1271153942.16881.233.camel@edumazet-laptop>
Le mardi 13 avril 2010 à 12:19 +0200, Eric Dumazet a écrit :
> Le mardi 13 avril 2010 à 17:50 +0800, Changli Gao a écrit :
> > On Tue, Apr 13, 2010 at 4:08 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > Use of 'volatile' is strongly discouraged, I would say, forbidden.
> > >
> >
> > volatile is used to avoid compiler optimization.
>
> volatile might be used on special macros only, not to guard a variable.
> volatile was pre SMP days. We need something better defined these days.
>
> >
> > > Its usually a sign of 'I dont exactly what memory ordering I need, so I
> > > throw a volatile just in case'. We live in a world full of RCU, read ,
> > > write, full barriers. And these apis are well documented.
> > >
> >
> > There isn't memory accessing order problem.
> >
>
> Really ? If you persist using this volatile thing, then you'll have to
> convince Mr Linus Torvald himself. Prepare your best guns, because in
> fact you'll have to convince about one hundred people that know for sure
> how volatile is weak.
$ cat Documentation/volatile-considered-harmful.txt
Why the "volatile" type class should not be used
------------------------------------------------
C programmers have often taken volatile to mean that the variable could be
changed outside of the current thread of execution; as a result, they are
sometimes tempted to use it in kernel code when shared data structures are
being used. In other words, they have been known to treat volatile types
as a sort of easy atomic variable, which they are not. The use of volatile in
kernel code is almost never correct; this document describes why.
The key point to understand with regard to volatile is that its purpose is
to suppress optimization, which is almost never what one really wants to
do. In the kernel, one must protect shared data structures against
unwanted concurrent access, which is very much a different task. The
process of protecting against unwanted concurrency will also avoid almost
all optimization-related problems in a more efficient way.
Like volatile, the kernel primitives which make concurrent access to data
safe (spinlocks, mutexes, memory barriers, etc.) are designed to prevent
unwanted optimization. If they are being used properly, there will be no
need to use volatile as well. If volatile is still necessary, there is
almost certainly a bug in the code somewhere. In properly-written kernel
code, volatile can only serve to slow things down.
Consider a typical block of kernel code:
spin_lock(&the_lock);
do_something_on(&shared_data);
do_something_else_with(&shared_data);
spin_unlock(&the_lock);
If all the code follows the locking rules, the value of shared_data cannot
change unexpectedly while the_lock is held. Any other code which might
want to play with that data will be waiting on the lock. The spinlock
primitives act as memory barriers - they are explicitly written to do so -
meaning that data accesses will not be optimized across them. So the
compiler might think it knows what will be in shared_data, but the
spin_lock() call, since it acts as a memory barrier, will force it to
forget anything it knows. There will be no optimization problems with
accesses to that data.
If shared_data were declared volatile, the locking would still be
necessary. But the compiler would also be prevented from optimizing access
to shared_data _within_ the critical section, when we know that nobody else
can be working with it. While the lock is held, shared_data is not
volatile. When dealing with shared data, proper locking makes volatile
unnecessary - and potentially harmful.
The volatile storage class was originally meant for memory-mapped I/O
registers. Within the kernel, register accesses, too, should be protected
by locks, but one also does not want the compiler "optimizing" register
accesses within a critical section. But, within the kernel, I/O memory
accesses are always done through accessor functions; accessing I/O memory
directly through pointers is frowned upon and does not work on all
architectures. Those accessors are written to prevent unwanted
optimization, so, once again, volatile is unnecessary.
Another situation where one might be tempted to use volatile is
when the processor is busy-waiting on the value of a variable. The right
way to perform a busy wait is:
while (my_variable != what_i_want)
cpu_relax();
The cpu_relax() call can lower CPU power consumption or yield to a
hyperthreaded twin processor; it also happens to serve as a compiler
barrier, so, once again, volatile is unnecessary. Of course, busy-
waiting is generally an anti-social act to begin with.
There are still a few rare situations where volatile makes sense in the
kernel:
- The above-mentioned accessor functions might use volatile on
architectures where direct I/O memory access does work. Essentially,
each accessor call becomes a little critical section on its own and
ensures that the access happens as expected by the programmer.
- Inline assembly code which changes memory, but which has no other
visible side effects, risks being deleted by GCC. Adding the volatile
keyword to asm statements will prevent this removal.
- The jiffies variable is special in that it can have a different value
every time it is referenced, but it can be read without any special
locking. So jiffies can be volatile, but the addition of other
variables of this type is strongly frowned upon. Jiffies is considered
to be a "stupid legacy" issue (Linus's words) in this regard; fixing it
would be more trouble than it is worth.
- Pointers to data structures in coherent memory which might be modified
by I/O devices can, sometimes, legitimately be volatile. A ring buffer
used by a network adapter, where that adapter changes pointers to
indicate which descriptors have been processed, is an example of this
type of situation.
For most code, none of the above justifications for volatile apply. As a
result, the use of volatile is likely to be seen as a bug and will bring
additional scrutiny to the code. Developers who are tempted to use
volatile should take a step back and think about what they are truly trying
to accomplish.
Patches to remove volatile variables are generally welcome - as long as
they come with a justification which shows that the concurrency issues have
been properly thought through.
NOTES
-----
[1] http://lwn.net/Articles/233481/
[2] http://lwn.net/Articles/233482/
CREDITS
-------
Original impetus and research by Randy Dunlap
Written by Jonathan Corbet
Improvements via comments from Satyam Sharma, Johannes Stezenbach, Jesper
Juhl, Heikki Orsila, H. Peter Anvin, Philipp Hahn, and Stefan
Richter.
^ permalink raw reply
* Re: [PATCH 1/2] PHY: fix typo in bcm63xx PHY driver table
From: David Miller @ 2010-04-13 10:30 UTC (permalink / raw)
To: ffainelli; +Cc: netdev
In-Reply-To: <201004091304.45392.ffainelli@freebox.fr>
From: Florian Fainelli <ffainelli@freebox.fr>
Date: Fri, 9 Apr 2010 13:04:45 +0200
>
> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] bcm63xx_enet: do not overwrite ENET_CTL_REG value
From: David Miller @ 2010-04-13 10:30 UTC (permalink / raw)
To: ffainelli; +Cc: netdev, mbizon
In-Reply-To: <201004091304.52540.ffainelli@freebox.fr>
From: Florian Fainelli <ffainelli@freebox.fr>
Date: Fri, 9 Apr 2010 13:04:52 +0200
> bcm_enet_hw_preinit will correctly set values in ENET_CTL_REG for internal
> or external MII operations, however, bcm_enet_open will blindly overwrite the
> ENET_CTL_REG register value and thus we will loose any changes to it that
> were made in bcm_enet_hw_preinit, rendering external MII operations non-working.
>
> This would lead to the driver not being able to check for link availability on
> external PHY setups, and thus we would never get to sending packets because
> link was down from the driver side.
>
> This was completely un-noticed because all boards out there but BCM6338-based
> ones use internal phy on their enet0 interface.
>
> Signed-off-by: Florian Fainelli <ffainelli@freebox.fr>
Applied.
^ permalink raw reply
* Re: [PATCH - resubmit] Fix some #includes in CAN drivers (rebased for net-next-2.6)
From: David Miller @ 2010-04-13 10:33 UTC (permalink / raw)
To: hjk
Cc: netdev, socketcan-core, kernel, 21cnbao, celston, chripell, wg,
haas, per.dalen, P.B.Cheblakov, oliver.hartkopp, anantgole
In-Reply-To: <20100413100324.GC2011@bluebox.local>
From: "Hans J. Koch" <hjk@linutronix.de>
Date: Tue, 13 Apr 2010 12:03:25 +0200
> In the current implementation, CAN drivers need to #include <linux/can.h>
> _before_ they #include <linux/can/dev.h>, which is both ugly and
> unnecessary.
>
> Fix this by including <linux/can.h> in <linux/can/dev.h> and remove the
> #include <linux/can.h> lines from drivers.
>
> Signed-off-by: Hans J. Koch <hjk@linutronix.de>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next-2.6] drivers: net: last_rx elimination
From: David Miller @ 2010-04-13 10:33 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1270975694.2078.2.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 11 Apr 2010 10:48:14 +0200
> Network drivers do not have to update last_rx, unless they need it for
> their private use.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks for taking care of this Eric.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: uninline skb_bond_should_drop()
From: David Miller @ 2010-04-13 10:33 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1271004971.2078.96.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 11 Apr 2010 18:56:11 +0200
> skb_bond_should_drop() is too big to be inlined.
>
> This patch reduces kernel text size, and its compilation time as well
> (shrinking include/linux/netdevice.h)
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] be2net: clarify promiscuous cmd with a comment
From: David Miller @ 2010-04-13 10:34 UTC (permalink / raw)
To: sathyap; +Cc: netdev, pugs
In-Reply-To: <20100412083527.GA27023@serverengines.com>
From: Sathya Perla <sathyap@serverengines.com>
Date: Mon, 12 Apr 2010 14:05:27 +0530
> The promiscous cmd config code gives an impression that
> setting a port to promisc mode will unset the other port.
> This is not the case and is clarified with a comment.
>
> Signed-off-by: Sathya Perla <sathyap@serverengines.com>
Applied.
^ permalink raw reply
* Re: [PATCH] dst: don't inline dst_ifdown
From: David Miller @ 2010-04-13 10:34 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20100412103805.13ac6339@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 12 Apr 2010 10:38:05 -0700
> The function dst_ifdown is called only two places but in a non-
> performance critical code path, there is no reason to inline it.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: SO_REUSEADDR with UDP (again)
From: Eric Dumazet @ 2010-04-13 10:39 UTC (permalink / raw)
To: Michal Svoboda; +Cc: netdev
In-Reply-To: <20100413093408.GA16595@myhost.felk.cvut.cz>
Le mardi 13 avril 2010 à 11:34 +0200, Michal Svoboda a écrit :
> Hello,
>
> (redirected here from LKML)
>
> I found SO_REUSEADDR on UDP sockets to behave somewhat nasty. If you
> create a UDP socket with that flag and bind it to a port, then anyone
> doing the same later will "steal" your packets, ie.
>
> 1. process A binds to port 12345 with SO_REUSEADDR, packets to that port
> go to process A
why process A sets SO_REUSEADDR ?
> 2. process B binds to port 12345 with SO_REUSEADDR, packets to that port
> now go to process B
Or not... this is implementation (un)defined.
> 3. A dies, fires up again, packets go back to A
Or not. Not documented.
> 4. A dies, does not fire up, packets go to B, as if they were stacked
>
> And this works even if A and B are owned by different users, thus anyone
> can "steal" packets from anyone as long as they use SO_REUSEADDR.
> However, in most programs that's the default.
Really ? They are very buggy then.
>
> Furthermore, one can lock-out a particular source from being "stolen" by
> using connect() to that source, ie.
>
> 1. process A binds to port 12345 with SO_REUSEADDR, gets the packets
> 2. B does the same, gets the packets, but also connect()s to the source
> of the packets
> 3. A can now restart or try to bind again, but does not get the packets
> (from that source)
>
> (I haven't tested the case if A also issues a connect() even if it does
> not receive packets.)
>
> All of this seems confusing to me, and the fact that users can steal
> packets from each other seems like a mild security risk. I've found some
> discussions about this from circa 2002, but the above cases were not
> mentioned. So - a problem or not?
>
>
Why do you use REUSEADDR ? This is doing what is documented.
SO_REUSEADDR
Indicates that the rules used in validating addresses supplied
in a bind(2) call should allow reuse of local addresses. For
AF_INET sockets this means that a socket may bind, except when
there is an active listening socket bound to the address. When
the listening socket is bound to INADDR_ANY with a specific
port then it is not possible to bind to this port for any local
address. Argument is an integer boolean flag.
An UDP application wanting a port for its exclusive use dont set
REUSEADDR, or basically allows anybody to bind an udp socket to same
port, and potentially steal incoming frames.
REUSEADDR is usually used when an application has several sockets bound
to same port, but different IP addresses (or bound to different devices)
^ permalink raw reply
* Re: forcedeth driver hangs under heavy load
From: Eric Dumazet @ 2010-04-13 10:49 UTC (permalink / raw)
To: stephen mulcahy; +Cc: netdev, Ben Hutchings, Ayaz Abdulla, 572201
In-Reply-To: <4BC44167.4080807@gmail.com>
Le mardi 13 avril 2010 à 11:03 +0100, stephen mulcahy a écrit :
> Eric Dumazet wrote:
> > OK it seems forcedeth has problem with checksums ?
> >
> > Try to change "ethtool -k eth0" settings ?
> >
> > ethtool -K eth0 tso off tx off
>
> Yes, that makes an unresponsive system responsive again immediately, nice!
>
> Should the driver default to disabling this until we problem is corrected?
>
> -stephen
Both flags need to be disabled, or only one is OK ?
^ permalink raw reply
* Re: forcedeth driver hangs under heavy load
From: stephen mulcahy @ 2010-04-13 11:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Ben Hutchings, Ayaz Abdulla, 572201
In-Reply-To: <1271155766.16881.245.camel@edumazet-laptop>
Eric Dumazet wrote:
> Le mardi 13 avril 2010 à 11:03 +0100, stephen mulcahy a écrit :
>> Eric Dumazet wrote:
>>> OK it seems forcedeth has problem with checksums ?
>>>
>>> Try to change "ethtool -k eth0" settings ?
>>>
>>> ethtool -K eth0 tso off tx off
>> Yes, that makes an unresponsive system responsive again immediately, nice!
>>
>> Should the driver default to disabling this until we problem is corrected?
>>
>> -stephen
>
> Both flags need to be disabled, or only one is OK ?
ethtool -K eth0 tx off
fixes the problem (without tso)
but running
ethtool -k eth0
Offload parameters for eth0:
rx-checksumming: on
tx-checksumming: off
scatter-gather: off
tcp-segmentation-offload: off
udp-fragmentation-offload: off
generic-segmentation-offload: on
generic-receive-offload: off
large-receive-offload: off
seems to indicate that tso is also disabled by this - does that sound
correct?
-stephen
^ permalink raw reply
* Re: [RFC PATCH 1/9] net: fib_rules: consolidate IPv4 and DECnet ->default_pref() functions.
From: David Miller @ 2010-04-13 11:06 UTC (permalink / raw)
To: kaber; +Cc: netdev
In-Reply-To: <1271007435-20035-2-git-send-email-kaber@trash.net>
From: kaber@trash.net
Date: Sun, 11 Apr 2010 19:37:07 +0200
> From: Patrick McHardy <kaber@trash.net>
>
> Both functions are equivalent, consolidate them since a following patch
> needs a third implementation for multicast routing.
>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
This patch is fine.
^ permalink raw reply
* [PATCH v2] xtables: make XT_ALIGN() usable in exported headers by exporting __ALIGN_KERNEL()
From: Alexey Dobriyan @ 2010-04-13 11:06 UTC (permalink / raw)
To: Patrick McHardy
Cc: linux-kernel, netdev, shemminger, bhutchings, andreas, hadi,
hideaki
In-Reply-To: <4BC44117.80901@trash.net>
XT_ALIGN() was rewritten through ALIGN() by commit 42107f5009da223daa800d6da6904d77297ae829
"netfilter: xtables: symmetric COMPAT_XT_ALIGN definition".
ALIGN() is not exported in userspace headers, which created compile problem for tc(8)
and will create problem for iptables(8).
We can't export generic looking name ALIGN() but we can export less generic
__ALIGN_KERNEL() (suggested by Ben Hutchings).
Google knows nothing about __ALIGN_KERNEL().
COMPAT_XT_ALIGN() changed for symmetry.
Reported-by: Andreas Henriksson <andreas@fatal.se>
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
include/linux/kernel.h | 6 ++++--
include/linux/netfilter/x_tables.h | 6 +++---
2 files changed, 7 insertions(+), 5 deletions(-)
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -4,6 +4,8 @@
/*
* 'kernel.h' contains some often-used function prototypes etc
*/
+#define __ALIGN_KERNEL(x, a) __ALIGN_KERNEL_MASK(x, (typeof(x))(a) - 1)
+#define __ALIGN_KERNEL_MASK(x, mask) (((x) + (mask)) & ~(mask))
#ifdef __KERNEL__
@@ -37,8 +39,8 @@ extern const char linux_proc_banner[];
#define STACK_MAGIC 0xdeadbeef
-#define ALIGN(x,a) __ALIGN_MASK(x,(typeof(x))(a)-1)
-#define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
+#define ALIGN(x, a) __ALIGN_KERNEL((x), (a))
+#define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask))
#define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))
#define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)
diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index 84c7c92..f01ddbe 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -1,6 +1,6 @@
#ifndef _X_TABLES_H
#define _X_TABLES_H
-
+#include <linux/kernel.h>
#include <linux/types.h>
#define XT_FUNCTION_MAXNAMELEN 30
@@ -93,7 +93,7 @@ struct _xt_align {
__u64 u64;
};
-#define XT_ALIGN(s) ALIGN((s), __alignof__(struct _xt_align))
+#define XT_ALIGN(s) __ALIGN_KERNEL((s), __alignof__(struct _xt_align))
/* Standard return verdict, or do jump. */
#define XT_STANDARD_TARGET ""
@@ -598,7 +598,7 @@ struct _compat_xt_align {
compat_u64 u64;
};
-#define COMPAT_XT_ALIGN(s) ALIGN((s), __alignof__(struct _compat_xt_align))
+#define COMPAT_XT_ALIGN(s) __ALIGN_KERNEL((s), __alignof__(struct _compat_xt_align))
extern void xt_compat_lock(u_int8_t af);
extern void xt_compat_unlock(u_int8_t af);
^ 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