Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1330075

Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter

From Jamal Hadi Salim <jhs@mojatatu.com>
Newsgroups linux.kernel
Subject Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter
Date 2016-02-09 12:00 +0100
Message-ID <r0ebw-2Tw-11@gated-at.bofh.it> (permalink)
References <qZYT9-Dx-29@gated-at.bofh.it> <qZZPc-1iv-1@gated-at.bofh.it> <r02WK-3dj-9@gated-at.bofh.it> <r0c9H-1Bp-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 16-02-09 03:40 AM, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> 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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH net v2 0/4] net: add and use rx_nohandler stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 17:00 +0100
  Re: [PATCH net v2 0/4] net: add and use rx_nohandler stat counter David Miller <davem@davemloft.net> - 2016-01-30 04:40 +0100
    Re: [PATCH net v2 0/4] net: add and use rx_nohandler stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-30 19:20 +0100
  [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-01-30 19:20 +0100
    Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in  netdev_stats_to_stats64 Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-30 19:40 +0100
      Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in  netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-01-30 21:40 +0100
        Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in  netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-01-30 22:00 +0100
          Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in  netdev_stats_to_stats64 David Miller <davem@davemloft.net> - 2016-01-31 00:30 +0100
            Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in  netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-01-31 19:10 +0100
  [PATCH net v3 4/4] bond: track sum of rx_nohandler for all slaves Jarod Wilson <jarod@redhat.com> - 2016-02-02 01:00 +0100
  [PATCH net v3 0/4] net: add and use rx_nohandler stat counter Jarod Wilson <jarod@redhat.com> - 2016-02-02 01:00 +0100
    [PATCH net v3 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-02-02 01:00 +0100
    [PATCH net v3 2/4] net: add rx_nohandler stat counter Jarod Wilson <jarod@redhat.com> - 2016-02-02 01:00 +0100
      Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter Stephen Hemminger <stephen@networkplumber.org> - 2016-02-07 20:40 +0100
        Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter David Miller <davem@davemloft.net> - 2016-02-07 20:50 +0100
          Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-02-07 21:20 +0100
            Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter Jarod Wilson <jarod@redhat.com> - 2016-02-08 19:40 +0100
              Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter Stephen Hemminger <stephen@networkplumber.org> - 2016-02-08 20:40 +0100
                Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-02-09 00:00 +0100
                Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter David Miller <davem@davemloft.net> - 2016-02-09 09:50 +0100
                Re: [PATCH net v3 2/4] net: add rx_nohandler stat counter Jamal Hadi Salim <jhs@mojatatu.com> - 2016-02-09 12:00 +0100
                [PATCH net-next iproute2] iplink: display rx nohandler stats Stephen Hemminger <stephen@networkplumber.org> - 2016-02-09 20:20 +0100
                Re: [PATCH net-next iproute2] iplink: display rx nohandler stats Jarod Wilson <jarod@redhat.com> - 2016-02-10 01:00 +0100
                Re: [PATCH net-next iproute2] iplink: display rx nohandler stats Stephen Hemminger <stephen@networkplumber.org> - 2016-02-10 02:50 +0100
                Re: [PATCH net-next iproute2] iplink: display rx nohandler stats Eric Dumazet <eric.dumazet@gmail.com> - 2016-02-10 06:00 +0100
                Re: [PATCH net-next iproute2] iplink: display rx nohandler stats Jarod Wilson <jarod@redhat.com> - 2016-02-10 14:30 +0100
                Re: [PATCH net-next iproute2] iplink: display rx nohandler stats Andy Gospodarek <gospo@cumulusnetworks.com> - 2016-02-10 16:10 +0100
    [PATCH net v3 3/4] team: track sum of rx_nohandler for all slaves Jarod Wilson <jarod@redhat.com> - 2016-02-02 01:00 +0100
    Re: [PATCH net v3 0/4] net: add and use rx_nohandler stat counter David Miller <davem@davemloft.net> - 2016-02-06 09:10 +0100

csiph-web