netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
@ 2004-07-30  8:12 Kazunori Miyazawa
  2004-08-02  2:51 ` David S. Miller
  2004-08-02  7:41 ` [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Herbert Xu
  0 siblings, 2 replies; 19+ messages in thread
From: Kazunori Miyazawa @ 2004-07-30  8:12 UTC (permalink / raw)
  To: davem, herbert; +Cc: netdev, usagi-core, kazunori

Hello,

This patch separates xfrm_lookup from ip6_dst_lookup
to support srcrt correctly. ip6_dst_lookup should be
called with next hop. however xfrm_lookup should be
called with final destination. It fixes them.

This patch makes AH support routing header with
previous Mr Hervert's patch. Of course this fixes ESP and IPcomp.

I consider copying flowi(fl_rt) uses too much stack at the moment.
I'll re-send the fixed patch again.

Thank you,

--Kazunori Miyazawa


diff -ruNBE a/net/ipv6/datagram.c b/net/ipv6/datagram.c
--- a/net/ipv6/datagram.c	2004-07-28 10:07:16.000000000 +0900
+++ b/net/ipv6/datagram.c	2004-07-28 10:29:13.000000000 +0900
@@ -40,7 +40,7 @@
 	struct ipv6_pinfo      	*np = inet6_sk(sk);
 	struct in6_addr		*daddr;
 	struct dst_entry	*dst;
-	struct flowi		fl;
+	struct flowi		fl, fl_rt, *flp = &fl;
 	struct ip6_flowlabel	*flowlabel = NULL;
 	int			addr_type;
 	int			err;
@@ -157,17 +157,27 @@
 	if (flowlabel) {
 		if (flowlabel->opt && flowlabel->opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) flowlabel->opt->srcrt;
-			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			memcpy(&fl_rt, &fl, sizeof(fl_rt));
+			ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+			flp = &fl_rt;
 		}
 	} else if (np->opt && np->opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *)np->opt->srcrt;
-		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		memcpy(&fl_rt, &fl, sizeof(fl_rt));
+		ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+		flp = &fl_rt;
 	}
 
-	err = ip6_dst_lookup(sk, &dst, &fl);
+	err = ip6_dst_lookup(sk, &dst, flp);
 	if (err)
 		goto out;
 
+	if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+		dst_release(dst);
+		dst = NULL;
+		goto out;
+	}
+
 	/* source address lookup done in ip6_dst_lookup */
 
 	if (ipv6_addr_any(&np->saddr))
diff -ruNBE a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
--- a/net/ipv6/ip6_output.c	2004-07-28 10:07:09.000000000 +0900
+++ b/net/ipv6/ip6_output.c	2004-07-28 10:29:20.000000000 +0900
@@ -796,10 +796,6 @@
 			goto out_err_release;
 		}
 	}
-	if ((err = xfrm_lookup(dst, fl, sk, 0)) < 0) {
-		err = -ENETUNREACH;
-		goto out_err_release;
-        }
 
 	return 0;
 
diff -ruNBE a/net/ipv6/raw.c b/net/ipv6/raw.c
--- a/net/ipv6/raw.c	2004-07-28 10:08:26.000000000 +0900
+++ b/net/ipv6/raw.c	2004-07-28 10:30:22.000000000 +0900
@@ -567,7 +567,7 @@
 	struct ipv6_txoptions *opt = NULL;
 	struct ip6_flowlabel *flowlabel = NULL;
 	struct dst_entry *dst = NULL;
-	struct flowi fl;
+	struct flowi fl, fl_rt, *flp = &fl;
 	int addr_len = msg->msg_namelen;
 	int hlimit = -1;
 	u16 proto;
@@ -674,6 +674,7 @@
 		opt = fl6_merge_options(&opt_space, flowlabel, opt);
 
 	fl.proto = proto;
+ 
 	ipv6_addr_copy(&fl.fl6_dst, daddr);
 	if (ipv6_addr_any(&fl.fl6_src) && !ipv6_addr_any(&np->saddr))
 		ipv6_addr_copy(&fl.fl6_src, &np->saddr);
@@ -681,16 +682,24 @@
 	/* merge ip6_build_xmit from ip6_output */
 	if (opt && opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
-		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		memcpy(&fl_rt, &fl, sizeof(fl_rt));
+		ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+		flp = &fl_rt;
 	}
 
 	if (!fl.oif && ipv6_addr_is_multicast(&fl.fl6_dst))
 		fl.oif = np->mcast_oif;
 
-	err = ip6_dst_lookup(sk, &dst, &fl);
+	err = ip6_dst_lookup(sk, &dst, flp);
 	if (err)
 		goto out;
 
+	if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+		dst_release(dst);
+		dst = NULL;
+		goto out;
+	}
+
 	if (hlimit < 0) {
 		if (ipv6_addr_is_multicast(&fl.fl6_dst))
 			hlimit = np->mcast_hops;
diff -ruNBE a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
--- a/net/ipv6/tcp_ipv6.c	2004-07-28 10:09:10.000000000 +0900
+++ b/net/ipv6/tcp_ipv6.c	2004-07-28 10:29:37.000000000 +0900
@@ -550,7 +550,7 @@
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct tcp_opt *tp = tcp_sk(sk);
 	struct in6_addr *saddr = NULL;
-	struct flowi fl;
+	struct flowi fl, fl_rt, *flp = &fl;
 	struct dst_entry *dst;
 	int addr_type;
 	int err;
@@ -666,14 +666,21 @@
 
 	if (np->opt && np->opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *)np->opt->srcrt;
-		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		memcpy(&fl_rt, &fl, sizeof(fl_rt));
+		ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+		flp = &fl_rt;
 	}
 
-	err = ip6_dst_lookup(sk, &dst, &fl);
-
+	err = ip6_dst_lookup(sk, &dst, flp);
 	if (err)
 		goto failure;
 
+	if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+		dst_release(dst);
+		dst = NULL;
+		goto failure;
+	}
+
 	if (saddr == NULL) {
 		saddr = &fl.fl6_src;
 		ipv6_addr_copy(&np->rcv_saddr, saddr);
@@ -793,6 +800,14 @@
 				sk->sk_err_soft = -err;
 				goto out;
 			}
+
+			if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+				sk->sk_err_soft = -err;
+				dst_release(dst);
+				dst = NULL;
+				goto out;
+			}
+
 		} else
 			dst_hold(dst);
 
@@ -863,7 +878,7 @@
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sk_buff * skb;
 	struct ipv6_txoptions *opt = NULL;
-	struct flowi fl;
+	struct flowi fl, fl_rt, *flp = &fl;
 	int err = -1;
 
 	memset(&fl, 0, sizeof(fl));
@@ -888,12 +903,22 @@
 
 		if (opt && opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
-			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			memcpy(&fl_rt, &fl, sizeof(fl_rt));
+			ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+			flp = &fl_rt;
 		}
 
-		err = ip6_dst_lookup(sk, &dst, &fl);
+		err = ip6_dst_lookup(sk, &dst, flp);
 		if (err)
 			goto done;
+
+		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+			dst_release(dst);
+			dst = NULL;
+			goto done;
+		}
+
+
 	}
 
 	skb = tcp_make_synack(sk, dst, req);
@@ -1021,6 +1046,12 @@
 
 	/* sk = NULL, but it is safe for now. RST socket required. */
 	if (!ip6_dst_lookup(NULL, &buff->dst, &fl)) {
+
+		if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0) {
+			dst_release(buff->dst);
+			return;
+		}
+
 		ip6_xmit(NULL, buff, &fl, NULL, 0);
 		TCP_INC_STATS_BH(TCP_MIB_OUTSEGS);
 		TCP_INC_STATS_BH(TCP_MIB_OUTRSTS);
@@ -1082,6 +1113,12 @@
 	fl.fl_ip_sport = t1->source;
 
 	if (!ip6_dst_lookup(NULL, &buff->dst, &fl)) {
+
+		if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0) {
+			dst_release(buff->dst);
+			return;
+		}
+
 		ip6_xmit(NULL, buff, &fl, NULL, 0);
 		TCP_INC_STATS_BH(TCP_MIB_OUTSEGS);
 		return;
@@ -1313,22 +1350,31 @@
 	}
 
 	if (dst == NULL) {
-		struct flowi fl;
+		struct flowi fl, fl_rt, *flp = &fl;
 
 		memset(&fl, 0, sizeof(fl));
 		fl.proto = IPPROTO_TCP;
 		ipv6_addr_copy(&fl.fl6_dst, &req->af.v6_req.rmt_addr);
 		if (opt && opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
-			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			memcpy(&fl_rt, &fl, sizeof(fl_rt));
+			ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+			flp = &fl_rt;
 		}
 		ipv6_addr_copy(&fl.fl6_src, &req->af.v6_req.loc_addr);
 		fl.oif = sk->sk_bound_dev_if;
 		fl.fl_ip_dport = req->rmt_port;
 		fl.fl_ip_sport = inet_sk(sk)->sport;
 
-		if (ip6_dst_lookup(sk, &dst, &fl))
+		if (ip6_dst_lookup(sk, &dst, flp))
 			goto out;
+
+		if ((xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+			dst_release(dst);
+			dst = NULL;
+			goto out;
+		}
+
 	} 
 
 	newsk = tcp_create_openreq_child(sk, req, skb);
@@ -1710,7 +1756,7 @@
 
 	if (dst == NULL) {
 		struct inet_opt *inet = inet_sk(sk);
-		struct flowi fl;
+		struct flowi fl, fl_rt, *flp = &fl;
 
 		memset(&fl, 0, sizeof(fl));
 		fl.proto = IPPROTO_TCP;
@@ -1723,16 +1769,25 @@
 
 		if (np->opt && np->opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
-			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			memcpy(&fl_rt, &fl, sizeof(fl_rt));
+			ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+			flp = &fl_rt;
 		}
 
-		err = ip6_dst_lookup(sk, &dst, &fl);
+		err = ip6_dst_lookup(sk, &dst, flp);
 
 		if (err) {
 			sk->sk_route_caps = 0;
 			return err;
 		}
 
+		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+			sk->sk_route_caps = 0;
+			dst_release(dst);
+			return err;
+		}
+
+
 		ip6_dst_store(sk, dst, NULL);
 		sk->sk_route_caps = dst->dev->features &
 			~(NETIF_F_IP_CSUM | NETIF_F_TSO);
@@ -1747,7 +1802,7 @@
 	struct sock *sk = skb->sk;
 	struct inet_opt *inet = inet_sk(sk);
 	struct ipv6_pinfo *np = inet6_sk(sk);
-	struct flowi fl;
+	struct flowi fl, fl_rt, *flp = &fl;
 	struct dst_entry *dst;
 
 	memset(&fl, 0, sizeof(fl));
@@ -1762,19 +1817,27 @@
 
 	if (np->opt && np->opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
-		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		memcpy(&fl_rt, &fl, sizeof(fl_rt));
+		ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+		flp = &fl_rt;
 	}
 
 	dst = __sk_dst_check(sk, np->dst_cookie);
 
 	if (dst == NULL) {
-		int err = ip6_dst_lookup(sk, &dst, &fl);
+		int err = ip6_dst_lookup(sk, &dst, flp);
 
 		if (err) {
 			sk->sk_err_soft = -err;
 			return err;
 		}
 
+		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+			sk->sk_err_soft = -err;
+			dst_release(dst);
+			return err;
+		}
+
 		ip6_dst_store(sk, dst, NULL);
 		sk->sk_route_caps = dst->dev->features &
 			~(NETIF_F_IP_CSUM | NETIF_F_TSO);
diff -ruNBE a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c	2004-07-28 10:10:23.000000000 +0900
+++ b/net/ipv6/udp.c	2004-07-28 10:29:41.000000000 +0900
@@ -630,13 +630,13 @@
 	struct in6_addr *daddr;
 	struct ipv6_txoptions *opt = NULL;
 	struct ip6_flowlabel *flowlabel = NULL;
-	struct flowi *fl = &inet->cork.fl;
-	struct dst_entry *dst;
+	struct flowi *fl = &inet->cork.fl, fl_rt, *flp = fl;
+	struct dst_entry *dst = NULL;
 	int addr_len = msg->msg_namelen;
 	int ulen = len;
 	int hlimit = -1;
 	int corkreq = up->corkflag || msg->msg_flags&MSG_MORE;
-	int err;
+	int err = 0;
 
 	/* destination address check */
 	if (sin6) {
@@ -779,20 +779,28 @@
 	if (ipv6_addr_any(&fl->fl6_src) && !ipv6_addr_any(&np->saddr))
 		ipv6_addr_copy(&fl->fl6_src, &np->saddr);
 	fl->fl_ip_sport = inet->sport;
-	
+
 	/* merge ip6_build_xmit from ip6_output */
 	if (opt && opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
-		ipv6_addr_copy(&fl->fl6_dst, rt0->addr);
+		memcpy(&fl_rt, &fl, sizeof(fl_rt));
+		ipv6_addr_copy(&fl_rt.fl6_dst, rt0->addr);
+		flp = &fl_rt;
 	}
 
 	if (!fl->oif && ipv6_addr_is_multicast(&fl->fl6_dst))
 		fl->oif = np->mcast_oif;
 
-	err = ip6_dst_lookup(sk, &dst, fl);
+	err = ip6_dst_lookup(sk, &dst, flp);
 	if (err)
 		goto out;
 
+	if ((err = xfrm_lookup(&dst, fl, sk, 0)) < 0) {
+		dst_release(dst);
+		dst = NULL;
+		goto out;
+	}
+
 	if (hlimit < 0) {
 		if (ipv6_addr_is_multicast(&fl->fl6_dst))
 			hlimit = np->mcast_hops;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-07-30  8:12 [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Kazunori Miyazawa
@ 2004-08-02  2:51 ` David S. Miller
  2004-08-03  9:00   ` YOSHIFUJI Hideaki / 吉藤英明
  2004-08-27 16:49   ` [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup) YOSHIFUJI Hideaki / 吉藤英明
  2004-08-02  7:41 ` [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Herbert Xu
  1 sibling, 2 replies; 19+ messages in thread
From: David S. Miller @ 2004-08-02  2:51 UTC (permalink / raw)
  To: Kazunori Miyazawa; +Cc: herbert, netdev, usagi-core, kazunori

On Fri, 30 Jul 2004 17:12:05 +0900
Kazunori Miyazawa <kazunori@miyazawa.org> wrote:

> I consider copying flowi(fl_rt) uses too much stack at the moment.
> I'll re-send the fixed patch again.

I agree, and let's defer this patch until we
resolve that.

It is simple to fix, I think.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-07-30  8:12 [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Kazunori Miyazawa
  2004-08-02  2:51 ` David S. Miller
@ 2004-08-02  7:41 ` Herbert Xu
  2004-08-03  2:09   ` David S. Miller
  1 sibling, 1 reply; 19+ messages in thread
From: Herbert Xu @ 2004-08-02  7:41 UTC (permalink / raw)
  To: Kazunori Miyazawa; +Cc: davem, netdev, usagi-core, Alexey Kuznetsov

On Fri, Jul 30, 2004 at 05:12:05PM +0900, Kazunori Miyazawa wrote:
> 
> This patch separates xfrm_lookup from ip6_dst_lookup
> to support srcrt correctly. ip6_dst_lookup should be
> called with next hop. however xfrm_lookup should be
> called with final destination. It fixes them.

Thanks for the patch.

This raises an interesting question.  Is it really correct to look at
the first hop address when doing the route lookup?

The problem is that if we use the first-hop address as the dst when
doing the route lookup then we may end up with incorrect MTU information.
This is because the MTU to the final destination may well be smaller than
the MTU to the first hop.

It seems that Alexey thought about this six years ago according to
the rthdr comment in icmpv6_rcv().

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-02  7:41 ` [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Herbert Xu
@ 2004-08-03  2:09   ` David S. Miller
  2004-08-03  8:19     ` YOSHIFUJI Hideaki / 吉藤英明
  2004-08-03 10:55     ` Herbert Xu
  0 siblings, 2 replies; 19+ messages in thread
From: David S. Miller @ 2004-08-03  2:09 UTC (permalink / raw)
  To: Herbert Xu; +Cc: kazunori, netdev, usagi-core, kuznet

On Mon, 2 Aug 2004 17:41:47 +1000
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> This raises an interesting question.  Is it really correct to look at
> the first hop address when doing the route lookup?
> 
> The problem is that if we use the first-hop address as the dst when
> doing the route lookup then we may end up with incorrect MTU information.
> This is because the MTU to the final destination may well be smaller than
> the MTU to the first hop.
> 
> It seems that Alexey thought about this six years ago according to
> the rthdr comment in icmpv6_rcv().

When the ICMP6 packet comes back in such a case, the type zero
routing header will be suitable edited.  So at least we can determine
which exact destination address the PMTU information applies to.

But I understand what the problem is.  We cannot update the destination
cache entry for destination "D" if the ICMP message is for hop "B"
specified in the routing header.  Furthermore, we cannot even update
a destination cache entry for hop "B" in the case where the routing
header specifies --> A --> B --> C --> D because a direct packet
destinated for "B" might use a different path than "A" does.

An intesting solution would be to use stacked destinations, which
do no actual encapsulation (or perhaps do the routing header work)
and merely represent the hop-by-hop path.  Then the PMTU propagation
machinery can be used, and route lookups will go through a slower path
to find these special stacked hop-by-hop routes.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-03  2:09   ` David S. Miller
@ 2004-08-03  8:19     ` YOSHIFUJI Hideaki / 吉藤英明
  2004-08-03 10:55     ` Herbert Xu
  1 sibling, 0 replies; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-08-03  8:19 UTC (permalink / raw)
  To: davem; +Cc: herbert, kazunori, netdev, usagi-core, kuznet, yoshfuji

In article <20040802190914.303ccfbe.davem@redhat.com> (at Mon, 2 Aug 2004 19:09:14 -0700), "David S. Miller" <davem@redhat.com> says:

> An intesting solution would be to use stacked destinations, which
> do no actual encapsulation (or perhaps do the routing header work)
> and merely represent the hop-by-hop path.  Then the PMTU propagation
> machinery can be used, and route lookups will go through a slower path
> to find these special stacked hop-by-hop routes.

Well, I think it would probably be another rt6_info{} member 
rt6i_srcrt (or something like that).

--yoshfuji

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-02  2:51 ` David S. Miller
@ 2004-08-03  9:00   ` YOSHIFUJI Hideaki / 吉藤英明
  2004-08-03  9:21     ` Kazunori Miyazawa
  2004-08-09 23:35     ` David S. Miller
  2004-08-27 16:49   ` [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup) YOSHIFUJI Hideaki / 吉藤英明
  1 sibling, 2 replies; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-08-03  9:00 UTC (permalink / raw)
  To: davem; +Cc: kazunori, herbert, netdev, usagi-core, yoshfuji

In article <20040801195135.16734846.davem@redhat.com> (at Sun, 1 Aug 2004 19:51:35 -0700), "David S. Miller" <davem@redhat.com> says:

> On Fri, 30 Jul 2004 17:12:05 +0900
> Kazunori Miyazawa <kazunori@miyazawa.org> wrote:
> 
> > I consider copying flowi(fl_rt) uses too much stack at the moment.
> > I'll re-send the fixed patch again.
> 
> I agree, and let's defer this patch until we
> resolve that.

Is the overhead for allocating memory okay?
Or, do we allcoate some per-cpu memory while ipv6.o initalization phase?
(check: lock? preemption?)
Or, will we allocate fl (and fl_rt) per sock{} (ipv6_pinfo{})?
(ditto.)

We have similar stack usage in other codes, and 
I would fix them at the same time.


Another question just for future reference: 
how many bytes (approx.) do we accept on stack?

Note: sizeof(struct flowi) is 72 bytes (on i386)

--yoshfuji

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-03  9:00   ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-08-03  9:21     ` Kazunori Miyazawa
  2004-08-09 23:35     ` David S. Miller
  1 sibling, 0 replies; 19+ messages in thread
From: Kazunori Miyazawa @ 2004-08-03  9:21 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: davem, herbert, netdev, usagi-core

YOSHIFUJI Hideaki wrote:

> In article <20040801195135.16734846.davem@redhat.com> (at Sun, 1 Aug 2004 19:51:35 -0700), "David S. Miller" <davem@redhat.com> says:
> 
> 
>>On Fri, 30 Jul 2004 17:12:05 +0900
>>Kazunori Miyazawa <kazunori@miyazawa.org> wrote:
>>
>>
>>>I consider copying flowi(fl_rt) uses too much stack at the moment.
>>>I'll re-send the fixed patch again.
>>
>>I agree, and let's defer this patch until we
>>resolve that.
> 
> 
> Is the overhead for allocating memory okay?
> Or, do we allcoate some per-cpu memory while ipv6.o initalization phase?
> (check: lock? preemption?)
> Or, will we allocate fl (and fl_rt) per sock{} (ipv6_pinfo{})?
> (ditto.)
> 

My intention is not high art, just using struct in6_addr
instead of struct flowi to store final destination.

> We have similar stack usage in other codes, and 
> I would fix them at the same time.
> 

These might be my changes. I will fix them.

> 
> Another question just for future reference: 
> how many bytes (approx.) do we accept on stack?
> 
> Note: sizeof(struct flowi) is 72 bytes (on i386)
> 
> --yoshfuji

--Kazunori Miyazawa

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-03  2:09   ` David S. Miller
  2004-08-03  8:19     ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-08-03 10:55     ` Herbert Xu
  1 sibling, 0 replies; 19+ messages in thread
From: Herbert Xu @ 2004-08-03 10:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: kazunori, netdev, usagi-core, kuznet

On Mon, Aug 02, 2004 at 07:09:14PM -0700, David S. Miller wrote:
> 
> An intesting solution would be to use stacked destinations, which
> do no actual encapsulation (or perhaps do the routing header work)
> and merely represent the hop-by-hop path.  Then the PMTU propagation
> machinery can be used, and route lookups will go through a slower path
> to find these special stacked hop-by-hop routes.

Yes that's brilliant.

We can replace the current rt dst with an rthdr dst + an rt dst.
The rt dst will be the same one pointing to the first hop.  The
rthdr will contain MTU information for that exact path.

This will work since any ICMP messages we receive due to MTU
issues must carry the entire IP header including the rthdr
(or in the case of IPv4 the ?SR option).  We can then attribute
that MTU to the rthdr dst.

The rthdr can even add the option/extension header as a transform.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-03  9:00   ` YOSHIFUJI Hideaki / 吉藤英明
  2004-08-03  9:21     ` Kazunori Miyazawa
@ 2004-08-09 23:35     ` David S. Miller
  2004-08-10  1:38       ` YOSHIFUJI Hideaki / 吉藤英明
  1 sibling, 1 reply; 19+ messages in thread
From: David S. Miller @ 2004-08-09 23:35 UTC (permalink / raw)
  To: yoshfuji; +Cc: kazunori, herbert, netdev, usagi-core, yoshfuji

On Tue, 03 Aug 2004 02:00:15 -0700 (PDT)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:

> In article <20040801195135.16734846.davem@redhat.com> (at Sun, 1 Aug 2004 19:51:35 -0700), "David S. Miller" <davem@redhat.com> says:
> 
> > On Fri, 30 Jul 2004 17:12:05 +0900
> > Kazunori Miyazawa <kazunori@miyazawa.org> wrote:
> > 
> > > I consider copying flowi(fl_rt) uses too much stack at the moment.
> > > I'll re-send the fixed patch again.
> > 
> > I agree, and let's defer this patch until we
> > resolve that.
> 
> Is the overhead for allocating memory okay?
> Or, do we allcoate some per-cpu memory while ipv6.o initalization phase?
> (check: lock? preemption?)
> Or, will we allocate fl (and fl_rt) per sock{} (ipv6_pinfo{})?
> (ditto.)
> 
> We have similar stack usage in other codes, and 
> I would fix them at the same time.

I think memory allocation will make it worse.

Instead, I would try to arrange order of events such that
single stack copy can be modified.  Something like:

	fl.foo = a;
	fl.bar = b;
	x = flow_lookup(&fl);

	fl.foo = a_2;
	y = other_lookup(&fl);

And I believe this is possible in most if not all of
these cases.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup
  2004-08-09 23:35     ` David S. Miller
@ 2004-08-10  1:38       ` YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 0 replies; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-08-10  1:38 UTC (permalink / raw)
  To: davem; +Cc: kazunori, herbert, netdev, usagi-core

In article <20040809163520.2873a3c8.davem@redhat.com> (at Mon, 9 Aug 2004 16:35:20 -0700), "David S. Miller" <davem@redhat.com> says:

> Instead, I would try to arrange order of events such that
> single stack copy can be modified.  Something like:

Thanks for clarification.

Miyazawa-san will take care of this when he has returned from another 
conference.

--yoshfuji

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-08-02  2:51 ` David S. Miller
  2004-08-03  9:00   ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-08-27 16:49   ` YOSHIFUJI Hideaki / 吉藤英明
  2004-08-28  0:02     ` David S. Miller
  2004-10-25 21:27     ` Brian Haley
  1 sibling, 2 replies; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-08-27 16:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, usagi-core, kazunori, yoshfuji

Hello.

In article <20040801195135.16734846.davem@redhat.com> (at Sun, 1 Aug 2004 19:51:35 -0700), "David S. Miller" <davem@redhat.com> says:

> On Fri, 30 Jul 2004 17:12:05 +0900
> Kazunori Miyazawa <kazunori@miyazawa.org> wrote:
> 
> > I consider copying flowi(fl_rt) uses too much stack at the moment.
> > I'll re-send the fixed patch again.
> 
> I agree, and let's defer this patch until we
> resolve that.
> 
> It is simple to fix, I think.

Here's the updated patch.

From: Kazunori Miyazawa <kazunori@miyazawa.org>

(Because Miyazawa-san is very busy now, 
I'm sending this patch as the proxy of him.)

Please pull from <bk://bk.skbuff.net:20609/linux-2.6-xfrm/>

Thank you.

DIFFSTAT
--------
 datagram.c   |   13 +++++++++++-
 ip6_output.c |    4 ---
 raw.c        |   11 +++++++++-
 tcp_ipv6.c   |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 udp.c        |   11 +++++++++-
 5 files changed, 91 insertions(+), 10 deletions(-)

CHANGESET
---------
ChangeSet@1.1841, 2004-08-28 01:35:09+09:00, kazunori@miyazawa.org
  [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() to support source routing appropriately.
  
  This patch extracts xfrm_lookup() from ip6_dst_lookup()
  to support source routing appropriately.
  
  This is because xfrm_lookup() should be performed with the final
  destination while ip6_dst_lookup() is called with the next-hop.
  
  Signed-off-by: Kazunori Miyazawa <kazunori@miyazawa.org>
  Signed-off-by: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>

diff -Nru a/net/ipv6/datagram.c b/net/ipv6/datagram.c
--- a/net/ipv6/datagram.c	2004-08-28 01:36:43 +09:00
+++ b/net/ipv6/datagram.c	2004-08-28 01:36:43 +09:00
@@ -38,7 +38,7 @@
 	struct sockaddr_in6	*usin = (struct sockaddr_in6 *) uaddr;
 	struct inet_opt      	*inet = inet_sk(sk);
 	struct ipv6_pinfo      	*np = inet6_sk(sk);
-	struct in6_addr		*daddr;
+	struct in6_addr		*daddr, *final_p = NULL, final;
 	struct dst_entry	*dst;
 	struct flowi		fl;
 	struct ip6_flowlabel	*flowlabel = NULL;
@@ -157,16 +157,27 @@
 	if (flowlabel) {
 		if (flowlabel->opt && flowlabel->opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) flowlabel->opt->srcrt;
+			ipv6_addr_copy(&final, &fl.fl6_dst);
 			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			final_p = &final;
 		}
 	} else if (np->opt && np->opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *)np->opt->srcrt;
+		ipv6_addr_copy(&final, &fl.fl6_dst);
 		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		final_p = &final;
 	}
 
 	err = ip6_dst_lookup(sk, &dst, &fl);
 	if (err)
 		goto out;
+	if (final_p)
+		ipv6_addr_copy(&fl.fl6_dst, final_p);
+
+	if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+		dst_release(dst);
+		goto out;
+	}
 
 	/* source address lookup done in ip6_dst_lookup */
 
diff -Nru a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
--- a/net/ipv6/ip6_output.c	2004-08-28 01:36:43 +09:00
+++ b/net/ipv6/ip6_output.c	2004-08-28 01:36:43 +09:00
@@ -796,10 +796,6 @@
 			goto out_err_release;
 		}
 	}
-	if ((err = xfrm_lookup(dst, fl, sk, 0)) < 0) {
-		err = -ENETUNREACH;
-		goto out_err_release;
-        }
 
 	return 0;
 
diff -Nru a/net/ipv6/raw.c b/net/ipv6/raw.c
--- a/net/ipv6/raw.c	2004-08-28 01:36:43 +09:00
+++ b/net/ipv6/raw.c	2004-08-28 01:36:43 +09:00
@@ -606,7 +606,7 @@
 {
 	struct ipv6_txoptions opt_space;
 	struct sockaddr_in6 * sin6 = (struct sockaddr_in6 *) msg->msg_name;
-	struct in6_addr *daddr;
+	struct in6_addr *daddr, *final_p = NULL, final;
 	struct inet_opt *inet = inet_sk(sk);
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct raw6_opt *raw_opt = raw6_sk(sk);
@@ -729,7 +729,9 @@
 	/* merge ip6_build_xmit from ip6_output */
 	if (opt && opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
+		ipv6_addr_copy(&final, &fl.fl6_dst);
 		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		final_p = &final;
 	}
 
 	if (!fl.oif && ipv6_addr_is_multicast(&fl.fl6_dst))
@@ -738,6 +740,13 @@
 	err = ip6_dst_lookup(sk, &dst, &fl);
 	if (err)
 		goto out;
+	if (final_p)
+		ipv6_addr_copy(&fl.fl6_dst, final_p);
+
+	if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+		dst_release(dst);
+		goto out;
+	}
 
 	if (hlimit < 0) {
 		if (ipv6_addr_is_multicast(&fl.fl6_dst))
diff -Nru a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
--- a/net/ipv6/tcp_ipv6.c	2004-08-28 01:36:43 +09:00
+++ b/net/ipv6/tcp_ipv6.c	2004-08-28 01:36:43 +09:00
@@ -549,7 +549,7 @@
 	struct inet_opt *inet = inet_sk(sk);
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct tcp_opt *tp = tcp_sk(sk);
-	struct in6_addr *saddr = NULL;
+	struct in6_addr *saddr = NULL, *final_p = NULL, final;
 	struct flowi fl;
 	struct dst_entry *dst;
 	int addr_type;
@@ -666,13 +666,21 @@
 
 	if (np->opt && np->opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *)np->opt->srcrt;
+		ipv6_addr_copy(&final, &fl.fl6_dst);
 		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		final_p = &final;
 	}
 
 	err = ip6_dst_lookup(sk, &dst, &fl);
-
 	if (err)
 		goto failure;
+	if (final_p)
+		ipv6_addr_copy(&fl.fl6_dst, final_p);
+
+	if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+		dst_release(dst);
+		goto failure;
+	}
 
 	if (saddr == NULL) {
 		saddr = &fl.fl6_src;
@@ -793,6 +801,12 @@
 				sk->sk_err_soft = -err;
 				goto out;
 			}
+
+			if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+				sk->sk_err_soft = -err;
+				goto out;
+			}
+
 		} else
 			dst_hold(dst);
 
@@ -863,6 +877,7 @@
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sk_buff * skb;
 	struct ipv6_txoptions *opt = NULL;
+	struct in6_addr * final_p = NULL, final;
 	struct flowi fl;
 	int err = -1;
 
@@ -888,12 +903,18 @@
 
 		if (opt && opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
+			ipv6_addr_copy(&final, &fl.fl6_dst);
 			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			final_p = &final;
 		}
 
 		err = ip6_dst_lookup(sk, &dst, &fl);
 		if (err)
 			goto done;
+		if (final_p)
+			ipv6_addr_copy(&fl.fl6_dst, final_p);
+		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
+			goto done;
 	}
 
 	skb = tcp_make_synack(sk, dst, req);
@@ -1021,6 +1042,12 @@
 
 	/* sk = NULL, but it is safe for now. RST socket required. */
 	if (!ip6_dst_lookup(NULL, &buff->dst, &fl)) {
+
+		if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0) {
+			dst_release(buff->dst);
+			return;
+		}
+
 		ip6_xmit(NULL, buff, &fl, NULL, 0);
 		TCP_INC_STATS_BH(TCP_MIB_OUTSEGS);
 		TCP_INC_STATS_BH(TCP_MIB_OUTRSTS);
@@ -1082,6 +1109,10 @@
 	fl.fl_ip_sport = t1->source;
 
 	if (!ip6_dst_lookup(NULL, &buff->dst, &fl)) {
+		if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0) {
+			dst_release(buff->dst);
+			return;
+		}
 		ip6_xmit(NULL, buff, &fl, NULL, 0);
 		TCP_INC_STATS_BH(TCP_MIB_OUTSEGS);
 		return;
@@ -1313,6 +1344,7 @@
 	}
 
 	if (dst == NULL) {
+		struct in6_addr *final_p = NULL, final;
 		struct flowi fl;
 
 		memset(&fl, 0, sizeof(fl));
@@ -1320,7 +1352,9 @@
 		ipv6_addr_copy(&fl.fl6_dst, &req->af.v6_req.rmt_addr);
 		if (opt && opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
+			ipv6_addr_copy(&final, &fl.fl6_dst);
 			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			final_p = &final;
 		}
 		ipv6_addr_copy(&fl.fl6_src, &req->af.v6_req.loc_addr);
 		fl.oif = sk->sk_bound_dev_if;
@@ -1329,6 +1363,12 @@
 
 		if (ip6_dst_lookup(sk, &dst, &fl))
 			goto out;
+
+		if (final_p)
+			ipv6_addr_copy(&fl.fl6_dst, final_p);
+
+		if ((xfrm_lookup(&dst, &fl, sk, 0)) < 0)
+			goto out;
 	} 
 
 	newsk = tcp_create_openreq_child(sk, req, skb);
@@ -1710,6 +1750,7 @@
 
 	if (dst == NULL) {
 		struct inet_opt *inet = inet_sk(sk);
+		struct in6_addr *final_p = NULL, final;
 		struct flowi fl;
 
 		memset(&fl, 0, sizeof(fl));
@@ -1723,15 +1764,24 @@
 
 		if (np->opt && np->opt->srcrt) {
 			struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
+			ipv6_addr_copy(&final, &fl.fl6_dst);
 			ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+			final_p = &final;
 		}
 
 		err = ip6_dst_lookup(sk, &dst, &fl);
-
 		if (err) {
 			sk->sk_route_caps = 0;
 			return err;
 		}
+		if (final_p)
+			ipv6_addr_copy(&fl.fl6_dst, final_p);
+
+		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+			sk->sk_err_soft = -err;
+			dst_release(dst);
+			return err;
+		}
 
 		ip6_dst_store(sk, dst, NULL);
 		sk->sk_route_caps = dst->dev->features &
@@ -1772,6 +1822,12 @@
 
 		if (err) {
 			sk->sk_err_soft = -err;
+			return err;
+		}
+
+		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+			sk->sk_route_caps = 0;
+			dst_release(dst);
 			return err;
 		}
 
diff -Nru a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c	2004-08-28 01:36:43 +09:00
+++ b/net/ipv6/udp.c	2004-08-28 01:36:43 +09:00
@@ -627,7 +627,7 @@
 	struct inet_opt *inet = inet_sk(sk);
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *) msg->msg_name;
-	struct in6_addr *daddr;
+	struct in6_addr *daddr, *final_p = NULL, final;
 	struct ipv6_txoptions *opt = NULL;
 	struct ip6_flowlabel *flowlabel = NULL;
 	struct flowi *fl = &inet->cork.fl;
@@ -783,7 +783,9 @@
 	/* merge ip6_build_xmit from ip6_output */
 	if (opt && opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
+		ipv6_addr_copy(&final, &fl->fl6_dst);
 		ipv6_addr_copy(&fl->fl6_dst, rt0->addr);
+		final_p = &final;
 	}
 
 	if (!fl->oif && ipv6_addr_is_multicast(&fl->fl6_dst))
@@ -792,6 +794,13 @@
 	err = ip6_dst_lookup(sk, &dst, fl);
 	if (err)
 		goto out;
+	if (final_p)
+		ipv6_addr_copy(&fl->fl6_dst, final_p);
+
+	if ((err = xfrm_lookup(&dst, fl, sk, 0)) < 0) {
+		dst_release(dst);
+		goto out;
+	}
 
 	if (hlimit < 0) {
 		if (ipv6_addr_is_multicast(&fl->fl6_dst))



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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-08-27 16:49   ` [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup) YOSHIFUJI Hideaki / 吉藤英明
@ 2004-08-28  0:02     ` David S. Miller
  2004-10-25 21:27     ` Brian Haley
  1 sibling, 0 replies; 19+ messages in thread
From: David S. Miller @ 2004-08-28  0:02 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, usagi-core, kazunori, yoshfuji

On Sat, 28 Aug 2004 01:49:35 +0900 (JST)
YOSHIFUJI Hideaki / ^[$B5HF#1QL@^[(B <yoshfuji@linux-ipv6.org> wrote:

> > It is simple to fix, I think.
> 
> Here's the updated patch.
> 
> From: Kazunori Miyazawa <kazunori@miyazawa.org>
> 
> (Because Miyazawa-san is very busy now, 
> I'm sending this patch as the proxy of him.)
> 
> Please pull from <bk://bk.skbuff.net:20609/linux-2.6-xfrm/>

Looks great, pulled.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-08-27 16:49   ` [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup) YOSHIFUJI Hideaki / 吉藤英明
  2004-08-28  0:02     ` David S. Miller
@ 2004-10-25 21:27     ` Brian Haley
  2004-10-26  3:55       ` YOSHIFUJI Hideaki / 吉藤英明
  1 sibling, 1 reply; 19+ messages in thread
From: Brian Haley @ 2004-10-25 21:27 UTC (permalink / raw)
  To: yoshfuji; +Cc: davem, netdev, kazunori

[-- Attachment #1: Type: text/plain, Size: 539 bytes --]


YOSHIFUJI Hideaki / 吉藤英明 wrote:

> Here's the updated patch.

<snip>

Hi,

I finally tracked-down the patch that fixed the IPv6 routing header 
problems I'd been seeing :)  But I think you missed one code path from 
the original patch Kazunori posted in tcp_v6_xmit(), which I think is 
only called from the ND code.

Attached is a patch that fixes that and also brings the rt0_hdr struct 
in-line with RFC 3542 by removing the "bitmap" field.

Thanks,

-Brian

--
Brian Haley, HP Linux and Open Source Lab

[-- Attachment #2: rt0.diffs --]
[-- Type: text/plain, Size: 2800 bytes --]

Signed-off-by: Brian.Haley@hp.com

--- linux-2.6.9.orig/include/linux/ipv6.h	2004-10-18 17:54:39.000000000 -0400
+++ linux-2.6.9.rt0/include/linux/ipv6.h	2004-10-22 17:15:52.000000000 -0400
@@ -68,7 +68,7 @@
 
 struct rt0_hdr {
 	struct ipv6_rt_hdr	rt_hdr;
-	__u32			bitmap;		/* strict/loose bit map */
+	__u32			reserved;
 	struct in6_addr		addr[0];
 
 #define rt0_type		rt_hdr.type
--- linux-2.6.9.orig/net/ipv6/netfilter/ip6t_rt.c	2004-10-22 16:09:32.000000000 -0400
+++ linux-2.6.9.rt0/net/ipv6/netfilter/ip6t_rt.c	2004-10-22 17:14:55.410800617 -0400
@@ -159,8 +159,8 @@
                            ((rtinfo->hdrlen == hdrlen) ^
                            !!(rtinfo->invflags & IP6T_RT_INV_LEN))));
        DEBUGP("res %02X %02X %02X ", 
-       		(rtinfo->flags & IP6T_RT_RES), ((struct rt0_hdr *)route)->bitmap,
-       		!((rtinfo->flags & IP6T_RT_RES) && (((struct rt0_hdr *)route)->bitmap)));
+       		(rtinfo->flags & IP6T_RT_RES), ((struct rt0_hdr *)route)->reserved,
+       		!((rtinfo->flags & IP6T_RT_RES) && (((struct rt0_hdr *)route)->reserved)));
 
        ret = (route != NULL)
        		&&
@@ -176,7 +176,7 @@
                            ((rtinfo->rt_type == route->type) ^
                            !!(rtinfo->invflags & IP6T_RT_INV_TYP)))
 		&&
-       		!((rtinfo->flags & IP6T_RT_RES) && (((struct rt0_hdr *)route)->bitmap));
+       		!((rtinfo->flags & IP6T_RT_RES) && (((struct rt0_hdr *)route)->reserved));
 
 	DEBUGP("#%d ",rtinfo->addrnr);
        temp = len = ptr = 0;
--- linux-2.6.9.orig/net/ipv6/tcp_ipv6.c	2004-10-18 17:54:32.000000000 -0400
+++ linux-2.6.9.rt0/net/ipv6/tcp_ipv6.c	2004-10-22 17:13:43.386387437 -0400
@@ -1802,6 +1802,7 @@
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct flowi fl;
 	struct dst_entry *dst;
+	struct in6_addr *final_p = NULL, final;
 
 	memset(&fl, 0, sizeof(fl));
 	fl.proto = IPPROTO_TCP;
@@ -1815,7 +1816,9 @@
 
 	if (np->opt && np->opt->srcrt) {
 		struct rt0_hdr *rt0 = (struct rt0_hdr *) np->opt->srcrt;
+		ipv6_addr_copy(&final, &fl.fl6_dst);
 		ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
+		final_p = &final;
 	}
 
 	dst = __sk_dst_check(sk, np->dst_cookie);
@@ -1828,6 +1831,9 @@
 			return err;
 		}
 
+		if (final_p)
+			ipv6_addr_copy(&fl.fl6_dst, final_p);
+
 		if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
 			sk->sk_route_caps = 0;
 			dst_release(dst);
--- linux-2.6.9.orig/net/ipv6/exthdrs.c	2004-10-18 17:53:07.000000000 -0400
+++ linux-2.6.9.rt0/net/ipv6/exthdrs.c	2004-10-22 17:14:12.413730831 -0400
@@ -404,8 +404,7 @@
 
 	memcpy(opt->srcrt, hdr, sizeof(*hdr));
 	irthdr = (struct rt0_hdr*)opt->srcrt;
-	/* Obsolete field, MBZ, when originated by us */
-	irthdr->bitmap = 0;
+	irthdr->reserved = 0;
 	opt->srcrt->segments_left = n;
 	for (i=0; i<n; i++)
 		memcpy(irthdr->addr+i, rthdr->addr+(n-1-i), 16);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-10-25 21:27     ` Brian Haley
@ 2004-10-26  3:55       ` YOSHIFUJI Hideaki / 吉藤英明
  2004-10-26 14:29         ` Brian Haley
  0 siblings, 1 reply; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-10-26  3:55 UTC (permalink / raw)
  To: Brian.Haley, davem; +Cc: netdev, kazunori, yoshfuji

In article <417D6FD0.7010903@hp.com> (at Mon, 25 Oct 2004 17:27:44 -0400), Brian Haley <Brian.Haley@hp.com> says:

> Attached is a patch that fixes that and also brings the rt0_hdr struct 
> in-line with RFC 3542 by removing the "bitmap" field.

Good catch but please do not change bitmap.
I applied it into my tree, without changing bitmap.

David, I'll send bk URL including this very soon.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-10-26  3:55       ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-10-26 14:29         ` Brian Haley
  2004-10-26 15:43           ` YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 1 reply; 19+ messages in thread
From: Brian Haley @ 2004-10-26 14:29 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev


YOSHIFUJI Hideaki / 吉藤英明 wrote:

>>Attached is a patch that fixes that and also brings the rt0_hdr struct 
>>in-line with RFC 3542 by removing the "bitmap" field.
> 
> Good catch but please do not change bitmap.

I understand that was more cosmetic since it's a kernel header, but can
you explain it any further?  I couldn't find any other occurences than 
those I fixed.  The user-space version in netinet/ip6.h is even more 
broken and I'm pushing that to the glibc people now too.

Thanks,

-Brian

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-10-26 14:29         ` Brian Haley
@ 2004-10-26 15:43           ` YOSHIFUJI Hideaki / 吉藤英明
  2004-10-26 18:09             ` Brian Haley
  0 siblings, 1 reply; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-10-26 15:43 UTC (permalink / raw)
  To: Brian.Haley; +Cc: netdev, yoshfuji

In article <417E5F37.4010501@hp.com> (at Tue, 26 Oct 2004 10:29:11 -0400), Brian Haley <Brian.Haley@hp.com> says:

> >>Attached is a patch that fixes that and also brings the rt0_hdr struct 
> >>in-line with RFC 3542 by removing the "bitmap" field.
> > 
> > Good catch but please do not change bitmap.
> 
> I understand that was more cosmetic since it's a kernel header, but can
> you explain it any further?  I couldn't find any other occurences than 
> those I fixed.  The user-space version in netinet/ip6.h is even more 
> broken and I'm pushing that to the glibc people now too.

First, in general, please do not mix two things.

Second, because we're not ready to migrate.
Please do not do that (pushing it to the glibc people) until we're ready.
We really need to do in consistent way; kernel header / glibc header,
and even with other systems.

--yoshfuji

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-10-26 15:43           ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-10-26 18:09             ` Brian Haley
  2004-10-27  0:10               ` YOSHIFUJI Hideaki / 吉藤英明
  0 siblings, 1 reply; 19+ messages in thread
From: Brian Haley @ 2004-10-26 18:09 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev


YOSHIFUJI Hideaki / 吉藤英明 wrote:

> First, in general, please do not mix two things.

Sorry, next time I'll split the patch.

> Second, because we're not ready to migrate.
> Please do not do that (pushing it to the glibc people) until we're ready.
> We really need to do in consistent way; kernel header / glibc header,
> and even with other systems.

The kernel struct is the correct size (since addr[0] has no size), 
ip6_rthdr0 in ip6.h is not, it's 128 bits too big.  Let me know the 
right steps to go through to correct this and I'll get it done, you can 
send it off-line if you like.

-Brian

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-10-26 18:09             ` Brian Haley
@ 2004-10-27  0:10               ` YOSHIFUJI Hideaki / 吉藤英明
  2004-10-27 15:21                 ` Brian Haley
  0 siblings, 1 reply; 19+ messages in thread
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2004-10-27  0:10 UTC (permalink / raw)
  To: Brian.Haley; +Cc: netdev, yoshfuji

In article <417E92D9.1050305@hp.com> (at Tue, 26 Oct 2004 14:09:29 -0400), Brian Haley <Brian.Haley@hp.com> says:

> > Second, because we're not ready to migrate.
> > Please do not do that (pushing it to the glibc people) until we're ready.
> > We really need to do in consistent way; kernel header / glibc header,
> > and even with other systems.
> 
> The kernel struct is the correct size (since addr[0] has no size), 
> ip6_rthdr0 in ip6.h is not, it's 128 bits too big.  Let me know the 
> right steps to go through to correct this and I'll get it done, you can 
> send it off-line if you like.

Because we do not support RFC3542, extra 128bits is definitely correct.

--yoshfuji

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup)
  2004-10-27  0:10               ` YOSHIFUJI Hideaki / 吉藤英明
@ 2004-10-27 15:21                 ` Brian Haley
  0 siblings, 0 replies; 19+ messages in thread
From: Brian Haley @ 2004-10-27 15:21 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev


> Because we do not support RFC3542, extra 128bits is definitely correct.

2292 was deprecated by 3542 in May 2003, when do we plan on supporting it?  I am 
willing to do this update, just please tell me the way you want me to go about it.

Thanks,

-Brian

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2004-10-27 15:21 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-07-30  8:12 [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Kazunori Miyazawa
2004-08-02  2:51 ` David S. Miller
2004-08-03  9:00   ` YOSHIFUJI Hideaki / 吉藤英明
2004-08-03  9:21     ` Kazunori Miyazawa
2004-08-09 23:35     ` David S. Miller
2004-08-10  1:38       ` YOSHIFUJI Hideaki / 吉藤英明
2004-08-27 16:49   ` [PATCH, TAKE 2] [IPV6] XFRM: extract xfrm_lookup() from ip6_dst_lookup() (is Re: [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup) YOSHIFUJI Hideaki / 吉藤英明
2004-08-28  0:02     ` David S. Miller
2004-10-25 21:27     ` Brian Haley
2004-10-26  3:55       ` YOSHIFUJI Hideaki / 吉藤英明
2004-10-26 14:29         ` Brian Haley
2004-10-26 15:43           ` YOSHIFUJI Hideaki / 吉藤英明
2004-10-26 18:09             ` Brian Haley
2004-10-27  0:10               ` YOSHIFUJI Hideaki / 吉藤英明
2004-10-27 15:21                 ` Brian Haley
2004-08-02  7:41 ` [PATCH][IPv6] separation xfrm_lookup from ip6_dst_lookup Herbert Xu
2004-08-03  2:09   ` David S. Miller
2004-08-03  8:19     ` YOSHIFUJI Hideaki / 吉藤英明
2004-08-03 10:55     ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).