[PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Shawn Lu]

Binding RST packet outgoing interface to incomming interface
for tcp v4 when there is no socket associate with it.
This has few benefits:
1. tcp_v6_send_reset already did that.

2. This helps tcp connect with SO_BINDTODEVICE set. When connection
is lost, we still able to sending out RST using same interface.

3. we are send reply, it is most likely to be succeed if iif
is used

Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
---
V2: amend as Eric Dumazet suggested
 net/ipv4/tcp_ipv4.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 90e4793..f2fde8d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -676,6 +676,12 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
 				      arg.iov[0].iov_len, IPPROTO_TCP, 0);
 	arg.csumoffset = offsetof(struct tcphdr, check) / 2;
 	arg.flags = (sk && inet_sk(sk)->transparent) ? IP_REPLY_ARG_NOSRCCHECK : 0;
+	/*
+	 * When socket is gone, all binding information is lost.
+	 * routing might fail in this case. using iif for oif to
+	 * make sure we can deliver it
+	 */
+	arg.bound_dev_if = sk ? sk->sk_bound_dev_if : inet_iif(skb);
 
 	net = dev_net(skb_dst(skb)->dev);
 	arg.tos = ip_hdr(skb)->tos;
-- 
1.7.0.4

Re: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[David Miller]

From: Shawn Lu <shawn.lu@ericsson.com>
Date: Sat, 4 Feb 2012 13:22:36 -0800

> +	/*
> +	 * When socket is gone, all binding information is lost.
> +	 * routing might fail in this case. using iif for oif to
> +	 * make sure we can deliver it
> +	 */

Comment is poorly formatted, and I'm getting tired of teaching
people

	/* That comments are formatted like
	 * this.
	 */

	/*
	 * Not like this.
	 */

or do you have infinite vertical lines on your monitor so that
this empty "/*" line has no impact on your screen real estate
like it does for the rest of us?

RE: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Shawn Lu]

Sorry, will resubmit with right format. 

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net] 
> Sent: Saturday, February 04, 2012 1:26 PM
> To: Shawn Lu
> Cc: eric.dumazet@gmail.com; netdev@vger.kernel.org; xiaoclu@gmail.com
> Subject: Re: [PATCH] tcp_v4_send_reset: binding oif to iif in 
> no sock case
> 
> From: Shawn Lu <shawn.lu@ericsson.com>
> Date: Sat, 4 Feb 2012 13:22:36 -0800
> 
> > +	/*
> > +	 * When socket is gone, all binding information is lost.
> > +	 * routing might fail in this case. using iif for oif to
> > +	 * make sure we can deliver it
> > +	 */
> 
> Comment is poorly formatted, and I'm getting tired of teaching people
> 
> 	/* That comments are formatted like
> 	 * this.
> 	 */
> 
> 	/*
> 	 * Not like this.
> 	 */
> 
> or do you have infinite vertical lines on your monitor so 
> that this empty "/*" line has no impact on your screen real 
> estate like it does for the rest of us?
> 

[PATCH] CodingStyle: Add net and drivers/net preferred commenting style

[Joe Perches]

Perhaps adding something to CodingStyle is useful?

---

 Documentation/CodingStyle |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
index 2b90d32..65e82e4 100644
--- a/Documentation/CodingStyle
+++ b/Documentation/CodingStyle
@@ -454,6 +454,14 @@ The preferred style for long (multi-line) comments is:
 	 * with beginning and ending almost-blank lines.
 	 */
 
+Unless the files are in net or drivers/net where David Miller
+prefers the vertically shorter:
+
+	 /* This is the preferred style for multi-line
+	  * comments in the net and drivers/net directories.
+	  * Please use it consistently.
+	  */
+
 It's also important to comment data, whether they are basic types or derived
 types.  To this end, use just one data declaration per line (no commas for
 multiple data declarations).  This leaves you room for a small comment on each

Re: [PATCH] CodingStyle: Add net and drivers/net preferred commenting style

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Sat, 04 Feb 2012 13:34:43 -0800

> Perhaps adding something to CodingStyle is useful?

Come on, get real.

[PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Shawn Lu]

Binding RST packet outgoing interface to incoming interface
for tcp v4 when there is no socket associate with it.
This has few benefits:
1. tcp_v6_send_reset already did that.
2. This helps tcp connect with SO_BINDTODEVICE set. When
connection is lost, we still able to sending out RST using
same interface.
3. we are sending reply, it is most likely to be succeed
if iif is used

Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
---
V3: reformat comments as suggested
 net/ipv4/tcp_ipv4.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 90e4793..4d6f81c 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -676,6 +676,11 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
 				      arg.iov[0].iov_len, IPPROTO_TCP, 0);
 	arg.csumoffset = offsetof(struct tcphdr, check) / 2;
 	arg.flags = (sk && inet_sk(sk)->transparent) ? IP_REPLY_ARG_NOSRCCHECK : 0;
+	/* When socket is gone, all binding information is lost.
+	 * routing might fail in this case. using iif for oif to
+	 * make sure we can deliver it
+	 */
+	arg.bound_dev_if = sk ? sk->sk_bound_dev_if : inet_iif(skb);
 
 	net = dev_net(skb_dst(skb)->dev);
 	arg.tos = ip_hdr(skb)->tos;
-- 
1.7.0.4

Re: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[David Miller]

From: Shawn Lu <shawn.lu@ericsson.com>
Date: Sat, 4 Feb 2012 13:44:57 -0800

> Binding RST packet outgoing interface to incoming interface
> for tcp v4 when there is no socket associate with it.
> This has few benefits:
> 1. tcp_v6_send_reset already did that.
> 2. This helps tcp connect with SO_BINDTODEVICE set. When
> connection is lost, we still able to sending out RST using
> same interface.
> 3. we are sending reply, it is most likely to be succeed
> if iif is used
> 
> Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>

Sigh...

Thanks for giving Eric not credit whatsoever for his contributions
and feedback for this fix. :-/

Re: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Eric Dumazet]

Le samedi 04 février 2012 à 13:44 -0800, Shawn Lu a écrit :
> Binding RST packet outgoing interface to incoming interface
> for tcp v4 when there is no socket associate with it.
> This has few benefits:
> 1. tcp_v6_send_reset already did that.
> 2. This helps tcp connect with SO_BINDTODEVICE set. When
> connection is lost, we still able to sending out RST using
> same interface.
> 3. we are sending reply, it is most likely to be succeed
> if iif is used
> 
> Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
> ---
> V3: reformat comments as suggested
>  net/ipv4/tcp_ipv4.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 90e4793..4d6f81c 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -676,6 +676,11 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
>  				      arg.iov[0].iov_len, IPPROTO_TCP, 0);
>  	arg.csumoffset = offsetof(struct tcphdr, check) / 2;
>  	arg.flags = (sk && inet_sk(sk)->transparent) ? IP_REPLY_ARG_NOSRCCHECK : 0;
> +	/* When socket is gone, all binding information is lost.
> +	 * routing might fail in this case. using iif for oif to
> +	 * make sure we can deliver it
> +	 */
> +	arg.bound_dev_if = sk ? sk->sk_bound_dev_if : inet_iif(skb);
>  
>  	net = dev_net(skb_dst(skb)->dev);
>  	arg.tos = ip_hdr(skb)->tos;

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Re: [PATCH] CodingStyle: Add net and drivers/net preferred commenting style

[Joe Perches]

On Sat, 2012-02-04 at 16:40 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sat, 04 Feb 2012 13:34:43 -0800
> > Perhaps adding something to CodingStyle is useful?
> Come on, get real.

;)

I'm trying to get you to understand that your
commenting style is not what's in CodingStyle
and that you are a bit of an outlier for using
it.

You can repeat it again and again on the list,
but not everyone reads it.

Notice I didn't sign it either, but I suppose
checkpatch could always add something instead...

cheers, Joe

Re: [PATCH] CodingStyle: Add net and drivers/net preferred commenting style

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Sat, 04 Feb 2012 13:50:59 -0800

> On Sat, 2012-02-04 at 16:40 -0500, David Miller wrote:
>> From: Joe Perches <joe@perches.com>
>> Date: Sat, 04 Feb 2012 13:34:43 -0800
>> > Perhaps adding something to CodingStyle is useful?
>> Come on, get real.
> 
> ;)
> 
> I'm trying to get you to understand that your
> commenting style is not what's in CodingStyle
> and that you are a bit of an outlier for using
> it.

So do you think my way is superior or inferior to the CodingStyle
suggestion?

Explain.

If you think my way is better, then submit a patch to change it
for everyone, not just networking.

Re: [PATCH] CodingStyle: Add net and drivers/net preferred commenting style

[Joe Perches]

On Sat, 2012-02-04 at 16:53 -0500, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> > I'm trying to get you to understand that your
> > commenting style is not what's in CodingStyle
> > and that you are a bit of an outlier for using
> > it.
> So do you think my way is superior or inferior to the CodingStyle
> suggestion?

I think it doesn't matter enough to care about it
and I generally use relatively height limited
screens (100x 50y or so).

cheers, Joe

[PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Shawn Lu]

Binding RST packet outgoing interface to incoming interface
for tcp v4 when there is no socket associate with it.
when sk is not NULL, using sk->sk_bound_dev_if instead.
(suggested by Eric Dumazet).

This has few benefits:
1. tcp_v6_send_reset already did that.
2. This helps tcp connect with SO_BINDTODEVICE set. When
connection is lost, we still able to sending out RST using
same interface.
3. we are sending reply, it is most likely to be succeed
if iif is used

Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
---
V4: change log. Thanks for Eric Dumazet to take care
of "sk is not NULL" case. He also provide code
for this fix. 

 net/ipv4/tcp_ipv4.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 90e4793..4d6f81c 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -676,6 +676,11 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
 				      arg.iov[0].iov_len, IPPROTO_TCP, 0);
 	arg.csumoffset = offsetof(struct tcphdr, check) / 2;
 	arg.flags = (sk && inet_sk(sk)->transparent) ? IP_REPLY_ARG_NOSRCCHECK : 0;
+	/* When socket is gone, all binding information is lost.
+	 * routing might fail in this case. using iif for oif to
+	 * make sure we can deliver it
+	 */
+	arg.bound_dev_if = sk ? sk->sk_bound_dev_if : inet_iif(skb);
 
 	net = dev_net(skb_dst(skb)->dev);
 	arg.tos = ip_hdr(skb)->tos;
-- 
1.7.0.4

RE: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Shawn Lu]

Hi,David:

I do appreciate Eric's helpand contribution in this fix. 
I am a newer here. I just don't know how I can give credit to him. I thought
That you will put "Acked-by: Eric Dumazet <eric.dumazet@gmail.com>" to 
Credit his contribution.

Thanks!
Shawn Lu  

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net] 
> Sent: Saturday, February 04, 2012 1:47 PM
> To: Shawn Lu
> Cc: eric.dumazet@gmail.com; netdev@vger.kernel.org; xiaoclu@gmail.com
> Subject: Re: [PATCH] tcp_v4_send_reset: binding oif to iif in 
> no sock case
> 
> From: Shawn Lu <shawn.lu@ericsson.com>
> Date: Sat, 4 Feb 2012 13:44:57 -0800
> 
> > Binding RST packet outgoing interface to incoming interface 
> for tcp v4 
> > when there is no socket associate with it.
> > This has few benefits:
> > 1. tcp_v6_send_reset already did that.
> > 2. This helps tcp connect with SO_BINDTODEVICE set. When 
> connection is 
> > lost, we still able to sending out RST using same interface.
> > 3. we are sending reply, it is most likely to be succeed if iif is 
> > used
> > 
> > Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
> 
> Sigh...
> 
> Thanks for giving Eric not credit whatsoever for his 
> contributions and feedback for this fix. :-/
> 

Re: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[David Miller]

From: Shawn Lu <shawn.lu@ericsson.com>
Date: Sat, 4 Feb 2012 14:38:09 -0800

> Binding RST packet outgoing interface to incoming interface
> for tcp v4 when there is no socket associate with it.
> when sk is not NULL, using sk->sk_bound_dev_if instead.
> (suggested by Eric Dumazet).
> 
> This has few benefits:
> 1. tcp_v6_send_reset already did that.
> 2. This helps tcp connect with SO_BINDTODEVICE set. When
> connection is lost, we still able to sending out RST using
> same interface.
> 3. we are sending reply, it is most likely to be succeed
> if iif is used
> 
> Signed-off-by: Shawn Lu <shawn.lu@ericsson.com>
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied and queued up for -stable, thanks.

Re: [PATCH] tcp_v4_send_reset: binding oif to iif in no sock case

[Ben Greear]

On 02/04/2012 01:26 PM, David Miller wrote:
> From: Shawn Lu<shawn.lu@ericsson.com>
> Date: Sat, 4 Feb 2012 13:22:36 -0800
>
>> +	/*
>> +	 * When socket is gone, all binding information is lost.
>> +	 * routing might fail in this case. using iif for oif to
>> +	 * make sure we can deliver it
>> +	 */
>
> Comment is poorly formatted, and I'm getting tired of teaching
> people
>
> 	/* That comments are formatted like
> 	 * this.
> 	 */
>
> 	/*
> 	 * Not like this.
> 	 */

I agree with your taste in comments, but the linux style guide
says basically the opposite.  Maybe see if you can sneak a patch
into the style guide? :)

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com