Patchwork tcp_v4_send_reset: binding oif to iif in no sock case

login
register
mail settings
Submitter Shawn Lu
Date Feb. 4, 2012, 9:22 p.m.
Message ID <1328390556-14906-1-git-send-email-shawn.lu@ericsson.com>
Download mbox | patch
Permalink /patch/139591/
State Rejected
Delegated to: David Miller
Headers show

Comments

Shawn Lu - Feb. 4, 2012, 9:22 p.m.
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(-)
David Miller - Feb. 4, 2012, 9:26 p.m.
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?
--
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
Shawn Lu - Feb. 4, 2012, 9:30 p.m.
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?
> --
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
Ben Greear - Feb. 5, 2012, 1:52 a.m.
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

Patch

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;