Re: [RFC iproute2] tos: interpret ToS in natural numeral system

On Wed, Feb 16, 2022 at 10:44:19PM +0000, David Laight wrote:

From: Guillaume Nault

quoted

Sent: 16 February 2022 22:24

On Wed, Feb 16, 2022 at 11:42:05AM -0800, Jakub Kicinski wrote:

quoted

Silently forcing a base numeral system is very painful for users.
ip currently interprets tos 10 as 0x10. Imagine user's bash script
does:

  .. tos $((TOS * 2)) ..

or any numerical operation on the ToS.

This patch breaks existing scripts if they expect 10 to be 0x10.

I agree that we shouldn't have forced base 16 in the first place.
But after so many years I find it a bit dangerous to change that.

Aren't the TOS values made up of several multi-bit fields and
very likely to be documented in hex?

In theory, they are. But as far as the kernel is concerned, they're
just plain integers with no significance (apart from some constraints
preventing the use of some values). Users are free to choose their own
values.

I'm not sure $((TOS * 2)) (or even + 2) makes any sense at all.

What it more horrid that that base 0 treats numbers that start
with a 0 as octal - has anyone really used octal since the 1970s
(except for file permissions).

Right, but that'd be consistent with the rest of iproute2, so users
should be aware of this trap at this point (or most likely, they never
prefix their values with 0). Anyway, I think we agreed that it's now
too late to modify the base.

I have written command line parsers that treat 0tnnn as decimal
while defaulting to hex.
That does make it easier to use shell arithmetic for field (like
addresses) that you would never normally specify in decimal.

quoted

What about just printing a warning when the value isn't prefixed with
'0x'? Something like (completely untested):

@@ -535,6 +535,12 @@ int rtnl_dsfield_a2n(__u32 *id, const char *arg)
 	if (!end || end == arg || *end || res > 255)
 		return -1;
 	*id = res;
+
+	if (strncmp("0x", arg, 2))
+		fprintf(stderr,
+			"Warning: dsfield and tos parameters are interpreted as hexadecimal values\n"
+			"Use 'dsfield 0x%02x' to avoid this message\n", res);

Ugg.

Not nice, I agree. But what else can we do without breaking backward
compatibility?
This is similar to the warning we have when creating a new vxlan device
without specifying the destination port:

  # ip link add type vxlan vni 200 remote 2001:db8::1
  vxlan: destination port not specified
  Will use Linux kernel default (non-standard value)
  Use 'dstport 4789' to get the IANA assigned value
  Use 'dstport 0' to get default and quiet this message

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)