* Packet capture and Bonding asymmetries
From: Paul LeoNerd Evans @ 2010-06-09 21:27 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]
We use ethernet bonding to bond eth0 + eth1 into bond0, in an
active/standby failover pair. Given this is for redundancy, we put the
two physical ethernet links into different switches that follow
different paths in the data centre.
Given this topology, it can be really useful to know which physical
interface packets are received on. It seems the bonding driver doesn't
make this happen:
# uname -r
2.6.31.12
# head -1 /proc/net/bonding/bond0
Ethernet Channel Bonding Driver: v3.5.0 (November 4, 2008)
# pktdump -f icmp
[15:27:12] RX(bond0): ICMP| 192.168.57.6->192.168.57.1 echo-request seq=1
[15:27:12] TX(bond0): ICMP| 192.168.57.1->192.168.57.6 echo-reply seq=1
[15:27:12] TX(eth0): ICMP| 192.168.57.1->192.168.57.6 echo-reply seq=1
I.e. when we transmit we see both the virtual bond0 interface and the
physical eth0 doing so; but when we receive only the virtual bond0
appears to do so.
I believe this should be fixable with a one-line patch; just adding a
call to netif_nit_deliver(skb) from within the bonding driver... though
just offhand I'm unable to find exactly the line where packets received
on slaves gets passed up to the master. :)
Can anyone advise on the sensibility or otherwise of this plan? I really
would like the behaviour where I can see how packets are received - is
this a good plan to acheive it?
I may sometime have a hack at writing a patch for this anyway, presuming
no major objections...
--
Paul "LeoNerd" Evans
leonerd@leonerd.org.uk
ICQ# 4135350 | Registered Linux# 179460
http://www.leonerd.org.uk/
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]
^ permalink raw reply
* Re: [PATCH v3] netfilter: Xtables: idletimer target implementation
From: Luciano Coelho @ 2010-06-09 21:28 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: ext Patrick McHardy, netfilter-devel@vger.kernel.org,
netdev@vger.kernel.org, Timo Teras
In-Reply-To: <alpine.LSU.2.01.1006092304270.16916@obet.zrqbmnf.qr>
On Wed, 2010-06-09 at 23:05 +0200, ext Jan Engelhardt wrote:
> On Wednesday 2010-06-09 20:42, Luciano Coelho wrote:
> >
> >> I'll move the sysfs file creation to outside that function so I can keep
> >> the lock until after the timer is added to the list. Thanks for
> >> clarifying!
> >
> >Hmmm... after struggling with this for a while, I think it's not really
> >possible to simply create the sysfs file outside of the lock, because if
> >the sysfs creation fails, we will again risk a race condition.
>
> Well if sysfs_add can return an error code when a file already
> exists (instead of adding it again), it's much easier. Try checking.
Unfortunately sysfs_add and sysfs_create will throw a kernel warning if
we try to create a file that already exists. But this was not really
the problem.
Now I just throw an WARN_ON if the sysfs file creation fails, the other
sysfs functions I'm calling can handle this situation. This shouldn't
happen in normal cases and, if it does, the timer will run normally, but
there won't be a sysfs file associated with it (and there won't be any
notification).
As I just wrote in another email, I thought I had figured out a way to
do it without a workqueue. But now that I looked into the code again, I
think there might still be some race conditions... For example: If
someone deletes the timer immediately after I released the lock and
before I created the sysfs entry... The deletion won't cause problems if
the file is not there, it will just nop. But the creation of the file
after the timer has been deleted will cause the file to be dangling in
sysfs without any timer associated with it... :(
--
Cheers,
Luca.
^ permalink raw reply
* [PATCH 1/2 -next] of_device.h: provide struct of_device even when not enabled
From: Randy Dunlap @ 2010-06-09 22:44 UTC (permalink / raw)
To: David Miller, Grant Likely; +Cc: sfr, netdev, linux-next, linux-kernel
In-Reply-To: <20100609.110638.112605100.davem@davemloft.net>
From: Randy Dunlap <randy.dunlap@oracle.com>
Drivers may use struct of_device (struct platform_device), even when
CONFIG_OF_DEVICE is not enabled, so minimally provide that struct
for that kconfig case.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Dave Miller <davem@davemloft.net>
---
include/linux/of_device.h | 4 ++++
1 file changed, 4 insertions(+)
--- linux-next-20100609.orig/include/linux/of_device.h
+++ linux-next-20100609/include/linux/of_device.h
@@ -47,6 +47,10 @@ extern ssize_t of_device_get_modalias(st
extern int of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
+#else
+
+#include <linux/platform_device.h>
+#define of_device platform_device
#endif /* CONFIG_OF_DEVICE */
^ permalink raw reply
* [PATCH 2/2 -next] niu: always include of_device.h
From: Randy Dunlap @ 2010-06-09 22:44 UTC (permalink / raw)
To: David Miller, Grant Likely; +Cc: sfr, netdev, linux-next, linux-kernel
In-Reply-To: <20100609.110638.112605100.davem@davemloft.net>
From: Randy Dunlap <randy.dunlap@oracle.com>
The niu driver uses struct of_device when built on any arch, not
only SPARC64, so always #include <linux/of_device.h>.
drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list
drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want
drivers/net/niu.c:9716: warning: assignment from incompatible pointer type
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Dave Miller <davem@davemloft.net>
---
drivers/net/niu.c | 3 ---
1 file changed, 3 deletions(-)
--- linux-next-20100609.orig/drivers/net/niu.c
+++ linux-next-20100609/drivers/net/niu.c
@@ -28,10 +28,7 @@
#include <linux/slab.h>
#include <linux/io.h>
-
-#ifdef CONFIG_SPARC64
#include <linux/of_device.h>
-#endif
#include "niu.h"
^ permalink raw reply
* Re: Packet capture and Bonding asymmetries
From: Jay Vosburgh @ 2010-06-09 22:52 UTC (permalink / raw)
To: Paul LeoNerd Evans; +Cc: netdev
In-Reply-To: <20100609212704.GY11110@cel.leo>
Paul LeoNerd Evans <leonerd@leonerd.org.uk> wrote:
>We use ethernet bonding to bond eth0 + eth1 into bond0, in an
>active/standby failover pair. Given this is for redundancy, we put the
>two physical ethernet links into different switches that follow
>different paths in the data centre.
>
>Given this topology, it can be really useful to know which physical
>interface packets are received on. It seems the bonding driver doesn't
>make this happen:
>
> # uname -r
> 2.6.31.12
>
> # head -1 /proc/net/bonding/bond0
> Ethernet Channel Bonding Driver: v3.5.0 (November 4, 2008)
>
> # pktdump -f icmp
> [15:27:12] RX(bond0): ICMP| 192.168.57.6->192.168.57.1 echo-request seq=1
> [15:27:12] TX(bond0): ICMP| 192.168.57.1->192.168.57.6 echo-reply seq=1
> [15:27:12] TX(eth0): ICMP| 192.168.57.1->192.168.57.6 echo-reply seq=1
>
>I.e. when we transmit we see both the virtual bond0 interface and the
>physical eth0 doing so; but when we receive only the virtual bond0
>appears to do so.
>
>I believe this should be fixable with a one-line patch; just adding a
>call to netif_nit_deliver(skb) from within the bonding driver... though
>just offhand I'm unable to find exactly the line where packets received
>on slaves gets passed up to the master. :)
This won't work, because bonding does not have a receive
function in the usual sense. Instead, the slaves do their normal
receive logic, and then in __netif_receive_skb, packets are assigned to
the bonding master if the device is a slave.
On the TX side, packet capture can happen at both the bonding
device and at the slave, because the packet will pass through both.
>Can anyone advise on the sensibility or otherwise of this plan? I really
>would like the behaviour where I can see how packets are received - is
>this a good plan to acheive it?
For your own private testing, you could add a call to
__netif_nit_deliver in netif_receive_skb prior to this part:
master = ACCESS_ONCE(orig_dev->master);
if (master) {
if (skb_bond_should_drop(skb, master))
null_or_orig = orig_dev; /* deliver only exact match */
else
skb->dev = master;
}
This will give you multiple captures of the same packet, as is
seen for transmit (i.e., one on the slave, one on the bond). For
non-bonding devices, tcpdump will see each packet twice on the same
device, so it's not really suitable for general use.
>I may sometime have a hack at writing a patch for this anyway, presuming
>no major objections...
If merely knowing the traffic counts is sufficient, the slaves
do count their received packets individually, so, e.g., ifconfig will
show how many packets a particular slave has received, regardless of
what bonding does with them. The packet counts for the bonding device
itself are merely a sum of all of its slaves.
Also, generally speaking, IP protocol traffic that arrives on an
inactive bonding slave is not delivered. If you're using active-backup
mode, and your traffic makes it through, it likely arrived on the active
slave.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH net-next-2.6] phonet: use call_rcu for phonet device free
From: David Miller @ 2010-06-09 23:15 UTC (permalink / raw)
To: eric.dumazet; +Cc: remi, jpirko, netdev
In-Reply-To: <1275918583.2545.84.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 07 Jun 2010 15:49:43 +0200
> Le lundi 07 juin 2010 à 15:43 +0200, Rémi Denis-Courmont a écrit :
>> On Mon, 7 Jun 2010 15:27:39 +0200, Jiri Pirko <jpirko@redhat.com> wrote:
>> > Use call_rcu rather than synchronize_rcu.
>> >
>> > Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>
>> This looks fine to me, but what is the goal here? The RCU documentation
>> seems to imply that synchronize_rcu() is preferable over call_rcu() when at
>> all possible.
>>
>
> Thats not exactly that.
>
> synchronize_rcu() is easier, in respect of memory use.
> But its drawback is current thread is blocked for several milli seconds.
>
> In the end, call_rcu() is more scalable.
>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: David Miller @ 2010-06-09 23:17 UTC (permalink / raw)
To: richardcochran; +Cc: afleming, netdev
In-Reply-To: <20100607153932.GA2800@riccoc20.at.omicron.at>
From: Richard Cochran <richardcochran@gmail.com>
Date: Mon, 7 Jun 2010 17:39:32 +0200
> This patch implements a work around for Erratum 5, "3.3 V Fiber Speed
> Selection." If the hardware wiring does not respect this erratum, then
> fiber optic mode will not work properly.
>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Applied, thanks.
^ permalink raw reply
* Re: [patch] caif: fix a couple range checks
From: David Miller @ 2010-06-09 23:19 UTC (permalink / raw)
To: sjur.brandeland; +Cc: error27, netdev, kernel-janitors
In-Reply-To: <81C3A93C17462B4BBD7E272753C10579169C4D9E64@EXDCVYMBSTM005.EQ1STM.local>
From: Sjur BRENDELAND <sjur.brandeland@stericsson.com>
Date: Wed, 9 Jun 2010 11:11:38 +0200
>> From: Dan Carpenter [mailto:error27@gmail.com]
>>
>> The extra ! character means that these conditions are always false.
>
>
> Looks good, thanks. This is embarrassing, I should caught this ages ago.
> Acked-by: Sjur Braendeland <sjur.brandeland@stericsson.com>
Applied, thanks guys.
^ permalink raw reply
* Re: [PATCH] gianfar: Revive the driver for eTSEC devices (disable timestamping)
From: David Miller @ 2010-06-09 23:27 UTC (permalink / raw)
To: avorontsov; +Cc: netdev, Manfred.Rudigier, linuxppc-dev
In-Reply-To: <20100609193219.GA8629@oksana.dev.rtsoft.ru>
From: Anton Vorontsov <avorontsov@mvista.com>
Date: Wed, 9 Jun 2010 23:32:19 +0400
> Since commit cc772ab7cdcaa24d1fae332d92a1602788644f7a ("gianfar: Add
> hardware RX timestamping support"), the driver no longer works on
> at least MPC8313ERDB and MPC8568EMDS boards (and possibly much more
> boards as well).
>
> That's how MPC8313 Reference Manual describes RCTRL_TS_ENABLE bit:
>
> Timestamp incoming packets as padding bytes. PAL field is set
> to 8 if the PAL field is programmed to less than 8. Must be set
> to zero if TMR_CTRL[TE]=0.
>
> I see that the commit above sets this bit, but it doesn't handle
> TMR_CTRL. Manfred probably had this bit set by the firmware for
> his boards. But obviously this isn't true for all boards in the
> wild.
>
> Also, I recall that Freescale BSPs were explicitly disabling the
> timestamping because of a performance drop.
>
> For now, the best way to deal with this is just disable the
> timestamping, and later we can discuss proper device tree bindings
> and implement enabling this feature via some property.
>
> Signed-off-by: Anton Vorontsov <avorontsov@mvista.com>
Agreed, applied, thanks Anton.
^ permalink raw reply
* Re: 2.6.25-rc2 pull request, stop log noise from CONFIG_RPS
From: David Miller @ 2010-06-09 23:28 UTC (permalink / raw)
To: timg; +Cc: eric.dumazet, netdev
In-Reply-To: <20100609195110.02FEAF89E9@sepang.rtg.net>
From: timg@tpi.com (Tim Gardner)
Date: Wed, 9 Jun 2010 13:51:09 -0600 (MDT)
> Please consider applying this patch in order to stop
> log noise under certain configurations.
Pulled, thanks Tim.
^ permalink raw reply
* Re: [PATCH 2/2 -next] niu: always include of_device.h
From: Grant Likely @ 2010-06-09 23:45 UTC (permalink / raw)
To: Randy Dunlap; +Cc: David Miller, sfr, netdev, linux-next, linux-kernel
In-Reply-To: <20100609154409.bf024c0c.randy.dunlap@oracle.com>
On Wed, Jun 9, 2010 at 4:44 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
>
> The niu driver uses struct of_device when built on any arch, not
> only SPARC64, so always #include <linux/of_device.h>.
>
> drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list
> drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want
> drivers/net/niu.c:9716: warning: assignment from incompatible pointer type
>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Dave Miller <davem@davemloft.net>
> ---
> drivers/net/niu.c | 3 ---
Looks okay to me (but I haven't build tested it yet). The bulk of
of_device.h is compiled out when CONFIG_OF_DEVICE is not selected.
David, are you okay with me taking this via my tree as it depends on
Randy's other patch?
Thanks,
g.
> 1 file changed, 3 deletions(-)
>
> --- linux-next-20100609.orig/drivers/net/niu.c
> +++ linux-next-20100609/drivers/net/niu.c
> @@ -28,10 +28,7 @@
> #include <linux/slab.h>
>
> #include <linux/io.h>
> -
> -#ifdef CONFIG_SPARC64
> #include <linux/of_device.h>
> -#endif
>
> #include "niu.h"
>
>
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/2 -next] of_device.h: provide struct of_device even when not enabled
From: Grant Likely @ 2010-06-09 23:47 UTC (permalink / raw)
To: Randy Dunlap; +Cc: David Miller, sfr, netdev, linux-next, linux-kernel
In-Reply-To: <20100609154405.20095178.randy.dunlap@oracle.com>
On Wed, Jun 9, 2010 at 4:44 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> From: Randy Dunlap <randy.dunlap@oracle.com>
>
> Drivers may use struct of_device (struct platform_device), even when
> CONFIG_OF_DEVICE is not enabled, so minimally provide that struct
> for that kconfig case.
>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Dave Miller <davem@davemloft.net>
> ---
> include/linux/of_device.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> --- linux-next-20100609.orig/include/linux/of_device.h
> +++ linux-next-20100609/include/linux/of_device.h
> @@ -47,6 +47,10 @@ extern ssize_t of_device_get_modalias(st
>
> extern int of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
>
> +#else
> +
> +#include <linux/platform_device.h>
> +#define of_device platform_device
I should probably just move these 2 lines out of the #if/else/endif
block entirely. I'll make that change and test it out.
g.
^ permalink raw reply
* Re: [PATCH 2/2 -next] niu: always include of_device.h
From: David Miller @ 2010-06-10 0:29 UTC (permalink / raw)
To: grant.likely; +Cc: randy.dunlap, sfr, netdev, linux-next, linux-kernel
In-Reply-To: <AANLkTikuIgeSMC5_IzAQGEQRso4udBvBGPcioAiArWb5@mail.gmail.com>
From: Grant Likely <grant.likely@secretlab.ca>
Date: Wed, 9 Jun 2010 17:45:54 -0600
> On Wed, Jun 9, 2010 at 4:44 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>> From: Randy Dunlap <randy.dunlap@oracle.com>
>>
>> The niu driver uses struct of_device when built on any arch, not
>> only SPARC64, so always #include <linux/of_device.h>.
>>
>> drivers/net/niu.c:9700: warning: 'struct of_device' declared inside parameter list
>> drivers/net/niu.c:9700: warning: its scope is only this definition or declaration, which is probably not what you want
>> drivers/net/niu.c:9716: warning: assignment from incompatible pointer type
>>
>> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
>> Cc: Grant Likely <grant.likely@secretlab.ca>
>> Cc: Dave Miller <davem@davemloft.net>
>> ---
>> drivers/net/niu.c | 3 ---
>
> Looks okay to me (but I haven't build tested it yet). The bulk of
> of_device.h is compiled out when CONFIG_OF_DEVICE is not selected.
>
> David, are you okay with me taking this via my tree as it depends on
> Randy's other patch?
Yep, that's totally fine with me:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH] r8169: fix mdio_read and update mdio_write according to hw specs
From: David Miller @ 2010-06-10 0:32 UTC (permalink / raw)
To: romieu; +Cc: timo.teras, netdev, hayeswang
In-Reply-To: <20100609061825.GA7087@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed, 9 Jun 2010 08:18:25 +0200
> Timo Teräs <timo.teras@iki.fi> :
>> Realtek confirmed that a 20us delay is needed after mdio_read and
>> mdio_write operations. Reduce the delay in mdio_write, and add it
>> to mdio_read too. Also add a comment that the 20us is from hw specs.
>>
>> Signed-off-by: Timo Teräs <timo.teras@iki.fi>
>
> Acked-off-by: Francois Romieu <romieu@fr.zoreil.com>
I think you meant "Acked-by: " :-) I fixed this and committed
Timo's patch, thanks everyone!
^ permalink raw reply
* [PATCH] tc: revert "echo" in install target
From: Mike Frysinger @ 2010-06-10 0:52 UTC (permalink / raw)
To: stephen.hemminger, netdev; +Cc: Andreas Henriksson
The recent commit "iproute2: add option to build m_xt as a tc module"
(ab814d635529787) looks like it wrongly included debug changes in the
install target. So drop the `echo` so the tc binary actually gets
installed again.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
tc/Makefile | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tc/Makefile b/tc/Makefile
index 01a16fc..3aa9f26 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -99,10 +99,10 @@ libtc.a: $(TCLIB)
$(AR) rcs $@ $(TCLIB)
install: all
- echo mkdir -p $(MODDESTDIR)
- echo install -m 0755 tc $(DESTDIR)$(SBINDIR)
+ mkdir -p $(MODDESTDIR)
+ install -m 0755 tc $(DESTDIR)$(SBINDIR)
for i in $(TCSO); \
- do echo install -m 755 $$i $(MODDESTDIR); \
+ do install -m 755 $$i $(MODDESTDIR); \
done
if [ ! -f $(MODDESTDIR)/m_ipt.so ]; then \
if [ -f $(MODDESTDIR)/m_xt.so ]; \
--
1.7.1
^ permalink raw reply related
* [PATCH] netem: fix installs of dist files
From: Mike Frysinger @ 2010-06-10 0:52 UTC (permalink / raw)
To: stephen.hemminger, netdev
The tc program searches LIBDIR by default for the .dist files, and that
defaults to /usr/lib. But the netem subdir has /lib/ hardcoded which
means the default build+install results in the files not being found.
Further, these are plain text files which are read at runtime, so it
doesn't make sense to give them executable bits.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
netem/Makefile | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/netem/Makefile b/netem/Makefile
index b6ccfc6..e52e125 100644
--- a/netem/Makefile
+++ b/netem/Makefile
@@ -20,9 +20,9 @@ stats: stats.c
$(HOSTCC) $(CCOPTS) -I../include -o $@ $@.c -lm
install: all
- mkdir -p $(DESTDIR)/lib/tc
+ mkdir -p $(DESTDIR)$(LIBDIR)/tc
for i in $(DISTDATA); \
- do install -m 755 $$i $(DESTDIR)/lib/tc; \
+ do install -m 644 $$i $(DESTDIR)$(LIBDIR)/tc; \
done
clean:
--
1.7.1
^ permalink raw reply related
* [PATCH] dnet: fix strict aliasing warnings
From: Mike Frysinger @ 2010-06-10 0:52 UTC (permalink / raw)
To: stephen.hemminger, netdev
Recent gcc doesn't like it when you cast char pointers to uint16_t
pointers and then dereference it. So use memcpy() instead and let
gcc take care of optimizing things away (when appropriate). This
should also fix alignment issues on arches where gcc packs the char
pointer tighter than 16bits.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
lib/dnet_ntop.c | 8 ++++++--
lib/dnet_pton.c | 5 ++++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/lib/dnet_ntop.c b/lib/dnet_ntop.c
index 9500df8..507a7eb 100644
--- a/lib/dnet_ntop.c
+++ b/lib/dnet_ntop.c
@@ -1,4 +1,5 @@
#include <errno.h>
+#include <string.h>
#include <sys/types.h>
#include <netinet/in.h>
@@ -35,11 +36,14 @@ static __inline__ int do_digit(char *str, u_int16_t *addr, u_int16_t scale, size
static const char *dnet_ntop1(const struct dn_naddr *dna, char *str, size_t len)
{
- u_int16_t addr = dn_ntohs(*(u_int16_t *)dna->a_addr);
- u_int16_t area = addr >> 10;
+ u_int16_t addr, area;
size_t pos = 0;
int started = 0;
+ memcpy(&addr, dna->a_addr, sizeof(addr));
+ addr = dn_ntohs(addr);
+ area = addr >> 10;
+
if (dna->a_len != 2)
return NULL;
diff --git a/lib/dnet_pton.c b/lib/dnet_pton.c
index bd7727a..7385756 100644
--- a/lib/dnet_pton.c
+++ b/lib/dnet_pton.c
@@ -1,4 +1,5 @@
#include <errno.h>
+#include <string.h>
#include <sys/types.h>
#include <netinet/in.h>
@@ -37,6 +38,7 @@ static int dnet_num(const char *src, u_int16_t * dst)
static int dnet_pton1(const char *src, struct dn_naddr *dna)
{
+ u_int16_t addr;
u_int16_t area = 0;
u_int16_t node = 0;
int pos;
@@ -48,7 +50,8 @@ static int dnet_pton1(const char *src, struct dn_naddr *dna)
if ((pos == 0) || (node > 1023))
return 0;
dna->a_len = 2;
- *(u_int16_t *)dna->a_addr = dn_htons((area << 10) | node);
+ addr = dn_htons((area << 10) | node);
+ memcpy(dna->a_addr, &addr, sizeof(addr));
return 1;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH 2/2] [PATCH] ucc_geth: fix for RX skb buffers recycling
From: David Miller @ 2010-06-10 1:02 UTC (permalink / raw)
To: geomatsi; +Cc: netdev, leoli, avorontsov
In-Reply-To: <1275935894-30483-2-git-send-email-geomatsi@gmail.com>
From: Sergey Matyukevich <geomatsi@gmail.com>
Date: Mon, 7 Jun 2010 22:38:14 +0400
> This patch implements a proper recycling of skb buffers belonging to RX error
> path. The suggested fix actually follows the recycling scheme implemented for
> TX skb buffers in the same driver (see 'ucc_geth_tx' function): skb buffers
> are checked by 'skb_recycle_check' function and deleted if can't be recycled.
>
> This problem in recycling of skb buffers was discovered by accident in a setup
> when ethernet interface on one link end was full-duplex while another was
> half-duplex. In this case numerous corrupted frames were received by
> full-duplex interface due to late collisions. RX skb buffers with error
> frames were not properly recycled, that is why overflow occured from time to
> time on the next use of those buffers. Here is example of crush dump:
The lack of skb_recycle_check() is not the true cause of this bug.
You should never, ever, need to make skb_recycle_check() tests on
packets in this situation. Once the skb pointers are properly adjusted
it will have sufficient room.
And that points to what the real problem is, the problem is the
skb->data assignment. It's trying to get the SKB data pointers back
into the same state they are in when dev_alloc_skb() returns a packet
buffer.
But this assignment isn't accomplishing that, in fact it's corrupting
the SKB because after adjusting skb->data, skb->tail and skb->len will
become incorrect. And this is what you need to fix.
That's why you get the skb_put() over panics, not because you lack
a skb_recycle_check() call here.
In fact, what your patch makes happen is that the error packets will
never get recycled. The skb_recycle_check() will always fail.
Please fix this bug properly by correctly restoring the SKB pointers
and lengths to their initial state, then you can retain the
unconditional queueing of the error packet onto the recycle list.
Once you do that, all of the checks done by skb_recycle_check() are
superfluous and will always pass, and we know this. The buffer is
not fragmented, there aren't any clones or external references to it,
and once you fix up the data pointers properly it will have enough
room as necessary for the RX buffer size the driver is currently using.
There are numerous helper routines in linux/skbuff.h that can be used
to do this properly, which will adjust a pointer and make the
corresponding adjustment to skb->len as well when necessary.
^ permalink raw reply
* Re: [PATCH net-next-2.6] [PPPOE] cleanup: remove pppoe_xmit() declaration.
From: David Miller @ 2010-06-10 1:07 UTC (permalink / raw)
To: ramirose; +Cc: netdev
In-Reply-To: <AANLkTim_Jg51_xyfeLD3m4i88TGv8B2SRHFZ-H31w1YK@mail.gmail.com>
From: Rami Rosen <ramirose@gmail.com>
Date: Wed, 9 Jun 2010 08:07:56 +0300
> Signed-off-by: Rami Rosen <ramirose@gmail.com>
Applied, thanks Rami.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ipv6: mcast: RCU conversions
From: David Miller @ 2010-06-10 1:07 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, yoshfuji
In-Reply-To: <1275980702.2775.193.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 08 Jun 2010 09:05:02 +0200
> - ipv6_sock_mc_join() : doesnt touch dev refcount
>
> - ipv6_sock_mc_drop() : doesnt touch dev/idev refcounts
>
> - ip6_mc_find_dev() becomes ip6_mc_find_dev_rcu() (called from rcu),
> and doesnt touch dev/idev refcounts
>
> - ipv6_sock_mc_close() : doesnt touch dev/idev refcounts
>
> - ip6_mc_source() uses ip6_mc_find_dev_rcu()
>
> - ip6_mc_msfilter() uses ip6_mc_find_dev_rcu()
>
> - ip6_mc_msfget() uses ip6_mc_find_dev_rcu()
>
> - ipv6_dev_mc_dec(), ipv6_chk_mcast_addr(),
> igmp6_event_query(), igmp6_event_report(),
> mld_sendpack(), igmp6_send() dont touch idev refcount
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next-2.6] icmp: RCU conversion in icmp_address_reply()
From: David Miller @ 2010-06-10 1:07 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1275986075.2475.73.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 08 Jun 2010 10:34:35 +0200
> - rcu_read_lock() already held by caller
> - use __in_dev_get_rcu() instead of in_dev_get() / in_dev_put()
> - remove goto out;
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied.
^ permalink raw reply
* Interesting issue when using IPv6 autoconf sysctl.
From: Ben Greear @ 2010-06-10 1:09 UTC (permalink / raw)
To: NetDev
I just tried using the /proc/sys/net/ipv6/conf/[dev]/autoconf
feature to run off auto-configured link-local addresses
for one of my interfaces.
It worked like a charm, the address is gone,
and now the /proc/sys/net/ipv6/conf/[dev]
directory does not exist so I can't turn it back on!
I can work around this by forcing an ipv6 address back
onto the interface and then removing it, but that
is pretty crufty!
There has got to be a better way to implement this!
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH net-2.6] ipv6: fix ICMP6_MIB_OUTERRORS
From: David Miller @ 2010-06-10 1:39 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1275985484.2475.59.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 08 Jun 2010 10:24:44 +0200
> In commit 1f8438a85366 (icmp: Account for ICMP out errors), I did a typo
> on IPV6 side, using ICMP6_MIB_OUTMSGS instead of ICMP6_MIB_OUTERRORS
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Applied, thanks Eric.
^ permalink raw reply
* [PATCH net-next-2.6] ip: ip_ra_control() rcu fix
From: Eric Dumazet @ 2010-06-10 2:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100607.212612.35795010.davem@davemloft.net>
Le lundi 07 juin 2010 à 21:26 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 07 Jun 2010 15:12:08 +0200
>
> > Straightforward conversion to RCU.
> >
> > One rwlock becomes a spinlock, and is static.
> >
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Applied.
Hmm, rcu is tricky, I should re-read all my previous patches :(
Here is a followup on this patch, thanks !
[PATCH net-next-2.6] ip: ip_ra_control() rcu fix
commit 66018506e15b (ip: Router Alert RCU conversion) introduced RCU
lookups to ip_call_ra_chain(). It missed proper deinit phase :
When ip_ra_control() deletes an ip_ra_chain, it should make sure
ip_call_ra_chain() users can not start to use socket during the rcu
grace period. It should also delay the sock_put() after the grace
period, or we risk a premature socket freeing and corruptions, as
raw sockets are not rcu protected yet.
This delay avoids using expensive atomic_inc_not_return() in
ip_call_ra_chain().
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/ip.h | 5 ++++-
net/ipv4/ip_sockglue.c | 19 +++++++++++++++----
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 9982c97..d52f011 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -61,7 +61,10 @@ struct ipcm_cookie {
struct ip_ra_chain {
struct ip_ra_chain *next;
struct sock *sk;
- void (*destructor)(struct sock *);
+ union {
+ void (*destructor)(struct sock *);
+ struct sock *saved_sk;
+ };
struct rcu_head rcu;
};
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 08b9519..47fff52 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -241,9 +241,13 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc)
struct ip_ra_chain *ip_ra_chain;
static DEFINE_SPINLOCK(ip_ra_lock);
-static void ip_ra_free_rcu(struct rcu_head *head)
+
+static void ip_ra_destroy_rcu(struct rcu_head *head)
{
- kfree(container_of(head, struct ip_ra_chain, rcu));
+ struct ip_ra_chain *ra = container_of(head, struct ip_ra_chain, rcu);
+
+ sock_put(ra->saved_sk);
+ kfree(ra);
}
int ip_ra_control(struct sock *sk, unsigned char on,
@@ -264,13 +268,20 @@ int ip_ra_control(struct sock *sk, unsigned char on,
kfree(new_ra);
return -EADDRINUSE;
}
+ /* dont let ip_call_ra_chain() use sk again */
+ ra->sk = NULL;
rcu_assign_pointer(*rap, ra->next);
spin_unlock_bh(&ip_ra_lock);
if (ra->destructor)
ra->destructor(sk);
- sock_put(sk);
- call_rcu(&ra->rcu, ip_ra_free_rcu);
+ /*
+ * Delay sock_put(sk) and kfree(ra) after one rcu grace
+ * period. This guarantee ip_call_ra_chain() dont need
+ * to mess with socket refcounts.
+ */
+ ra->saved_sk = sk;
+ call_rcu(&ra->rcu, ip_ra_destroy_rcu);
return 0;
}
}
^ permalink raw reply related
* Re: [PATCH net-next-2.6] ip: ip_ra_control() rcu fix
From: Eric Dumazet @ 2010-06-10 2:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1276136109.2475.9.camel@edumazet-laptop>
Le jeudi 10 juin 2010 à 04:15 +0200, Eric Dumazet a écrit :
> [PATCH net-next-2.6] ip: ip_ra_control() rcu fix
>
> commit 66018506e15b (ip: Router Alert RCU conversion) introduced RCU
> lookups to ip_call_ra_chain(). It missed proper deinit phase :
> When ip_ra_control() deletes an ip_ra_chain, it should make sure
> ip_call_ra_chain() users can not start to use socket during the rcu
> grace period. It should also delay the sock_put() after the grace
> period, or we risk a premature socket freeing and corruptions, as
> raw sockets are not rcu protected yet.
>
> This delay avoids using expensive atomic_inc_not_return() in
Grrr... should be atomic_inc_not_zero(), sorry for the typo in
ChangeLog :(
[PATCH net-next-2.6 v2] ip: ip_ra_control() rcu fix
commit 66018506e15b (ip: Router Alert RCU conversion) introduced RCU
lookups to ip_call_ra_chain(). It missed proper deinit phase :
When ip_ra_control() deletes an ip_ra_chain, it should make sure
ip_call_ra_chain() users can not start to use socket during the rcu
grace period. It should also delay the sock_put() after the grace
period, or we risk a premature socket freeing and corruptions, as
raw sockets are not rcu protected yet.
This delay avoids using expensive atomic_inc_not_zero() in
ip_call_ra_chain().
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/net/ip.h | 5 ++++-
net/ipv4/ip_sockglue.c | 19 +++++++++++++++----
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 9982c97..d52f011 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -61,7 +61,10 @@ struct ipcm_cookie {
struct ip_ra_chain {
struct ip_ra_chain *next;
struct sock *sk;
- void (*destructor)(struct sock *);
+ union {
+ void (*destructor)(struct sock *);
+ struct sock *saved_sk;
+ };
struct rcu_head rcu;
};
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 08b9519..47fff52 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -241,9 +241,13 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc)
struct ip_ra_chain *ip_ra_chain;
static DEFINE_SPINLOCK(ip_ra_lock);
-static void ip_ra_free_rcu(struct rcu_head *head)
+
+static void ip_ra_destroy_rcu(struct rcu_head *head)
{
- kfree(container_of(head, struct ip_ra_chain, rcu));
+ struct ip_ra_chain *ra = container_of(head, struct ip_ra_chain, rcu);
+
+ sock_put(ra->saved_sk);
+ kfree(ra);
}
int ip_ra_control(struct sock *sk, unsigned char on,
@@ -264,13 +268,20 @@ int ip_ra_control(struct sock *sk, unsigned char on,
kfree(new_ra);
return -EADDRINUSE;
}
+ /* dont let ip_call_ra_chain() use sk again */
+ ra->sk = NULL;
rcu_assign_pointer(*rap, ra->next);
spin_unlock_bh(&ip_ra_lock);
if (ra->destructor)
ra->destructor(sk);
- sock_put(sk);
- call_rcu(&ra->rcu, ip_ra_free_rcu);
+ /*
+ * Delay sock_put(sk) and kfree(ra) after one rcu grace
+ * period. This guarantee ip_call_ra_chain() dont need
+ * to mess with socket refcounts.
+ */
+ ra->saved_sk = sk;
+ call_rcu(&ra->rcu, ip_ra_destroy_rcu);
return 0;
}
}
^ 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