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


Groups > linux.kernel > #1315232 > unrolled thread

[RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves

Started byJarod Wilson <jarod@redhat.com>
First post2016-01-22 20:20 +0100
Last post2016-01-28 17:30 +0100
Articles 20 on this page of 57 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Jarod Wilson <jarod@redhat.com> - 2016-01-22 20:20 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Jay Vosburgh <jay.vosburgh@canonical.com> - 2016-01-22 22:00 +0100
      Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jiri Pirko <jiri@resnulli.us> - 2016-01-23 09:30 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jiri Pirko <jiri@resnulli.us> - 2016-01-23 09:10 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Andy Gospodarek <gospo@cumulusnetworks.com> - 2016-01-23 15:30 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on  inactive slaves Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-23 16:30 +0100
      Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 22:20 +0100
        Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive slaves Eric Dumazet <edumazet@google.com> - 2016-01-26 22:30 +0100
          Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 22:40 +0100
        Re: [RFC PATCH net] net/core: don't increment rx_dropped on  inactive slaves David Miller <davem@davemloft.net> - 2016-01-26 22:30 +0100
          Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 22:40 +0100
    Re: [RFC PATCH net] net/core: don't increment rx_dropped on  inactive slaves David Miller <davem@davemloft.net> - 2016-01-25 07:50 +0100
      Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-25 15:30 +0100
        Re: [RFC PATCH net] net/core: don't increment rx_dropped on inactive  slaves Jarod Wilson <jarod@redhat.com> - 2016-01-26 05:50 +0100
    [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
      [PATCH net 1/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
      [PATCH net 2/4] net-procfs: show rx_unhandled counters Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
      [PATCH net 4/4] bond: track sum of rx_unhandled for all slaves Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
      [PATCH net 3/4] team: track sum of rx_unhandled for all slaves Jarod Wilson <jarod@redhat.com> - 2016-01-27 21:30 +0100
      Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-27 22:10 +0100
        Re: [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 07:10 +0100
          Re: [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 07:20 +0100
            Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-28 14:10 +0100
              Re: [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 15:40 +0100
                Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-28 15:50 +0100
                  Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-28 15:50 +0100
                    Re: [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 16:20 +0100
                  Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-28 15:50 +0100
          Re: [PATCH net 0/4] net: add rx_unhandled stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 07:20 +0100
          Re: [PATCH net 0/4] net: add rx_unhandled stat counter Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-28 14:10 +0100
    [PATCH net v2 2/4] net: add rx_nohandler stat counter Jarod Wilson <jarod@redhat.com> - 2016-01-28 17:00 +0100
    [PATCH net v2 3/4] team: track sum of rx_nohandler for all slaves Jarod Wilson <jarod@redhat.com> - 2016-01-28 17:00 +0100
    [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-01-28 17:00 +0100
    [PATCH net v2 4/4] bond: track sum of rx_nohandler for all slaves Jarod Wilson <jarod@redhat.com> - 2016-01-28 17:00 +0100
    [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
    [PATCH net v3 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64 Jarod Wilson <jarod@redhat.com> - 2016-01-28 17:30 +0100

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


#1320321 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 07:10 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVNWh-7D7-11@gated-at.bofh.it>
In reply to#1319825
On Wed, Jan 27, 2016 at 01:09:47PM -0800, Eric Dumazet wrote:
> On Wed, 2016-01-27 at 15:21 -0500, Jarod Wilson wrote:
> 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index 289c231..7973ab5 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -180,6 +180,7 @@ struct net_device_stats {
> >  	unsigned long	tx_window_errors;
> >  	unsigned long	rx_compressed;
> >  	unsigned long	tx_compressed;
> > +	unsigned long	rx_unhandled;
> >  };
> >  
> 
> This structure is deprecated, please do not add new fields in it,
> as it will increase netlink answers for no good reason.
> 
> rtnl_link_stats64 is what really matters these days.

I'll respin the set without that, along with s/unhandled/nohandler/, which
I somehow got screwed up in my head and realized a split second after
hitting send. Outside of that, does this approach look sane? Should I
bother with touching /proc/net/dev output or not?

Thanks much,

-- 
Jarod Wilson
jarod@redhat.com

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


#1320324 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 07:20 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVO5X-7IF-1@gated-at.bofh.it>
In reply to#1320321
On Thu, Jan 28, 2016 at 01:02:15AM -0500, Jarod Wilson wrote:
> On Wed, Jan 27, 2016 at 01:09:47PM -0800, Eric Dumazet wrote:
> > On Wed, 2016-01-27 at 15:21 -0500, Jarod Wilson wrote:
> > 
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 289c231..7973ab5 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -180,6 +180,7 @@ struct net_device_stats {
> > >  	unsigned long	tx_window_errors;
> > >  	unsigned long	rx_compressed;
> > >  	unsigned long	tx_compressed;
> > > +	unsigned long	rx_unhandled;
> > >  };
> > >  
> > 
> > This structure is deprecated, please do not add new fields in it,
> > as it will increase netlink answers for no good reason.
> > 
> > rtnl_link_stats64 is what really matters these days.
> 
> I'll respin the set without that

Or not. Now I remember why I added that in the first place:

In file included from ./arch/x86/include/asm/uaccess.h:7:0,
                 from net/core/dev.c:75:
net/core/dev.c: In function 'netdev_stats_to_stats64':
include/linux/compiler.h:484:20: error: call to '__compiletime_assert_7263' declared with attribute error: BUILD_BUG_ON failed: sizeof(*stats64) != sizeof(*netdev_stats)
    prefix ## suffix();    \
                    ^

Things are actually hard-wired to require that addition at the moment, or
you get the above build failure. Not sure if it's safe to remove that
BUILD_BUG_ON() yet, haven't looked closely, it's past my bed time. :)

-- 
Jarod Wilson
jarod@redhat.com

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


#1320647 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromEric Dumazet <eric.dumazet@gmail.com>
Date2016-01-28 14:10 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVUuK-3R3-17@gated-at.bofh.it>
In reply to#1320324
On Thu, 2016-01-28 at 01:18 -0500, Jarod Wilson wrote:

> Or not. Now I remember why I added that in the first place:
> 
> In file included from ./arch/x86/include/asm/uaccess.h:7:0,
>                  from net/core/dev.c:75:
> net/core/dev.c: In function 'netdev_stats_to_stats64':
> include/linux/compiler.h:484:20: error: call to '__compiletime_assert_7263' declared with attribute error: BUILD_BUG_ON failed: sizeof(*stats64) != sizeof(*netdev_stats)
>     prefix ## suffix();    \
>                     ^
> 
> Things are actually hard-wired to require that addition at the moment, or
> you get the above build failure. Not sure if it's safe to remove that
> BUILD_BUG_ON() yet, haven't looked closely, it's past my bed time. :)
> 

This was done for the transition from "unsigned long" to "u64", which is
a nop on 64bit arches.

But as we do not need to be compatible, since no linux kernel in the
past had this new field in struct net_device_stats,

and we do not need to add this new field as it is only accessed from
core networking stack [1], you need to adapt this helper.

[1] And maybe some virtual devices like bonding/team 

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


#1320741 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 15:40 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVVTR-4Lm-47@gated-at.bofh.it>
In reply to#1320647
On Thu, Jan 28, 2016 at 05:00:02AM -0800, Eric Dumazet wrote:
> On Thu, 2016-01-28 at 01:18 -0500, Jarod Wilson wrote:
> 
> > Or not. Now I remember why I added that in the first place:
> > 
> > In file included from ./arch/x86/include/asm/uaccess.h:7:0,
> >                  from net/core/dev.c:75:
> > net/core/dev.c: In function 'netdev_stats_to_stats64':
> > include/linux/compiler.h:484:20: error: call to '__compiletime_assert_7263' declared with attribute error: BUILD_BUG_ON failed: sizeof(*stats64) != sizeof(*netdev_stats)
> >     prefix ## suffix();    \
> >                     ^
> > 
> > Things are actually hard-wired to require that addition at the moment, or
> > you get the above build failure. Not sure if it's safe to remove that
> > BUILD_BUG_ON() yet, haven't looked closely, it's past my bed time. :)
> > 
> 
> This was done for the transition from "unsigned long" to "u64", which is
> a nop on 64bit arches.
> 
> But as we do not need to be compatible, since no linux kernel in the
> past had this new field in struct net_device_stats,
> 
> and we do not need to add this new field as it is only accessed from
> core networking stack [1], you need to adapt this helper.

Something like this then:

diff --git a/net/core/dev.c b/net/core/dev.c
index 82334c6..2ca3eab 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7262,15 +7262,16 @@ void netdev_run_todo(void)
 void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
 			     const struct net_device_stats *netdev_stats)
 {
+	memset(stats64, 0, sizeof(*stats64));
 #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];

Compiles locally w/o that net_device_stats addition, seems sane to me.

-- 
Jarod Wilson
jarod@redhat.com

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


#1320743 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromEric Dumazet <eric.dumazet@gmail.com>
Date2016-01-28 15:50 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVW3v-4OQ-9@gated-at.bofh.it>
In reply to#1320741
On Thu, 2016-01-28 at 09:38 -0500, Jarod Wilson wrote:

> Something like this then:
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 82334c6..2ca3eab 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -7262,15 +7262,16 @@ void netdev_run_todo(void)
>  void netdev_stats_to_stats64(struct rtnl_link_stats64 *stats64,
>  			     const struct net_device_stats *netdev_stats)
>  {
> +	memset(stats64, 0, sizeof(*stats64));
>  #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];
> 
> Compiles locally w/o that net_device_stats addition, seems sane to me.
> 

Sure, you also can set stats64->rx_unhandled to 0 here, just to be 100%
safe.

Thanks.

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


#1320744 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromEric Dumazet <eric.dumazet@gmail.com>
Date2016-01-28 15:50 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVW3v-4OQ-7@gated-at.bofh.it>
In reply to#1320743
On Thu, 2016-01-28 at 06:44 -0800, Eric Dumazet wrote:
> On Thu, 2016-01-28 at 06:42 -0800, Eric Dumazet wrote:
> 
> > 
> > Sure, you also can set stats64->rx_unhandled to 0 here, just to be 100%
> > safe.
> 
> And not add the memset(stats64, 0, sizeof(*stats64)), since we have the
> guarantee to properly init whole stats64 structure.

Or a more tricky

memset((char *)stats64 + sizeof(struct net_device_stats),
       0,
       sizeof(*stats64) - sizeof(struct net_device_stats));

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


#1320780 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 16:20 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVWwy-5jp-27@gated-at.bofh.it>
In reply to#1320744
On Thu, Jan 28, 2016 at 06:46:50AM -0800, Eric Dumazet wrote:
> On Thu, 2016-01-28 at 06:44 -0800, Eric Dumazet wrote:
> > On Thu, 2016-01-28 at 06:42 -0800, Eric Dumazet wrote:
> > 
> > > 
> > > Sure, you also can set stats64->rx_unhandled to 0 here, just to be 100%
> > > safe.
> > 
> > And not add the memset(stats64, 0, sizeof(*stats64)), since we have the
> > guarantee to properly init whole stats64 structure.
> 
> Or a more tricky
> 
> memset((char *)stats64 + sizeof(struct net_device_stats),
>        0,
>        sizeof(*stats64) - sizeof(struct net_device_stats));

I think I like this best, since it won't require someone to notice they
have to add an explicit initialization if they add a new counter, as well
as saving us from memset'ing the entire struct, the majority of which
we'll initialize moments later. Just needs some documentation updates,
which I've done locally. Will rebuild, re-test, then hopefully get an
updated patchset out the door.

-- 
Jarod Wilson
jarod@redhat.com

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


#1320748 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromEric Dumazet <eric.dumazet@gmail.com>
Date2016-01-28 15:50 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVW3v-4OQ-11@gated-at.bofh.it>
In reply to#1320743
On Thu, 2016-01-28 at 06:42 -0800, Eric Dumazet wrote:

> 
> Sure, you also can set stats64->rx_unhandled to 0 here, just to be 100%
> safe.

And not add the memset(stats64, 0, sizeof(*stats64)), since we have the
guarantee to properly init whole stats64 structure.

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


#1320325 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 07:20 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVO5Y-7IF-11@gated-at.bofh.it>
In reply to#1320321
On Thu, Jan 28, 2016 at 01:02:15AM -0500, Jarod Wilson wrote:
> On Wed, Jan 27, 2016 at 01:09:47PM -0800, Eric Dumazet wrote:
> > On Wed, 2016-01-27 at 15:21 -0500, Jarod Wilson wrote:
> > 
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index 289c231..7973ab5 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -180,6 +180,7 @@ struct net_device_stats {
> > >  	unsigned long	tx_window_errors;
> > >  	unsigned long	rx_compressed;
> > >  	unsigned long	tx_compressed;
> > > +	unsigned long	rx_unhandled;
> > >  };
> > >  
> > 
> > This structure is deprecated, please do not add new fields in it,
> > as it will increase netlink answers for no good reason.
> > 
> > rtnl_link_stats64 is what really matters these days.
> 
> I'll respin the set without that, along with s/unhandled/nohandler/, which
> I somehow got screwed up in my head and realized a split second after
> hitting send. Outside of that, does this approach look sane? Should I
> bother with touching /proc/net/dev output or not?

Also, please excuse the poor excuse for a cover-letter that had a
duplicate of patch 1 in it. I'll fix that the next pass too.

/me hangs head in shame...

-- 
Jarod Wilson
jarod@redhat.com

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


#1320644 — Re: [PATCH net 0/4] net: add rx_unhandled stat counter

FromEric Dumazet <eric.dumazet@gmail.com>
Date2016-01-28 14:10 +0100
SubjectRe: [PATCH net 0/4] net: add rx_unhandled stat counter
Message-ID<qVUuK-3R3-15@gated-at.bofh.it>
In reply to#1320321
On Thu, 2016-01-28 at 01:02 -0500, Jarod Wilson wrote:
> Outside of that, does this approach look sane? Should I
> bother with touching /proc/net/dev output or not?

Please do not touch /proc/net/dev 

This is legacy stuff and really should not be touched anymore.

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


#1320812 — [PATCH net v2 2/4] net: add rx_nohandler stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 17:00 +0100
Subject[PATCH net v2 2/4] net: add rx_nohandler stat counter
Message-ID<qVX9f-5Ao-1@gated-at.bofh.it>
In reply to#1315232
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 575a7df..fef2351 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. :-)
@@ -7305,6 +7308,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]


#1320813 — [PATCH net v2 3/4] team: track sum of rx_nohandler for all slaves

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 17:00 +0100
Subject[PATCH net v2 3/4] team: track sum of rx_nohandler for all slaves
Message-ID<qVX9f-5Ao-7@gated-at.bofh.it>
In reply to#1315232
CC: Jiri Pirko <jiri@resnulli.us>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/team/team.c | 10 +++++++---
 include/linux/if_team.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 718ceea..00558e1 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -758,6 +758,8 @@ static rx_handler_result_t team_handle_frame(struct sk_buff **pskb)
 		u64_stats_update_end(&pcpu_stats->syncp);
 
 		skb->dev = team->dev;
+	} else if (res == RX_HANDLER_EXACT) {
+		this_cpu_inc(team->pcpu_stats->rx_nohandler);
 	} else {
 		this_cpu_inc(team->pcpu_stats->rx_dropped);
 	}
@@ -1807,7 +1809,7 @@ team_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	struct team *team = netdev_priv(dev);
 	struct team_pcpu_stats *p;
 	u64 rx_packets, rx_bytes, rx_multicast, tx_packets, tx_bytes;
-	u32 rx_dropped = 0, tx_dropped = 0;
+	u32 rx_dropped = 0, tx_dropped = 0, rx_nohandler = 0;
 	unsigned int start;
 	int i;
 
@@ -1828,14 +1830,16 @@ team_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
 		stats->tx_packets	+= tx_packets;
 		stats->tx_bytes		+= tx_bytes;
 		/*
-		 * rx_dropped & tx_dropped are u32, updated
-		 * without syncp protection.
+		 * rx_dropped, tx_dropped & rx_nohandler are u32,
+		 * updated without syncp protection.
 		 */
 		rx_dropped	+= p->rx_dropped;
 		tx_dropped	+= p->tx_dropped;
+		rx_nohandler	+= p->rx_nohandler;
 	}
 	stats->rx_dropped	= rx_dropped;
 	stats->tx_dropped	= tx_dropped;
+	stats->rx_nohandler	= rx_nohandler;
 	return stats;
 }
 
diff --git a/include/linux/if_team.h b/include/linux/if_team.h
index b84e49c..174f43f 100644
--- a/include/linux/if_team.h
+++ b/include/linux/if_team.h
@@ -24,6 +24,7 @@ struct team_pcpu_stats {
 	struct u64_stats_sync	syncp;
 	u32			rx_dropped;
 	u32			tx_dropped;
+	u32			rx_nohandler;
 };
 
 struct team;
-- 
1.8.3.1

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


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

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 17:00 +0100
Subject[PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
Message-ID<qVX9g-5Ao-21@gated-at.bofh.it>
In reply to#1315232
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>
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 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]


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

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 17:00 +0100
Subject[PATCH net v2 4/4] bond: track sum of rx_nohandler for all slaves
Message-ID<qVX9g-5Ao-29@gated-at.bofh.it>
In reply to#1315232
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]


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

FromJarod Wilson <jarod@redhat.com>
Date2016-01-28 17:00 +0100
Subject[PATCH net v2 0/4] net: add and use rx_nohandler stat counter
Message-ID<qVX9f-5Ao-3@gated-at.bofh.it>
In reply to#1315232
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] | [prev] | [next] | [standalone]


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

FromDavid Miller <davem@davemloft.net>
Date2016-01-30 04:40 +0100
SubjectRe: [PATCH net v2 0/4] net: add and use rx_nohandler stat counter
Message-ID<qWuye-51K-31@gated-at.bofh.it>
In reply to#1320824
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 — Re: [PATCH net v2 0/4] net: add and use rx_nohandler stat counter

FromJarod Wilson <jarod@redhat.com>
Date2016-01-30 19:20 +0100
SubjectRe: [PATCH net v2 0/4] net: add and use rx_nohandler stat counter
Message-ID<qWIhP-7Qz-11@gated-at.bofh.it>
In reply to#1322250
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

FromJarod Wilson <jarod@redhat.com>
Date2016-01-30 19:20 +0100
Subject[PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
Message-ID<qWIhP-7Qz-9@gated-at.bofh.it>
In reply to#1320824
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

FromEric Dumazet <eric.dumazet@gmail.com>
Date2016-01-30 19:40 +0100
SubjectRe: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
Message-ID<qWIBc-7Y1-3@gated-at.bofh.it>
In reply to#1322479
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

FromJarod Wilson <jarod@redhat.com>
Date2016-01-30 21:40 +0100
SubjectRe: [PATCH net v2 1/4] net/core: relax BUILD_BUG_ON in netdev_stats_to_stats64
Message-ID<qWKtj-Mu-3@gated-at.bofh.it>
In reply to#1322485
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]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web