Bug 2948 - SetPriority does not support value 7
SetPriority does not support value 7
Status: RESOLVED FIXED
Product: ns-3
Classification: Unclassified
Component: network
ns-3.28
All All
: P3 normal
Assigned To: ns-bugs
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-07-11 03:56 UTC by sebastien.deronne
Modified: 2018-08-15 19:02 UTC (History)
2 users (show)

See Also:


Attachments
Allow full range and fix doxygen (1.62 KB, patch)
2018-08-15 17:41 UTC, Stefano Avallone
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sebastien.deronne 2018-07-11 03:56:50 UTC
I can set priority from value 0 till 6, getting correct AC mapping in wifi.
Once I set priority to 7, wifi QoS utils will select AC_BE (priority 0) since there is no priority tag.

Why is there a limitation to value 6?
Wifi supports up to priority 7.

I suggest to allow priority settings up to value 7, as follows:

 void
 Socket::SetPriority (uint8_t priority)
 {
-  if (priority <= 6)
+  if (priority <= 7)
     {
       m_priority = priority;
     }
Comment 1 Tom Henderson 2018-07-20 23:31:56 UTC
I don't know the history of why it was limited to 6; I think your change looks fine.
Comment 2 sebastien.deronne 2018-07-29 08:34:30 UTC
(In reply to Tom Henderson from comment #1)
> I don't know the history of why it was limited to 6; I think your change
> looks fine.

Tom, thanks for your feedback.
I am planning to push this, just I need to know whether I do before or after the release.
Comment 3 sebastien.deronne 2018-08-07 06:47:58 UTC
pushed in changeset 13729:ad7ed739b5e0
Comment 4 Stefano Avallone 2018-08-11 10:58:10 UTC
The SO_PRIORITY socket option can be set to a value in the range 0 to 6 (see the socket man page). The socket priority is not directly mapped onto wifi user priority. On Linux, the wifi user priority is set to three most significant bits of the DS field (corresponding to the precedence bits according to the ToS definition), so a value in the range 0 to 7. This is exactly what happens with ns-3. Indeed, the traffic control layer calls WifiNetDevice::SelectQueue () before enqueuing packets in the queue disc. WifiNetDevice::SelectQueue () replaces the priority with the three most significant bits of the DS field.

You can use the test/ns3wifi/wifi-ac-mapping-test-suite.cc to verify what I am saying. Packets marked with PHB CS7 are correctly enqueued into VO AC queue (I think you can modify the test or enable some log messages to check that the wifi user priority is 7).

Of course, alternative strategies can be defined to set the wifi user priority. I am working on some changes to the traffic control layer that will make it easier to support different mappings (expect a message on the mailing list soon...)

In the meantime, can we please revert this commit?
Comment 5 sebastien.deronne 2018-08-14 10:20:04 UTC
Stefano, I can revert, but I have the impression we cannot open a packet socket that will map to a priority 7. 

If I am correct, there is no example using a packet socket (I do not want to use TCP/IP stack in my case). In Linux, It worked fine by setting skb priority to 7.
Comment 6 Stefano Avallone 2018-08-15 07:09:58 UTC
(In reply to sebastien.deronne from comment #5)
> Stefano, I can revert, but I have the impression we cannot open a packet
> socket that will map to a priority 7. 

Yes, you are correct. I did not think of the packet socket case. The thing is that setting a priority outside the range 0..6 requires to set the CAP_NET_ADMIN capability on Linux. So, I thought to implement in ns-3 the case where this capability is not available. Of course, I am open to make changes here. Do we want to simulate the availability of the CAP_NET_ADMIN capability by default? Or do we want to simulate the setting of such capability? 
 
> If I am correct, there is no example using a packet socket (I do not want to
> use TCP/IP stack in my case). In Linux, It worked fine by setting skb
> priority to 7.

Did you set the CAP_NET_ADMIN capability for your process? How did you set the skb priority to 7? I am interested to understand why it worked in your case.
Comment 7 sebastien.deronne 2018-08-15 08:15:36 UTC
(In reply to Stefano Avallone from comment #6)
> (In reply to sebastien.deronne from comment #5)
> > Stefano, I can revert, but I have the impression we cannot open a packet
> > socket that will map to a priority 7. 
> 
> Yes, you are correct. I did not think of the packet socket case. The thing
> is that setting a priority outside the range 0..6 requires to set the
> CAP_NET_ADMIN capability on Linux. So, I thought to implement in ns-3 the
> case where this capability is not available. Of course, I am open to make
> changes here. Do we want to simulate the availability of the CAP_NET_ADMIN
> capability by default? Or do we want to simulate the setting of such
> capability? 
>  
> > If I am correct, there is no example using a packet socket (I do not want to
> > use TCP/IP stack in my case). In Linux, It worked fine by setting skb
> > priority to 7.
> 
> Did you set the CAP_NET_ADMIN capability for your process? How did you set
> the skb priority to 7? I am interested to understand why it worked in your
> case.

Maybe my case is a bit different since I generate the packet in the Linux kernel, not in the user space. I simply do something like that:

...
struct sk_buff *skb;
memset(skb_put(skb, total_size), 0, total_size);
skb->dev = net_dev;
skb->len = total_size;
skb->priority = 7;
...

Then the sk_buff is sent to mac80211.

I honestly have no idea about CAP_NET_ADMIN.
Comment 8 Tom Henderson 2018-08-15 09:41:40 UTC
My sense is to allow [0,7] but add a note to Doxygen such as:

  /**
   * \brief Manually set the socket priority
   *
   * This method corresponds to using setsockopt () SO_PRIORITY of
   * real network or BSD sockets.
   *
   * With Linux IP sockets, the typical operational range is [0,6] and
   * the value 7 requires administrator privileges (see the man page
   * for socket).  ns-3 allows the full range because users may want
   * to use packet sockets and have access to the full range.
   *
   * \param priority The socket priority (in the range [0,7])
   */
Comment 9 Stefano Avallone 2018-08-15 13:16:40 UTC
> Maybe my case is a bit different since I generate the packet in the Linux
> kernel, not in the user space. I simply do something like that:
> 
> ...
> struct sk_buff *skb;
> memset(skb_put(skb, total_size), 0, total_size);
> skb->dev = net_dev;
> skb->len = total_size;
> skb->priority = 7;
> ...
> 
> Then the sk_buff is sent to mac80211.

Ok, you are setting skb->priority directly, so I expect it to work. My understanding is that user processes instead cannot set the priority to 7, unless the process has the requested capability.
Comment 10 Stefano Avallone 2018-08-15 13:21:01 UTC
(In reply to Tom Henderson from comment #8)
> My sense is to allow [0,7] but add a note to Doxygen such as:
> 
>   /**
>    * \brief Manually set the socket priority
>    *
>    * This method corresponds to using setsockopt () SO_PRIORITY of
>    * real network or BSD sockets.
>    *
>    * With Linux IP sockets, the typical operational range is [0,6] and
>    * the value 7 requires administrator privileges (see the man page
>    * for socket).  ns-3 allows the full range because users may want
>    * to use packet sockets and have access to the full range.
>    *
>    * \param priority The socket priority (in the range [0,7])
>    */

This is basically what I was going to suggest. The only difference is that I would allow any value in the range [0..255], which is what Linux permits to processes with the CAP_NET_ADMIN capability. If you agree, I will post a possible patch.
Comment 11 Tom Henderson 2018-08-15 14:28:58 UTC
(In reply to Stefano Avallone from comment #10)
> (In reply to Tom Henderson from comment #8)
> > My sense is to allow [0,7] but add a note to Doxygen such as:
> > 
> >   /**
> >    * \brief Manually set the socket priority
> >    *
> >    * This method corresponds to using setsockopt () SO_PRIORITY of
> >    * real network or BSD sockets.
> >    *
> >    * With Linux IP sockets, the typical operational range is [0,6] and
> >    * the value 7 requires administrator privileges (see the man page
> >    * for socket).  ns-3 allows the full range because users may want
> >    * to use packet sockets and have access to the full range.
> >    *
> >    * \param priority The socket priority (in the range [0,7])
> >    */
> 
> This is basically what I was going to suggest. The only difference is that I
> would allow any value in the range [0..255], which is what Linux permits to
> processes with the CAP_NET_ADMIN capability. If you agree, I will post a
> possible patch.

OK for me if you just commit directly along these lines.
Comment 12 Stefano Avallone 2018-08-15 17:41:13 UTC
Created attachment 3158 [details]
Allow full range and fix doxygen

Please find attached a git commit that allows users to set the socket priority to any 8-bit non-negative value and fixes doxygen. Please feel free to amend and push to ns-3-dev, thanks.
Comment 13 Tom Henderson 2018-08-15 19:02:37 UTC
latest patch pushed in changeset 13746:fa0765852e89