Re: [Ecn-sane] Fwd: [PATCH net] ipv4: Ignore ECN bits for fib lookups in fib_compute_spec_dst()

[Jonathan Morton <chromatix99@gmail.com>]

> On 29 Dec, 2020, at 3:33 pm, Rodney W. Grimes <4bone@gndrsh.dnsmgr.net> wrote:
> 
> A better fix might be to create some new macros, make the
> old macro emmit a compile time warning to weed out the
> bad use cases:
> a)  Modify RT_TOS to assert a compile time warning to
>    flag old usage.
> 
> b)  create RT_TOSBYTE, this returns the whole byte, used
>    to replace RT_TOS when the whole byte is needed and
>    dealt with properly.
> 
> c)  create RT_TOSTOSBITS, this returns ONLY the TOS bits,
>    and probably should replace most of the current calls
>    to RT_TOS with this, after inspecting the code to
>    make sure it does the right thing.
> 
> d)  create RT_TOSECNBITS, this returns ONLY the ECN bits

Agree with the principle here, quibble with the terminology.  Better names for these new macros:

RT_TOSBYTE
RT_TOS_DSFIELD
RT_TOS_ECNFIELD

This emphasises that the "TOS bits" and Precedence field have been absorbed into the Diffserv field for the past 20 years already, and that generally the talk is nowadays of a "field" or a "flag" rather than "bits".

I would seriously consider also making the DSFIELD macro return a right-justified value, matching most Diffserv documentation tables, rather than just masking out the left-justified position it occupies within the TOS byte.  This requires a second macro to correctly insert a fresh right-justified value into the left-justified position, ideally without disturbing any ECN field value already present.  There is some evidence that mishandling this is a significant source of bugs in user code, if not also in kernel code.

 - Jonathan Morton