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

["Rodney W. Grimes" <4bone@gndrsh.dnsmgr.net>]

> > 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:

:-)  Its, blue, I tell you that bike shed must be blue!  :-)  

> 
> RT_TOSBYTE
> RT_TOS_DSFIELD
> RT_TOS_ECNFIELD

I agree that these are better names.

> 
> 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.

That does make since but my concern is this would require more than just a drop in replacement of RT_TOS as now you would need to replace the macro and probably modify other nerby code.

If these are purely kernel macros that scope of work is well contained.  Are these macro's exported from a header to userland visible code?

>  - Jonathan Morton

-- 
Rod Grimes                                                 rgrimes@freebsd.org