Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1315232 > unrolled thread
| Started by | Jarod Wilson <jarod@redhat.com> |
|---|---|
| First post | 2016-01-22 20:20 +0100 |
| Last post | 2016-01-28 17:30 +0100 |
| Articles | 20 on this page of 57 — 8 participants |
Back to article view | Back to linux.kernel
[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 →
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-28 07:10 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-28 07:20 +0100 |
| Subject | Re: [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]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-28 14:10 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-28 15:40 +0100 |
| Subject | Re: [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]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-28 15:50 +0100 |
| Subject | Re: [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]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-28 15:50 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-28 16:20 +0100 |
| Subject | Re: [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]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-28 15:50 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-28 07:20 +0100 |
| Subject | Re: [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]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-28 14:10 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-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]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2016-01-30 04:40 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-30 19:20 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-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]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-30 19:40 +0100 |
| Subject | Re: [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]
| From | Jarod Wilson <jarod@redhat.com> |
|---|---|
| Date | 2016-01-30 21:40 +0100 |
| Subject | Re: [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