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

Groups > linux.kernel > #1320824 > unrolled thread

[PATCH net v2 0/4] net: add and use rx_nohandler stat counter

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

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

Page 1 of 2  [1] 2  Next page →

#1320824 — [PATCH net v2 0/4] net: add and use rx_nohandler stat counter

The network core tries to keep track of dropped packets, but some packets
you wouldn't really call dropped, so much as intentionally ignored, under
certain circumstances. One such case is that of bonding and team device
slaves that are currently inactive. Their respective rx_handler functions
return RX_HANDLER_EXACT (the only places in the kernel that return that),
which ends up tracking into the network core's __netif_receive_skb_core()
function's drop path, with no pt_prev set. On a noisy network, this can
result in a very rapidly incrementing rx_dropped counter, not only on the
inactive slave(s), but also on the master device, such as the following:

$ cat /proc/net/dev
Inter-|   Receive                                                |  Transmit
 face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
  p7p1: 14783346  140430    0 140428    0     0          0      2040      680       8    0    0    0     0       0          0
  p7p2: 14805198  140648    0    0    0     0          0      2034        0       0    0    0    0     0       0          0
 bond0: 53365248  532798    0 421160    0     0          0    115151     2040      24    0    0    0     0       0          0
    lo:    5420      54    0    0    0     0          0         0     5420      54    0    0    0     0       0          0
  p5p1: 19292195  196197    0 140368    0     0          0     56564      680       8    0    0    0     0       0          0
  p5p2: 19289707  196171    0 140364    0     0          0     56547      680       8    0    0    0     0       0          0
   em3: 20996626  158214    0    0    0     0          0       383        0       0    0    0    0     0       0          0
   em2: 14065122  138462    0    0    0     0          0       310        0       0    0    0    0     0       0          0
   em1: 14063162  138440    0    0    0     0          0       308        0       0    0    0    0     0       0          0
   em4: 21050830  158729    0    0    0     0          0       385    71662     469    0    0    0     0       0          0
   ib0:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0

In this scenario, p5p1, p5p2 and p7p1 are all inactive slaves in an
active-backup bond0, and you can see that all three have high drop counts,
with the master bond0 showing a tally of all three.

I know that this was previously discussed some here:

    http://www.spinics.net/lists/netdev/msg226341.html

It seems additional counters never came to fruition, so this is a first
attempt at creating one of them, so that we stop calling these drops,
which for users monitoring rx_dropped, causes great alarm, and renders the
counter much less useful for them.

This adds a sysfs statistics node and makes the counter available via
netlink.

Additionally, I'm not certain if this set qualifies for net, or if it
should be put aside and resubmitted for net-next after 4.5 is put to
bed, but I do have users who consider this an important bugfix.

Jarod Wilson (4):
  net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
  net: add rx_nohandler stat counter
  team: track sum of rx_nohandler for all slaves
  bond: track sum of rx_nohandler for all slaves

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@mellanox.com>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Tom Herbert <tom@herbertland.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <gospo@cumulusnetworks.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>

 drivers/net/bonding/bond_main.c |  1 +
 drivers/net/team/team.c         | 10 +++++++---
 include/linux/if_team.h         |  1 +
 include/linux/netdevice.h       |  3 +++
 include/uapi/linux/if_link.h    |  4 ++++
 net/core/dev.c                  | 19 ++++++++++++++-----
 net/core/net-sysfs.c            |  2 ++
 net/core/rtnetlink.c            |  2 ++
 8 files changed, 34 insertions(+), 8 deletions(-)

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1322250

From: Jarod Wilson <jarod@redhat.com>
Date: Thu, 28 Jan 2016 10:49:44 -0500

> The network core tries to keep track of dropped packets, but some packets
> you wouldn't really call dropped, so much as intentionally ignored, under
> certain circumstances. One such case is that of bonding and team device
> slaves that are currently inactive. Their respective rx_handler functions
> return RX_HANDLER_EXACT (the only places in the kernel that return that),
> which ends up tracking into the network core's __netif_receive_skb_core()
> function's drop path, with no pt_prev set. On a noisy network, this can
> result in a very rapidly incrementing rx_dropped counter, not only on the
> inactive slave(s), but also on the master device, such as the following:
 ...

Both my inbox and patchwork only show patch 2, 3, and 4.  Where is #1?

Thanks.

[toc] | [prev] | [next] | [standalone]

#1322480

On Fri, Jan 29, 2016 at 07:37:51PM -0800, David Miller wrote:
> From: Jarod Wilson <jarod@redhat.com>
> Date: Thu, 28 Jan 2016 10:49:44 -0500
> 
> > The network core tries to keep track of dropped packets, but some packets
> > you wouldn't really call dropped, so much as intentionally ignored, under
> > certain circumstances. One such case is that of bonding and team device
> > slaves that are currently inactive. Their respective rx_handler functions
> > return RX_HANDLER_EXACT (the only places in the kernel that return that),
> > which ends up tracking into the network core's __netif_receive_skb_core()
> > function's drop path, with no pt_prev set. On a noisy network, this can
> > result in a very rapidly incrementing rx_dropped counter, not only on the
> > inactive slave(s), but also on the master device, such as the following:
>  ...
> 
> Both my inbox and patchwork only show patch 2, 3, and 4.  Where is #1?

Crap. It seems I fat-fingered netdev@vger.kernel.org without the v in
front of ger. Will re-send in a sec, hopefully finallly not screwing it up
this time.

-- 
Jarod Wilson
jarod@redhat.com

[toc] | [prev] | [next] | [standalone]

#1322479 — [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

The netdev_stats_to_stats64 function copies the deprecated
net_device_stats format stats into rtnl_link_stats64 for legacy support
purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
extend rtnl_link_stats64 without also extending net_device_stats. Relax
the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
zero out all the stat counters that aren't present in net_device_stats.

CC: Eric Dumazet <edumazet@google.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
Re-re-sending, hopefully getting the patch to the right list this time.

 net/core/dev.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8cba3d8..575a7df 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7253,25 +7253,30 @@ void netdev_run_todo(void)
 	}
 }
 
-/* Convert net_device_stats to rtnl_link_stats64.  They have the same
- * fields in the same order, with only the type differing.
+/* Convert net_device_stats to rtnl_link_stats64. rtnl_link_stats64 has
+ * all the same fields in the same order as net_device_stats, with only
+ * the type differing, but rtnl_link_stats64 may have additional fields
+ * at the end for newer counters.
  */
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 			     const struct net_device_stats *netdev_stats)
 {
 #if BITS_PER_LONG == 64
-	BUILD_BUG_ON(sizeof(*stats64) != sizeof(*netdev_stats));
+	BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
 	memcpy(stats64, netdev_stats, sizeof(*stats64));
 #else
 	size_t i, n = sizeof(*stats64) / sizeof(u64);
 	const unsigned long *src = (const unsigned long *)netdev_stats;
 	u64 *dst = (u64 *)stats64;
 
-	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) !=
+	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) >
 		     sizeof(*stats64) / sizeof(u64));
 	for (i = 0; i < n; i++)
 		dst[i] = src[i];
 #endif
+	/* zero out counters that only exist in rtnl_link_stats64 */
+	memset((char *)stats64 + sizeof(*netdev_stats), 0,
+	       sizeof(*stats64) - sizeof(*netdev_stats));
 }
 EXPORT_SYMBOL(netdev_stats_to_stats64);
 
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1322485 — Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

On Sat, 2016-01-30 at 13:19 -0500, Jarod Wilson wrote:
> The netdev_stats_to_stats64 function copies the deprecated
> net_device_stats format stats into rtnl_link_stats64 for legacy support
> purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
> extend rtnl_link_stats64 without also extending net_device_stats. Relax
> the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
> zero out all the stat counters that aren't present in net_device_stats.
> 
> CC: Eric Dumazet <edumazet@google.com>
> CC: netdev@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
> Re-re-sending, hopefully getting the patch to the right list this time.
> 
>  net/core/dev.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 8cba3d8..575a7df 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7253,25 +7253,30 @@ void netdev_run_todo(void)
>  	}
>  }
>  
> -/* Convert net_device_stats to rtnl_link_stats64.  They have the same
> - * fields in the same order, with only the type differing.
> +/* Convert net_device_stats to rtnl_link_stats64. rtnl_link_stats64 has
> + * all the same fields in the same order as net_device_stats, with only
> + * the type differing, but rtnl_link_stats64 may have additional fields
> + * at the end for newer counters.
>   */
>  void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
>  			     const struct net_device_stats *netdev_stats)
>  {
>  #if BITS_PER_LONG == 64
> -	BUILD_BUG_ON(sizeof(*stats64) != sizeof(*netdev_stats));
> +	BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
>  	memcpy(stats64, netdev_stats, sizeof(*stats64));
>  #else
>  	size_t i, n = sizeof(*stats64) / sizeof(u64);
>  	const unsigned long *src = (const unsigned long *)netdev_stats;
>  	u64 *dst = (u64 *)stats64;
>  
> -	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) !=
> +	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) >
>  		     sizeof(*stats64) / sizeof(u64));
>  	for (i = 0; i < n; i++)
>  		dst[i] = src[i];
>  #endif
> +	/* zero out counters that only exist in rtnl_link_stats64 */
> +	memset((char *)stats64 + sizeof(*netdev_stats), 0,
> +	       sizeof(*stats64) - sizeof(*netdev_stats));

Are you sure it works on 32bit arches ?

[toc] | [prev] | [next] | [standalone]

#1322504 — Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

On Sat, Jan 30, 2016 at 10:34:32AM -0800, Eric Dumazet wrote:
> On Sat, 2016-01-30 at 13:19 -0500, Jarod Wilson wrote:
> > The netdev_stats_to_stats64 function copies the deprecated
> > net_device_stats format stats into rtnl_link_stats64 for legacy support
> > purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
> > extend rtnl_link_stats64 without also extending net_device_stats. Relax
> > the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
> > zero out all the stat counters that aren't present in net_device_stats.
> > 
> > CC: Eric Dumazet <edumazet@google.com>
> > CC: netdev@vger.kernel.org
> > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > ---
> > Re-re-sending, hopefully getting the patch to the right list this time.
> > 
> >  net/core/dev.c | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 8cba3d8..575a7df 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -7253,25 +7253,30 @@ void netdev_run_todo(void)
> >  	}
> >  }
> >  
> > -/* Convert net_device_stats to rtnl_link_stats64.  They have the same
> > - * fields in the same order, with only the type differing.
> > +/* Convert net_device_stats to rtnl_link_stats64. rtnl_link_stats64 has
> > + * all the same fields in the same order as net_device_stats, with only
> > + * the type differing, but rtnl_link_stats64 may have additional fields
> > + * at the end for newer counters.
> >   */
> >  void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
> >  			     const struct net_device_stats *netdev_stats)
> >  {
> >  #if BITS_PER_LONG == 64
> > -	BUILD_BUG_ON(sizeof(*stats64) != sizeof(*netdev_stats));
> > +	BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
> >  	memcpy(stats64, netdev_stats, sizeof(*stats64));
> >  #else
> >  	size_t i, n = sizeof(*stats64) / sizeof(u64);
> >  	const unsigned long *src = (const unsigned long *)netdev_stats;
> >  	u64 *dst = (u64 *)stats64;
> >  
> > -	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) !=
> > +	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) >
> >  		     sizeof(*stats64) / sizeof(u64));
> >  	for (i = 0; i < n; i++)
> >  		dst[i] = src[i];
> >  #endif
> > +	/* zero out counters that only exist in rtnl_link_stats64 */
> > +	memset((char *)stats64 + sizeof(*netdev_stats), 0,
> > +	       sizeof(*stats64) - sizeof(*netdev_stats));
> 
> Are you sure it works on 32bit arches ?

Ew, no, it won't work correctly on 32-bit. The for loop is going to copy
data into dst from beyond the end of netdev_stats, and the range looks
like it won't be right either, only half of the added stats64 space will
get zeroed out. Okay, I'll fix that up correctly.

-- 
Jarod Wilson
jarod@redhat.com

[toc] | [prev] | [next] | [standalone]

#1322509 — Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

On Sat, Jan 30, 2016 at 03:39:01PM -0500, Jarod Wilson wrote:
> On Sat, Jan 30, 2016 at 10:34:32AM -0800, Eric Dumazet wrote:
> > On Sat, 2016-01-30 at 13:19 -0500, Jarod Wilson wrote:
> > > The netdev_stats_to_stats64 function copies the deprecated
> > > net_device_stats format stats into rtnl_link_stats64 for legacy support
> > > purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
> > > extend rtnl_link_stats64 without also extending net_device_stats. Relax
> > > the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
> > > zero out all the stat counters that aren't present in net_device_stats.
> > > 
> > > CC: Eric Dumazet <edumazet@google.com>
> > > CC: netdev@vger.kernel.org
> > > Signed-off-by: Jarod Wilson <jarod@redhat.com>
> > > ---
> > > Re-re-sending, hopefully getting the patch to the right list this time.
> > > 
> > >  net/core/dev.c | 13 +++++++++----
> > >  1 file changed, 9 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 8cba3d8..575a7df 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -7253,25 +7253,30 @@ void netdev_run_todo(void)
> > >  	}
> > >  }
> > >  
> > > -/* Convert net_device_stats to rtnl_link_stats64.  They have the same
> > > - * fields in the same order, with only the type differing.
> > > +/* Convert net_device_stats to rtnl_link_stats64. rtnl_link_stats64 has
> > > + * all the same fields in the same order as net_device_stats, with only
> > > + * the type differing, but rtnl_link_stats64 may have additional fields
> > > + * at the end for newer counters.
> > >   */
> > >  void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
> > >  			     const struct net_device_stats *netdev_stats)
> > >  {
> > >  #if BITS_PER_LONG == 64
> > > -	BUILD_BUG_ON(sizeof(*stats64) != sizeof(*netdev_stats));
> > > +	BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
> > >  	memcpy(stats64, netdev_stats, sizeof(*stats64));
> > >  #else
> > >  	size_t i, n = sizeof(*stats64) / sizeof(u64);
> > >  	const unsigned long *src = (const unsigned long *)netdev_stats;
> > >  	u64 *dst = (u64 *)stats64;
> > >  
> > > -	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) !=
> > > +	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) >
> > >  		     sizeof(*stats64) / sizeof(u64));
> > >  	for (i = 0; i < n; i++)
> > >  		dst[i] = src[i];
> > >  #endif
> > > +	/* zero out counters that only exist in rtnl_link_stats64 */
> > > +	memset((char *)stats64 + sizeof(*netdev_stats), 0,
> > > +	       sizeof(*stats64) - sizeof(*netdev_stats));
> > 
> > Are you sure it works on 32bit arches ?
> 
> Ew, no, it won't work correctly on 32-bit. The for loop is going to copy
> data into dst from beyond the end of netdev_stats, and the range looks
> like it won't be right either, only half of the added stats64 space will
> get zeroed out. Okay, I'll fix that up correctly.

Completely untested as of yet, but I think something like the following
looks correct. I'll give it a spin as soon as I can.

diff --git a/net/core/dev.c b/net/core/dev.c
index 8cba3d8..65863e5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7253,24 +7253,31 @@ void netdev_run_todo(void)
 	}
 }
 
-/* Convert net_device_stats to rtnl_link_stats64.  They have the same
- * fields in the same order, with only the type differing.
+/* Convert net_device_stats to rtnl_link_stats64. rtnl_link_stats64 has
+ * all the same fields in the same order as net_device_stats, with only
+ * the type differing, but rtnl_link_stats64 may have additional fields
+ * at the end for newer counters.
  */
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 			     const struct net_device_stats *netdev_stats)
 {
 #if BITS_PER_LONG == 64
-	BUILD_BUG_ON(sizeof(*stats64) != sizeof(*netdev_stats));
+	BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
 	memcpy(stats64, netdev_stats, sizeof(*stats64));
+	/* zero out counters that only exist in rtnl_link_stats64 */
+	memset((char *)stats64 + sizeof(*netdev_stats), 0,
+	       sizeof(*stats64) - sizeof(*netdev_stats));
 #else
-	size_t i, n = sizeof(*stats64) / sizeof(u64);
+	size_t i, n = sizeof(*netdev_stats) / sizeof(unsigned long);
 	const unsigned long *src = (const unsigned long *)netdev_stats;
 	u64 *dst = (u64 *)stats64;
 
-	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) !=
-		     sizeof(*stats64) / sizeof(u64));
+	BUILD_BUG_ON(n > sizeof(*stats64) / sizeof(u64));
 	for (i = 0; i < n; i++)
 		dst[i] = src[i];
+	/* zero out counters that only exist in rtnl_link_stats64 */
+	memset((char *)stats64 + n * sizeof(u64), 0,
+	       sizeof(*stats64) - n * sizeof(u64));
 #endif
 }
 EXPORT_SYMBOL(netdev_stats_to_stats64);

-- 
Jarod Wilson
jarod@redhat.com

[toc] | [prev] | [next] | [standalone]

#1322524 — Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

From: Jarod Wilson <jarod@redhat.com>
Date: Sat, 30 Jan 2016 15:53:05 -0500

> On Sat, Jan 30, 2016 at 03:39:01PM -0500, Jarod Wilson wrote:
>> Ew, no, it won't work correctly on 32-bit. The for loop is going to copy
>> data into dst from beyond the end of netdev_stats, and the range looks
>> like it won't be right either, only half of the added stats64 space will
>> get zeroed out. Okay, I'll fix that up correctly.
> 
> Completely untested as of yet, but I think something like the following
> looks correct. I'll give it a spin as soon as I can.

Jarod, please respin your entire series as a v3 once you sort this out.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1322699 — Re: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

On Sat, Jan 30, 2016 at 03:26:35PM -0800, David Miller wrote:
> From: Jarod Wilson <jarod@redhat.com>
> Date: Sat, 30 Jan 2016 15:53:05 -0500
> 
> > On Sat, Jan 30, 2016 at 03:39:01PM -0500, Jarod Wilson wrote:
> >> Ew, no, it won't work correctly on 32-bit. The for loop is going to copy
> >> data into dst from beyond the end of netdev_stats, and the range looks
> >> like it won't be right either, only half of the added stats64 space will
> >> get zeroed out. Okay, I'll fix that up correctly.
> > 
> > Completely untested as of yet, but I think something like the following
> > looks correct. I'll give it a spin as soon as I can.
> 
> Jarod, please respin your entire series as a v3 once you sort this out.

Will do. I should be able to get it tested out on a 32-bit setup Monday.

-- 
Jarod Wilson
jarod@redhat.com

[toc] | [prev] | [next] | [standalone]

#1323631 — [PATCH net v3 4/4] bond: track sum of rx_nohandler for all slaves

Sample output with this set applied for an active-backup bond:

$ cat /sys/devices/virtual/net/bond0/lower_p7p1/statistics/rx_nohandler
16568
$ cat /sys/devices/virtual/net/bond0/lower_p5p2/statistics/rx_nohandler
16583
$ cat /sys/devices/virtual/net/bond0/statistics/rx_nohandler
33151

CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <gospo@cumulusnetworks.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 56b5605..6587929 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3309,6 +3309,7 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
 		stats->rx_bytes += sstats->rx_bytes - pstats->rx_bytes;
 		stats->rx_errors += sstats->rx_errors - pstats->rx_errors;
 		stats->rx_dropped += sstats->rx_dropped - pstats->rx_dropped;
+		stats->rx_nohandler += sstats->rx_nohandler - pstats->rx_nohandler;
 
 		stats->tx_packets += sstats->tx_packets - pstats->tx_packets;;
 		stats->tx_bytes += sstats->tx_bytes - pstats->tx_bytes;
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1323632 — [PATCH net v3 0/4] net: add and use rx_nohandler stat counter

The network core tries to keep track of dropped packets, but some packets
you wouldn't really call dropped, so much as intentionally ignored, under
certain circumstances. One such case is that of bonding and team device
slaves that are currently inactive. Their respective rx_handler functions
return RX_HANDLER_EXACT (the only places in the kernel that return that),
which ends up tracking into the network core's __netif_receive_skb_core()
function's drop path, with no pt_prev set. On a noisy network, this can
result in a very rapidly incrementing rx_dropped counter, not only on the
inactive slave(s), but also on the master device, such as the following:

$ cat /proc/net/dev
Inter-|   Receive                                                |  Transmit
 face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
  p7p1: 14783346  140430    0 140428    0     0          0      2040      680       8    0    0    0     0       0          0
  p7p2: 14805198  140648    0    0    0     0          0      2034        0       0    0    0    0     0       0          0
 bond0: 53365248  532798    0 421160    0     0          0    115151     2040      24    0    0    0     0       0          0
    lo:    5420      54    0    0    0     0          0         0     5420      54    0    0    0     0       0          0
  p5p1: 19292195  196197    0 140368    0     0          0     56564      680       8    0    0    0     0       0          0
  p5p2: 19289707  196171    0 140364    0     0          0     56547      680       8    0    0    0     0       0          0
   em3: 20996626  158214    0    0    0     0          0       383        0       0    0    0    0     0       0          0
   em2: 14065122  138462    0    0    0     0          0       310        0       0    0    0    0     0       0          0
   em1: 14063162  138440    0    0    0     0          0       308        0       0    0    0    0     0       0          0
   em4: 21050830  158729    0    0    0     0          0       385    71662     469    0    0    0     0       0          0
   ib0:       0       0    0    0    0     0          0         0        0       0    0    0    0     0       0          0

In this scenario, p5p1, p5p2 and p7p1 are all inactive slaves in an
active-backup bond0, and you can see that all three have high drop counts,
with the master bond0 showing a tally of all three.

I know that this was previously discussed some here:

    http://www.spinics.net/lists/netdev/msg226341.html

It seems additional counters never came to fruition, so this is a first
attempt at creating one of them, so that we stop calling these drops,
which for users monitoring rx_dropped, causes great alarm, and renders the
counter much less useful for them.

This adds a sysfs statistics node and makes the counter available via
netlink.

Additionally, I'm not certain if this set qualifies for net, or if it
should be put aside and resubmitted for net-next after 4.5 is put to
bed, but I do have users who consider this an important bugfix.

This has been tested quite a bit on x86_64, and now lightly on i686 as
well, to verify functionality of updates to netdev_stats_to_stats64()
on 32-bit arches.

Jarod Wilson (4):
  net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
  net: add rx_nohandler stat counter
  team: track sum of rx_nohandler for all slaves
  bond: track sum of rx_nohandler for all slaves

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@mellanox.com>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Tom Herbert <tom@herbertland.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <gospo@cumulusnetworks.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>

 drivers/net/bonding/bond_main.c |  1 +
 drivers/net/team/team.c         | 10 +++++++---
 include/linux/if_team.h         |  1 +
 include/linux/netdevice.h       |  3 +++
 include/uapi/linux/if_link.h    |  4 ++++
 net/core/dev.c                  | 25 ++++++++++++++++++-------
 net/core/net-sysfs.c            |  2 ++
 net/core/rtnetlink.c            |  2 ++
 8 files changed, 38 insertions(+), 10 deletions(-)

-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1323633 — [PATCH net v3 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64

The netdev_stats_to_stats64 function copies the deprecated
net_device_stats format stats into rtnl_link_stats64 for legacy support
purposes, but with the BUILD_BUG_ON as it was, it wasn't possible to
extend rtnl_link_stats64 without also extending net_device_stats. Relax
the BUILD_BUG_ON to only require that rtnl_link_stats64 is larger, and
zero out all the stat counters that aren't present in net_device_stats.

CC: Eric Dumazet <edumazet@google.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 net/core/dev.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 8cba3d8..65863e5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7253,24 +7253,31 @@ void netdev_run_todo(void)
 	}
 }
 
-/* Convert net_device_stats to rtnl_link_stats64.  They have the same
- * fields in the same order, with only the type differing.
+/* Convert net_device_stats to rtnl_link_stats64. rtnl_link_stats64 has
+ * all the same fields in the same order as net_device_stats, with only
+ * the type differing, but rtnl_link_stats64 may have additional fields
+ * at the end for newer counters.
  */
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 			     const struct net_device_stats *netdev_stats)
 {
 #if BITS_PER_LONG == 64
-	BUILD_BUG_ON(sizeof(*stats64) != sizeof(*netdev_stats));
+	BUILD_BUG_ON(sizeof(*stats64) < sizeof(*netdev_stats));
 	memcpy(stats64, netdev_stats, sizeof(*stats64));
+	/* zero out counters that only exist in rtnl_link_stats64 */
+	memset((char *)stats64 + sizeof(*netdev_stats), 0,
+	       sizeof(*stats64) - sizeof(*netdev_stats));
 #else
-	size_t i, n = sizeof(*stats64) / sizeof(u64);
+	size_t i, n = sizeof(*netdev_stats) / sizeof(unsigned long);
 	const unsigned long *src = (const unsigned long *)netdev_stats;
 	u64 *dst = (u64 *)stats64;
 
-	BUILD_BUG_ON(sizeof(*netdev_stats) / sizeof(unsigned long) !=
-		     sizeof(*stats64) / sizeof(u64));
+	BUILD_BUG_ON(n > sizeof(*stats64) / sizeof(u64));
 	for (i = 0; i < n; i++)
 		dst[i] = src[i];
+	/* zero out counters that only exist in rtnl_link_stats64 */
+	memset((char *)stats64 + n * sizeof(u64), 0,
+	       sizeof(*stats64) - n * sizeof(u64));
 #endif
 }
 EXPORT_SYMBOL(netdev_stats_to_stats64);
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1323635 — [PATCH net v3 2/4] net: add rx_nohandler stat counter

This adds an rx_nohandler stat counter, along with a sysfs statistics
node, and copies the counter out via netlink as well.

CC: "David S. Miller" <davem@davemloft.net>
CC: Eric Dumazet <edumazet@google.com>
CC: Jiri Pirko <jiri@mellanox.com>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Tom Herbert <tom@herbertland.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <gospo@cumulusnetworks.com>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 include/linux/netdevice.h    | 3 +++
 include/uapi/linux/if_link.h | 4 ++++
 net/core/dev.c               | 6 +++++-
 net/core/net-sysfs.c         | 2 ++
 net/core/rtnetlink.c         | 2 ++
 5 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 289c231..78a20ce 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1397,6 +1397,8 @@ enum netdev_priv_flags {
  *			do not use this in drivers
  *	@tx_dropped:	Dropped packets by core network,
  *			do not use this in drivers
+ *	@rx_nohandler:	nohandler dropped packets by core network on
+ *			inactive devices, do not use this in drivers
  *
  *	@wireless_handlers:	List of functions to handle Wireless Extensions,
  *				instead of ioctl,
@@ -1611,6 +1613,7 @@ struct net_device {
 
 	atomic_long_t		rx_dropped;
 	atomic_long_t		tx_dropped;
+	atomic_long_t		rx_nohandler;
 
 #ifdef CONFIG_WIRELESS_EXT
 	const struct iw_handler_def *	wireless_handlers;
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index a30b780..d3e90b9 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -35,6 +35,8 @@ struct rtnl_link_stats {
 	/* for cslip etc */
 	__u32	rx_compressed;
 	__u32	tx_compressed;
+
+	__u32	rx_nohandler;		/* dropped, no handler found	*/
 };
 
 /* The main device statistics structure */
@@ -68,6 +70,8 @@ struct rtnl_link_stats64 {
 	/* for cslip etc */
 	__u64	rx_compressed;
 	__u64	tx_compressed;
+
+	__u64	rx_nohandler;		/* dropped, no handler found	*/
 };
 
 /* The struct should be in sync with struct ifmap */
diff --git a/net/core/dev.c b/net/core/dev.c
index 65863e5..f128483 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4154,7 +4154,10 @@ ncls:
 			ret = pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
 	} else {
 drop:
-		atomic_long_inc(&skb->dev->rx_dropped);
+		if (!deliver_exact)
+			atomic_long_inc(&skb->dev->rx_dropped);
+		else
+			atomic_long_inc(&skb->dev->rx_nohandler);
 		kfree_skb(skb);
 		/* Jamal, now you will not able to escape explaining
 		 * me how you were going to use this. :-)
@@ -7307,6 +7310,7 @@ struct rtnl_link_stats64 *dev_get_stats(struct net_device *dev,
 	}
 	storage->rx_dropped += atomic_long_read(&dev->rx_dropped);
 	storage->tx_dropped += atomic_long_read(&dev->tx_dropped);
+	storage->rx_nohandler += atomic_long_read(&dev->rx_nohandler);
 	return storage;
 }
 EXPORT_SYMBOL(dev_get_stats);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index b6c8a66..da7dbc2 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -574,6 +574,7 @@ NETSTAT_ENTRY(tx_heartbeat_errors);
 NETSTAT_ENTRY(tx_window_errors);
 NETSTAT_ENTRY(rx_compressed);
 NETSTAT_ENTRY(tx_compressed);
+NETSTAT_ENTRY(rx_nohandler);
 
 static struct attribute *netstat_attrs[] = {
 	&dev_attr_rx_packets.attr,
@@ -599,6 +600,7 @@ static struct attribute *netstat_attrs[] = {
 	&dev_attr_tx_window_errors.attr,
 	&dev_attr_rx_compressed.attr,
 	&dev_attr_tx_compressed.attr,
+	&dev_attr_rx_nohandler.attr,
 	NULL
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d735e85..20d7135 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -804,6 +804,8 @@ static void copy_rtnl_link_stats(struct rtnl_link_stats *a,
 
 	a->rx_compressed = b->rx_compressed;
 	a->tx_compressed = b->tx_compressed;
+
+	a->rx_nohandler = b->rx_nohandler;
 }
 
 static void copy_rtnl_link_stats64(void *v, const struct rtnl_link_stats64 *b)
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

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

On Mon,  1 Feb 2016 18:51:05 -0500
Jarod Wilson <jarod@redhat.com> wrote:

> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -35,6 +35,8 @@ struct rtnl_link_stats {
>  	/* for cslip etc */
>  	__u32	rx_compressed;
>  	__u32	tx_compressed;
> +
> +	__u32	rx_nohandler;		/* dropped, no handler found	*/
>  };
>  
>  /* The main device statistics structure */
> @@ -68,6 +70,8 @@ struct rtnl_link_stats64 {
>  	/* for cslip etc */
>  	__u64	rx_compressed;
>  	__u64	tx_compressed;
> +
> +	__u64	rx_nohandler;		/* dropped, no handler found	*/
>  };

Why was this userspace ABI change allowed?
The stats structure is exposed to user space via netlink
and changing the size of responses will break iproute2 commands.

The code will be expecting one size and the response will vary and
break existing code.  Yes, the code should check the size
of the response, but it doesn't and I am sure iproute2 is not
the only code that does this.

[toc] | [prev] | [next] | [standalone]

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

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 7 Feb 2016 11:37:32 -0800

> On Mon,  1 Feb 2016 18:51:05 -0500
> Jarod Wilson <jarod@redhat.com> wrote:
> 
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -35,6 +35,8 @@ struct rtnl_link_stats {
>>  	/* for cslip etc */
>>  	__u32	rx_compressed;
>>  	__u32	tx_compressed;
>> +
>> +	__u32	rx_nohandler;		/* dropped, no handler found	*/
>>  };
>>  
>>  /* The main device statistics structure */
>> @@ -68,6 +70,8 @@ struct rtnl_link_stats64 {
>>  	/* for cslip etc */
>>  	__u64	rx_compressed;
>>  	__u64	tx_compressed;
>> +
>> +	__u64	rx_nohandler;		/* dropped, no handler found	*/
>>  };
> 
> Why was this userspace ABI change allowed?
> The stats structure is exposed to user space via netlink
> and changing the size of responses will break iproute2 commands.
> 
> The code will be expecting one size and the response will vary and
> break existing code.  Yes, the code should check the size
> of the response, but it doesn't and I am sure iproute2 is not
> the only code that does this.

Jarod, please look into this.

[toc] | [prev] | [next] | [standalone]

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

On Sun, 2016-02-07 at 14:46 -0500, David Miller wrote:

> > Why was this userspace ABI change allowed?
> > The stats structure is exposed to user space via netlink
> > and changing the size of responses will break iproute2 commands.

I do not think it breaks anything.

iproute2 always assumed kernel was sending at least 23 u64, and does not
check at all if the kernel sends more. (or less, so iproute2 can print
garbage if kernel is malicious)

an iproute2 patch will be needed to automatically detect if new kernels
are sending more data and print it accordingly.

> > 
> > The code will be expecting one size and the response will vary and
> > break existing code.  Yes, the code should check the size
> > of the response, but it doesn't and I am sure iproute2 is not
> > the only code that does this.
> 
> Jarod, please look into this.

Running latest net-next, and old iproute2 is just fine.

# ip -s link sh dev eth0
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq portid
001a11fffec30d80 state UP mode DEFAULT group default qlen 16000
    link/ether 00:1a:11:c3:0d:7f brd ff:ff:ff:ff:ff:ff
    RX: bytes  packets  errors  dropped overrun mcast   
    533766     1875     0       0       0       135     
    TX: bytes  packets  errors  dropped carrier collsns 
    209204     1858     0       0       0       0       

[toc] | [prev] | [next] | [standalone]

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

On Sun, Feb 07, 2016 at 12:19:28PM -0800, Eric Dumazet wrote:
> On Sun, 2016-02-07 at 14:46 -0500, David Miller wrote:
> 
> > > Why was this userspace ABI change allowed?
> > > The stats structure is exposed to user space via netlink
> > > and changing the size of responses will break iproute2 commands.
> 
> I do not think it breaks anything.
> 
> iproute2 always assumed kernel was sending at least 23 u64, and does not
> check at all if the kernel sends more. (or less, so iproute2 can print
> garbage if kernel is malicious)
> 
> an iproute2 patch will be needed to automatically detect if new kernels
> are sending more data and print it accordingly.

My TODO list did include poking at iproute2 to expose the new info, I can
take a closer look for possible issues as well, but...

> > > The code will be expecting one size and the response will vary and
> > > break existing code.  Yes, the code should check the size
> > > of the response, but it doesn't and I am sure iproute2 is not
> > > the only code that does this.
> > 
> > Jarod, please look into this.
> 
> Running latest net-next, and old iproute2 is just fine.

...I haven't run into anything that didn't work with current iproute2
either while testing out functionality of these patches. If there's
something in particular that seems most suspect that I perhaps simply
haven't tried, I can give that a go as well.

In any case, I'm definitely due to take a look at iproute2 as it relates
to this patchset.

-- 
Jarod Wilson
jarod@redhat.com

[toc] | [prev] | [next] | [standalone]

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

On Mon, 8 Feb 2016 13:32:54 -0500
Jarod Wilson <jarod@redhat.com> wrote:

> On Sun, Feb 07, 2016 at 12:19:28PM -0800, Eric Dumazet wrote:
> > On Sun, 2016-02-07 at 14:46 -0500, David Miller wrote:
> > 
> > > > Why was this userspace ABI change allowed?
> > > > The stats structure is exposed to user space via netlink
> > > > and changing the size of responses will break iproute2 commands.
> > 
> > I do not think it breaks anything.
> > 
> > iproute2 always assumed kernel was sending at least 23 u64, and does not
> > check at all if the kernel sends more. (or less, so iproute2 can print
> > garbage if kernel is malicious)
> > 
> > an iproute2 patch will be needed to automatically detect if new kernels
> > are sending more data and print it accordingly.
> 
> My TODO list did include poking at iproute2 to expose the new info, I can
> take a closer look for possible issues as well, but...
> 
> > > > The code will be expecting one size and the response will vary and
> > > > break existing code.  Yes, the code should check the size
> > > > of the response, but it doesn't and I am sure iproute2 is not
> > > > the only code that does this.
> > > 
> > > Jarod, please look into this.
> > 
> > Running latest net-next, and old iproute2 is just fine.
> 
> ...I haven't run into anything that didn't work with current iproute2
> either while testing out functionality of these patches. If there's
> something in particular that seems most suspect that I perhaps simply
> haven't tried, I can give that a go as well.
> 
> In any case, I'm definitely due to take a look at iproute2 as it relates
> to this patchset.
> 

The iproute2 command can be fixed, but adding dependency on size of response
gets gross fast.  Imagine when 4 more fields get added, this doesn't scale well.

Also, the definition of userspace ABI is that structures can't change.
There are many other utilities that are not visible that may get broken.
Traditionally Linux has guaranteed that programs will continue to work
no matter how they were coded.

[toc] | [prev] | [next] | [standalone]

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

On Mon, 2016-02-08 at 11:38 -0800, Stephen Hemminger wrote:

> The iproute2 command can be fixed, but adding dependency on size of response
> gets gross fast.  Imagine when 4 more fields get added, this doesn't scale well.

Really ? I see no problem at all doing the proper tests.

> 
> Also, the definition of userspace ABI is that structures can't change.
> There are many other utilities that are not visible that may get broken.
> Traditionally Linux has guaranteed that programs will continue to work
> no matter how they were coded.

Stephen, we have been doing that for years.

Whole point of TLV is that it allows us to add new fields at the end of
the structures.

Yes, some buggy programs might need a fix.

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.

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web