diff mbox

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

Message ID 52327F6F.8050508@cn.fujitsu.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Duan Jiong Sept. 13, 2013, 2:58 a.m. UTC
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>

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(-)

Comments

Hannes Frederic Sowa Sept. 13, 2013, 8:37 p.m. UTC | #1
[added linux-sctp, Daniel and Vlad + full quote]

On Fri, Sep 13, 2013 at 10:58:55AM +0800, Duan Jiong wrote:
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> 
> 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;
> +

Minor: Could you move this above the if (...) ICMP6_INC_STATS_BH. We don't
want to count errors here under no circumstance. These are in fact no errors.
(This is also the scheme in the other *_err functions).

Please Cc linux-sctp@vger.kernel.org on the sctp bits in the next round,
too.

Otherwise I looked at all patches and they seemed fine to me.

Duan, I would suggest the following:

I cannot judge if the patch Daniel proposed regarding EPROTO in sk->sk_err
on redirects should go to stable. If it should, perhaps this patch should
go in first and you could later on rebase this series as soon as Daniel's
patch has landed in the net repo? Only some minor edits should be needed
then. This way there is a clean patch for stable and David can consider
taking this in for net or net-next (this fixes a glitch where we do not
apply redirects generated by packets of ipv6 tunnels, otherwise a bit of
code removal).

Thanks,

  Hannes

>  	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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Daniel Borkmann Sept. 14, 2013, 8:20 a.m. UTC | #2
> [added linux-sctp, Daniel and Vlad + full quote]
> 
> On Fri, Sep 13, 2013 at 10:58:55AM +0800, Duan Jiong wrote:
> > From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
> > 
> > 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>
[...]
> Please Cc linux-sctp@vger.kernel.org on the sctp bits in the next round,
> too.
> 
> Otherwise I looked at all patches and they seemed fine to me.
> 
> Duan, I would suggest the following:
> 
> I cannot judge if the patch Daniel proposed regarding EPROTO in sk->sk_err
> on redirects should go to stable. If it should, perhaps this patch should
> go in first and you could later on rebase this series as soon as Daniel's
> patch has landed in the net repo? Only some minor edits should be needed
> then. This way there is a clean patch for stable and David can consider
> taking this in for net or net-next (this fixes a glitch where we do not
> apply redirects generated by packets of ipv6 tunnels, otherwise a bit of
> code removal).

Thanks Hannes !

I'm on travel over the weekend and mostly offline, so I'll look into this
on right on Monday morning.

Cheers and thanks,

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

Patch

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;
 	}