class of service

In ns-3-dev, we do not have a way to express class of service at the application layer so that packets are prioritized in the Wi-Fi layer.

This is a proposal for how to pass PIEEE802.1p class of service value from IP layer (IPv4 only right now) to Wi-Fi so that Wi-Fi access category can be set without Wi-Fi having to peek at the IP header (Wi-Fi does not have an internet module dependency).

This is implemented as a tag that is passed from IP to Wi-Fi; it is stripped in most legacy NetDevice types but converted to Wi-Fi QoS tag.

Thoughts on the approach in general?  We now have a somewhat clumsy way to pass TOS from socket to the device-- there is a SocketIpTosTag to get this info from the socket level to Ipv4L3Protocol, then converted to a ClassOfServiceTag to get from IPv4 to Wi-Fi, then converted internal to Wi-Fi to a QosTag.  Should we get rid of some, or all, of these tags (seems to me that QosTag could be retired if we just kept ClassOfService tag deeper into this device)?  

Tags were introduced to avoid API change between layers, but we could do it a different way (have WifiNetDevice expose a special Send() method, and use GetObject<>() at the Ipv4L3Protocol to see if the device supported that special interface).

If you don't have time to look at all files, look at ipv4-l3-protocol.cc, wifi-net-device.cc, udp-client.cc, and class-of-service-tag.{cc,h} for the main modifications and example of how it is plumbed into an application.  I also added proposed documentation on setting TOS to socket.h.

See https://codereview.appspot.com/277570043/ for an alternative implementation.

[Tommaso Pecorella, 2015-12-05 12:18:48]

https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-heade...
File src/internet/model/ipv4-header.cc (right):

https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-heade...
src/internet/model/ipv4-header.cc:113: Ipv4Header::DscpTypeToClassOfService
(DscpType dscp) const
Please add to IPv6 header as well

https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-l3-pr...
File src/internet/model/ipv4-l3-protocol.cc (right):

https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-l3-pr...
src/internet/model/ipv4-l3-protocol.cc:881: packet->AddPacketTag (cosTag);
Please add this to IPv6L3Protocol as well.

https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h
File src/network/model/socket.h (right):

https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h#ne...
src/network/model/socket.h:652: * RFC 1349 specifies the following encoding of
TOS in IPv4 headers:
RFC 1349 is obsolete, please use RFC 2474.
Moreover, 1) it's for IPv6 as well and 2) it's 6 bits only (not 7).

A slight longer point is: I know that in IPv4's header the field is still named
"TOS", but its use has been obsoleted by RFC 2474 (plus other ones). Note that
the same applies to IPv6 Traffic Class.
These functions allows to set/get the "raw" field value, but this is not really
a good idea.
We should deprecate them and use the new definition/use of these fields.

https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
File src/sixlowpan/model/sixlowpan-net-device.cc (right):

https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
src/sixlowpan/model/sixlowpan-net-device.cc:434: 
For consistency, add the above to LrWpanNetDevice as well please

[Tom Henderson, 2015-12-28 22:57:08]

https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h
File src/network/model/socket.h (right):

https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h#ne...
src/network/model/socket.h:652: * RFC 1349 specifies the following encoding of
TOS in IPv4 headers:
On 2015/12/05 12:18:48, Tommaso Pecorella wrote:
> RFC 1349 is obsolete, please use RFC 2474.
> Moreover, 1) it's for IPv6 as well and 2) it's 6 bits only (not 7).
> 
> A slight longer point is: I know that in IPv4's header the field is still
named
> "TOS", but its use has been obsoleted by RFC 2474 (plus other ones). Note that
> the same applies to IPv6 Traffic Class.
> These functions allows to set/get the "raw" field value, but this is not
really
> a good idea.
> We should deprecate them and use the new definition/use of these fields.

I can paste in the RFC 2474 definition of this field to the comment, but
unfortunately the socket API just provides the raw value.  If you want to set
DSCP directly using a DSCP value, another method will be needed, but that wasn't
the main point of my patch.

https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
File src/sixlowpan/model/sixlowpan-net-device.cc (right):

https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
src/sixlowpan/model/sixlowpan-net-device.cc:434: 
On 2015/12/05 12:18:48, Tommaso Pecorella wrote:
> For consistency, add the above to LrWpanNetDevice as well please

but IP does not (cannot?) sit on top of LrWpanNetDevice, so can it ever see this
tag?  That's why I left it out of that one.

[Tommaso Pecorella, 2015-12-29 15:23:13]

https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h
File src/network/model/socket.h (right):

https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h#ne...
src/network/model/socket.h:652: * RFC 1349 specifies the following encoding of
TOS in IPv4 headers:
The comment was more about the fact to refer to RFC 1349 instead of 2474 and the
use of 3+4 bits (they're 3+3).


On 2015/12/28 22:57:08, Tom Henderson wrote:
> On 2015/12/05 12:18:48, Tommaso Pecorella wrote:
> > RFC 1349 is obsolete, please use RFC 2474.
> > Moreover, 1) it's for IPv6 as well and 2) it's 6 bits only (not 7).
> > 
> > A slight longer point is: I know that in IPv4's header the field is still
> named
> > "TOS", but its use has been obsoleted by RFC 2474 (plus other ones). Note
that
> > the same applies to IPv6 Traffic Class.
> > These functions allows to set/get the "raw" field value, but this is not
> really
> > a good idea.
> > We should deprecate them and use the new definition/use of these fields.
> 
> I can paste in the RFC 2474 definition of this field to the comment, but
> unfortunately the socket API just provides the raw value.  If you want to set
> DSCP directly using a DSCP value, another method will be needed, but that
wasn't
> the main point of my patch.
>

https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
File src/sixlowpan/model/sixlowpan-net-device.cc (right):

https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
src/sixlowpan/model/sixlowpan-net-device.cc:434: 
On 2015/12/28 22:57:08, Tom Henderson wrote:
> On 2015/12/05 12:18:48, Tommaso Pecorella wrote:
> > For consistency, add the above to LrWpanNetDevice as well please
> 
> but IP does not (cannot?) sit on top of LrWpanNetDevice, so can it ever see
this
> tag?  That's why I left it out of that one.

True, but in another comment I did suggest to extend the patch to IPv6 as well.

[Tom Henderson, 2015-12-29 19:16:27]

On 2015/12/29 15:23:13, Tommaso Pecorella wrote:
> https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h
> File src/network/model/socket.h (right):
> 
>
https://codereview.appspot.com/277230044/diff/1/src/network/model/socket.h#ne...
> src/network/model/socket.h:652: * RFC 1349 specifies the following encoding of
> TOS in IPv4 headers:
> The comment was more about the fact to refer to RFC 1349 instead of 2474 and
the
> use of 3+4 bits (they're 3+3).

I am working on a different implementation of this feature (will post a second
code review for it) and in that one, I will fix this documentation as you
suggest (deleting the RFC1349 figure and using 2474 instead).

> 
> 
> On 2015/12/28 22:57:08, Tom Henderson wrote:
> > On 2015/12/05 12:18:48, Tommaso Pecorella wrote:
> > > RFC 1349 is obsolete, please use RFC 2474.
> > > Moreover, 1) it's for IPv6 as well and 2) it's 6 bits only (not 7).
> > > 
> > > A slight longer point is: I know that in IPv4's header the field is still
> > named
> > > "TOS", but its use has been obsoleted by RFC 2474 (plus other ones). Note
> that
> > > the same applies to IPv6 Traffic Class.
> > > These functions allows to set/get the "raw" field value, but this is not
> > really
> > > a good idea.
> > > We should deprecate them and use the new definition/use of these fields.
> > 
> > I can paste in the RFC 2474 definition of this field to the comment, but
> > unfortunately the socket API just provides the raw value.  If you want to
set
> > DSCP directly using a DSCP value, another method will be needed, but that
> wasn't
> > the main point of my patch.
> >
> 
>
https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
> File src/sixlowpan/model/sixlowpan-net-device.cc (right):
> 
>
https://codereview.appspot.com/277230044/diff/1/src/sixlowpan/model/sixlowpan...
> src/sixlowpan/model/sixlowpan-net-device.cc:434: 
> On 2015/12/28 22:57:08, Tom Henderson wrote:
> > On 2015/12/05 12:18:48, Tommaso Pecorella wrote:
> > > For consistency, add the above to LrWpanNetDevice as well please
> > 
> > but IP does not (cannot?) sit on top of LrWpanNetDevice, so can it ever see
> this
> > tag?  That's why I left it out of that one.
> 
> True, but in another comment I did suggest to extend the patch to IPv6 as
well.

yes, but SixLowPan device should remove this tag.

I don't mind to add it though if you prefer.

[Stefano Avallone, 2016-01-04 16:17:05]

The work we are doing on the introduction of a traffic control (TC) layer might
provide an alternative solution:

- before passing a packet to the queue disc, the TC layer calls
NetDevice::GetSelectedQueue (this is done to determine which queue to use, in
case of multi-queue aware queue disc)

- in the current implementation, GetSelectedQueue takes a Ptr<Packet> as input
but, considering that we do not want to add a dependency on the Internet module,
we might change it to take the DSCP, too, as input, if we assume that the
selection of a transmission queue is based on the DSCP (this is true for Wifi)

- WifiNetDevice::GetSelectedQueue implements the mapping which you put in
Ipv4Header::DscpTypeToClassOfService, stores the CoS in a ClassOfServiceTag that
attaches to the packet and returns the selected transmission queue (which is a
function of the CoS)

- when the packet is actually passed to the WifiNetDevice, the ClassOfService
tag carries the CoS which is passed to hdr.SetQosTid () and is used to determine
the transmission queue

Notes:

- the ClassOfServiceTag is only added to packets destined to a WifiNetDevice,
thus avoiding to "pollute" the other netdevices with a Wifi-specific tag. Given
that it is added and removed by WifiNetDevices only, I think it can live within
the wifi module.

- with such a ClassOfService tag, there is no need for me to introduce a tag
carrying the selected transmission queue

It is likely I am missing something, but I thought it was worth to propose such
a solution. In any case, I am in favor of removing the QosTag.

[Stefano Avallone, 2016-01-04 16:17:32]

https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-heade...
File src/internet/model/ipv4-header.cc (right):

https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-heade...
src/internet/model/ipv4-header.cc:116: switch (dscp)
Did you take this mapping from:

https://datatracker.ietf.org/doc/draft-szigeti-tsvwg-ieee-802-11e/  ?

Maybe it might be worth to mention this IETF draft.

Moreover, I think this mapping is Wifi-specific and hence it should be moved to
WifiNetDevice (see the text of my email).

https://codereview.appspot.com/277230044/diff/1/src/network/utils/class-of-se...
File src/network/utils/class-of-service-tag.h (right):

https://codereview.appspot.com/277230044/diff/1/src/network/utils/class-of-se...
src/network/utils/class-of-service-tag.h:96: #endif /* QOS_TAG_H */
s/QOS_TAG_H/CLASS_OF_SERVICE_TAG_H

[Tom Henderson, 2016-01-04 20:57:51]

On 2016/01/04 16:17:05, Stefano Avallone wrote:
> The work we are doing on the introduction of a traffic control (TC) layer
might
> provide an alternative solution:
> 
> - before passing a packet to the queue disc, the TC layer calls
> NetDevice::GetSelectedQueue (this is done to determine which queue to use, in
> case of multi-queue aware queue disc)
> 
> - in the current implementation, GetSelectedQueue takes a Ptr<Packet> as input
> but, considering that we do not want to add a dependency on the Internet
module,
> we might change it to take the DSCP, too, as input, if we assume that the
> selection of a transmission queue is based on the DSCP (this is true for Wifi)

I am not sure what you mean by avoiding internet dependency yet taking the DSCP
as an input parameter.

> 
> - WifiNetDevice::GetSelectedQueue implements the mapping which you put in
> Ipv4Header::DscpTypeToClassOfService, stores the CoS in a ClassOfServiceTag
that
> attaches to the packet and returns the selected transmission queue (which is a
> function of the CoS)
> 
> - when the packet is actually passed to the WifiNetDevice, the ClassOfService
> tag carries the CoS which is passed to hdr.SetQosTid () and is used to
determine
> the transmission queue
> 
> Notes:
> 
> - the ClassOfServiceTag is only added to packets destined to a WifiNetDevice,
> thus avoiding to "pollute" the other netdevices with a Wifi-specific tag.
Given
> that it is added and removed by WifiNetDevices only, I think it can live
within
> the wifi module.
> 
> - with such a ClassOfService tag, there is no need for me to introduce a tag
> carrying the selected transmission queue
> 
> It is likely I am missing something, but I thought it was worth to propose
such
> a solution. In any case, I am in favor of removing the QosTag.

Yes, there are interactions with the proposed new Traffic Control layer, I
agree.

I just posted an alternative implementation here:
https://codereview.appspot.com/277570043/

but I am not that fond of it after implementing it; it is a bit messy to plumb
this into the API to avoid the tag.

I am thinking lately that we could just get rid of the Wifi QoS tag and use the
class of service tag in its place, and such tag could be used also by the TC
layer.  The Class of Service tag is defined to be independent of the internet
module; it is an IEEE concept, which is why I put it into src/network module.

Basically, this amounts to deciding for a tag approach to pass this information
down from internet layer, and we should (it is not imperative, but nice to do)
modify all other net devices to at least scrub this tag if unused.

[Tom Henderson, 2016-01-04 21:00:06]

On 2016/01/04 16:17:32, Stefano Avallone wrote:
>
https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-heade...
> File src/internet/model/ipv4-header.cc (right):
> 
>
https://codereview.appspot.com/277230044/diff/1/src/internet/model/ipv4-heade...
> src/internet/model/ipv4-header.cc:116: switch (dscp)
> Did you take this mapping from:
> 
> https://datatracker.ietf.org/doc/draft-szigeti-tsvwg-ieee-802-11e/  ?
> 
> Maybe it might be worth to mention this IETF draft.

I did not know about this draft; will need to review and compare.

> 
> Moreover, I think this mapping is Wifi-specific and hence it should be moved
to
> WifiNetDevice (see the text of my email).

It is broader than wifi, it is IEEE P802.1p.  See
https://en.wikipedia.org/wiki/IEEE_P802.1p

> 
>
https://codereview.appspot.com/277230044/diff/1/src/network/utils/class-of-se...
> File src/network/utils/class-of-service-tag.h (right):
> 
>
https://codereview.appspot.com/277230044/diff/1/src/network/utils/class-of-se...
> src/network/utils/class-of-service-tag.h:96: #endif /* QOS_TAG_H */
> s/QOS_TAG_H/CLASS_OF_SERVICE_TAG_H