Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter
From: Jamal Hadi Salim
Date: Tue Feb 09 2016 - 05:56:36 EST
On 16-02-09 03:40 AM, David Miller wrote:
From: Eric Dumazet <eric.dumazet@xxxxxxxxx>
Date: Mon, 08 Feb 2016 14:57:40 -0800
Whole point of TLV is that it allows us to add new fields at the end of
the structures.
...
Look at iproute2, you were the one adding in 2004 code to cope with
various tcp_info sizes.
So 12 years later, you cannot say it does not work anymore.
+1
The TLV L should be canonical way to determine length. i.e should be
sufficient to just look at L and understand that content has changed.
But:
Using sizeof could be dangerous unless the data is packed to be
32-bit aligned. Looking INET_DIAG_INFO check for sizeof
there is a small 8 bit hole in tcp_info I think between
these two fields:
----
__u8 tcpi_snd_wscale : 4, tcpi_rcv_wscale : 4;
__u32 tcpi_rto;
---
The kernel will pad to make sure the TLV data is 32-bit aligned.
I am not sure if that will be the same length as sizeof() in all
hardware + compilers... For this case,
it is almost safe to just add a version field - probably in the hole.
Or have a #define to say what the expected length should be. Or add
an 8 bit pad.
In general adding new fields that are non-optional is problematic. i.e
by non-optional i mean always expected to be present.
I think a good test is old kernel with new iproute2. If the new field
is non-optional, it will fail (example iproute2 may try to print a value
that it expects but because old kernel doesnt understand it; it is
non-existent).
cheers,
jamal