Bug 2358

Summary: code review: query routing once per TCP socket
Product: ns-3 Reporter: Tom Henderson <tomh>
Component: tcpAssignee: natale.patriciello
Status: NEEDINFO ---    
Severity: enhancement CC: m.kheirkhah, ns-bugs, tommaso.pecorella
Priority: P5    
Version: unspecified   
Hardware: All   
OS: All   

Description Tom Henderson 2016-03-31 15:30:02 EDT
contributed by Morteza:
https://codereview.appspot.com/241870043/
Comment 1 natale.patriciello 2016-04-05 11:19:29 EDT
There are some pratical examples where this proposed path could fail ? From what I could say, if the oif changes, the connection is destroyed since the 5-tuple changes (two oif cannot share the same IP address). But at this point I don't know why currently it was coded with that fear..
Comment 2 Tommaso Pecorella 2016-04-05 11:31:44 EDT
The problem isn't the oif, the problem is in the router.

The "This Has Been Written" argument is: Routing is a L3 process, and L4 shouldn't be interested in it.

The real point is another one: the router can change and L4 shouldn't be affected by it.
One practical case: ICMP redirect in IPv6. You start with a router, and you change to another one without interrupting the stream (and keeping the same exit interface).

The only things you can save in L4 are the outgoing interface data (oif and source IP address), not the whole route.

As a side note, fixing this (as is, be able to force an oif from a socket) should also benefit bug #1041
Comment 3 natale.patriciello 2016-04-05 12:06:52 EDT
(In reply to Tommaso Pecorella from comment #2)

> The real point is another one: the router can change and L4 shouldn't be
> affected by it.
> One practical case: ICMP redirect in IPv6. You start with a router, and you
> change to another one without interrupting the stream (and keeping the same
> exit interface).

Isn't that a process that happens somewhere in the network? From TCP point of view, the physical interface can not change.. unless two interfaces share the same IP address. Maybe my misunderstanding is due to a lack of the knowledge of Ptr<Ipv{4,6}Route>. Imagine a simple topology:

          (*)
A ----- R -- B
\           /
 \         /
  \---- R1 ---- C

A has two NetDevice, address 10.0.0.1 (upper) and 192.168.0.1 (lower). We are inside the node A. We want to open a connection to B through 10.0.0.1, port 53433. B has two IP address (one to R, the other to R1). If A choose the upper address, it is forced to pass through R, and the route for A will be the same. TCP is build around the tuple (10.0.0.1, 53433, Addr(B).Upper, dstport). Changing the route would mean change also the destination address.

Let's now pretend that the network portion represented by (*) is composed by multiple routers. Inside this portion, router can change freely, but A will never know that, and I hope that the Ipv4Route object inside of A does not have also these mid-path routers, but only the first hop.

The last case to consider is NetDevices that are multi-point (e.g. wifi). In that case would the above fail in case of a gateway change? The oif would be the same, but the first hop is different.

> The only things you can save in L4 are the outgoing interface data (oif and
> source IP address)

In general, don't you think that the current socket API is a bit odd? I mean, DoForwardUp in TCP means "I need to process this packet", not "Give that packet to app" that would be common sense.. The same happens downlink: why Ipv4L3Protocol::Send takes an Ipv4Route as argument ?!? Shouldn't be his duty to retrieve it?

But this is clearly impossible, since all is stacked up with inheritance .. TcpL4Protocol is-a IpL4Protocol, which is NOT true. Aaah, I agree with Linus Torvalds.. http://harmful.cat-v.org/software/c++/linus (I'm clearly joking here :P)

Being serious, can we use a = 0 specifier on the route parameter of Ipv4L3Protocol::Send (same for Ipv6)? TCP and UDP will ignore this parameter, and if it's 0 the *L3Protocol would take care of searching a route, or to store it I don't know.
Comment 4 Tommaso Pecorella 2016-04-05 16:20:48 EDT
Hi,

maybe I didn't clarify my point enough.
I'm not against storing some informations in TCP, and I'm not ready to start a religion fight about who should do what.
*However* the routing does its job, and it isn't going to signal the L4 protocols.

Now, in both Ipv4 and Ipv6 the Route class stores these:
  Ipv4Address m_dest;             //!< Destination address.
  Ipv4Address m_source;           //!< Source address.
  Ipv4Address m_gateway;          //!< Gateway address.
  Ptr<NetDevice> m_outputDevice;  //!< Output device.

When you call RouteOutput the routing protocol knows *only* m_dest.
It should (must) honor m_outputDevice if it is already set (a.k.a. the oif).
It should (must) honor m_source if it is already set.

If m_source is *not* set it does this:
Given m_dest, and constrained to m_outputDevice (if set) it will decide m_source and m_gateway.

If m_source is *already* set it does this:
Given m_dest, and constrained to m_outputDevice (if set) it will decide m_gateway.

Depending on the IP settings, RouteOutput could (or could not) complain about a mismatch between m_outputDevice and m_source, i.e., if the source address is set on the m_outputDevice or not.

I must stress that m_gateway can be updated dynamically without any consequence on the TCP or UDP flow, without changing the output interface and the source address.

Use case: a host attached to a network. The network has a default gateway and a secondary gateway. The default gateway fails and the secondary gateway takes over.
The secondary gateway had a different MAC and a different IP number. Problems ? None.

A more graceful case. The flow is for a network reachable from the default gateway. After some time the secondary gateway is able to establish a preferred route toward the destination and exports it with a router announcement.
The source happily changes the gateway from the default one to the preferred one.

Summarizing, I'd say that the fist (and only) thing to do is to make sure that the "static" parts are honored by the routing protocol, i.e.: Destination address (obvious), Source address (if set) and output device (if set).
The whole route is counterintuitive, because the gateway IS the route.


(In reply to natale.patriciello from comment #3)
> (In reply to Tommaso Pecorella from comment #2)
> 
> > The real point is another one: the router can change and L4 shouldn't be
> > affected by it.
> > One practical case: ICMP redirect in IPv6. You start with a router, and you
> > change to another one without interrupting the stream (and keeping the same
> > exit interface).
> 
> Isn't that a process that happens somewhere in the network? From TCP point
> of view, the physical interface can not change.. unless two interfaces share
> the same IP address. Maybe my misunderstanding is due to a lack of the
> knowledge of Ptr<Ipv{4,6}Route>. Imagine a simple topology:
> 
>           (*)
> A ----- R -- B
> \           /
>  \         /
>   \---- R1 ---- C
> 
> A has two NetDevice, address 10.0.0.1 (upper) and 192.168.0.1 (lower). We
> are inside the node A. We want to open a connection to B through 10.0.0.1,
> port 53433. B has two IP address (one to R, the other to R1). If A choose
> the upper address, it is forced to pass through R, and the route for A will
> be the same. TCP is build around the tuple (10.0.0.1, 53433, Addr(B).Upper,
> dstport). Changing the route would mean change also the destination address.
> 
> Let's now pretend that the network portion represented by (*) is composed by
> multiple routers. Inside this portion, router can change freely, but A will
> never know that, and I hope that the Ipv4Route object inside of A does not
> have also these mid-path routers, but only the first hop.
> 
> The last case to consider is NetDevices that are multi-point (e.g. wifi). In
> that case would the above fail in case of a gateway change? The oif would be
> the same, but the first hop is different.
> 
> > The only things you can save in L4 are the outgoing interface data (oif and
> > source IP address)
> 
> In general, don't you think that the current socket API is a bit odd? I
> mean, DoForwardUp in TCP means "I need to process this packet", not "Give
> that packet to app" that would be common sense.. The same happens downlink:
> why Ipv4L3Protocol::Send takes an Ipv4Route as argument ?!? Shouldn't be his
> duty to retrieve it?
> 
> But this is clearly impossible, since all is stacked up with inheritance ..
> TcpL4Protocol is-a IpL4Protocol, which is NOT true. Aaah, I agree with Linus
> Torvalds.. http://harmful.cat-v.org/software/c++/linus (I'm clearly joking
> here :P)
> 
> Being serious, can we use a = 0 specifier on the route parameter of
> Ipv4L3Protocol::Send (same for Ipv6)? TCP and UDP will ignore this
> parameter, and if it's 0 the *L3Protocol would take care of searching a
> route, or to store it I don't know.
Comment 5 natale.patriciello 2016-04-06 04:06:57 EDT
(In reply to Tommaso Pecorella from comment #4)
> maybe I didn't clarify my point enough.
> I'm not against storing some informations in TCP, and I'm not ready to start
> a religion fight about who should do what.

Me neither


> I must stress that m_gateway can be updated dynamically without any
> consequence on the TCP or UDP flow, without changing the output interface
> and the source address.

Ok, now I understood perfectly. So, it is clear that we cannot store Ipv{4,6}Route. And I cannot find the reference of where this patch come from (to ready why this patch is needed). Without more informations from the author, I'm prone to not accept this patch.
Comment 6 natale.patriciello 2016-05-22 16:21:29 EDT
Should we close this as invalid ?
Comment 7 Tommaso Pecorella 2016-05-22 17:48:00 EDT
No, please. I want to review the whole problem (it could be wider than just that).


(In reply to natale.patriciello from comment #6)
> Should we close this as invalid ?
Comment 8 natale.patriciello 2016-11-06 05:12:15 EST
(In reply to Tommaso Pecorella from comment #7)
> No, please. I want to review the whole problem (it could be wider than just
> that).


Hi Tommaso,

any news on that?
Comment 9 Tommaso Pecorella 2016-11-08 17:34:08 EST
(In reply to natale.patriciello from comment #8)
> (In reply to Tommaso Pecorella from comment #7)
> > No, please. I want to review the whole problem (it could be wider than just
> > that).
> 
> 
> Hi Tommaso,
> 
> any news on that?

Yes and no.

No, no news.
Yes, I'm more than convinced that Morteza's approach was not totally right (the route can't be cached) BUT that a problem exists and should be fixed.

If you want we can close this particular bug and open another one, or I can extend/modify this one.