[PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[Duan Jiong]

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

the ip6_redirect() could be replaced with
ip6_redirect_no_header(), we could always use ip6_redirect()
for route updating in ndisc layer and use the data of the
redirected header option just for finding the socket to be
notified and then notify user in protocols' err_handler.

---
 Changes for v3:
 1.del the ICMP6_INC_STATS_BH  error count, these are in fact
   no errors.

 Changes for v2:
 1.handle the update of the NDISC_REDIRECT error code directly in
   icmpv6_err_convert.
 2.squash some patchs into one patch.
 3.modify the subject of those patchs.

Duan Jiong (6):
  ipv6: del the statements for updating route in (dccp|tcp|sctp)_v6_err
  ipv6: just match on ICMPV6_PKT_TOOBIG in those err_handle
  ipv6: del statements for dealing with NDISC_REDIRECT
  ip6tnl: move route updating for redirect to ndisc layer
  ipv6: modify the err to 0 when dealing with NDISC_REDIRECT
  ipv6: Do route updating for redirect in ndisc layer

 include/net/ip6_route.h |  3 ---
 net/dccp/ipv6.c         | 13 +------------
 net/ipv6/ah6.c          |  9 ++-------
 net/ipv6/esp6.c         |  9 ++-------
 net/ipv6/icmp.c         |  5 +++--
 net/ipv6/ip6_tunnel.c   |  5 -----
 net/ipv6/ipcomp6.c      |  9 ++-------
 net/ipv6/ndisc.c        |  6 ++----
 net/ipv6/raw.c          |  3 +--
 net/ipv6/route.c        | 29 ++---------------------------
 net/ipv6/tcp_ipv6.c     | 12 ++++--------
 net/ipv6/udp.c          |  2 --
 net/sctp/input.c        | 12 ------------
 net/sctp/ipv6.c         |  6 +++---
 14 files changed, 22 insertions(+), 101 deletions(-)

-- 
1.8.3.1

[PATCH 1/6] ipv6: del the statements for updating route in, (dccp|tcp|sctp)_v6_err

[Duan Jiong]

Because we will do route updating for redirect in nidsc layer. And
when dealing with redirect message, the dccp and sctp should like
tcp return directly.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 net/dccp/ipv6.c     | 10 +++-------
 net/ipv6/tcp_ipv6.c | 12 ++++--------
 net/sctp/input.c    | 12 ------------
 net/sctp/ipv6.c     |  6 +++---
 4 files changed, 10 insertions(+), 30 deletions(-)

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 9c61f9c..300840c 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -98,6 +98,9 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		return;
 	}
 
+	if (type == NDISC_REDIRECT)
+		return;
+
 	sk = inet6_lookup(net, &dccp_hashinfo,
 			&hdr->daddr, dh->dccph_dport,
 			&hdr->saddr, dh->dccph_sport, inet6_iif(skb));
@@ -130,13 +133,6 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 	np = inet6_sk(sk);
 
-	if (type == NDISC_REDIRECT) {
-		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
-
-		if (dst)
-			dst->ops->redirect(dst, sk, skb);
-	}
-
 	if (type == ICMPV6_PKT_TOOBIG) {
 		struct dst_entry *dst = NULL;
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 5c71501..d3ca8a4 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -346,6 +346,10 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	__u32 seq;
 	struct net *net = dev_net(skb->dev);
 
+
+	if (type == NDISC_REDIRECT)
+		return;
+
 	sk = inet6_lookup(net, &tcp_hashinfo, &hdr->daddr,
 			th->dest, &hdr->saddr, th->source, skb->dev->ifindex);
 
@@ -382,14 +386,6 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 	np = inet6_sk(sk);
 
-	if (type == NDISC_REDIRECT) {
-		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
-
-		if (dst)
-			dst->ops->redirect(dst, sk, skb);
-		goto out;
-	}
-
 	if (type == ICMPV6_PKT_TOOBIG) {
 		/* We are not interested in TCP_LISTEN and open_requests
 		 * (SYN-ACKs send out by Linux are always <576bytes so
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 5f20686..0d2d4b7 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -413,18 +413,6 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
 	sctp_retransmit(&asoc->outqueue, t, SCTP_RTXR_PMTUD);
 }
 
-void sctp_icmp_redirect(struct sock *sk, struct sctp_transport *t,
-			struct sk_buff *skb)
-{
-	struct dst_entry *dst;
-
-	if (!t)
-		return;
-	dst = sctp_transport_dst_check(t);
-	if (dst)
-		dst->ops->redirect(dst, sk, skb);
-}
-
 /*
  * SCTP Implementer's Guide, 2.37 ICMP handling procedures
  *
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index da613ce..ee12d87 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -151,6 +151,9 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	int err;
 	struct net *net = dev_net(skb->dev);
 
+	if (type == NDISC_REDIRECT)
+		return;
+
 	idev = in6_dev_get(skb->dev);
 
 	/* Fix up skb to look at the embedded net header. */
@@ -181,9 +184,6 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 			goto out_unlock;
 		}
 		break;
-	case NDISC_REDIRECT:
-		sctp_icmp_redirect(sk, transport, skb);
-		break;
 	default:
 		break;
 	}
-- 
1.8.3.1

Re: [PATCH 1/6] ipv6: del the statements for updating route in, (dccp|tcp|sctp)_v6_err

[Duan Jiong]

Sorry, i forgot to add "v3" in title.

Thanks,
Duan
于 2013/9/16 19:49, Duan Jiong 写道:
> Because we will do route updating for redirect in nidsc layer. And
> when dealing with redirect message, the dccp and sctp should like
> tcp return directly.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> ---
>  net/dccp/ipv6.c     | 10 +++-------
>  net/ipv6/tcp_ipv6.c | 12 ++++--------
>  net/sctp/input.c    | 12 ------------
>  net/sctp/ipv6.c     |  6 +++---
>  4 files changed, 10 insertions(+), 30 deletions(-)
>
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 9c61f9c..300840c 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -98,6 +98,9 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  		return;
>  	}
>  
> +	if (type == NDISC_REDIRECT)
> +		return;
> +
>  	sk = inet6_lookup(net, &dccp_hashinfo,
>  			&hdr->daddr, dh->dccph_dport,
>  			&hdr->saddr, dh->dccph_sport, inet6_iif(skb));
> @@ -130,13 +133,6 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  
>  	np = inet6_sk(sk);
>  
> -	if (type == NDISC_REDIRECT) {
> -		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
> -
> -		if (dst)
> -			dst->ops->redirect(dst, sk, skb);
> -	}
> -
>  	if (type == ICMPV6_PKT_TOOBIG) {
>  		struct dst_entry *dst = NULL;
>  
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 5c71501..d3ca8a4 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -346,6 +346,10 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  	__u32 seq;
>  	struct net *net = dev_net(skb->dev);
>  
> +
> +	if (type == NDISC_REDIRECT)
> +		return;
> +
>  	sk = inet6_lookup(net, &tcp_hashinfo, &hdr->daddr,
>  			th->dest, &hdr->saddr, th->source, skb->dev->ifindex);
>  
> @@ -382,14 +386,6 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  
>  	np = inet6_sk(sk);
>  
> -	if (type == NDISC_REDIRECT) {
> -		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
> -
> -		if (dst)
> -			dst->ops->redirect(dst, sk, skb);
> -		goto out;
> -	}
> -
>  	if (type == ICMPV6_PKT_TOOBIG) {
>  		/* We are not interested in TCP_LISTEN and open_requests
>  		 * (SYN-ACKs send out by Linux are always <576bytes so
> diff --git a/net/sctp/input.c b/net/sctp/input.c
> index 5f20686..0d2d4b7 100644
> --- a/net/sctp/input.c
> +++ b/net/sctp/input.c
> @@ -413,18 +413,6 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
>  	sctp_retransmit(&asoc->outqueue, t, SCTP_RTXR_PMTUD);
>  }
>  
> -void sctp_icmp_redirect(struct sock *sk, struct sctp_transport *t,
> -			struct sk_buff *skb)
> -{
> -	struct dst_entry *dst;
> -
> -	if (!t)
> -		return;
> -	dst = sctp_transport_dst_check(t);
> -	if (dst)
> -		dst->ops->redirect(dst, sk, skb);
> -}
> -
>  /*
>   * SCTP Implementer's Guide, 2.37 ICMP handling procedures
>   *
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index da613ce..ee12d87 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -151,6 +151,9 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  	int err;
>  	struct net *net = dev_net(skb->dev);
>  
> +	if (type == NDISC_REDIRECT)
> +		return;
> +
>  	idev = in6_dev_get(skb->dev);
>  
>  	/* Fix up skb to look at the embedded net header. */
> @@ -181,9 +184,6 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>  			goto out_unlock;
>  		}
>  		break;
> -	case NDISC_REDIRECT:
> -		sctp_icmp_redirect(sk, transport, skb);
> -		break;
>  	default:
>  		break;
>  	}

[PATCH v3 2/6] ipv6: just match on ICMPV6_PKT_TOOBIG in those err_handle

[Duan Jiong]

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

we don't handle NDISC_REDIRECTs here any more, and the match
on ICMPV6_DEST_UNREACH is meaningless.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 net/dccp/ipv6.c    | 7 -------
 net/ipv6/ah6.c     | 9 ++-------
 net/ipv6/esp6.c    | 9 ++-------
 net/ipv6/ipcomp6.c | 9 ++-------
 4 files changed, 6 insertions(+), 28 deletions(-)

diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 300840c..980cfba 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -91,13 +91,6 @@ static void dccp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	__u64 seq;
 	struct net *net = dev_net(skb->dev);
 
-	if (skb->len < offset + sizeof(*dh) ||
-	    skb->len < offset + __dccp_basic_hdr_len(dh)) {
-		ICMP6_INC_STATS_BH(net, __in6_dev_get(skb->dev),
-				   ICMP6_MIB_INERRORS);
-		return;
-	}
-
 	if (type == NDISC_REDIRECT)
 		return;
 
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 73784c3..79fb40f 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -618,19 +618,14 @@ static void ah6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	struct ip_auth_hdr *ah = (struct ip_auth_hdr*)(skb->data+offset);
 	struct xfrm_state *x;
 
-	if (type != ICMPV6_DEST_UNREACH &&
-	    type != ICMPV6_PKT_TOOBIG &&
-	    type != NDISC_REDIRECT)
+	if (type != ICMPV6_PKT_TOOBIG)
 		return;
 
 	x = xfrm_state_lookup(net, skb->mark, (xfrm_address_t *)&iph->daddr, ah->spi, IPPROTO_AH, AF_INET6);
 	if (!x)
 		return;
 
-	if (type == NDISC_REDIRECT)
-		ip6_redirect(skb, net, skb->dev->ifindex, 0);
-	else
-		ip6_update_pmtu(skb, net, info, 0, 0);
+	ip6_update_pmtu(skb, net, info, 0, 0);
 	xfrm_state_put(x);
 }
 
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index d3618a7..6aa64e1 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -436,9 +436,7 @@ static void esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	struct ip_esp_hdr *esph = (struct ip_esp_hdr *)(skb->data + offset);
 	struct xfrm_state *x;
 
-	if (type != ICMPV6_DEST_UNREACH &&
-	    type != ICMPV6_PKT_TOOBIG &&
-	    type != NDISC_REDIRECT)
+	if (type != ICMPV6_PKT_TOOBIG)
 		return;
 
 	x = xfrm_state_lookup(net, skb->mark, (const xfrm_address_t *)&iph->daddr,
@@ -446,10 +444,7 @@ static void esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	if (!x)
 		return;
 
-	if (type == NDISC_REDIRECT)
-		ip6_redirect(skb, net, skb->dev->ifindex, 0);
-	else
-		ip6_update_pmtu(skb, net, info, 0, 0);
+	ip6_update_pmtu(skb, net, info, 0, 0);
 	xfrm_state_put(x);
 }
 
diff --git a/net/ipv6/ipcomp6.c b/net/ipv6/ipcomp6.c
index 5636a91..e943158 100644
--- a/net/ipv6/ipcomp6.c
+++ b/net/ipv6/ipcomp6.c
@@ -64,9 +64,7 @@ static void ipcomp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		(struct ip_comp_hdr *)(skb->data + offset);
 	struct xfrm_state *x;
 
-	if (type != ICMPV6_DEST_UNREACH &&
-	    type != ICMPV6_PKT_TOOBIG &&
-	    type != NDISC_REDIRECT)
+	if (type != ICMPV6_PKT_TOOBIG)
 		return;
 
 	spi = htonl(ntohs(ipcomph->cpi));
@@ -75,10 +73,7 @@ static void ipcomp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 	if (!x)
 		return;
 
-	if (type == NDISC_REDIRECT)
-		ip6_redirect(skb, net, skb->dev->ifindex, 0);
-	else
-		ip6_update_pmtu(skb, net, info, 0, 0);
+	ip6_update_pmtu(skb, net, info, 0, 0);
 	xfrm_state_put(x);
 }
 
-- 
1.8.3.1

[PATCH v3 3/6] ipv6: del statements for dealing with NDISC_REDIRECT

[Duan Jiong]

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

Now the route updating for redirect is done in ndisc
layer.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 net/ipv6/icmp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index eef8d94..4bde43c 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -91,8 +91,6 @@ static void icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 	if (type == ICMPV6_PKT_TOOBIG)
 		ip6_update_pmtu(skb, net, info, 0, 0);
-	else if (type == NDISC_REDIRECT)
-		ip6_redirect(skb, net, skb->dev->ifindex, 0);
 
 	if (!(type & ICMPV6_INFOMSG_MASK))
 		if (icmp6->icmp6_type == ICMPV6_ECHO_REQUEST)
-- 
1.8.3.1

[PATCH 4/6] ip6tnl: move route updating for redirect to ndisc layer

[Duan Jiong]

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

In rfc2473, we can know that the tunnel ICMP redirect
message should not be reported to the source of the
original packet, so after calling ip6_tnl_err(), the
rel_msg is set to 0 in function ip4ip6_err(), and the
redirect will never be handled.

In order to deal with this, we move route updating for
redirect to ndisc layer.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 net/ipv6/ip6_tunnel.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 61355f7..3ea834b 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -576,9 +576,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		rel_type = ICMP_DEST_UNREACH;
 		rel_code = ICMP_FRAG_NEEDED;
 		break;
-	case NDISC_REDIRECT:
-		rel_type = ICMP_REDIRECT;
-		rel_code = ICMP_REDIR_HOST;
 	default:
 		return 0;
 	}
@@ -637,8 +634,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 		skb_dst(skb2)->ops->update_pmtu(skb_dst(skb2), NULL, skb2, rel_info);
 	}
-	if (rel_type == ICMP_REDIRECT)
-		skb_dst(skb2)->ops->redirect(skb_dst(skb2), NULL, skb2);
 
 	icmp_send(skb2, rel_type, rel_code, htonl(rel_info));
 
-- 
1.8.3.1

[PATCH v3 5/6] ipv6: modify the err to 0 when dealing with, NDISC_REDIRECT

[Duan Jiong]

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

when dealing with redirect message, the err shoud
be assigned to 0, not EPROTO. And del the statements
for updating route.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 net/ipv6/icmp.c | 3 +++
 net/ipv6/raw.c  | 3 +--
 net/ipv6/udp.c  | 2 --
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 4bde43c..6bcedcc 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -975,6 +975,9 @@ int icmpv6_err_convert(u8 type, u8 code, int *err)
 	case ICMPV6_TIME_EXCEED:
 		*err = EHOSTUNREACH;
 		break;
+	case NDISC_REDIRECT:
+		*err = 0;
+		break;
 	}
 
 	return fatal;
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 58916bb..baf86b8 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -335,8 +335,7 @@ static void rawv6_err(struct sock *sk, struct sk_buff *skb,
 		ip6_sk_update_pmtu(skb, sk, info);
 		harderr = (np->pmtudisc == IPV6_PMTUDISC_DO);
 	}
-	if (type == NDISC_REDIRECT)
-		ip6_sk_redirect(skb, sk);
+
 	if (np->recverr) {
 		u8 *payload = skb->data;
 		if (!inet->hdrincl)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index f405815..a40b392 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -525,8 +525,6 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 	if (type == ICMPV6_PKT_TOOBIG)
 		ip6_sk_update_pmtu(skb, sk, info);
-	if (type == NDISC_REDIRECT)
-		ip6_sk_redirect(skb, sk);
 
 	np = inet6_sk(sk);
 
-- 
1.8.3.1

[PATCH v3 6/6] ipv6: Do route updating for redirect in ndisc layer

[Duan Jiong]

From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

Do the whole verification and route updating in ndisc
lay and then just call into icmpv6_notify() to notify
the upper protocols.

Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
 include/net/ip6_route.h |  3 ---
 net/ipv6/ndisc.c        |  6 ++----
 net/ipv6/route.c        | 29 ++---------------------------
 3 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index f525e70..5db259e 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -133,9 +133,6 @@ extern void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu,
 extern void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk,
 			       __be32 mtu);
 extern void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark);
-extern void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
-				   u32 mark);
-extern void ip6_sk_redirect(struct sk_buff *skb, struct sock *sk);
 
 struct netlink_callback;
 
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f8a55ff..6bd1b41 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1368,11 +1368,9 @@ static void ndisc_redirect_rcv(struct sk_buff *skb)
 	if (!ndisc_parse_options(msg->opt, ndoptlen, &ndopts))
 		return;
 
-	if (!ndopts.nd_opts_rh) {
-		ip6_redirect_no_header(skb, dev_net(skb->dev),
-					skb->dev->ifindex, 0);
+	ip6_redirect(skb, dev_net(skb->dev), skb->dev->ifindex, 0);
+	if (!ndopts.nd_opts_rh)
 		return;
-	}
 
 	hdr = (u8 *)ndopts.nd_opts_rh;
 	hdr += 8;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c979dd9..151bd6c 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1227,27 +1227,7 @@ static struct dst_entry *ip6_route_redirect(struct net *net,
 				flags, __ip6_route_redirect);
 }
 
-void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark)
-{
-	const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
-	struct dst_entry *dst;
-	struct flowi6 fl6;
-
-	memset(&fl6, 0, sizeof(fl6));
-	fl6.flowi6_oif = oif;
-	fl6.flowi6_mark = mark;
-	fl6.flowi6_flags = 0;
-	fl6.daddr = iph->daddr;
-	fl6.saddr = iph->saddr;
-	fl6.flowlabel = ip6_flowinfo(iph);
-
-	dst = ip6_route_redirect(net, &fl6, &ipv6_hdr(skb)->saddr);
-	rt6_do_redirect(dst, NULL, skb);
-	dst_release(dst);
-}
-EXPORT_SYMBOL_GPL(ip6_redirect);
-
-void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
+void ip6_redirect(struct sk_buff *skb, struct net *net, int oif,
 			    u32 mark)
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
@@ -1266,12 +1246,7 @@ void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
 	rt6_do_redirect(dst, NULL, skb);
 	dst_release(dst);
 }
-
-void ip6_sk_redirect(struct sk_buff *skb, struct sock *sk)
-{
-	ip6_redirect(skb, sock_net(sk), sk->sk_bound_dev_if, sk->sk_mark);
-}
-EXPORT_SYMBOL_GPL(ip6_sk_redirect);
+EXPORT_SYMBOL_GPL(ip6_redirect);
 
 static unsigned int ip6_default_advmss(const struct dst_entry *dst)
 {
-- 
1.8.3.1

Re: [PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[Daniel Borkmann]

On 09/16/2013 01:47 PM, Duan Jiong wrote:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> 
> the ip6_redirect() could be replaced with
> ip6_redirect_no_header(), we could always use ip6_redirect()
> for route updating in ndisc layer and use the data of the
> redirected header option just for finding the socket to be
> notified and then notify user in protocols' err_handler.

If I get this right, it seems to me that this patchset actually consists of two
different kind of changes:

1) Not notifying user space on ICMP redirects (net material)
2) Simplify code for updating route in ndisc layer instead of error handlers (net-next)

Also, you do the *actual* change in the very last patch, which means that from
patch 1 to 5 we're in an inconsistent and buggy state unless we also apply patch
number 6. It should actually be the other way around, that you first do the actual
change and then migrate users (also commit messages are quite terse).

Moreover, just looking at the SCTP part (sctp_err_lookup() function) ...

/* RFC 4960, Appendix C. ICMP Handling
 *
 * ICMP6) An implementation MUST validate that the Verification Tag
 * contained in the ICMP message matches the Verification Tag of
 * the peer.  If the Verification Tag is not 0 and does NOT
 * match, discard the ICMP message.  If it is 0 and the ICMP
 * message contains enough bytes to verify that the chunk type is
 * an INIT chunk and that the Initiate Tag matches the tag of the
 * peer, continue with ICMP7.  If the ICMP message is too short
 * or the chunk type or the Initiate Tag does not match, silently
 * discard the packet.
 */

... it seems to me that we would simply ignore such RFC requirements with
your patch for the sctp_v6_err() part.

Care to elaborate? ;-)

> ---
>   Changes for v3:
>   1.del the ICMP6_INC_STATS_BH  error count, these are in fact
>     no errors.
> 
>   Changes for v2:
>   1.handle the update of the NDISC_REDIRECT error code directly in
>     icmpv6_err_convert.
>   2.squash some patchs into one patch.
>   3.modify the subject of those patchs.
> 
> Duan Jiong (6):
>    ipv6: del the statements for updating route in (dccp|tcp|sctp)_v6_err
>    ipv6: just match on ICMPV6_PKT_TOOBIG in those err_handle
>    ipv6: del statements for dealing with NDISC_REDIRECT
>    ip6tnl: move route updating for redirect to ndisc layer
>    ipv6: modify the err to 0 when dealing with NDISC_REDIRECT
>    ipv6: Do route updating for redirect in ndisc layer
> 
>   include/net/ip6_route.h |  3 ---
>   net/dccp/ipv6.c         | 13 +------------
>   net/ipv6/ah6.c          |  9 ++-------
>   net/ipv6/esp6.c         |  9 ++-------
>   net/ipv6/icmp.c         |  5 +++--
>   net/ipv6/ip6_tunnel.c   |  5 -----
>   net/ipv6/ipcomp6.c      |  9 ++-------
>   net/ipv6/ndisc.c        |  6 ++----
>   net/ipv6/raw.c          |  3 +--
>   net/ipv6/route.c        | 29 ++---------------------------
>   net/ipv6/tcp_ipv6.c     | 12 ++++--------
>   net/ipv6/udp.c          |  2 --
>   net/sctp/input.c        | 12 ------------
>   net/sctp/ipv6.c         |  6 +++---
>   14 files changed, 22 insertions(+), 101 deletions(-)
> 

Re: [PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[Duan Jiong]

于 2013/9/16 20:22, Daniel Borkmann 写道:
> On 09/16/2013 01:47 PM, Duan Jiong wrote:
>> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>>
>> the ip6_redirect() could be replaced with
>> ip6_redirect_no_header(), we could always use ip6_redirect()
>> for route updating in ndisc layer and use the data of the
>> redirected header option just for finding the socket to be
>> notified and then notify user in protocols' err_handler.
> If I get this right, it seems to me that this patchset actually consists of two
> different kind of changes:
>
> 1) Not notifying user space on ICMP redirects (net material)
> 2) Simplify code for updating route in ndisc layer instead of error handlers (net-next)
>
> Also, you do the *actual* change in the very last patch, which means that from
> patch 1 to 5 we're in an inconsistent and buggy state unless we also apply patch
> number 6. It should actually be the other way around, that you first do the actual
> change and then migrate users (also commit messages are quite terse).

I make the patch set on net tree, not on net-next. Maybe those
things should be done in two patch sets.

> Moreover, just looking at the SCTP part (sctp_err_lookup() function) ...
>
> /* RFC 4960, Appendix C. ICMP Handling
>  *
>  * ICMP6) An implementation MUST validate that the Verification Tag
>  * contained in the ICMP message matches the Verification Tag of
>  * the peer.  If the Verification Tag is not 0 and does NOT
>  * match, discard the ICMP message.  If it is 0 and the ICMP
>  * message contains enough bytes to verify that the chunk type is
>  * an INIT chunk and that the Initiate Tag matches the tag of the
>  * peer, continue with ICMP7.  If the ICMP message is too short
>  * or the chunk type or the Initiate Tag does not match, silently
>  * discard the packet.
>  */
>  
> ... it seems to me that we would simply ignore such RFC requirements with
> your patch for the sctp_v6_err() part.
>
> Care to elaborate? ;-)

Sorry, i didn't notice that.

According to the RFC requirements, it suggests that we can't update
route for redirect in ndisc layer before calling into sctp_err_lookup(),
because we must verify the ICMP Message. Now maybe we update route for
redirect in ndisc layer is wrong.

Do you have any idea?

>> ---
>>   Changes for v3:
>>   1.del the ICMP6_INC_STATS_BH  error count, these are in fact
>>     no errors.
>>
>>   Changes for v2:
>>   1.handle the update of the NDISC_REDIRECT error code directly in
>>     icmpv6_err_convert.
>>   2.squash some patchs into one patch.
>>   3.modify the subject of those patchs.
>>
>> Duan Jiong (6):
>>    ipv6: del the statements for updating route in (dccp|tcp|sctp)_v6_err
>>    ipv6: just match on ICMPV6_PKT_TOOBIG in those err_handle
>>    ipv6: del statements for dealing with NDISC_REDIRECT
>>    ip6tnl: move route updating for redirect to ndisc layer
>>    ipv6: modify the err to 0 when dealing with NDISC_REDIRECT
>>    ipv6: Do route updating for redirect in ndisc layer
>>
>>   include/net/ip6_route.h |  3 ---
>>   net/dccp/ipv6.c         | 13 +------------
>>   net/ipv6/ah6.c          |  9 ++-------
>>   net/ipv6/esp6.c         |  9 ++-------
>>   net/ipv6/icmp.c         |  5 +++--
>>   net/ipv6/ip6_tunnel.c   |  5 -----
>>   net/ipv6/ipcomp6.c      |  9 ++-------
>>   net/ipv6/ndisc.c        |  6 ++----
>>   net/ipv6/raw.c          |  3 +--
>>   net/ipv6/route.c        | 29 ++---------------------------
>>   net/ipv6/tcp_ipv6.c     | 12 ++++--------
>>   net/ipv6/udp.c          |  2 --
>>   net/sctp/input.c        | 12 ------------
>>   net/sctp/ipv6.c         |  6 +++---
>>   14 files changed, 22 insertions(+), 101 deletions(-)
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[Daniel Borkmann]

On 09/16/2013 04:08 PM, Duan Jiong wrote:
> 于 2013/9/16 20:22, Daniel Borkmann 写道:
>> On 09/16/2013 01:47 PM, Duan Jiong wrote:
>>> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>>>
>>> the ip6_redirect() could be replaced with
>>> ip6_redirect_no_header(), we could always use ip6_redirect()
>>> for route updating in ndisc layer and use the data of the
>>> redirected header option just for finding the socket to be
>>> notified and then notify user in protocols' err_handler.
>> If I get this right, it seems to me that this patchset actually consists of two
>> different kind of changes:
>>
>> 1) Not notifying user space on ICMP redirects (net material)
>> 2) Simplify code for updating route in ndisc layer instead of error handlers (net-next)
>>
>> Also, you do the *actual* change in the very last patch, which means that from
>> patch 1 to 5 we're in an inconsistent and buggy state unless we also apply patch
>> number 6. It should actually be the other way around, that you first do the actual
>> change and then migrate users (also commit messages are quite terse).
> 
> I make the patch set on net tree, not on net-next. Maybe those
> things should be done in two patch sets.
> 
>> Moreover, just looking at the SCTP part (sctp_err_lookup() function) ...
>>
>> /* RFC 4960, Appendix C. ICMP Handling
>>   *
>>   * ICMP6) An implementation MUST validate that the Verification Tag
>>   * contained in the ICMP message matches the Verification Tag of
>>   * the peer.  If the Verification Tag is not 0 and does NOT
>>   * match, discard the ICMP message.  If it is 0 and the ICMP
>>   * message contains enough bytes to verify that the chunk type is
>>   * an INIT chunk and that the Initiate Tag matches the tag of the
>>   * peer, continue with ICMP7.  If the ICMP message is too short
>>   * or the chunk type or the Initiate Tag does not match, silently
>>   * discard the packet.
>>   */
>>   
>> ... it seems to me that we would simply ignore such RFC requirements with
>> your patch for the sctp_v6_err() part.
>>
>> Care to elaborate? ;-)
> 
> Sorry, i didn't notice that.
> 
> According to the RFC requirements, it suggests that we can't update
> route for redirect in ndisc layer before calling into sctp_err_lookup(),
> because we must verify the ICMP Message. Now maybe we update route for
> redirect in ndisc layer is wrong.

Looking further into RFC4960 [1] ... it says:

Appendix C.  ICMP Handling

   Whenever an ICMP message is received by an SCTP endpoint, the
   following procedures MUST be followed to ensure proper utilization of
   the information being provided by layer 3.

   ICMP1) An implementation MAY ignore all ICMPv4 messages where the
          type field is not set to "Destination Unreachable".

   ICMP2) An implementation MAY ignore all ICMPv6 messages where the
          type field is not "Destination Unreachable", "Parameter
          Problem", or "Packet Too Big".
   ...

So this basically means that only packets in step 2 are interesting for us here,
we *may* ignore the rest. The verification comes at step 6, so we may have
ignored the packet first. ;-) Therefore, I think your proposal should not be an
issue for SCTP.

I'd however prefer that your patch handles this similarly as in sctp_v4_err(),
so that in the default switch-case, we just go to out_unlock.

In any case, your commit message would need to be more elaborate and reflect
"why" it is okay/safe to change that in SCTP or other cases, preferably with
reference to the RFC. Otherwise, looking at the Git history in future will just be
confusing as it won't clarify why it was reasonable doing so this way.

  [1] http://tools.ietf.org/html/rfc4960#appendix-C

Re: [PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[Hannes Frederic Sowa]

On Mon, Sep 16, 2013 at 10:08:12PM +0800, Duan Jiong wrote:
> 于 2013/9/16 20:22, Daniel Borkmann 写道:
> > On 09/16/2013 01:47 PM, Duan Jiong wrote:
> >> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> >>
> >> the ip6_redirect() could be replaced with
> >> ip6_redirect_no_header(), we could always use ip6_redirect()
> >> for route updating in ndisc layer and use the data of the
> >> redirected header option just for finding the socket to be
> >> notified and then notify user in protocols' err_handler.
> > If I get this right, it seems to me that this patchset actually consists of two
> > different kind of changes:
> >
> > 1) Not notifying user space on ICMP redirects (net material)
> > 2) Simplify code for updating route in ndisc layer instead of error handlers (net-next)
> >
> > Also, you do the *actual* change in the very last patch, which means that from
> > patch 1 to 5 we're in an inconsistent and buggy state unless we also apply patch
> > number 6. It should actually be the other way around, that you first do the actual
> > change and then migrate users (also commit messages are quite terse).
> 
> I make the patch set on net tree, not on net-next. Maybe those
> things should be done in two patch sets.

I have thought about going the other direction. Just make it one patch
and update the whole logic atomically in the git tree. So we won't have
any partial logic updates in the tree.

Please also rebase your series onto net from today, because Daniel's
patch regarding updating sk->sk_err in sctp went in yesterday.

> > Moreover, just looking at the SCTP part (sctp_err_lookup() function) ...
> >
> > /* RFC 4960, Appendix C. ICMP Handling
> >  *
> >  * ICMP6) An implementation MUST validate that the Verification Tag
> >  * contained in the ICMP message matches the Verification Tag of
> >  * the peer.  If the Verification Tag is not 0 and does NOT
> >  * match, discard the ICMP message.  If it is 0 and the ICMP
> >  * message contains enough bytes to verify that the chunk type is
> >  * an INIT chunk and that the Initiate Tag matches the tag of the
> >  * peer, continue with ICMP7.  If the ICMP message is too short
> >  * or the chunk type or the Initiate Tag does not match, silently
> >  * discard the packet.
> >  */
> >  
> > ... it seems to me that we would simply ignore such RFC requirements with
> > your patch for the sctp_v6_err() part.
> >
> > Care to elaborate? ;-)
> 
> Sorry, i didn't notice that.
> 
> According to the RFC requirements, it suggests that we can't update
> route for redirect in ndisc layer before calling into sctp_err_lookup(),
> because we must verify the ICMP Message. Now maybe we update route for
> redirect in ndisc layer is wrong.
> 
> Do you have any idea?

IMO updating of routes in ndisc layer is fine. We already accept redirects
(and also change routes) for sctp connections where the redirect packet does
not contain any tag. Also it is debatable if redirects are counted as
icmp packets or merely just belong to the kind of neighbour discovery
packets which just use icmp framing (and so the sctp rfc would not say
anything about them).

As Daniel already said, it would be better to update the commit message
to clarify the reasons why this is ok (with some pointers to RFCs).

Greetings,

  Hannes

Re: [PATCH v3 0/6] ipv6: Do route updating for redirect in ndisc layer

[Duan Jiong]

davem,

please just ignore this patch set this time, i
find some errors in patch 2/6, and i'm so sorry for
this.

thanks,
Duan

于 2013/9/16 19:47, Duan Jiong 写道:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> the ip6_redirect() could be replaced with
> ip6_redirect_no_header(), we could always use ip6_redirect()
> for route updating in ndisc layer and use the data of the
> redirected header option just for finding the socket to be
> notified and then notify user in protocols' err_handler.
>
> ---
>  Changes for v3:
>  1.del the ICMP6_INC_STATS_BH  error count, these are in fact
>    no errors.
>
>  Changes for v2:
>  1.handle the update of the NDISC_REDIRECT error code directly in
>    icmpv6_err_convert.
>  2.squash some patchs into one patch.
>  3.modify the subject of those patchs.
>
> Duan Jiong (6):
>   ipv6: del the statements for updating route in (dccp|tcp|sctp)_v6_err
>   ipv6: just match on ICMPV6_PKT_TOOBIG in those err_handle
>   ipv6: del statements for dealing with NDISC_REDIRECT
>   ip6tnl: move route updating for redirect to ndisc layer
>   ipv6: modify the err to 0 when dealing with NDISC_REDIRECT
>   ipv6: Do route updating for redirect in ndisc layer
>
>  include/net/ip6_route.h |  3 ---
>  net/dccp/ipv6.c         | 13 +------------
>  net/ipv6/ah6.c          |  9 ++-------
>  net/ipv6/esp6.c         |  9 ++-------
>  net/ipv6/icmp.c         |  5 +++--
>  net/ipv6/ip6_tunnel.c   |  5 -----
>  net/ipv6/ipcomp6.c      |  9 ++-------
>  net/ipv6/ndisc.c        |  6 ++----
>  net/ipv6/raw.c          |  3 +--
>  net/ipv6/route.c        | 29 ++---------------------------
>  net/ipv6/tcp_ipv6.c     | 12 ++++--------
>  net/ipv6/udp.c          |  2 --
>  net/sctp/input.c        | 12 ------------
>  net/sctp/ipv6.c         |  6 +++---
>  14 files changed, 22 insertions(+), 101 deletions(-)
>