internet: Add a tos field to InetSockAddress

internet: Add a tos field to InetSockAddress

[Stefano Avallone, 2016-04-20 17:17:44]

Hello,

I would kindly ask you to provide a preliminary feedback on the solution drafted
in this code review to make it easy (possible?) to set the ToS byte in IPv4
packets.

Currently, the ToS can be set via the Socket::SetIpTos method, then (some of
the) actual socket implementations add a SocketIpTosTag to convey the ToS value
to Ipv4L3Protocol::Send, which actually sets the value in the header.

I think the problem currently resides in the need to use the Socket::SetIpTos
method:

- sockets are usually hidden (or even not accessible) in examples, thus making
it difficult for users to set the ToS value

- we might modify the applications to provide a hook to modify the ToS in the
socket (as Tom did in issue #277570043), but I think the downside is that we
need to modify ALL the applications

Thus, considering that:

- examples usually handle InetSockAddresses

- in Linux, tos is one of the fields of the struct inet_sock (which
InetSockAddress aims to mimic)

- ToS is specific to IPv4 sockets (hence the socket base class should not
provide methods to get/set the ToS)

I thought to extend the InetSockAddress class so as to include a m_tos field. To
set the ToS, users could do something like:

InetSocketAddress inet (remoteAddr, port);
inet.SetTos (tos);
OnOffHelper onoff ("ns3::UdpSocketFactory", inet);

(for simplicity I did not add other InetSockAddress constructors, but that could
be done as well).

This patch drafts this solution (only UDP sockets are updated). I would be happy
to have a preliminary feedback on the feasibility of such an approach before
proceeding further.

Thanks!

[Tom Henderson, 2016-05-05 22:27:27]

Overall, like the idea; just think that it needs to be tied together with
existing Socket::SetIpTos ().

https://codereview.appspot.com/294280043/diff/1/src/internet/model/udp-socket...
File src/internet/model/udp-socket-impl.cc (right):

https://codereview.appspot.com/294280043/diff/1/src/internet/model/udp-socket...
src/internet/model/udp-socket-impl.cc:377: m_defaultTos = transport.GetTos ();
I like this idea, but I wonder if here (and above in Bind()) whether instead of
saving m_defaultTos, you just call Socket::SetIpTos () to this value.

https://codereview.appspot.com/294280043/diff/1/src/internet/model/udp-socket...
src/internet/model/udp-socket-impl.cc:523: if (tos)
here, I wonder about whether we should have logic such as

if (tos)
  {
    ... ipTosTag.SetTos (tos); ...
  }
else if (IsManualIpTos ())
  {
    ... ipTosTag.SetTos (GetIpTos ()); ...
  }

That is, the inclusion of the tos in the InetSocketAddress in a call to
"SendTo()" locally overrides whatever TOS was set on the socket previously.

This seems flexible in that users can set the Tos once for the socket using
Connect (InetSocketAddress) or set it on a packet-by-packet basis by using
SendTo (InetSocketAddress).

[Stefano Avallone, 2016-06-15 16:15:15]

https://codereview.appspot.com/294280043/diff/1/src/internet/model/udp-socket...
File src/internet/model/udp-socket-impl.cc (right):

https://codereview.appspot.com/294280043/diff/1/src/internet/model/udp-socket...
src/internet/model/udp-socket-impl.cc:377: m_defaultTos = transport.GetTos ();
Good idea! Done.

https://codereview.appspot.com/294280043/diff/1/src/internet/model/udp-socket...
src/internet/model/udp-socket-impl.cc:523: if (tos)
> This seems flexible in that users can set the Tos once for the socket using
> Connect (InetSocketAddress) or set it on a packet-by-packet basis by using
> SendTo (InetSocketAddress).

I think this is the current behavior: this DoSendTo function is eventually
called by both Send and SendTo. In the former case, the TOS set by Bind or
Connect is passed to this function (by DoSend). In the latter case, the TOS
included in the address passed to SendTo is passed to this function.

However, on second thought I realized that the sendto function of the C socket
API does not allow to specify a TOS value for a single packet. If we want to
adhere to Linux (and I am correct), we should ignore the Tos value in the
InetSocketAddress passed to SendTo.

What do you think? (btw, I am fine if we want to be more flexible than Linux)