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.
Created attachment 2132 [details] ipqos-shift.diff shift DSCP values 2 bits right to accommodate ECN flags
(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.
Sigh. Typo. That should have read: We take AF21 || 00 (i.e. shifted left 2 bits) and we get 010010.00 which is 0x48.
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?
(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.
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.
Have you been able to reproduce this code that's been patched with the fix in bug 1963?
(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?
Yes, the command ssh -4o IPQoS=0x08 -S none lotus also yields 0001 00.. = Differentiated Services Codepoint: Unknown (0x04)