Patchwork 2.6.34 + IPv6: Oops?

login
register
mail settings
Submitter stephen hemminger
Date June 21, 2010, 5:25 p.m.
Message ID <20100621102508.2075d677@nehalam>
Download mbox | patch
Permalink /patch/56335/
State RFC
Delegated to: David Miller
Headers show

Comments

stephen hemminger - June 21, 2010, 5:25 p.m.
The OOPS is here
static void ndisc_send_na(struct net_device *dev, struct neighbour *neigh,
			  const struct in6_addr *daddr,
			  const struct in6_addr *solicited_addr,
			  int router, int solicited, int override, int inc_opt)
{
...
	/* for anycast or proxy, solicited_addr != src_addr */
	ifp = ipv6_get_ifaddr(dev_net(dev), solicited_addr, dev, 1);
	if (ifp) {
		src_addr = solicited_addr;
		if (ifp->flags & IFA_F_OPTIMISTIC)
			override = 0;
		in6_ifa_put(ifp);
	} else {
		if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
				       inet6_sk(dev_net(dev)->ipv6.ndisc_sk)->srcprefs,
				       &tmpaddr))
			return;
		src_addr = &tmpaddr;
	}

	icmp6h.icmp6_router = router;
	icmp6h.icmp6_solicited = solicited;
	icmp6h.icmp6_override = override;

	inc_opt |= ifp->idev->cnf.force_tllao;

And it caused by this recent commit.

Author: Octavian Purdila <opurdila@ixiacom.com>  2009-10-02 04:39:15
Committer: David S. Miller <davem@davemloft.net>  2009-10-07 01:10:45
Parent: d1f8297a96b0d70f17704296a6666468f2087ce6 (Revert "sit: stateless autoconf for isatap")
Child:  d7fc02c7bae7b1cf69269992cf880a43a350cdaa (Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next-2.6)
Branches: addrconf, master, remotes/origin/master
Follows: v2.6.32-rc3
Precedes: v2.6.33-rc1

    make TLLAO option for NA packets configurable
    
    On Friday 02 October 2009 20:53:51 you wrote:
    
    > This is good although I would have shortened the name.
    
    Ah, I knew I forgot something :) Here is v4.
    
    tavi
    
    >From 24d96d825b9fa832b22878cc6c990d5711968734 Mon Sep 17 00:00:00 2001
    From: Octavian Purdila <opurdila@ixiacom.com>
    Date: Fri, 2 Oct 2009 00:51:15 +0300
    Subject: [PATCH] ipv6: new sysctl for sending TLLAO with unicast NAs
    
    Neighbor advertisements responding to unicast neighbor solicitations
    did not include the target link-layer address option. This patch adds
    a new sysctl option (disabled by default) which controls whether this
    option should be sent even with unicast NAs.
    
    The need for this arose because certain routers expect the TLLAO in
    some situations even as a response to unicast NS packets.
    
    Moreover, RFC 2461 recommends sending this to avoid a race condition
    (section 4.4, Target link-layer address)
    
    Signed-off-by: Cosmin Ratiu <cratiu@ixiacom.com>
    Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

It is not handling the case of ifp == NULL.

Maybe the following (move the assignment into the if block).




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Klauer - June 21, 2010, 8:04 p.m.
On Mon, Jun 21, 2010 at 10:25:08AM -0700, Stephen Hemminger wrote:
> It is not handling the case of ifp == NULL.
> 
> Maybe the following (move the assignment into the if block).
> 
> --- a/net/ipv6/ndisc.c	2010-06-21 10:22:20.825637690 -0700
> +++ b/net/ipv6/ndisc.c	2010-06-21 10:24:31.573011996 -0700
> @@ -586,6 +586,7 @@ static void ndisc_send_na(struct net_dev
>  		src_addr = solicited_addr;
>  		if (ifp->flags & IFA_F_OPTIMISTIC)
>  			override = 0;
> +		inc_opt |= ifp->idev->cnf.force_tllao;
>  		in6_ifa_put(ifp);
>  	} else {
>  		if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
> @@ -599,7 +600,6 @@ static void ndisc_send_na(struct net_dev
>  	icmp6h.icmp6_solicited = solicited;
>  	icmp6h.icmp6_override = override;
>  
> -	inc_opt |= ifp->idev->cnf.force_tllao;
>  	__ndisc_send(dev, neigh, daddr, src_addr,
>  		     &icmp6h, solicited_addr,
>  		     inc_opt ? ND_OPT_TARGET_LL_ADDR : 0);
> 
> 
> 

Thanks a lot! This fix seems to work fine for me (tested locally only).
I'll see what happens when I apply it to my server tomorrow.

Curious though as to why I wasn't able to reproduce it when compiling 
the kernel with Gentoo's GCC. It doesn't look like it should make any 
difference. Maybe I made a mistake when I tested it with Gentoo.

Regards
Andreas Klauer
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andreas Klauer - June 22, 2010, 12:29 p.m.
On Mon, Jun 21, 2010 at 10:04:13PM +0200, Andreas Klauer wrote:
> Thanks a lot! This fix seems to work fine for me (tested locally only).
> I'll see what happens when I apply it to my server tomorrow.

The fix also works for my server.

Thank you
Andreas Klauer
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- a/net/ipv6/ndisc.c	2010-06-21 10:22:20.825637690 -0700
+++ b/net/ipv6/ndisc.c	2010-06-21 10:24:31.573011996 -0700
@@ -586,6 +586,7 @@  static void ndisc_send_na(struct net_dev
 		src_addr = solicited_addr;
 		if (ifp->flags & IFA_F_OPTIMISTIC)
 			override = 0;
+		inc_opt |= ifp->idev->cnf.force_tllao;
 		in6_ifa_put(ifp);
 	} else {
 		if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
@@ -599,7 +600,6 @@  static void ndisc_send_na(struct net_dev
 	icmp6h.icmp6_solicited = solicited;
 	icmp6h.icmp6_override = override;
 
-	inc_opt |= ifp->idev->cnf.force_tllao;
 	__ndisc_send(dev, neigh, daddr, src_addr,
 		     &icmp6h, solicited_addr,
 		     inc_opt ? ND_OPT_TARGET_LL_ADDR : 0);