[PATCH][IPV6][NDISC] unify ipv6 output routine

[PATCH][IPV6][NDISC] unify ipv6 output routine

[Kazunori Miyazawa]

Hello, 

Yoshifuji-san and I send the patch which unifies IPv6 output routine and remove
RTF_NDISC again. It resolves an issue of IPv6 IPsec with neighbor discovery
without a flag.

This patch is against linux-2.6.2.

Best regards,

--Kazunori Miyazawa

===== include/linux/ipv6_route.h 1.4 vs edited =====
--- 1.4/include/linux/ipv6_route.h	Sun Aug 31 13:26:12 2003
+++ edited/include/linux/ipv6_route.h	Mon Jan 26 15:09:34 2004
@@ -24,7 +24,6 @@
 #define RTF_CACHE	0x01000000	/* cache entry			*/
 #define RTF_FLOW	0x02000000	/* flow significant route	*/
 #define RTF_POLICY	0x04000000	/* policy route			*/
-#define RTF_NDISC	0x08000000	/* ndisc route			*/
 
 #define RTF_LOCAL	0x80000000
 
===== include/net/ipv6.h 1.28 vs edited =====
--- 1.28/include/net/ipv6.h	Wed Jan 14 09:36:24 2004
+++ edited/include/net/ipv6.h	Mon Jan 26 15:10:50 2004
@@ -355,6 +355,7 @@
  */
 
 extern int			ip6_output(struct sk_buff *skb);
+extern int			ip6_output2(struct sk_buff *skb);
 extern int			ip6_forward(struct sk_buff *skb);
 extern int			ip6_input(struct sk_buff *skb);
 extern int			ip6_mc_input(struct sk_buff *skb);
===== net/ipv6/ndisc.c 1.64 vs edited =====
--- 1.64/net/ipv6/ndisc.c	Thu Jan 22 15:38:40 2004
+++ edited/net/ipv6/ndisc.c	Mon Jan 26 15:10:16 2004
@@ -390,20 +390,6 @@
  *	Send a Neighbour Advertisement
  */
 
-static int ndisc_output(struct sk_buff *skb)
-{
-	if (skb) {
-		struct neighbour *neigh = (skb->dst ? skb->dst->neighbour : NULL);
-		if (ndisc_build_ll_hdr(skb, skb->dev, &skb->nh.ipv6h->daddr, neigh, skb->len) == 0) {
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-		dev_queue_xmit(skb);
-		return 0;
-	}
-	return -EINVAL;
-}
-
 static inline void ndisc_flow_init(struct flowi *fl, u8 type,
 			    struct in6_addr *saddr, struct in6_addr *daddr)
 {
@@ -446,7 +432,7 @@
 
 	ndisc_flow_init(&fl, NDISC_NEIGHBOUR_ADVERTISEMENT, src_addr, daddr);
 
-	dst = ndisc_dst_alloc(dev, neigh, ndisc_output);
+	dst = ndisc_dst_alloc(dev, neigh, ip6_output2);
 	if (!dst)
 		return;
 
@@ -533,7 +519,7 @@
 
 	ndisc_flow_init(&fl, NDISC_NEIGHBOUR_SOLICITATION, saddr, daddr);
 
-	dst = ndisc_dst_alloc(dev, neigh, ndisc_output);
+	dst = ndisc_dst_alloc(dev, neigh, ip6_output2);
 	if (!dst)
 		return;
 
@@ -605,7 +591,7 @@
 
 	ndisc_flow_init(&fl, NDISC_ROUTER_SOLICITATION, saddr, daddr);
 
-	dst = ndisc_dst_alloc(dev, NULL, ndisc_output);
+	dst = ndisc_dst_alloc(dev, NULL, ip6_output2);
 	if (!dst)
 		return;
 
===== net/ipv6/route.c 1.63 vs edited =====
--- 1.63/net/ipv6/route.c	Sun Jan 25 03:09:52 2004
+++ edited/net/ipv6/route.c	Mon Jan 26 15:11:39 2004
@@ -578,7 +578,7 @@
 	rt->rt6i_dev	  = dev;
 	rt->rt6i_nexthop  = neigh;
 	rt->rt6i_expires  = 0;
-	rt->rt6i_flags    = RTF_LOCAL | RTF_NDISC;
+	rt->rt6i_flags    = RTF_LOCAL;
 	rt->rt6i_metric   = 0;
 	atomic_set(&rt->u.dst.__refcnt, 1);
 	rt->u.dst.metrics[RTAX_HOPLIMIT-1] = 255;
@@ -832,7 +832,7 @@
 		}
 	}
 
-	rt->rt6i_flags = rtmsg->rtmsg_flags & ~RTF_NDISC;
+	rt->rt6i_flags = rtmsg->rtmsg_flags;
 
 install_route:
 	if (rta && rta[RTA_METRICS-1]) {
@@ -1124,8 +1124,6 @@
 static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
 {
 	struct rt6_info *rt = ip6_dst_alloc();
-
-	BUG_ON(ort->rt6i_flags & RTF_NDISC);
 
 	if (rt) {
 		rt->u.dst.input = ort->u.dst.input;
===== net/ipv6/xfrm6_policy.c 1.14 vs edited =====
--- 1.14/net/ipv6/xfrm6_policy.c	Fri Oct 24 21:39:33 2003
+++ edited/net/ipv6/xfrm6_policy.c	Mon Jan 26 15:12:35 2004
@@ -55,13 +55,6 @@
 __xfrm6_find_bundle(struct flowi *fl, struct rtable *rt, struct xfrm_policy *policy)
 {
 	struct dst_entry *dst;
-	u32 ndisc_bit = 0;
-
-	if (fl->proto == IPPROTO_ICMPV6 &&
-	    (fl->fl_icmp_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
-	     fl->fl_icmp_type == NDISC_NEIGHBOUR_SOLICITATION  ||
-	     fl->fl_icmp_type == NDISC_ROUTER_SOLICITATION))
-		ndisc_bit = RTF_NDISC;
 
 	/* Still not clear if we should set fl->fl6_{src,dst}... */
 	read_lock_bh(&policy->lock);
@@ -69,9 +62,6 @@
 		struct xfrm_dst *xdst = (struct xfrm_dst*)dst;
 		struct in6_addr fl_dst_prefix, fl_src_prefix;
 
-		if ((xdst->u.rt6.rt6i_flags & RTF_NDISC) != ndisc_bit)
-			continue;
-
 		ipv6_addr_prefix(&fl_dst_prefix,
 				 &fl->fl6_dst,
 				 xdst->u.rt6.rt6i_dst.plen);
@@ -169,7 +159,7 @@
 		dst_prev->output	= dst_prev->xfrm->type->output;
 		/* Sheit... I remember I did this right. Apparently,
 		 * it was magically lost, so this code needs audit */
-		x->u.rt6.rt6i_flags    = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL|RTF_NDISC);
+		x->u.rt6.rt6i_flags    = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL);
 		x->u.rt6.rt6i_metric   = rt0->rt6i_metric;
 		x->u.rt6.rt6i_node     = rt0->rt6i_node;
 		x->u.rt6.rt6i_gateway  = rt0->rt6i_gateway;

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[Mika Penttilä]

Kazunori Miyazawa wrote:

>Hello, 
>
>Yoshifuji-san and I send the patch which unifies IPv6 output routine and remove
>RTF_NDISC again. It resolves an issue of IPv6 IPsec with neighbor discovery
>without a flag.
>  
>

You break multicast replies, see what ndisc_build_ll_hdr() does...

--Mika


>This patch is against linux-2.6.2.
>
>Best regards,
>
>--Kazunori Miyazawa
>
>===== include/linux/ipv6_route.h 1.4 vs edited =====
>--- 1.4/include/linux/ipv6_route.h	Sun Aug 31 13:26:12 2003
>+++ edited/include/linux/ipv6_route.h	Mon Jan 26 15:09:34 2004
>@@ -24,7 +24,6 @@
> #define RTF_CACHE	0x01000000	/* cache entry			*/
> #define RTF_FLOW	0x02000000	/* flow significant route	*/
> #define RTF_POLICY	0x04000000	/* policy route			*/
>-#define RTF_NDISC	0x08000000	/* ndisc route			*/
> 
> #define RTF_LOCAL	0x80000000
> 
>===== include/net/ipv6.h 1.28 vs edited =====
>--- 1.28/include/net/ipv6.h	Wed Jan 14 09:36:24 2004
>+++ edited/include/net/ipv6.h	Mon Jan 26 15:10:50 2004
>@@ -355,6 +355,7 @@
>  */
> 
> extern int			ip6_output(struct sk_buff *skb);
>+extern int			ip6_output2(struct sk_buff *skb);
> extern int			ip6_forward(struct sk_buff *skb);
> extern int			ip6_input(struct sk_buff *skb);
> extern int			ip6_mc_input(struct sk_buff *skb);
>===== net/ipv6/ndisc.c 1.64 vs edited =====
>--- 1.64/net/ipv6/ndisc.c	Thu Jan 22 15:38:40 2004
>+++ edited/net/ipv6/ndisc.c	Mon Jan 26 15:10:16 2004
>@@ -390,20 +390,6 @@
>  *	Send a Neighbour Advertisement
>  */
> 
>-static int ndisc_output(struct sk_buff *skb)
>-{
>-	if (skb) {
>-		struct neighbour *neigh = (skb->dst ? skb->dst->neighbour : NULL);
>-		if (ndisc_build_ll_hdr(skb, skb->dev, &skb->nh.ipv6h->daddr, neigh, skb->len) == 0) {
>-			kfree_skb(skb);
>-			return -EINVAL;
>-		}
>-		dev_queue_xmit(skb);
>-		return 0;
>-	}
>-	return -EINVAL;
>-}
>-
> static inline void ndisc_flow_init(struct flowi *fl, u8 type,
> 			    struct in6_addr *saddr, struct in6_addr *daddr)
> {
>@@ -446,7 +432,7 @@
> 
> 	ndisc_flow_init(&fl, NDISC_NEIGHBOUR_ADVERTISEMENT, src_addr, daddr);
> 
>-	dst = ndisc_dst_alloc(dev, neigh, ndisc_output);
>+	dst = ndisc_dst_alloc(dev, neigh, ip6_output2);
> 	if (!dst)
> 		return;
> 
>@@ -533,7 +519,7 @@
> 
> 	ndisc_flow_init(&fl, NDISC_NEIGHBOUR_SOLICITATION, saddr, daddr);
> 
>-	dst = ndisc_dst_alloc(dev, neigh, ndisc_output);
>+	dst = ndisc_dst_alloc(dev, neigh, ip6_output2);
> 	if (!dst)
> 		return;
> 
>@@ -605,7 +591,7 @@
> 
> 	ndisc_flow_init(&fl, NDISC_ROUTER_SOLICITATION, saddr, daddr);
> 
>-	dst = ndisc_dst_alloc(dev, NULL, ndisc_output);
>+	dst = ndisc_dst_alloc(dev, NULL, ip6_output2);
> 	if (!dst)
> 		return;
> 
>===== net/ipv6/route.c 1.63 vs edited =====
>--- 1.63/net/ipv6/route.c	Sun Jan 25 03:09:52 2004
>+++ edited/net/ipv6/route.c	Mon Jan 26 15:11:39 2004
>@@ -578,7 +578,7 @@
> 	rt->rt6i_dev	  = dev;
> 	rt->rt6i_nexthop  = neigh;
> 	rt->rt6i_expires  = 0;
>-	rt->rt6i_flags    = RTF_LOCAL | RTF_NDISC;
>+	rt->rt6i_flags    = RTF_LOCAL;
> 	rt->rt6i_metric   = 0;
> 	atomic_set(&rt->u.dst.__refcnt, 1);
> 	rt->u.dst.metrics[RTAX_HOPLIMIT-1] = 255;
>@@ -832,7 +832,7 @@
> 		}
> 	}
> 
>-	rt->rt6i_flags = rtmsg->rtmsg_flags & ~RTF_NDISC;
>+	rt->rt6i_flags = rtmsg->rtmsg_flags;
> 
> install_route:
> 	if (rta && rta[RTA_METRICS-1]) {
>@@ -1124,8 +1124,6 @@
> static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
> {
> 	struct rt6_info *rt = ip6_dst_alloc();
>-
>-	BUG_ON(ort->rt6i_flags & RTF_NDISC);
> 
> 	if (rt) {
> 		rt->u.dst.input = ort->u.dst.input;
>===== net/ipv6/xfrm6_policy.c 1.14 vs edited =====
>--- 1.14/net/ipv6/xfrm6_policy.c	Fri Oct 24 21:39:33 2003
>+++ edited/net/ipv6/xfrm6_policy.c	Mon Jan 26 15:12:35 2004
>@@ -55,13 +55,6 @@
> __xfrm6_find_bundle(struct flowi *fl, struct rtable *rt, struct xfrm_policy *policy)
> {
> 	struct dst_entry *dst;
>-	u32 ndisc_bit = 0;
>-
>-	if (fl->proto == IPPROTO_ICMPV6 &&
>-	    (fl->fl_icmp_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
>-	     fl->fl_icmp_type == NDISC_NEIGHBOUR_SOLICITATION  ||
>-	     fl->fl_icmp_type == NDISC_ROUTER_SOLICITATION))
>-		ndisc_bit = RTF_NDISC;
> 
> 	/* Still not clear if we should set fl->fl6_{src,dst}... */
> 	read_lock_bh(&policy->lock);
>@@ -69,9 +62,6 @@
> 		struct xfrm_dst *xdst = (struct xfrm_dst*)dst;
> 		struct in6_addr fl_dst_prefix, fl_src_prefix;
> 
>-		if ((xdst->u.rt6.rt6i_flags & RTF_NDISC) != ndisc_bit)
>-			continue;
>-
> 		ipv6_addr_prefix(&fl_dst_prefix,
> 				 &fl->fl6_dst,
> 				 xdst->u.rt6.rt6i_dst.plen);
>@@ -169,7 +159,7 @@
> 		dst_prev->output	= dst_prev->xfrm->type->output;
> 		/* Sheit... I remember I did this right. Apparently,
> 		 * it was magically lost, so this code needs audit */
>-		x->u.rt6.rt6i_flags    = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL|RTF_NDISC);
>+		x->u.rt6.rt6i_flags    = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL);
> 		x->u.rt6.rt6i_metric   = rt0->rt6i_metric;
> 		x->u.rt6.rt6i_node     = rt0->rt6i_node;
> 		x->u.rt6.rt6i_gateway  = rt0->rt6i_gateway;
>
>
>
>  
>

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[David S. Miller]

On Fri, 06 Feb 2004 20:03:39 +0200
Mika Penttilä <mika.penttila@kolumbus.fi> wrote:

> >Yoshifuji-san and I send the patch which unifies IPv6 output routine and remove
> >RTF_NDISC again. It resolves an issue of IPv6 IPsec with neighbor discovery
> >without a flag.
> 
> You break multicast replies, see what ndisc_build_ll_hdr() does...

That's right, and compiler should have warned about ndisc_build_ll_hdr()
(a static function in ndisc.c) since it was no longer referenced after
applying the patch, which would be a clue that something was wrong :-)

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[YOSHIFUJI Hideaki / 吉藤英明]

In article <4023D6FB.9010909@kolumbus.fi> (at Fri, 06 Feb 2004 20:03:39 +0200), Mika Penttilä <mika.penttila@kolumbus.fi> says:

> Kazunori Miyazawa wrote:
:
> >Yoshifuji-san and I send the patch which unifies IPv6 output routine and remove
> >RTF_NDISC again. It resolves an issue of IPv6 IPsec with neighbor discovery
> >without a flag.
:
> You break multicast replies, see what ndisc_build_ll_hdr() does...

It doesn't "break." 
The ip6_output2() resolves and inserts link-layer address appropriately.
If it did, we would have noticed (by conformance test or even by
usual operation). ;-)

BTW, Miyazawa-san, we do not need ndisc_build_ll_hdr() anymore.
Compiler warns it. Please remove it. Thanks.


Here's the description:

dst->neighbour owns the neighbour entry for the destination. 

For multicast, we know its link-layer address 
(filled in ndisc_constructor()).
For unicast, the link-layer address may be unknown.

ip6_output2() will call ip6_output_finish() and
it tries inserting the link-layer header.

The destination link-layer address is used if it is known (*).
(Note: you remember that we know link-layer address for multicast.)

If the link-layer address is not known or invalid, 
we queue the packet and resolve the link-layer address for the destination.
Here, since the neighbour entry is "invalid," *multicast* NS is sent. 
The NS packet passes similar path. 
We know the destination for the NS is multicast and 
what the link-layer address is, and the packet is sent via (*).
We will send the queued packet when we got NA from that node.

Real story:
A node may send NS without source link-layer address option 
while we do not know the link-layer address for the source of the NS.
We need to send NA in reply to this NS.
Original code cannot handle this case.  The new code path can.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[Kazunori Miyazawa]

Yoshifuji-san, thank you for following up.
I recreate the pactch. It cares multicast.

> > >Yoshifuji-san and I send the patch
> > > which unifies IPv6 output routine
> > > and remove RTF_NDISC again. It
> > > resolves an issue of IPv6 IPsec
> > > with neighbor discovery without a
> > > flag.
> >
> > You break multicast replies, see
> > what ndisc_build_ll_hdr() does...
>
> It doesn't "break."
> The ip6_output2() resolves and
> inserts link-layer address
> appropriately. If it did, we would
> have noticed (by conformance test or
> even by usual operation). ;-)
>
> BTW, Miyazawa-san, we do not need
> ndisc_build_ll_hdr() anymore.
> Compiler warns it. Please remove it.
> Thanks.
>
>
> Here's the description:
>
> dst->neighbour owns the neighbour
> entry for the destination.
>
> For multicast, we know its link-layer
> address (filled in
> ndisc_constructor()). For unicast,
> the link-layer address may be
> unknown.
>
> ip6_output2() will call
> ip6_output_finish() and it tries
> inserting the link-layer header.
>
> The destination link-layer address is
> used if it is known (*). (Note: you
> remember that we know link-layer
> address for multicast.)
>
--Kazunori Miyazawa

===== include/linux/ipv6_route.h 1.4 vs edited =====
--- 1.4/include/linux/ipv6_route.h	Sun Aug 31 13:26:12 2003
+++ edited/include/linux/ipv6_route.h	Mon Jan 26 15:09:34 2004
@@ -24,7 +24,6 @@
 #define RTF_CACHE	0x01000000	/* cache entry			*/
 #define RTF_FLOW	0x02000000	/* flow significant route	*/
 #define RTF_POLICY	0x04000000	/* policy route			*/
-#define RTF_NDISC	0x08000000	/* ndisc route			*/
 
 #define RTF_LOCAL	0x80000000
 
===== include/net/ip6_route.h 1.11 vs edited =====
--- 1.11/include/net/ip6_route.h	Sat Jul 19 15:42:53 2003
+++ edited/include/net/ip6_route.h	Sat Feb  7 04:11:39 2004
@@ -64,6 +64,7 @@
 
 extern struct dst_entry *ndisc_dst_alloc(struct net_device *dev,
 					 struct neighbour *neigh,
+					 struct in6_addr *addr,
 					 int (*output)(struct sk_buff *));
 extern int ndisc_dst_gc(int *more);
 extern void fib6_force_start_gc(void);
===== include/net/ipv6.h 1.28 vs edited =====
--- 1.28/include/net/ipv6.h	Wed Jan 14 09:36:24 2004
+++ edited/include/net/ipv6.h	Mon Jan 26 15:10:50 2004
@@ -355,6 +355,7 @@
  */
 
 extern int			ip6_output(struct sk_buff *skb);
+extern int			ip6_output2(struct sk_buff *skb);
 extern int			ip6_forward(struct sk_buff *skb);
 extern int			ip6_input(struct sk_buff *skb);
 extern int			ip6_mc_input(struct sk_buff *skb);
===== net/ipv6/ndisc.c 1.64 vs edited =====
--- 1.64/net/ipv6/ndisc.c	Thu Jan 22 15:38:40 2004
+++ edited/net/ipv6/ndisc.c	Sat Feb  7 13:06:24 2004
@@ -345,65 +345,10 @@
 	ipv6_dev_mc_dec(dev, &maddr);
 }
 
-
-
-static int
-ndisc_build_ll_hdr(struct sk_buff *skb, struct net_device *dev,
-		   struct in6_addr *daddr, struct neighbour *neigh, int len)
-{
-	unsigned char ha[MAX_ADDR_LEN];
-	unsigned char *h_dest = NULL;
-
-	if (dev->hard_header) {
-		if (ipv6_addr_type(daddr) & IPV6_ADDR_MULTICAST) {
-			ndisc_mc_map(daddr, ha, dev, 1);
-			h_dest = ha;
-		} else if (neigh) {
-			read_lock_bh(&neigh->lock);
-			if (neigh->nud_state&NUD_VALID) {
-				memcpy(ha, neigh->ha, dev->addr_len);
-				h_dest = ha;
-			}
-			read_unlock_bh(&neigh->lock);
-		} else {
-			neigh = neigh_lookup(&nd_tbl, daddr, dev);
-			if (neigh) {
-				read_lock_bh(&neigh->lock);
-				if (neigh->nud_state&NUD_VALID) {
-					memcpy(ha, neigh->ha, dev->addr_len);
-					h_dest = ha;
-				}
-				read_unlock_bh(&neigh->lock);
-				neigh_release(neigh);
-			}
-		}
-
-		if (dev->hard_header(skb, dev, ETH_P_IPV6, h_dest, NULL, len) < 0)
-			return 0;
-	}
-
-	return 1;
-}
-
-
 /*
  *	Send a Neighbour Advertisement
  */
 
-static int ndisc_output(struct sk_buff *skb)
-{
-	if (skb) {
-		struct neighbour *neigh = (skb->dst ? skb->dst->neighbour : NULL);
-		if (ndisc_build_ll_hdr(skb, skb->dev, &skb->nh.ipv6h->daddr, neigh, skb->len) == 0) {
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-		dev_queue_xmit(skb);
-		return 0;
-	}
-	return -EINVAL;
-}
-
 static inline void ndisc_flow_init(struct flowi *fl, u8 type,
 			    struct in6_addr *saddr, struct in6_addr *daddr)
 {
@@ -446,7 +391,7 @@
 
 	ndisc_flow_init(&fl, NDISC_NEIGHBOUR_ADVERTISEMENT, src_addr, daddr);
 
-	dst = ndisc_dst_alloc(dev, neigh, ndisc_output);
+	dst = ndisc_dst_alloc(dev, neigh, daddr, ip6_output2);
 	if (!dst)
 		return;
 
@@ -533,7 +478,7 @@
 
 	ndisc_flow_init(&fl, NDISC_NEIGHBOUR_SOLICITATION, saddr, daddr);
 
-	dst = ndisc_dst_alloc(dev, neigh, ndisc_output);
+	dst = ndisc_dst_alloc(dev, neigh, daddr, ip6_output2);
 	if (!dst)
 		return;
 
@@ -605,7 +550,7 @@
 
 	ndisc_flow_init(&fl, NDISC_ROUTER_SOLICITATION, saddr, daddr);
 
-	dst = ndisc_dst_alloc(dev, NULL, ndisc_output);
+	dst = ndisc_dst_alloc(dev, NULL, daddr, ip6_output2);
 	if (!dst)
 		return;
 
===== net/ipv6/route.c 1.63 vs edited =====
--- 1.63/net/ipv6/route.c	Sun Jan 25 03:09:52 2004
+++ edited/net/ipv6/route.c	Sat Feb  7 03:57:17 2004
@@ -563,6 +563,7 @@
 
 struct dst_entry *ndisc_dst_alloc(struct net_device *dev, 
 				  struct neighbour *neigh,
+				  struct in6_addr *addr,
 				  int (*output)(struct sk_buff *))
 {
 	struct rt6_info *rt = ip6_dst_alloc();
@@ -574,11 +575,13 @@
 		dev_hold(dev);
 	if (neigh)
 		neigh_hold(neigh);
+	else
+		neigh = ndisc_get_neigh(dev, addr);
 
 	rt->rt6i_dev	  = dev;
 	rt->rt6i_nexthop  = neigh;
 	rt->rt6i_expires  = 0;
-	rt->rt6i_flags    = RTF_LOCAL | RTF_NDISC;
+	rt->rt6i_flags    = RTF_LOCAL;
 	rt->rt6i_metric   = 0;
 	atomic_set(&rt->u.dst.__refcnt, 1);
 	rt->u.dst.metrics[RTAX_HOPLIMIT-1] = 255;
@@ -832,7 +835,7 @@
 		}
 	}
 
-	rt->rt6i_flags = rtmsg->rtmsg_flags & ~RTF_NDISC;
+	rt->rt6i_flags = rtmsg->rtmsg_flags;
 
 install_route:
 	if (rta && rta[RTA_METRICS-1]) {
@@ -1124,8 +1127,6 @@
 static struct rt6_info * ip6_rt_copy(struct rt6_info *ort)
 {
 	struct rt6_info *rt = ip6_dst_alloc();
-
-	BUG_ON(ort->rt6i_flags & RTF_NDISC);
 
 	if (rt) {
 		rt->u.dst.input = ort->u.dst.input;
===== net/ipv6/xfrm6_policy.c 1.14 vs edited =====
--- 1.14/net/ipv6/xfrm6_policy.c	Fri Oct 24 21:39:33 2003
+++ edited/net/ipv6/xfrm6_policy.c	Mon Jan 26 15:12:35 2004
@@ -55,13 +55,6 @@
 __xfrm6_find_bundle(struct flowi *fl, struct rtable *rt, struct xfrm_policy *policy)
 {
 	struct dst_entry *dst;
-	u32 ndisc_bit = 0;
-
-	if (fl->proto == IPPROTO_ICMPV6 &&
-	    (fl->fl_icmp_type == NDISC_NEIGHBOUR_ADVERTISEMENT ||
-	     fl->fl_icmp_type == NDISC_NEIGHBOUR_SOLICITATION  ||
-	     fl->fl_icmp_type == NDISC_ROUTER_SOLICITATION))
-		ndisc_bit = RTF_NDISC;
 
 	/* Still not clear if we should set fl->fl6_{src,dst}... */
 	read_lock_bh(&policy->lock);
@@ -69,9 +62,6 @@
 		struct xfrm_dst *xdst = (struct xfrm_dst*)dst;
 		struct in6_addr fl_dst_prefix, fl_src_prefix;
 
-		if ((xdst->u.rt6.rt6i_flags & RTF_NDISC) != ndisc_bit)
-			continue;
-
 		ipv6_addr_prefix(&fl_dst_prefix,
 				 &fl->fl6_dst,
 				 xdst->u.rt6.rt6i_dst.plen);
@@ -169,7 +159,7 @@
 		dst_prev->output	= dst_prev->xfrm->type->output;
 		/* Sheit... I remember I did this right. Apparently,
 		 * it was magically lost, so this code needs audit */
-		x->u.rt6.rt6i_flags    = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL|RTF_NDISC);
+		x->u.rt6.rt6i_flags    = rt0->rt6i_flags&(RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL);
 		x->u.rt6.rt6i_metric   = rt0->rt6i_metric;
 		x->u.rt6.rt6i_node     = rt0->rt6i_node;
 		x->u.rt6.rt6i_gateway  = rt0->rt6i_gateway;

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[Mika Penttilä]

YOSHIFUJI Hideaki / ???? wrote:

>In article <4023D6FB.9010909@kolumbus.fi> (at Fri, 06 Feb 2004 20:03:39 +0200), Mika Penttilä <mika.penttila@kolumbus.fi> says:
>
>  
>
>>Kazunori Miyazawa wrote:
>>    
>>
>:
>  
>
>>>Yoshifuji-san and I send the patch which unifies IPv6 output routine and remove
>>>RTF_NDISC again. It resolves an issue of IPv6 IPsec with neighbor discovery
>>>without a flag.
>>>      
>>>
>:
>  
>
>>You break multicast replies, see what ndisc_build_ll_hdr() does...
>>    
>>
>
>It doesn't "break." 
>The ip6_output2() resolves and inserts link-layer address appropriately.
>If it did, we would have noticed (by conformance test or even by
>usual operation). ;-)
>  
>
ip6_output2() doesn't resolve link-layer addresses. We don't even have a 
neighbour, in

ndisc_dst_alloc(dev, NULL, ip6_output2); case.


>BTW, Miyazawa-san, we do not need ndisc_build_ll_hdr() anymore.
>Compiler warns it. Please remove it. Thanks.
>
>
>Here's the description:
>
>dst->neighbour owns the neighbour entry for the destination. 
>
>For multicast, we know its link-layer address 
>(filled in ndisc_constructor()).
>For unicast, the link-layer address may be unknown.
>
>ip6_output2() will call ip6_output_finish() and
>it tries inserting the link-layer header.
>
>The destination link-layer address is used if it is known (*).
>(Note: you remember that we know link-layer address for multicast.)
>
>If the link-layer address is not known or invalid, 
>we queue the packet and resolve the link-layer address for the destination.
>Here, since the neighbour entry is "invalid," *multicast* NS is sent. 
>The NS packet passes similar path. 
>We know the destination for the NS is multicast and 
>what the link-layer address is, and the packet is sent via (*).
>We will send the queued packet when we got NA from that node.
>
>Real story:
>A node may send NS without source link-layer address option 
>while we do not know the link-layer address for the source of the NS.
>We need to send NA in reply to this NS.
>Original code cannot handle this case.  The new code path can.
>  
>

You just described how the normal link layer address resolving goes. It 
doesn't apply here. Again, there is no dst->neighbour.



--Mika

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[YOSHIFUJI Hideaki / 吉藤英明]

In article <4024A488.60203@kolumbus.fi> (at Sat, 07 Feb 2004 10:40:40 +0200), Mika Penttilä <mika.penttila@kolumbus.fi> says:

> >The ip6_output2() resolves and inserts link-layer address appropriately.
> >If it did, we would have noticed (by conformance test or even by
> >usual operation). ;-)
> >  
> >
> ip6_output2() doesn't resolve link-layer addresses. We don't even have a 
> neighbour, in
> ndisc_dst_alloc(dev, NULL, ip6_output2); case.

ip6_output2() calls ip6_output_flinish().
ip6_output_finish() calls dst->hh->hh_output() if hh is already built.
Otherwise, dst->neighbour->output() is called and it resolves 
link-layer address of neighbor.

I think you missed our ndsic_dst_alloc() change.
ndisc_dst_alloc() takes 4 argument:
   struct dst_entry *ndisc_dst_alloc(struct net_device *dev, 
                                     struct neighbour *neigh,
                                     struct in6_addr *addr,
                                     int (*output)(struct sk_buff *));
If neigh is NULL, we do ndisc_get_neigh(dev, addr) to get one.


> You just described how the normal link layer address resolving goes. It 
> doesn't apply here. Again, there is no dst->neighbour.

There IS with our patch.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[Mika Penttilä]

YOSHIFUJI Hideaki / ???? wrote:

>In article <4024A488.60203@kolumbus.fi> (at Sat, 07 Feb 2004 10:40:40 +0200), Mika Penttilä <mika.penttila@kolumbus.fi> says:
>
>  
>
>>>The ip6_output2() resolves and inserts link-layer address appropriately.
>>>If it did, we would have noticed (by conformance test or even by
>>>usual operation). ;-)
>>> 
>>>
>>>      
>>>
>>ip6_output2() doesn't resolve link-layer addresses. We don't even have a 
>>neighbour, in
>>ndisc_dst_alloc(dev, NULL, ip6_output2); case.
>>    
>>
>
>ip6_output2() calls ip6_output_flinish().
>ip6_output_finish() calls dst->hh->hh_output() if hh is already built.
>Otherwise, dst->neighbour->output() is called and it resolves 
>link-layer address of neighbor.
>
>I think you missed our ndsic_dst_alloc() change.
>ndisc_dst_alloc() takes 4 argument:
>   struct dst_entry *ndisc_dst_alloc(struct net_device *dev, 
>                                     struct neighbour *neigh,
>                                     struct in6_addr *addr,
>                                     int (*output)(struct sk_buff *));
>If neigh is NULL, we do ndisc_get_neigh(dev, addr) to get one.
>
>  
>
hmm. where is this ndisc_dst_alloc() change? In the patch it's called 
with three params, only the output routine is changed :

- dst = ndisc_dst_alloc(dev, NULL, ndisc_output);
+ dst = ndisc_dst_alloc(dev, NULL, ip6_output2);


--Mika

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[Mika Penttilä]

Mika Penttilä wrote:

>
>
> YOSHIFUJI Hideaki / ???? wrote:
>
>> In article <4024A488.60203@kolumbus.fi> (at Sat, 07 Feb 2004 10:40:40 
>> +0200), Mika Penttilä <mika.penttila@kolumbus.fi> says:
>>
>>  
>>
>>>> The ip6_output2() resolves and inserts link-layer address 
>>>> appropriately.
>>>> If it did, we would have noticed (by conformance test or even by
>>>> usual operation). ;-)
>>>>
>>>>
>>>>     
>>>
>>> ip6_output2() doesn't resolve link-layer addresses. We don't even 
>>> have a neighbour, in
>>> ndisc_dst_alloc(dev, NULL, ip6_output2); case.
>>>   
>>
>>
>> ip6_output2() calls ip6_output_flinish().
>> ip6_output_finish() calls dst->hh->hh_output() if hh is already built.
>> Otherwise, dst->neighbour->output() is called and it resolves 
>> link-layer address of neighbor.
>>
>> I think you missed our ndsic_dst_alloc() change.
>> ndisc_dst_alloc() takes 4 argument:
>>   struct dst_entry *ndisc_dst_alloc(struct net_device *dev, 
>>                                     struct neighbour *neigh,
>>                                     struct in6_addr *addr,
>>                                     int (*output)(struct sk_buff *));
>> If neigh is NULL, we do ndisc_get_neigh(dev, addr) to get one.
>>
>>  
>>
> hmm. where is this ndisc_dst_alloc() change? In the patch it's called 
> with three params, only the output routine is changed :
>
> - dst = ndisc_dst_alloc(dev, NULL, ndisc_output);
> + dst = ndisc_dst_alloc(dev, NULL, ip6_output2);
>
>
> --Mika
>
I see, it was in a follow-up patch. Ok, I was just looking at the 
original patch, which didn't work. But with this ndisc_dst_alloc() 
change it's ok.

Thanks,
--Mika

Re: (usagi-core 17380) Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[YOSHIFUJI Hideaki / 吉藤英明]

In article <4024C1D5.5030906@kolumbus.fi> (at Sat, 07 Feb 2004 12:45:41 +0200), Mika Penttilä <mika.penttila@kolumbus.fi> says:

> >> I think you missed our ndsic_dst_alloc() change.
> >> ndisc_dst_alloc() takes 4 argument:
> >>   struct dst_entry *ndisc_dst_alloc(struct net_device *dev, 
> >>                                     struct neighbour *neigh,
> >>                                     struct in6_addr *addr,
> >>                                     int (*output)(struct sk_buff *));
> >> If neigh is NULL, we do ndisc_get_neigh(dev, addr) to get one.
> >>
> >>  
> >>
> > hmm. where is this ndisc_dst_alloc() change? In the patch it's called 
> > with three params, only the output routine is changed :
> >
> > - dst = ndisc_dst_alloc(dev, NULL, ndisc_output);
> > + dst = ndisc_dst_alloc(dev, NULL, ip6_output2);
:
> I see, it was in a follow-up patch. Ok, I was just looking at the 
> original patch, which didn't work. But with this ndisc_dst_alloc() 
> change it's ok.

Oh my god!!

Miyazawa-san failed to export our "correct" code in original patch.
I didn't noticed that.

We're sorry about it. Thank you for spotting the bug.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[David S. Miller]

Is a fixed version of this output path unification patch
coming soon?

Re: [PATCH][IPV6][NDISC] unify ipv6 output routine

[YOSHIFUJI Hideaki / 吉藤英明]

In article <20040208130811.328225e7.davem@redhat.com> (at Sun, 8 Feb 2004 13:08:11 -0800), "David S. Miller" <davem@redhat.com> says:

> 
> Is a fixed version of this output path unification patch
> coming soon?

No. The second patch from Miyazawa-san is fine.

-- 
Hideaki YOSHIFUJI @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG FP: 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA