openssh bugs - Dec 2011 - [Bug 1964] New: QoS/DSCP names false translated to ToS hex value

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

             Bug #: 1964
           Summary: QoS/DSCP names false translated to ToS hex value
    Classification: Unclassified
           Product: Portable OpenSSH
           Version: 5.9p1
          Platform: amd64
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ssh
        AssignedTo: unassigned-bugs at mindrot.org
        ReportedBy: bugzilla.mindrot.org at pobox.madduck.net


The ToS field (8 bit) is partitioned as follows:

                                       ToS
<---------|---------|---------|---------|--------|---------|---------|--------->
<---------|---------|---------|---------|--------|---------><--------|--------->
                            DSCP                                    ECN

The OpenSSH client currently translates DSCP classes directly into
their hex code and fills the entire ToS field with that. Instead, it
should bitshift the DSCP number by 2 bits to the left and OR it with
the ECN number.

E.g. class cs1 ("throughput", 0x08) becomes "000010|00".
When
written directly into the ToS field, that yields a DSCP of 0x02,
which is undefined.

The 0x08 should only be written into the highest 6 bits, and the ToS
field should be 0x20 afterwards.

The same applies to class cs2 ("lowdelay", 0x10), which is written
as 00010000 (DSCP class 0x04, which is also undefined), when instead
it should be 0x40.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

martin f. krafft <bugzilla.mindrot.org at pobox.madduck.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://bugs.debian.org/6505
                   |                            |21

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #1 from Damien Miller <djm at mindrot.org> 2012-02-24 11:21:26
EST ---
Created attachment 2132
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2132
ipqos-shift.diff

shift DSCP values 2 bits right to accommodate ECN flags

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |djm at mindrot.org,
                   |                            |philipp at redfish-solutions.c
                   |                            |om

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #2 from Philip Prindeville <philipp at redfish-solutions.com>
2012-02-24 15:59:03 EST ---
(In reply to comment #1)
> Created attachment 2132 [details]
> ipqos-shift.diff
> 
> shift DSCP values 2 bits right to accommodate ECN flags
The values *ARE* normalized to the TOS field being octet-aligned.

There is no shifting required.

Quoting RFC-2597:

6. Recommended Codepoints

   Recommended codepoints for the four general use AF classes are given
   below. These codepoints do not overlap with any other general use
PHB
   groups.

   The RECOMMENDED values of the AF codepoints are as follows: AF11 = '
   001010', AF12 = '001100', AF13 = '001110', AF21 =
'010010', AF22 = '
   010100', AF23 = '010110', AF31 = '011010', AF32 =
'011100', AF33 = '
   011110', AF41 = '100010', AF42 = '100100', and AF43 =
'100110'.  The
   table below summarizes the recommended AF codepoint values.

We take AF11 || 00 (i.e. shifted left 2 bits) and we get 0010100.00
which is 0x48.

Grepping through the sources, we get:

defines.h:65:# define    IPTOS_DSCP_AF21        0x48

Likewise, quoting RFC-2474, Section 2:

   Class Selector Codepoint: any of the eight codepoints in the range '
   xxx000' (where 'x' may equal '0' or '1').  Class
Selector Codepoints
   are discussed in Sec. 4.2.2.

so CS1 would be 001000.00 or 0x20, indeed grepping through the source,
we get:

defines.h:78:# define    IPTOS_CLASS_CS1        0x20

Again, THE VALUES DON'T REQUIRE A SHIFT.

There is no bug!

Please close this out as NOTABUG.

In the future, please check glibc's /usr/include/netinet/ip.h:

#define IPTOS_CLASS_MASK                0xe0
#define IPTOS_CLASS(class)              ((class) & IPTOS_CLASS_MASK)
#define IPTOS_CLASS_CS0                 0x00
#define IPTOS_CLASS_CS1                 0x20

the value for the macro "class" is meant to be iph->ip_tos, which
represents all 8 bits of the TOS octet, including the ECN bits.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #3 from Philip Prindeville <philipp at redfish-solutions.com>
2012-02-24 16:02:41 EST ---
Sigh. Typo.

That should have read:

We take AF21 || 00 (i.e. shifted left 2 bits) and we get 010010.00
which is 0x48.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #4 from martin f. krafft <bugzilla.mindrot.org at
pobox.madduck.net> 2012-02-24 18:17:26 EST ---
So it looks as if your #defines do not actually specify the hex value
of the DSCP classes, but they define the hex value shifted left by two
bits. For instance, cs1 is 0x08 in the RFC, but you store 0x20, which
is <<2.

This is all good and well and an implementation detail, really, but I
think the problem is that when someone specifies IPQoS=0x08, they
expect the same behaviour as if they specfied "cs1". However, 0x08 is
used directly without shifting, and so OpenSSH interprets it as 0x02,
which is undefined.

What is the reason that #defines specify shifted values instead of the
standard values, making the shift happen explicitly later?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #5 from Philip Prindeville <philipp at redfish-solutions.com>
2012-02-25 13:55:06 EST ---
(In reply to comment #4)
> So it looks as if your #defines do not actually specify the hex value
> of the DSCP classes, but they define the hex value shifted left by two
> bits. For instance, cs1 is 0x08 in the RFC, but you store 0x20, which
> is <<2.
They follow the notation that's present in /usr/include/netinet/ip.h
which has been present for 20 years. It's a POSIX standard.

> This is all good and well and an implementation detail, really, but I
> think the problem is that when someone specifies IPQoS=0x08, they
> expect the same behaviour as if they specfied "cs1". However,
0x08 is
> used directly without shifting, and so OpenSSH interprets it as 0x02,
> which is undefined.
Well, hopefully that person is acquainted with the BSD Sockets source
code and it will all look very familiar to him.
> What is the reason that #defines specify shifted values instead of the
> standard values, making the shift happen explicitly later?
Precedent and compatibility with existing source.

If you look at "defines.h", it says:

#include <netinet/ip.h>
...

#ifndef IPTOS_DSCP_AF11

i.e. it can get the definition from netinet/ip.h (and it's recent
enough to contain my patches to glibc 2.12). If those definitions
aren't present, then it #define's then to duplicate names and values.

The openssh sources don't do anything that isn't exactly the same as
what's done in FreeBSD, Linux, QNX, HP-UX, Solaris, etc.

I'm really not sure what the issue is here, other than we're trying too
hard to be compatible.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #6 from martin f. krafft <bugzilla.mindrot.org at
pobox.madduck.net> 2012-02-25 18:04:55 EST ---
Your argument is comprehensible.

I don't know anything about the implementation details and would not
want to questions its correctness.

I am just a user, and I noticed that a call like

  ssh -4o IPQoS=0x10 -S none lotus

yields (according to `tshark`:

  0001 00.. = Differentiated Services Codepoint: Unknown (0x04)

which is wrong. <<2 would make this right.

Maybe this is related to #1963 or #1965.

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #7 from Philip Prindeville <philipp at redfish-solutions.com>
2012-02-28 18:30:12 EST ---
Have you been able to reproduce this code that's been patched with the
fix in bug 1963?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=1964

--- Comment #8 from Philip Prindeville <philipp at redfish-solutions.com>
2012-02-28 18:39:38 EST ---
(In reply to comment #6)
> Your argument is comprehensible.
> 
> I don't know anything about the implementation details and would not
> want to questions its correctness.
> 
> I am just a user, and I noticed that a call like
> 
>   ssh -4o IPQoS=0x10 -S none lotus
> 
> yields (according to `tshark`:
> 
>   0001 00.. = Differentiated Services Codepoint: Unknown (0x04)
> 
> which is wrong. <<2 would make this right.
> 
> Maybe this is related to #1963 or #1965.
For that matter, what happens when you run:

ssh -4o IPQoS=0x08 -S none lotus

Do you also see 0x04?

-- 
Configure bugmail: https://bugzilla.mindrot.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.