Netdev List
 help / color / mirror / Atom feed
* Re: [Bloat] shaper team forming up
From: Eric Dumazet @ 2011-03-15 23:00 UTC (permalink / raw)
  To: David Miller; +Cc: chromatix99, d, netdev
In-Reply-To: <20110315.155305.71104836.davem@davemloft.net>

Le mardi 15 mars 2011 à 15:53 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 15 Mar 2011 05:42:46 +0100
> 
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 6ed6603..fabfe81 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -171,7 +171,7 @@ static struct dst_ops ipv4_dst_ops = {
> >  
> >  const __u8 ip_tos2prio[16] = {
> >  	TC_PRIO_BESTEFFORT,
> > -	ECN_OR_COST(FILLER),
> > +	ECN_OR_COST(BESTEFFORT),
> >  	TC_PRIO_BESTEFFORT,
> >  	ECN_OR_COST(BESTEFFORT),
> >  	TC_PRIO_BULK,
> > 
> > 
> 
> Eric can you please formally submit this fix?  I'd like to apply it soon.
> 
> Thanks.

Sure, I can do this right now, but I want to fully credit Dan Siemon on
this one, as the author of the patch.



^ permalink raw reply

* net-next-2.6 status...
From: David Miller @ 2011-03-15 23:01 UTC (permalink / raw)
  To: netdev
  Cc: linux-wireless, netfilter-devel, eric.dumazet, jpirko, linville,
	kaber


Please do not submit feature patches until the merge window is over and
Linus releases 2.6.39-rc1, thank you.

Bug fixes are, of course, still ok.

I have merged all of the stragglers from net-2.6 that didn't make it
into 2.6.38 into net-next-2.6, and also just about everything that
is relevant from patchwork.

Jiri, I know there is your patch set there, but I think you and Changli
still need to go back and forth one more time wrt. orig_dev semantics.
Since you have been posting this patch set for some time I'm still
willing to apply it for this merge window, but please make haste and
work out the remaining discussion.  Thank you.

Eric, I'd like to have the ECN one-liner in the tree soon.

John and Patrick, I assume that after today's pulls I am completely
up to date with everything, feature wise, that will go into the merge
window for wireless and netfilter.

Thanks.

^ permalink raw reply

* Re: [Bloat] shaper team forming up
From: David Miller @ 2011-03-15 22:53 UTC (permalink / raw)
  To: eric.dumazet; +Cc: chromatix99, d, netdev
In-Reply-To: <1300164166.2649.70.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Mar 2011 05:42:46 +0100

> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 6ed6603..fabfe81 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -171,7 +171,7 @@ static struct dst_ops ipv4_dst_ops = {
>  
>  const __u8 ip_tos2prio[16] = {
>  	TC_PRIO_BESTEFFORT,
> -	ECN_OR_COST(FILLER),
> +	ECN_OR_COST(BESTEFFORT),
>  	TC_PRIO_BESTEFFORT,
>  	ECN_OR_COST(BESTEFFORT),
>  	TC_PRIO_BULK,
> 
> 

Eric can you please formally submit this fix?  I'd like to apply it soon.

Thanks.

^ permalink raw reply

* Re: IPsec tunnel forwarding in net-next-2.6 since 452edd59
From: Michael Smith @ 2011-03-15 22:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, David Miller
In-Reply-To: <1300227411.2565.7.camel@edumazet-laptop>

On Tue, 15 Mar 2011, Eric Dumazet wrote:

> Could you try following patch ?
> 
> [PATCH] xfrm: fix __xfrm_route_forward()
> 
> This function should return 0 in case of error, 1 if OK
> commit 452edd598f60522 (xfrm: Return dst directly from xfrm_lookup())
> got it wrong.

That does the trick, thanks! Nice catch.

Mike

^ permalink raw reply

* Re: [Bloat] shaper team forming up
From: David Miller @ 2011-03-15 22:51 UTC (permalink / raw)
  To: eric.dumazet; +Cc: chromatix99, d, netdev
In-Reply-To: <1300164166.2649.70.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Mar 2011 05:42:46 +0100

> Le lundi 14 mars 2011 à 21:24 +0100, Eric Dumazet a écrit :
> 
> David, it seems ip_tos2prio is wrong on its 2nd entry :

Indeed, and in context, this is simply a thinko which has survived since
ECN support first got added to the tree, here's the relevant hunk:

--------------------
@@ -152,23 +152,29 @@ struct dst_ops ipv4_dst_ops =
 	sizeof(struct rtable),
 };
 
+#ifdef CONFIG_INET_ECN
+#define ECN_OR_COST(class)	TC_PRIO_##class
+#else
+#define ECN_OR_COST(class)	TC_PRIO_FILLER
+#endif
+
 __u8 ip_tos2prio[16] = {
 	TC_PRIO_BESTEFFORT,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(FILLER),
 	TC_PRIO_BESTEFFORT,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(BESTEFFORT),
 	TC_PRIO_BULK,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(BULK),
 	TC_PRIO_BULK,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(BULK),
 	TC_PRIO_INTERACTIVE,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(INTERACTIVE),
 	TC_PRIO_INTERACTIVE,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(INTERACTIVE),
 	TC_PRIO_INTERACTIVE_BULK,
-	TC_PRIO_FILLER,
+	ECN_OR_COST(INTERACTIVE_BULK),
 	TC_PRIO_INTERACTIVE_BULK,
-	TC_PRIO_FILLER
+	ECN_OR_COST(INTERACTIVE_BULK)
 };

^ permalink raw reply

* Re: IPsec tunnel forwarding in net-next-2.6 since 452edd59
From: Eric Dumazet @ 2011-03-15 22:31 UTC (permalink / raw)
  To: David Miller; +Cc: msmith, netdev
In-Reply-To: <20110315.152825.193708028.davem@davemloft.net>

Le mardi 15 mars 2011 à 15:28 -0700, David Miller a écrit :

> Thanks so much for fixing this Eric, sheesh you are so awesome that
> you don't even give me enough time to fix my own bugs. :-))))
> 

Well, I know you must be pretty busy now linux-2.6.38 is released ;)

> Applied, and indeed thanks to Michael for the excellent report.
> 



^ permalink raw reply

* Re: IPsec tunnel forwarding in net-next-2.6 since 452edd59
From: David Miller @ 2011-03-15 22:28 UTC (permalink / raw)
  To: eric.dumazet; +Cc: msmith, netdev
In-Reply-To: <1300227411.2565.7.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 15 Mar 2011 23:16:51 +0100

> Could you try following patch ?
> 
> [PATCH] xfrm: fix __xfrm_route_forward()
> 
> This function should return 0 in case of error, 1 if OK
> commit 452edd598f60522 (xfrm: Return dst directly from xfrm_lookup())
> got it wrong.
> 
> Reported-and-bisected-by: Michael Smith <msmith@cbnco.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks so much for fixing this Eric, sheesh you are so awesome that
you don't even give me enough time to fix my own bugs. :-))))

Applied, and indeed thanks to Michael for the excellent report.


^ permalink raw reply

* Re: [RFC v3 3/6] can: make struct proto const
From: Eric Dumazet @ 2011-03-15 22:19 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: Kurt Van Dijck, socketcan-core, netdev
In-Reply-To: <4D7FD9ED.1080004@hartkopp.net>

Le mardi 15 mars 2011 à 22:28 +0100, Oliver Hartkopp a écrit :

> Hello Eric,
> 
> the RCU code in phonet is worth to consider. But loading and removing of
> protocols is far away from being a hot path ... what would be the advantages
> to move to RCU here?
> 

can_create() could avoid taking the spinlock.

I have no idea if its a critical path, I guess not.



^ permalink raw reply

* Re: IPsec tunnel forwarding in net-next-2.6 since 452edd59
From: Eric Dumazet @ 2011-03-15 22:16 UTC (permalink / raw)
  To: Michael Smith; +Cc: netdev, David Miller
In-Reply-To: <4D7FB054.2090902@cbnco.com>

Le mardi 15 mars 2011 à 14:30 -0400, Michael Smith a écrit :
> Hi,
> 
> I'm able to ping across a tunnel to a peer running net-next-2.6, but 
> only to an interface on the peer; trying to ping a host behind the peer 
> fails. The incoming packet shows up in encrypted and decrypted form in 
> tcpdump, but it's not forwarded. None of the XFRM error counters are 
> incremented; the packets just silently fail to be forwarded.
> 
> There are no iptables rules and net.ipv4.ip_forward=1. The same config 
> works on 2.6.38-rc8. git bisect pointed me to commit 452edd59 from March 2:
> 
>      xfrm: Return dst directly from xfrm_lookup()
> 
>      Instead of on the stack.
> 
> 
> ip xfrm policy:
> 
> src 192.168.136.0/24 dst 192.168.137.0/24
>          dir out priority 2344 ptype main
>          tmpl src 1.1.1.136 dst 1.1.1.137
>                  proto esp reqid 16385 mode tunnel
> 
> src 192.168.137.0/24 dst 192.168.136.0/24
>          dir fwd priority 2344 ptype main
>          tmpl src 1.1.1.137 dst 1.1.1.136
>                  proto esp reqid 16385 mode tunnel
> 
> src 192.168.137.0/24 dst 192.168.136.0/24
>          dir in priority 2344 ptype main
>          tmpl src 1.1.1.137 dst 1.1.1.136
>                  proto esp reqid 16385 mode tunnel
> 
> net-next-2.6 host is at 1.1.1.136 and 192.168.136.1. 2.6.35.10 host is 
> at 1.1.1.137 and 192.168.137.1. From that host:
> 
> ping -I 192.168.137.1 192.168.136.1 -> success
> ping -I 192.168.137.1 192.168.136.2 -> silent failure
> 
> Thanks,

Thanks for this excellent bug report !

Could you try following patch ?

[PATCH] xfrm: fix __xfrm_route_forward()

This function should return 0 in case of error, 1 if OK
commit 452edd598f60522 (xfrm: Return dst directly from xfrm_lookup())
got it wrong.

Reported-and-bisected-by: Michael Smith <msmith@cbnco.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 1ba0258..027e3c6 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2175,7 +2175,7 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 	struct net *net = dev_net(skb->dev);
 	struct flowi fl;
 	struct dst_entry *dst;
-	int res = 0;
+	int res = 1;
 
 	if (xfrm_decode_session(skb, &fl, family) < 0) {
 		XFRM_INC_STATS(net, LINUX_MIB_XFRMFWDHDRERROR);
@@ -2186,7 +2186,7 @@ int __xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 
 	dst = xfrm_lookup(net, skb_dst(skb), &fl, NULL, 0);
 	if (IS_ERR(dst)) {
-		res = 1;
+		res = 0;
 		dst = NULL;
 	}
 	skb_dst_set(skb, dst);




^ permalink raw reply related

* Re: [RFC v3 3/6] can: make struct proto const
From: Kurt Van Dijck @ 2011-03-15 22:12 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Eric Dumazet
In-Reply-To: <4D7FD9ED.1080004-fJ+pQTUTwRTk1uMJSBkQmQ@public.gmane.org>

On Tue, Mar 15, 2011 at 10:28:13PM +0100, Oliver Hartkopp wrote:
> On 14.03.2011 18:17, Kurt Van Dijck wrote:
> > On Mon, Mar 14, 2011 at 05:42:23PM +0100, Eric Dumazet wrote:
> >> Le lundi 14 mars 2011 à 16:02 +0100, Kurt Van Dijck a écrit :
> >>
> >>> I see your point.
> >>> The reason I modified this was, as described, checkpatch.pl was complaining.
> >>> Anyway, this sequence was protected by a spinlock 'proto_tab_lock'.
> >>> I don't think it was a race.
> >>>
> >>> Did I miss something?
> >>
> >> As soon as proto_tab[proto] = cp; is done, another thread on another cpu
> >> can read the pointer and follow it.
> >>
> >> Hmm, I missed can_create() also uses the spinlock protection, so you're
> >> probably right.
> >>
> >> It seems a bit overkill :(
> >> phonet uses RCU for example.
> 
> Hello Eric,
> 
> the RCU code in phonet is worth to consider. But loading and removing of
> protocols is far away from being a hot path ... what would be the advantages
> to move to RCU here?
Does RCU avoid a spinlock?

> 
> > I'll keep that in mind.
> > Switching CAN protos to RCU is far beyond to goal of this patch :-)
> 
> Anyway making the struct proto a const (and set the default defines in the CAN
> protocols) is a nice cleanup. Let's wait for the feedback about the RCU from
> Eric ... then this patch could be put into mainline independently from the
> j1939 protocol.
ack.

Kurt
_______________________________________________
Socketcan-core mailing list
Socketcan-core@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: linux-next: build failure after merge of the net tree
From: David Miller @ 2011-03-15 22:07 UTC (permalink / raw)
  To: tglx; +Cc: sfr, bhutchings, netdev, linux-next, linux-kernel, mingo, hpa,
	peterz
In-Reply-To: <alpine.LFD.2.00.1103151128070.2787@localhost6.localdomain6>

From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 15 Mar 2011 11:29:18 +0100 (CET)

> On Tue, 15 Mar 2011, Stephen Rothwell wrote:
> 
>> Hi Dave,
>> 
>> On Mon, 14 Mar 2011 21:07:12 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>> >
>> > From: Stephen Rothwell <sfr@canb.auug.org.au>
>> > Date: Tue, 15 Mar 2011 11:30:49 +1100
>> > 
>> > > So, if you merge the commit 1fb0ef31f428 ("genirq: Fix affinity notifier
>> > > fallout") from the tip tree, you will get exactly one new commit and will
>> > > fix your tree properly with no adverse effect on anyone else.
>> > 
>> > Thomas, how would you like me to do this?  Do you want to put that
>> > commit on a branch for me to pull from or should I apply it
>> > directly?
>> 
>> That commit is already the head of the irq/numa branch in the tip tree
>> (it is also on the irq/core branch).
>> 
>> Please merge that rather than cherry-picking the commit as that will
>> minimise the possibility of conflicts.  Pulling the irq/numa branch will
>> get you just that one fixup commit (as you have already merged up to the
>> immediately preceding commit).
>> 
>> I assume this makes sense, Thomas?  I just did a merge of the actual
>> fixup commit after merging the net tree.
> 
> Yes. irq/numa should do the trick.

Great, I'll do that now.

^ permalink raw reply

* Re: [PATCH net-next-2.6] be2net: Fix UDP packet detected status in RX compl
From: David Miller @ 2011-03-15 21:57 UTC (permalink / raw)
  To: padmanabh.ratnakar; +Cc: netdev, sathya.perla
In-Reply-To: <fa0d690e-8b0a-4608-be17-d65c22246f37@exht1.ad.emulex.com>

From: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
Date: Tue, 15 Mar 2011 15:59:01 +0530

> Status of UDP packet detection not getting populated in RX completion
> structure. This is required in csum_passed() routine.
> 
> Signed-off-by: Padmanabh Ratnakar <padmanabh.ratnakar@emulex.com>
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] Phonet: fix aligned-mode pipe socket buffer header reserve
From: David Miller @ 2011-03-15 21:56 UTC (permalink / raw)
  To: remi.denis-courmont; +Cc: netdev
In-Reply-To: <1300178222-2877-1-git-send-email-remi.denis-courmont@nokia.com>

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Tue, 15 Mar 2011 10:37:02 +0200

> When the pipe uses aligned-mode data packets, we must reserve 4 bytes
> instead of 3 for the pipe protocol header. Otherwise the Phonet header
> would not be aligned, resulting in potentially corrupted headers with
> later unaligned memory writes.
> 
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCHv2 NEXT 1/1] netxen: support for GbE port settings
From: David Miller @ 2011-03-15 21:55 UTC (permalink / raw)
  To: amit.salecha; +Cc: netdev, ameen.rahman, anirban.chakraborty, sony.chacko
In-Reply-To: <1300172175-16853-1-git-send-email-amit.salecha@qlogic.com>

From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Mon, 14 Mar 2011 23:56:15 -0700

> From: Sony Chacko <sony.chacko@qlogic.com>
> 
> o Enable setting speed and auto negotiation parameters for GbE ports.
> o Hardware do not support half duplex setting currently.
> 
> David Miller:
> 	Amit please update your patch to silently reject link setting
> 	attempts that are unsupported by the device.
> 
> Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] smsc911x: Fix build error when SMSC_TRACE() used
From: David Miller @ 2011-03-15 21:52 UTC (permalink / raw)
  To: jtp.park; +Cc: netdev, steve.glendinning, kirjanov
In-Reply-To: <1300166180-19103-1-git-send-email-jtp.park@samsung.com>

From: Jeongtae Park <jtp.park@samsung.com>
Date: Tue, 15 Mar 2011 14:16:20 +0900

> This patch fixes build error when SMSC_TRACE() used.
> 
> Signed-off-by: Jeongtae Park <jtp.park@samsung.com>

Applied, thanks.

^ permalink raw reply

* Re: [net-next-2.6 0/2][pull request] Intel Wired LAN Driver Update
From: David Miller @ 2011-03-15 21:51 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips
In-Reply-To: <1300168464-29546-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 14 Mar 2011 22:54:22 -0700

> The following series contains a fix for timestamping and the
> addition of thermal sensor support for igb.
> 
> The following are changes since commit 0c0217b016ba8a970a6f6ab62ad0d858f39881ca:
>   net: dcbnl: Add IEEE app selector value definitions
> and are available in the git repository at:
>   master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master
> 
> Anders Berggren (1):
>   igb: fix hw timestamping
> 
> Carolyn Wyborny (1):
>   igb: Add messaging for thermal sensor events on i350 devices

Pulled, thanks Jeff.

^ permalink raw reply

* Re: [PATCH] Fix overflow of name in struct net_device, replaced sprintf with snprintf.
From: Stephen Hemminger @ 2011-03-15 21:48 UTC (permalink / raw)
  To: Sasikanth V; +Cc: davem, netdev
In-Reply-To: <1300219208-2708-1-git-send-email-sasikanth.v19@gmail.com>

On Wed, 16 Mar 2011 01:30:08 +0530
Sasikanth V <sasikanth.v19@gmail.com> wrote:

> 
> Signed-off-by: Sasikanth V <sasikanth.v19@gmail.com>
> ---
>  net/core/dev.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6561021..a74efa9 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -539,7 +539,7 @@ unsigned long netdev_boot_base(const char *prefix, int unit)
>  	char name[IFNAMSIZ];
>  	int i;
>  
> -	sprintf(name, "%s%d", prefix, unit);
> +	snprintf(name, IFNAMSIZ, "%s%d", prefix, unit);
>  
>  	/*
>  	 * If device already registered then return base of 1

that code is only called with a really small values "eth" and "tr" so
it is really not necessary. The whole drivers/net/Space.c code is totally
legacy and should eventually be eliminated anyway.


-- 

^ permalink raw reply

* Re: Multicast Fails Over Multipoint GRE Tunnel
From: Doug Kehn @ 2011-03-15 21:35 UTC (permalink / raw)
  To: Eric Dumazet, Timo Teräs; +Cc: netdev
In-Reply-To: <4D7F9592.5050408@iki.fi>

Hi Timo,


--- On Tue, 3/15/11, Timo Teräs <timo.teras@iki.fi> wrote:

> From: Timo Teräs <timo.teras@iki.fi>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Eric Dumazet" <eric.dumazet@gmail.com>
> Cc: "Doug Kehn" <rdkehn@yahoo.com>, netdev@vger.kernel.org
> Date: Tuesday, March 15, 2011, 12:36 PM
> On 03/15/2011 05:34 PM, Eric Dumazet
> wrote:
> > Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a
> écrit :
> >> I'm running kernel version 2.6.36 on ARM XSCALE
> (big-endian) and multicast over a multipoint GRE tunnel
> isn't working.  For my architecture, this worked on
> 2.6.26.8.  For x86, multicast over a multipoint GRE
> tunnel worked with kernel version 2.6.31 but failed with
> version 2.6.35.  Multicast over a multipoint GRE tunnel
> fails because ipgre_header() fails the 'if (iph->daddr)'
> check and reutrns -t->hlen.  ipgre_header() is being
> called, from neigh_connected_output(), with a non-null
> daddr; the contents of daddr is zero.
> >>
> >> Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2
> resolves the problem.  (Reviewing the HEAD of
> net-next-2.6 it appears that ipgre_header() remains
> unchanged from 2.6.36.)
> >>
> >> The configuration used to discover/diagnose the
> problem:
> >>
> >> ip tunnel add tun1 mode gre key 11223344 ttl 64
> csum remote any
> >> ip link set dev tun1 up
> >> ip link set dev tun1 multicast on
> >> ip addr flush dev tun1
> >> ip addr add 10.40.92.114/24 broadcast 10.40.92.255
> dev tun1
> >>
> >> 12: tun1: <MULTICAST,NOARP,UP,10000> mtu
> 1468 qdisc noqueue
> >>     link/gre 0.0.0.0 brd
> 0.0.0.0
> >>     inet 10.40.92.114/24 brd
> 10.40.92.255 scope global tun1
> >>
> >> Then attempt:
> >> ping -I tun1 224.0.0.9
> >>
> >> Are additional configuration steps now required
> for multicast over multipoint GRE tunnel or is
> ipgre_header() in error?
> > 
> > Hi Doug
> > 
> > CC Timo Teras <timo.teras@iki.fi>
> > 
> > I would do a partial revert of Timo patch, but this
> means initial
> > concern should be addressed ?
> > 
> > (Timo mentioned : 
> >     If the NOARP packets are not
> dropped, ipgre_tunnel_xmit() will
> >     take rt->rt_gateway (= NBMA IP)
> and use that for route
> >     look up (and may lead to bogus xfrm
> acquires).)
> > 
> > 
> > Is the following works for you ?
> 
> I have memory that _header() is called with daddr being
> valid pointer,
> but pointing to zero memory. So basically my situation
> would break with
> this.
> 
> The above configuration would be fixable by setting
> broadcast to tun1
> interface explicitly. But I'm not sure if the above
> configuration is
> somehow different that it'd need to work.
> 
> Basically how things work on send path is:
>  1. arp_constructor maps multicast address to NUD_NOARP
>  2. arp_mc_map in turn copies dev->broadcast to haddr
>     (so you get the ip packet pointing to zero
> ip)
>     - i assumed normally gre tunnels would have
> the
>       broadcast address set
>  3. ipgre_tunnel_xmit checks for daddr==0 and uses the
>     rt_gateway then, which would map to the
> multicast
>     address
> 
> So I assume that the above ping command not working would
> end up sending
> gre encapsulated packet where both the inner and outer IP
> address is the
> same multicast address?
> 
> Looks like my patch also broke the default behaviour that
> if one has
> such GRE tunnel, and you send unicast packets, it would
> default the link
> destination to be same as the inner destination. Not sure
> if this would
> be useful.
> 
> So basically my situation is undistinguishable from the
> above one. The
> only difference is that the above problems only with
> multicast, and I'm
> having with unicast.
> 
> I think the fundamental problem is that arp_mc_map maps the
> multicast
> address to zeroes (due to device broadcast being zero).
> 
> Could we instead maybe do something like:
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7927589..372448a 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -215,6 +215,12 @@ int arp_mc_map(__be32 addr, u8 *haddr,
> struct
> net_device *dev, int dir)
>         case ARPHRD_INFINIBAND:
>                
> ip_ib_mc_map(addr, dev->broadcast, haddr);
>                
> return 0;
> +       case ARPHRD_IPGRE:
> +           
>    if (dev->broadcast)
> +               
>        memcpy(haddr,
> dev->broadcast, dev->addr_len);
> +           
>    else
> +               
>        memcpy(haddr, &addr,
> sizeof(addr));
> +           
>    return 0;
>         default:
>                 if
> (dir) {
>                
>         memcpy(haddr, dev->broadcast,
> dev->addr_len);
> 


I wasn't sure if the above patch was in addition too or in lieu of the partial revert of ipgre_header() suggested by Eric; both cases were attempted.

The multicast scenario described does not work if only the arp_mc_map() patch is applied.

The multicast scenario described does work if the partial revert of ipgre_header(), suggested by Eric, and the arp_mc_map patch are applied.

Regards,
...doug



      

^ permalink raw reply

* Re: Multicast Fails Over Multipoint GRE Tunnel
From: Doug Kehn @ 2011-03-15 21:33 UTC (permalink / raw)
  To: Eric Dumazet, Timo Teräs; +Cc: netdev
In-Reply-To: <4D7F9592.5050408@iki.fi>

Hi Timo,


--- On Tue, 3/15/11, Timo Teräs <timo.teras@iki.fi> wrote:

> From: Timo Teräs <timo.teras@iki.fi>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Eric Dumazet" <eric.dumazet@gmail.com>
> Cc: "Doug Kehn" <rdkehn@yahoo.com>, netdev@vger.kernel.org
> Date: Tuesday, March 15, 2011, 12:36 PM
> On 03/15/2011 05:34 PM, Eric Dumazet
> wrote:
> > Le lundi 14 mars 2011 à 16:34 -0700, Doug Kehn a
> écrit :
> >> I'm running kernel version 2.6.36 on ARM XSCALE
> (big-endian) and multicast over a multipoint GRE tunnel
> isn't working.  For my architecture, this worked on
> 2.6.26.8.  For x86, multicast over a multipoint GRE
> tunnel worked with kernel version 2.6.31 but failed with
> version 2.6.35.  Multicast over a multipoint GRE tunnel
> fails because ipgre_header() fails the 'if (iph->daddr)'
> check and reutrns -t->hlen.  ipgre_header() is being
> called, from neigh_connected_output(), with a non-null
> daddr; the contents of daddr is zero.
> >>
> >> Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2
> resolves the problem.  (Reviewing the HEAD of
> net-next-2.6 it appears that ipgre_header() remains
> unchanged from 2.6.36.)
> >>
> >> The configuration used to discover/diagnose the
> problem:
> >>
> >> ip tunnel add tun1 mode gre key 11223344 ttl 64
> csum remote any
> >> ip link set dev tun1 up
> >> ip link set dev tun1 multicast on
> >> ip addr flush dev tun1
> >> ip addr add 10.40.92.114/24 broadcast 10.40.92.255
> dev tun1
> >>
> >> 12: tun1: <MULTICAST,NOARP,UP,10000> mtu
> 1468 qdisc noqueue
> >>     link/gre 0.0.0.0 brd
> 0.0.0.0
> >>     inet 10.40.92.114/24 brd
> 10.40.92.255 scope global tun1
> >>
> >> Then attempt:
> >> ping -I tun1 224.0.0.9
> >>
> >> Are additional configuration steps now required
> for multicast over multipoint GRE tunnel or is
> ipgre_header() in error?
> > 
> > Hi Doug
> > 
> > CC Timo Teras <timo.teras@iki.fi>
> > 
> > I would do a partial revert of Timo patch, but this
> means initial
> > concern should be addressed ?
> > 
> > (Timo mentioned : 
> >     If the NOARP packets are not
> dropped, ipgre_tunnel_xmit() will
> >     take rt->rt_gateway (= NBMA IP)
> and use that for route
> >     look up (and may lead to bogus xfrm
> acquires).)
> > 
> > 
> > Is the following works for you ?
> 
> I have memory that _header() is called with daddr being
> valid pointer,
> but pointing to zero memory. So basically my situation
> would break with
> this.
> 
> The above configuration would be fixable by setting
> broadcast to tun1
> interface explicitly. But I'm not sure if the above
> configuration is
> somehow different that it'd need to work.
> 
> Basically how things work on send path is:
>  1. arp_constructor maps multicast address to NUD_NOARP
>  2. arp_mc_map in turn copies dev->broadcast to haddr
>     (so you get the ip packet pointing to zero
> ip)
>     - i assumed normally gre tunnels would have
> the
>       broadcast address set
>  3. ipgre_tunnel_xmit checks for daddr==0 and uses the
>     rt_gateway then, which would map to the
> multicast
>     address
> 
> So I assume that the above ping command not working would
> end up sending
> gre encapsulated packet where both the inner and outer IP
> address is the
> same multicast address?
> 
> Looks like my patch also broke the default behaviour that
> if one has
> such GRE tunnel, and you send unicast packets, it would
> default the link
> destination to be same as the inner destination. Not sure
> if this would
> be useful.
> 
> So basically my situation is undistinguishable from the
> above one. The
> only difference is that the above problems only with
> multicast, and I'm
> having with unicast.
> 
> I think the fundamental problem is that arp_mc_map maps the
> multicast
> address to zeroes (due to device broadcast being zero).
> 
> Could we instead maybe do something like:
> 
> diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
> index 7927589..372448a 100644
> --- a/net/ipv4/arp.c
> +++ b/net/ipv4/arp.c
> @@ -215,6 +215,12 @@ int arp_mc_map(__be32 addr, u8 *haddr,
> struct
> net_device *dev, int dir)
>         case ARPHRD_INFINIBAND:
>                
> ip_ib_mc_map(addr, dev->broadcast, haddr);
>                
> return 0;
> +       case ARPHRD_IPGRE:
> +           
>    if (dev->broadcast)
> +               
>        memcpy(haddr,
> dev->broadcast, dev->addr_len);
> +           
>    else
> +               
>        memcpy(haddr, &addr,
> sizeof(addr));
> +           
>    return 0;
>         default:
>                 if
> (dir) {
>                
>         memcpy(haddr, dev->broadcast,
> dev->addr_len);
> 


I wasn't sure if the above patch was in addition too or in lieu of the partial revert of ipgre_header() suggested by Eric; both cases were attempted.

The multicast scenario described does not work if only the arp_mc_map() patch is applied.

The multicast scenario described does work if the partial revert of ipgre_header(), suggested by Eric, and the arp_mc_map patch are applied.

Regards,
...doug



      

^ permalink raw reply

* Re: [RFC v3 3/6] can: make struct proto const
From: Oliver Hartkopp @ 2011-03-15 21:28 UTC (permalink / raw)
  To: Kurt Van Dijck, Eric Dumazet; +Cc: socketcan-core, netdev
In-Reply-To: <20110314171752.GL333@e-circ.dyndns.org>

On 14.03.2011 18:17, Kurt Van Dijck wrote:
> On Mon, Mar 14, 2011 at 05:42:23PM +0100, Eric Dumazet wrote:
>> Le lundi 14 mars 2011 à 16:02 +0100, Kurt Van Dijck a écrit :
>>
>>> I see your point.
>>> The reason I modified this was, as described, checkpatch.pl was complaining.
>>> Anyway, this sequence was protected by a spinlock 'proto_tab_lock'.
>>> I don't think it was a race.
>>>
>>> Did I miss something?
>>
>> As soon as proto_tab[proto] = cp; is done, another thread on another cpu
>> can read the pointer and follow it.
>>
>> Hmm, I missed can_create() also uses the spinlock protection, so you're
>> probably right.
>>
>> It seems a bit overkill :(
>> phonet uses RCU for example.

Hello Eric,

the RCU code in phonet is worth to consider. But loading and removing of
protocols is far away from being a hot path ... what would be the advantages
to move to RCU here?

> I'll keep that in mind.
> Switching CAN protos to RCU is far beyond to goal of this patch :-)

Anyway making the struct proto a const (and set the default defines in the CAN
protocols) is a nice cleanup. Let's wait for the feedback about the RCU from
Eric ... then this patch could be put into mainline independently from the
j1939 protocol.

Thanks,
Oliver

^ permalink raw reply

* Re: Multicast Fails Over Multipoint GRE Tunnel
From: Doug Kehn @ 2011-03-15 21:24 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Timo Teras
In-Reply-To: <1300203277.2927.9.camel@edumazet-laptop>

Hi Eric,

--- On Tue, 3/15/11, Eric Dumazet <eric.dumazet@gmail.com> wrote:

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Subject: Re: Multicast Fails Over Multipoint GRE Tunnel
> To: "Doug Kehn" <rdkehn@yahoo.com>
> Cc: netdev@vger.kernel.org, "Timo Teras" <timo.teras@iki.fi>
> Date: Tuesday, March 15, 2011, 11:34 AM
> Le lundi 14 mars 2011 à 16:34 -0700,
> Doug Kehn a écrit :
> > Hi All,
> > 
> > I'm running kernel version 2.6.36 on ARM XSCALE
> (big-endian) and multicast over a multipoint GRE tunnel
> isn't working.  For my architecture, this worked on
> 2.6.26.8.  For x86, multicast over a multipoint GRE
> tunnel worked with kernel version 2.6.31 but failed with
> version 2.6.35.  Multicast over a multipoint GRE tunnel
> fails because ipgre_header() fails the 'if (iph->daddr)'
> check and reutrns -t->hlen.  ipgre_header() is being
> called, from neigh_connected_output(), with a non-null
> daddr; the contents of daddr is zero.
> > 
> > Reverting the ip_gre.c patch posted in http://marc.info/?l=linux-netdev&m=126762491525281&w=2
> resolves the problem.  (Reviewing the HEAD of
> net-next-2.6 it appears that ipgre_header() remains
> unchanged from 2.6.36.)
> > 
> > The configuration used to discover/diagnose the
> problem:
> > 
> > ip tunnel add tun1 mode gre key 11223344 ttl 64 csum
> remote any
> > ip link set dev tun1 up
> > ip link set dev tun1 multicast on
> > ip addr flush dev tun1
> > ip addr add 10.40.92.114/24 broadcast 10.40.92.255 dev
> tun1
> > 
> > 12: tun1: <MULTICAST,NOARP,UP,10000> mtu 1468
> qdisc noqueue
> >     link/gre 0.0.0.0 brd 0.0.0.0
> >     inet 10.40.92.114/24 brd
> 10.40.92.255 scope global tun1
> > 
> > Then attempt:
> > ping -I tun1 224.0.0.9
> > 
> > Are additional configuration steps now required for
> multicast over multipoint GRE tunnel or is ipgre_header() in
> error?
> 
> Hi Doug
> 
> CC Timo Teras <timo.teras@iki.fi>
> 
> I would do a partial revert of Timo patch, but this means
> initial
> concern should be addressed ?
> 
> (Timo mentioned : 
>     If the NOARP packets are not dropped,
> ipgre_tunnel_xmit() will
>     take rt->rt_gateway (= NBMA IP) and
> use that for route
>     look up (and may lead to bogus xfrm
> acquires).)
> 
> 
> Is the following works for you ?
> 
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index da5941f..47844fa2 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -1170,8 +1170,10 @@ static int ipgre_header(struct
> sk_buff *skb, struct net_device *dev,
>  
>      if (saddr)
>         
> memcpy(&iph->saddr, saddr, 4);
> -    if (daddr)
> +    if (daddr) {
>         
> memcpy(&iph->daddr, daddr, 4);
> +        return t->hlen;
> +    }
>      if (iph->daddr)
>          return t->hlen;
>  

Yes, the partial revert does work.

Regards,
...doug



      

^ permalink raw reply

* Re: [PATCH 17/17] net,act_police,rcu: remove rcu_barrier()
From: David Miller @ 2011-03-15 21:04 UTC (permalink / raw)
  To: laijs
  Cc: mingo, paulmck, axboe, Trond.Myklebust, vladislav.yasevich, sri,
	viro, eparis, menage, kuznet, pekkas, jmorris, yoshfuji, kaber,
	johannes, linville, hadi, jpirko, eric.dumazet, hagen,
	randy.dunlap, jkosina, joe, u.kleine-koenig, ebiederm, gregkh,
	xemul, linux-kernel, linux-nfs, linux-sctp, netdev, containers,
	linux-decnet-user, linux-wireless, linux-security-module
In-Reply-To: <4D7F3B62.6000005@cn.fujitsu.com>

From: Lai Jiangshan <laijs@cn.fujitsu.com>
Date: Tue, 15 Mar 2011 18:11:46 +0800

> 
> 
> There is no callback of this module maybe queued
> since we use kfree_rcu(), we can safely remove the rcu_barrier().
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH 15/17] net,rcu: convert call_rcu(ha_rcu_free) to kfree_rcu()
From: David Miller @ 2011-03-15 21:04 UTC (permalink / raw)
  To: laijs-BthXqXjhjHXQFUHtdCDX3A
  Cc: mingo-X9Un+BFzKDI, paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	axboe-tSWWG44O7X1aa/9Udqfwiw,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	vladislav.yasevich-VXdhtT5mjnY, sri-r/Jw6+rmf7HQT0dZR+AlfA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
	eparis-H+wXaHxf7aLQT0dZR+AlfA, menage-hpIqsD4AKlfQT0dZR+AlfA,
	kuznet-v/Mj1YrvjDBInbfyfbPRSQ, pekkas-UjJjq++bwZ7HOG6cAo2yLw,
	jmorris-gx6/JNMH7DfYtjvyW6yDsg, yoshfuji-VfPWfsRibaP+Ru+s062T9g,
	kaber-dcUjhNyLwpNeoWH0uzbU5w, johannes-cdvu00un1VgdHxzADdlk8Q,
	linville-2XuSBdqkA4R54TAoqtyWWQ, hadi-fAAogVwAN2Kw5LPnMra/2Q,
	jpirko-H+wXaHxf7aLQT0dZR+AlfA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, hagen-GvnIQ6b/HdU,
	randy.dunlap-QHcLZuEGTsvQT0dZR+AlfA, jkosina-AlSwsSmVLrQ,
	joe-6d6DIl74uiNBDgjK7y7TUQ,
	u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, gregkh-l3A5Bk7waGM,
	xemul-GEFAQzZX7r8dnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-sctp-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-decnet-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4D7F3ABA.7020709-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

From: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Date: Tue, 15 Mar 2011 18:08:58 +0800

> 
> 
> The rcu callback ha_rcu_free() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(ha_rcu_free).
> 
> Signed-off-by: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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 16/17] net,rcu: convert call_rcu(dn_dev_free_ifa_rcu) to kfree_rcu()
From: David Miller @ 2011-03-15 21:04 UTC (permalink / raw)
  To: laijs
  Cc: mingo, paulmck, axboe, Trond.Myklebust, vladislav.yasevich, sri,
	viro, eparis, menage, kuznet, pekkas, jmorris, yoshfuji, kaber,
	johannes, linville, hadi, jpirko, eric.dumazet, hagen,
	randy.dunlap, jkosina, joe, u.kleine-koenig, ebiederm, gregkh,
	xemul, linux-kernel, linux-nfs, linux-sctp, netdev, containers,
	linux-decnet-user, linux-wireless, linux-security-module
In-Reply-To: <4D7F3B04.9080504@cn.fujitsu.com>

From: Lai Jiangshan <laijs@cn.fujitsu.com>
Date: Tue, 15 Mar 2011 18:10:12 +0800

> 
> The rcu callback dn_dev_free_ifa_rcu() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(dn_dev_free_ifa_rcu).
> 
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH 11/17] net,rcu: convert call_rcu(sctp_local_addr_free) to kfree_rcu()
From: David Miller @ 2011-03-15 21:03 UTC (permalink / raw)
  To: laijs-BthXqXjhjHXQFUHtdCDX3A
  Cc: mingo-X9Un+BFzKDI, paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	axboe-tSWWG44O7X1aa/9Udqfwiw,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	vladislav.yasevich-VXdhtT5mjnY, sri-r/Jw6+rmf7HQT0dZR+AlfA,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
	eparis-H+wXaHxf7aLQT0dZR+AlfA, menage-hpIqsD4AKlfQT0dZR+AlfA,
	kuznet-v/Mj1YrvjDBInbfyfbPRSQ, pekkas-UjJjq++bwZ7HOG6cAo2yLw,
	jmorris-gx6/JNMH7DfYtjvyW6yDsg, yoshfuji-VfPWfsRibaP+Ru+s062T9g,
	kaber-dcUjhNyLwpNeoWH0uzbU5w, johannes-cdvu00un1VgdHxzADdlk8Q,
	linville-2XuSBdqkA4R54TAoqtyWWQ, hadi-fAAogVwAN2Kw5LPnMra/2Q,
	jpirko-H+wXaHxf7aLQT0dZR+AlfA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w, hagen-GvnIQ6b/HdU,
	randy.dunlap-QHcLZuEGTsvQT0dZR+AlfA, jkosina-AlSwsSmVLrQ,
	joe-6d6DIl74uiNBDgjK7y7TUQ,
	u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ,
	ebiederm-aS9lmoZGLiVWk0Htik3J/w, gregkh-l3A5Bk7waGM,
	xemul-GEFAQzZX7r8dnm+yROfE0A, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-sctp-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-decnet-user-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4D7F39CE.80403-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

From: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Date: Tue, 15 Mar 2011 18:05:02 +0800

> 
> 
> The rcu callback sctp_local_addr_free() just calls a kfree(),
> so we use kfree_rcu() instead of the call_rcu(sctp_local_addr_free).
> 
> Signed-off-by: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>

Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox