Networking: send-to-self

This patch allows one to use the SO_BINDTODEVICE and a new ioctl against net_device objects to send and receive regular routed traffic between two interfaces on the same machine. It also fixes a problem when using BINDTODEVICE:
the old code does not set the bound_if correctly when sending back the syn-ack during TCP connection initialization.

I'm not sure how useful others will find it, but it amuses me ;)

Comments and suggestions welcome.

Thanks,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

@@ -114,6 +114,16 @@
 #define SIOCBONDINFOQUERY      0x8994	/* rtn info about bond state    */
 #define SIOCBONDCHANGEACTIVE   0x8995   /* update to a new active slave */
 			
+
+/* Ben's little hack land */
+#define SIOCSACCEPTLOCALADDRS  0x89a0   /*  Allow interfaces to accept pkts from
+                                         * local interfaces...use with SO_BINDTODEVICE
+                                         */
+#define SIOCGACCEPTLOCALADDRS  0x89a1   /*  Allow interfaces to accept pkts from
+                                         * local interfaces...use with SO_BINDTODEVICE
+                                         */
+
+
 /* Device private ioctl calls */
 
 /*

@@ -97,6 +97,7 @@
 mainmenu_option next_comment
 comment 'Network testing'
 tristate 'Packet Generator (USE WITH CAUTION)' CONFIG_NET_PKTGEN
+bool 'Send-to-Self' CONFIG_NET_SENDTOSELF
 endmenu
 
 endmenu

@@ -27,6 +27,7 @@
 #include <linux/config.h>
 #include <linux/tcp.h>
 #include <linux/slab.h>
+#include <linux/cache.h>
 #include <net/checksum.h>
 #include <net/sock.h>
 

@@ -117,8 +118,7 @@
 	 * Now align to a new cache line as all the following members
 	 * are often dirty.
 	 */
-	rwlock_t __tcp_lhash_lock
-		__attribute__((__aligned__(SMP_CACHE_BYTES)));
+	rwlock_t __tcp_lhash_lock ____cacheline_aligned;
 	atomic_t __tcp_lhash_users;
 	wait_queue_head_t __tcp_lhash_wait;
 	spinlock_t __tcp_portalloc_lock;

@@ -447,7 +447,6 @@
 extern int sysctl_tcp_retrans_collapse;
 extern int sysctl_tcp_stdurg;
 extern int sysctl_tcp_rfc1337;
-extern int sysctl_tcp_tw_recycle;
 extern int sysctl_tcp_abort_on_overflow;
 extern int sysctl_tcp_max_orphans;
 extern int sysctl_tcp_max_tw_buckets;

@@ -520,6 +519,14 @@
 		struct tcp_v6_open_req v6_req;
 #endif
 	} af;
+#ifdef CONFIG_NET_SENDTOSELF
+        int bound_dev_if; /* This is so we can connect to ourselves and not collide
+                           * in the open-request hash (the addresses and ports are the
+                           * same, but we need two ends, so use the interface to determine
+                           * one from the other.  Active when SO_BINDTODEVICE is used.
+                           * --Ben
+                           */
+#endif
 };
 
 /* SLAB cache for open requests. */

@@ -1,4 +1,4 @@
-/* linux/net/inet/arp.c
+/* linux/net/inet/arp.c  -*-linux-c-*-
  *
  * Version:	$Id: sts_2.4.19.patch,v 1.3 2002/09/17 07:01:55 greear Exp $
  *

@@ -351,12 +351,26 @@
 	int flag = 0; 
 	/*unsigned long now; */
 
-	if (ip_route_output(&rt, sip, tip, 0, 0) < 0) 
+	if (ip_route_output(&rt, sip, tip, 0, 0) < 0)
 		return 1;
-	if (rt->u.dst.dev != dev) { 
-		NET_INC_STATS_BH(ArpFilter);
-		flag = 1;
-	} 
+        
+	if (rt->u.dst.dev != dev) {
+#ifdef CONFIG_NET_SENDTOSELF
+                if ((dev->priv_flags & IFF_ACCEPT_LOCAL_ADDRS) &&
+                    (rt->u.dst.dev == &loopback_dev))  {
+                        /* OK, we'll let this special case slide, so that we can arp from one
+                         * local interface to another.  This seems to work, but could use some
+                         * review. --Ben
+                         */
+                }
+                else {
+#endif
+                        NET_INC_STATS_BH(ArpFilter);
+                        flag = 1;
+#ifdef CONFIG_NET_SENDTOSELF
+                }
+#endif
+        }
 	ip_rt_put(rt); 
 	return flag; 
 } 

@@ -233,8 +233,19 @@
 
 	if (fib_lookup(&key, &res))
 		goto last_resort;
-	if (res.type != RTN_UNICAST)
-		goto e_inval_res;
+        
+	if (res.type != RTN_UNICAST) {
+#ifdef CONFIG_NET_SENDTOSELF
+           if ((res.type == RTN_LOCAL) &&
+                    (dev->priv_flags & IFF_ACCEPT_LOCAL_ADDRS)) {
+                     /* All is OK */
+           }
+           else
+#endif
+              goto e_inval_res;
+                
+        }
+        
 	*spec_dst = FIB_RES_PREFSRC(res);
 	fib_combine_itag(itag, &res);
 #ifdef CONFIG_IP_ROUTE_MULTIPATH

@@ -520,8 +520,18 @@
 		/*
 		 *	Get the memory we require with some space left for alignment.
 		 */
-
-		skb = sock_alloc_send_skb(sk, fraglen+hh_len+15, flags&MSG_DONTWAIT, &err);
+		if (!(flags & MSG_DONTWAIT) || nfrags == 0) {
+			skb = sock_alloc_send_skb(sk, fraglen + hh_len + 15,
+						  (flags & MSG_DONTWAIT), &err);
+		} else {
+			/* On a non-blocking write, we check for send buffer
+			 * usage on the first fragment only.
+			 */
+			skb = sock_wmalloc(sk, fraglen + hh_len + 15, 1,
+					   sk->allocation);
+			if (!skb)
+				err = -ENOBUFS;
+		}
 		if (skb == NULL)
 			goto error;
 

@@ -965,7 +975,11 @@
 			daddr = replyopts.opt.faddr;
 	}
 
+#ifdef CONFIG_NET_SENDTOSELF
+	if (ip_route_output(&rt, daddr, rt->rt_spec_dst, RT_TOS(skb->nh.iph->tos), sk->bound_dev_if))
+#else
 	if (ip_route_output(&rt, daddr, rt->rt_spec_dst, RT_TOS(skb->nh.iph->tos), 0))
+#endif
 		return;
 
 	/* And let IP do all the hard work.

We saw it the first two times.

Gack, sorry for the last patch..it seems I screwed up the patch process somehow. Plz don't apply it as is!

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

So, I feel bad about wasting bandwidth and spamming folks with busted patches. The corrected patch(es) can be found here for those who are interested:

http://www.candelatech.com/sts_2.4.19.patch
http://www.candelatech.com/sts2_hack.patch

Thanks,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

From: Ben Greear <[email protected]>
Date: Wed, 18 Sep 2002 00:09:50 -0700

http://www.candelatech.com/sts_2.4.19.patch

I don't think I'll be applying this:

1) No tcp ipv6 bits
2) SIOC{S,G}ACCEPTLOCALADDRS added, but no 32-bit translation code added to varions 64-bit/32-bit biarch port ioctl handling. Also, no code added to the ioctl dispatch in the networking so that devices could actually receive these requests.
3) Finally, it's just too damn ugly. If you have to ifdef it then it really doesn't belong in the tree. Maybe if the device number comparison logic changes existed via macros in tcp.h and thus removing all the CONFIG_NET_SENDTOSELF ifdefs from tcp*.c code it might be more palatable.
4) I haven't reviewed the ramifications of the route lookup changes, that is Alexey's territory.

Sorry, these changes are pretty ugly right now.

I know squat about this, so am reluctant to hack code there.

See http://www.candelatech.com/sts2_hack.patch (32-bit only), it contains the missing bits, I'm not good at generating two patch sets (ie pktgen and send-to-self) when they touch the same file...

The #ifdefs were per request, I personally would like them not to be there either. As far as I can tell, the changes are backwards compatible, so there should be no need for ifdefs.

Thanks for looking at them. I can fix the #ifdef cruft, but adding 64bit support or hacking ipv6 is beyond my means of testing at this point, so I cannot make those changes.

Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

From: Ben Greear <[email protected]>
Date: Wed, 18 Sep 2002 16:20:49 -0700

I know squat about this, so am reluctant to hack code there.

It's hash lookup code, nearly identical to ipv4 version except it's dealing with 128-bit IP addresses instead of 32-bit.

You give up way too easily, which leads me to belive you'll disappear just as easily if complicated bugs stop popping up as a result of your changes.

This is one of the most important issues I consider when I get a delicate patch to the networking for someone, how fast they throw their arms up in the air.

For example, someone like Arnaldo, when he sends me a patch and the whole kernel explodes as a result I know he'll stick around for however long it takes to fix the problems and he won't go "ipv6 looks too complicated" when I ask him to submit a complete version of his changes.

You patch should not be a maintainence burden to me. Your attitude tells me it is going to become one.

See http://www.candelatech.com/sts2_hack.patch (32-bit only), it contains the missing bits, I'm not good at generating two patch sets (ie pktgen and send-to-self) when they touch the same file...

Don't include stuff in the patch that doesn't belong there, this isn't so difficult.

The #ifdefs were per request, I personally would like them not to be there either. As far as I can tell, the changes are backwards compatible, so there should be no need for ifdefs.

I mean put the ifdefs in a header file such as tcp.h, not in the *.c code.

Thanks for looking at them. I can fix the #ifdef cruft, but adding 64bit support or hacking ipv6 is beyond my means of testing at this point, so I cannot make those changes.

I don't require you to test the ipv6 portions, I will be able to eyeball them and know if they are right or not, this is how simple the ipv6 version of the tcp bits will be.

I'll maintain this patch for myself if no one else, so I will not go away. But, since I am new to this code, and do not have a test setup to test the ipv6 changes, I was hesitant. I can at the least patch it how I think it should be and test that ipv4 still works and it compiles. If you or someone else can do more profound testing on it, then that would be great.

Would you object to me just removing all of them and having the patch unconditionally compiled in?

Ok, I'll work up a patch with the ipv6 support and try to get that out sometime next week.

Thanks,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

From: Ben Greear <[email protected]>
Date: Wed, 18 Sep 2002 19:07:33 -0700

Would you object to me just removing all of them and having the patch unconditionally compiled in?

Your comments say that SIOCBINDTODEVICE behavior is changed, how can we legitimately do that all the time without breaking apps?

The old way is broken, it sets the bound-device to 0 when sending the syn-ack. I am not sure exactly how this worked before, or if it even worked at all. I changed it to use the bound_dev_if of the parent socket, which I believe is more correct.

@@ -975,7 +975,11 @@
 			daddr = replyopts.opt.faddr;
 	}
 
+#ifdef CONFIG_NET_SENDTOSELF
+	if (ip_route_output(&rt, daddr, rt->rt_spec_dst, RT_TOS(skb->nh.iph->tos), sk->bound_dev_if))
+#else
 	if (ip_route_output(&rt, daddr, rt->rt_spec_dst, RT_TOS(skb->nh.iph->tos), 0))
+#endif
 		return;
 
 	/* And let IP do all the hard work.

It also failed due to the hashing issue, but with the current code, packets to self from self will be dropped before reaching the IP code, so that is not really a bug in the current code.

Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Hello!

Ben, this function is _not_ used to send syn-acks...

I am not so sure, untill I changed that method, it most certainly did not work. It could have been something other than the syn-ack that failed though...

Unfortunately, I ripped out all of the printks, so I cannot easily back up my claims w/out poluting the code again.

Think about this: Suppose you are connecting to a listening socket that has been bound to a device. That creates the the temporary socket structure on the receive side, which
is used to send the syn-ack. That temp socket structure must also be bound to the same device, or the ack will not get routed correctly back out of the right interface.

As far as I can tell, the code must be patched as above or the temp socket will not use the correct bound device. Please explain how the syn-ack can get routed based on the parent's bound_dev_if if my assumption here is not correct.

I will investigate that code...I haven't used sysctl on purpose before :)

Thanks for the review, and I look forward to your response to my assertions!

Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Hello!

That does _not_. All transmits use listening socket structure.

Find function tcp_v4_send_synack(), then tcp_v4_route_req() and derive that your modifications have exactly zero impact on it, sk->bound_dev_if is setup and used exactly like it was used before.

Alexey

Ok, I took out my changes above and sure enough, it still seems to work.

I have a question though: If this method is ever called to send an RST, will it work? It seems to me that it may not be routed correctly, but I don't understand all the circumstances that would cause this method to be called either...

What about this part, do you think it is not needed either? It appeared to me that w/out this, the sending socket and the receiving socket could hash to the same thing, and so greatly confused themselves. For instance,

src=1.2.3.4, port 9999, dst=1.2.3.5 port 9999

I believe I was seeing the first packet (SYN) from the originator being delivered back to the socket that sent the SYN because no new socket on the receiving side was created because the hash found one (it found the sending one, unfortunately).

+ /* Will only take netdevice_id into the equation if neither are

+         * 0.  This should be backwards compatible with older code, and also
+         * let us connect to ourselves over external ports.  Otherwise, we
+         * get confused about which connection is the originator v/s the
+         * receiver of the open request. --Ben
+         */

for (prev = &lopt->syn_table[tcp_v4_synq_hash(raddr, rport)];
(req = *prev) != NULL;
prev = &req->dl_next) {
if (req->rmt_port == rport &&
req->af.v4_req.rmt_addr == raddr && req->af.v4_req.loc_addr == laddr &&
- TCP_INET_FAMILY(req->class->family)) {
+ TCP_INET_FAMILY(req->class->family)
+#ifdef CONFIG_NET_SENDTOSELF
+ && ((!netdevice_id) || (!req->bound_dev_if) ||
+ (req->bound_dev_if == netdevice_id))) {
+#else
+ ) {
+#endif

Thanks again,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Hello!

Work for what? This method is used when you have no socket in the system matching to this skb. :-)

...

No, this is impossible. addrs/ports identity identifies socket unambiguously.

But even this does not matter, your patch is noop by much simpler pure "syntactical" reason, it copies req->bound_dev_if from listening socket. And after this the check in lookup req degenerates to TRUE __identitically__.
:-)

Alexey

Assume we are sending to ourself, and we bind to the local ip/port before we connect:
we have source IP = 1.2.3.4, source-port = 7

Assume we are connecting to a socket on a second interface, with dest ip = 1.2.3.5, dest-port = 7

When the SYN is received on the second port after connect() is called, the code looks for a socket based on source ip 1.2.3.4 and source port 7. In this case, it should not find any socket because this SYN is the first communication....

However, because the sender is on the same machine, using the same kernel structures, the socket **DOES** exist, and so the hash lookup finds it. At this point, I believe it sends the RST because a socket in the SYN-sent state just received a SYN packet, which is not how things should work.

It will not match the case where the sending socket is bound to interface 3 and the listening socket is bound on interface 4, but both sockets have the same source IP and port. That was what I was trying
to do...but I may have not done it quite right :)

Thanks,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear

Hello!

I would say "quite not right". :-)

Anyway, this is supposed to work without any modifications.

Alexey

I will run some tests with that part of the code commented out.

Thanks,
Ben

Ben Greear <[email protected]>       <Ben_Greear AT excite.com>
President of Candela Technologies Inc      http://www.candelatech.com
ScryMUD:  http://scry.wanfear.com     http://scry.wanfear.com/~greear