| Message ID | 1268337450-6749-4-git-send-email-jengelh@medozas.de |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, 11 Mar 2010 20:57:29 +0100 Jan Engelhardt <jengelh@medozas.de> wrote: > `ip -s link` shows interface counters truncated to 32 bit. This is > because interface statistics are transported only in 32-bit quantity > to userspace. This commit adds a new IFLA_STATS64 attribute that > exports them in full 64 bit. > > References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html > Signed-off-by: Jan Engelhardt <jengelh@medozas.de> > What is the point since net_device_stats has only 32 bit counters. The last time this came up the issue was is that is not possible to atomically increment a 64 bit counter on most 32 bit platforms. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le jeudi 11 mars 2010 à 20:57 +0100, Jan Engelhardt a écrit : > `ip -s link` shows interface counters truncated to 32 bit. This is > because interface statistics are transported only in 32-bit quantity > to userspace. This commit adds a new IFLA_STATS64 attribute that > exports them in full 64 bit. > > References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html oh oh, back to 2003 :) > Signed-off-by: Jan Engelhardt <jengelh@medozas.de> > --- > > static inline int rtnl_vfinfo_size(const struct net_device *dev) > { > @@ -698,6 +730,14 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, > stats = dev_get_stats(dev); OK, stats is up2date with device stats > copy_rtnl_link_stats(nla_data(attr), stats); > > + attr = nla_reserve(skb, IFLA_STATS64, > + sizeof(struct rtnl_link_stats64)); > + if (attr == NULL) > + goto nla_put_failure; > + > + stats = dev_get_stats(dev); Why calling dev_get_stats(dev) a second time ? Its can be pretty expensive on some devices. > + copy_rtnl_link_stats64(nla_data(attr), stats); > + > if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent) { > int i; > struct ifla_vf_info ivi; > -- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Stephen Hemminger <shemminger@vyatta.com> Date: Thu, 11 Mar 2010 12:12:39 -0800 > On Thu, 11 Mar 2010 20:57:29 +0100 > Jan Engelhardt <jengelh@medozas.de> wrote: > >> `ip -s link` shows interface counters truncated to 32 bit. This is >> because interface statistics are transported only in 32-bit quantity >> to userspace. This commit adds a new IFLA_STATS64 attribute that >> exports them in full 64 bit. >> >> References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html >> Signed-off-by: Jan Engelhardt <jengelh@medozas.de> >> > > What is the point since net_device_stats has only 32 bit counters. > The last time this came up the issue was is that is not possible > to atomically increment a 64 bit counter on most 32 bit platforms. On 64-bit it has 64-bit counters, yet we only report 32-bit counters to userspace via netlink even in that case. That's what Jan is fixing here. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 2010-03-11 21:26, David Miller wrote: >> On Thu, 11 Mar 2010 20:57:29 +0100 >> Jan Engelhardt <jengelh@medozas.de> wrote: >> >>> `ip -s link` shows interface counters truncated to 32 bit. This is >>> because interface statistics are transported only in 32-bit quantity >>> to userspace. This commit adds a new IFLA_STATS64 attribute that >>> exports them in full 64 bit. >>> >>> References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html >>> Signed-off-by: Jan Engelhardt <jengelh@medozas.de> >>> >> >> What is the point since net_device_stats has only 32 bit counters. >> The last time this came up the issue was is that is not possible >> to atomically increment a 64 bit counter on most 32 bit platforms. > >On 64-bit it has 64-bit counters, yet we only report 32-bit >counters to userspace via netlink even in that case. > >That's what Jan is fixing here. On a side note, is it perhaps possible to use jiffies-like magic to also have 64-bit counters on 32-bit, or is that still too expensive? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Jan Engelhardt <jengelh@medozas.de> Date: Thu, 11 Mar 2010 22:07:07 +0100 (CET) > On a side note, is it perhaps possible to use jiffies-like magic to > also have 64-bit counters on 32-bit, or is that still too expensive? I think there might still be atomicity issues. We've been telling people to poll periodically in userspace to avoid these problems, but that has limitations as network speeds increase. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 11 Mar 2010 12:26:39 -0800 (PST) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Thu, 11 Mar 2010 12:12:39 -0800 > > > On Thu, 11 Mar 2010 20:57:29 +0100 > > Jan Engelhardt <jengelh@medozas.de> wrote: > > > >> `ip -s link` shows interface counters truncated to 32 bit. This is > >> because interface statistics are transported only in 32-bit quantity > >> to userspace. This commit adds a new IFLA_STATS64 attribute that > >> exports them in full 64 bit. > >> > >> References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html > >> Signed-off-by: Jan Engelhardt <jengelh@medozas.de> > >> > > > > What is the point since net_device_stats has only 32 bit counters. > > The last time this came up the issue was is that is not possible > > to atomically increment a 64 bit counter on most 32 bit platforms. > > On 64-bit it has 64-bit counters, yet we only report 32-bit > counters to userspace via netlink even in that case. That make sense, but maybe we shouldn't send IFLA_STATS64 on 32bit platforms. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 2010-03-11 23:04, Stephen Hemminger wrote: >> >> `ip -s link` shows interface counters truncated to 32 bit. This is >> >> because interface statistics are transported only in 32-bit quantity >> >> to userspace. This commit adds a new IFLA_STATS64 attribute that >> >> exports them in full 64 bit. >> >> On 64-bit it has 64-bit counters, yet we only report 32-bit >> counters to userspace via netlink even in that case. > >That make sense, but maybe we shouldn't send IFLA_STATS64 on >32bit platforms. Somehow I'd prefer to have consistency. Platform-specific actions and/or payload I think we already were plagued enough by syscalls and iptables. (Just to name two.) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Stephen Hemminger <shemminger@vyatta.com> Date: Thu, 11 Mar 2010 14:04:06 -0800 > That make sense, but maybe we shouldn't send IFLA_STATS64 on > 32bit platforms. Let's at least be optimistic that we'll be able to support 64-bit stats on 32-bit at some point :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday 2010-03-11 23:13, David Miller wrote: > >> That make sense, but maybe we shouldn't send IFLA_STATS64 on >> 32bit platforms. > >Let's at least be optimistic that we'll be able to support 64-bit >stats on 32-bit at some point :-) I was about to say. There is hope: whenever you least expect it, academia comes up with a perfectly foundated theory and plan to solve the problem. (N.B.: But the initial implementation generally sucks ;-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 11 Mar 2010 23:13:18 +0100 (CET) Jan Engelhardt <jengelh@medozas.de> wrote: > On Thursday 2010-03-11 23:04, Stephen Hemminger wrote: > >> >> `ip -s link` shows interface counters truncated to 32 bit. This is > >> >> because interface statistics are transported only in 32-bit quantity > >> >> to userspace. This commit adds a new IFLA_STATS64 attribute that > >> >> exports them in full 64 bit. > >> > >> On 64-bit it has 64-bit counters, yet we only report 32-bit > >> counters to userspace via netlink even in that case. > > > >That make sense, but maybe we shouldn't send IFLA_STATS64 on > >32bit platforms. > > Somehow I'd prefer to have consistency. Platform-specific > actions and/or payload I think we already were plagued enough by > syscalls and iptables. (Just to name two.) But if you send 32bit truncated values when 64 bit is expected then users are going to complain
From: Stephen Hemminger <shemminger@vyatta.com> Date: Thu, 11 Mar 2010 14:50:59 -0800 > But if you send 32bit truncated values when 64 bit is expected > then users are going to complain They already complain, a lot. This isn't going to change the situation at all, and I agree with Jan that consistency is the trumping factor here. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 11 Mar 2010 14:54:12 -0800 (PST) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Thu, 11 Mar 2010 14:50:59 -0800 > > > But if you send 32bit truncated values when 64 bit is expected > > then users are going to complain > > They already complain, a lot. > > This isn't going to change the situation at all, and > I agree with Jan that consistency is the trumping factor > here. But if non utilities will already adapt to new/old kernel, so just don't put out 64 bit values if the platform is 32 bit. When/if 32 bit platforms support it, great add the extra stats.
From: Stephen Hemminger <shemminger@vyatta.com> Date: Thu, 11 Mar 2010 15:10:50 -0800 > But if non utilities will already adapt to new/old kernel, so > just don't put out 64 bit values if the platform is 32 bit. > When/if 32 bit platforms support it, great add the extra stats. How about we just make userland ready for it and: 1) Emitting it now will get it tested even on 32-bit 2) They'll be ready when the drivers can keep track of 64-bit stats too. Stephen I've heard your side of the story, I just don't agree with it :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 11 Mar 2010 15:15:29 -0800 (PST) David Miller <davem@davemloft.net> wrote: > From: Stephen Hemminger <shemminger@vyatta.com> > Date: Thu, 11 Mar 2010 15:10:50 -0800 > > > But if non utilities will already adapt to new/old kernel, so > > just don't put out 64 bit values if the platform is 32 bit. > > When/if 32 bit platforms support it, great add the extra stats. > > How about we just make userland ready for it and: > > 1) Emitting it now will get it tested even on 32-bit > > 2) They'll be ready when the drivers can keep track of > 64-bit stats too. > > Stephen I've heard your side of the story, I just don't > agree with it :-) I'm okay with it. just raising the issue.
From: Jan Engelhardt <jengelh@medozas.de> Date: Thu, 11 Mar 2010 20:57:29 +0100 > `ip -s link` shows interface counters truncated to 32 bit. This is > because interface statistics are transported only in 32-bit quantity > to userspace. This commit adds a new IFLA_STATS64 attribute that > exports them in full 64 bit. > > References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html > Signed-off-by: Jan Engelhardt <jengelh@medozas.de> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/if_link.h b/include/linux/if_link.h index c9bf92c..cfd420b 100644 --- a/include/linux/if_link.h +++ b/include/linux/if_link.h @@ -37,6 +37,38 @@ struct rtnl_link_stats { __u32 tx_compressed; }; +struct rtnl_link_stats64 { + __u64 rx_packets; /* total packets received */ + __u64 tx_packets; /* total packets transmitted */ + __u64 rx_bytes; /* total bytes received */ + __u64 tx_bytes; /* total bytes transmitted */ + __u64 rx_errors; /* bad packets received */ + __u64 tx_errors; /* packet transmit problems */ + __u64 rx_dropped; /* no space in linux buffers */ + __u64 tx_dropped; /* no space available in linux */ + __u64 multicast; /* multicast packets received */ + __u64 collisions; + + /* detailed rx_errors: */ + __u64 rx_length_errors; + __u64 rx_over_errors; /* receiver ring buff overflow */ + __u64 rx_crc_errors; /* recved pkt with crc error */ + __u64 rx_frame_errors; /* recv'd frame alignment error */ + __u64 rx_fifo_errors; /* recv'r fifo overrun */ + __u64 rx_missed_errors; /* receiver missed packet */ + + /* detailed tx_errors */ + __u64 tx_aborted_errors; + __u64 tx_carrier_errors; + __u64 tx_fifo_errors; + __u64 tx_heartbeat_errors; + __u64 tx_window_errors; + + /* for cslip etc */ + __u64 rx_compressed; + __u64 tx_compressed; +}; + /* The struct should be in sync with struct ifmap */ struct rtnl_link_ifmap { __u64 mem_start; @@ -83,6 +115,7 @@ enum { IFLA_VF_VLAN, IFLA_VF_TX_RATE, /* TX Bandwidth Allocation */ IFLA_VFINFO, + IFLA_STATS64, __IFLA_MAX }; diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 4568120..e1121f0 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -600,7 +600,39 @@ static void copy_rtnl_link_stats(struct rtnl_link_stats *a, a->rx_compressed = b->rx_compressed; a->tx_compressed = b->tx_compressed; -}; +} + +static void copy_rtnl_link_stats64(struct rtnl_link_stats64 *a, + const struct net_device_stats *b) +{ + a->rx_packets = b->rx_packets; + a->tx_packets = b->tx_packets; + a->rx_bytes = b->rx_bytes; + a->tx_bytes = b->tx_bytes; + a->rx_errors = b->rx_errors; + a->tx_errors = b->tx_errors; + a->rx_dropped = b->rx_dropped; + a->tx_dropped = b->tx_dropped; + + a->multicast = b->multicast; + a->collisions = b->collisions; + + a->rx_length_errors = b->rx_length_errors; + a->rx_over_errors = b->rx_over_errors; + a->rx_crc_errors = b->rx_crc_errors; + a->rx_frame_errors = b->rx_frame_errors; + a->rx_fifo_errors = b->rx_fifo_errors; + a->rx_missed_errors = b->rx_missed_errors; + + a->tx_aborted_errors = b->tx_aborted_errors; + a->tx_carrier_errors = b->tx_carrier_errors; + a->tx_fifo_errors = b->tx_fifo_errors; + a->tx_heartbeat_errors = b->tx_heartbeat_errors; + a->tx_window_errors = b->tx_window_errors; + + a->rx_compressed = b->rx_compressed; + a->tx_compressed = b->tx_compressed; +} static inline int rtnl_vfinfo_size(const struct net_device *dev) { @@ -698,6 +730,14 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev, stats = dev_get_stats(dev); copy_rtnl_link_stats(nla_data(attr), stats); + attr = nla_reserve(skb, IFLA_STATS64, + sizeof(struct rtnl_link_stats64)); + if (attr == NULL) + goto nla_put_failure; + + stats = dev_get_stats(dev); + copy_rtnl_link_stats64(nla_data(attr), stats); + if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent) { int i; struct ifla_vf_info ivi;
`ip -s link` shows interface counters truncated to 32 bit. This is because interface statistics are transported only in 32-bit quantity to userspace. This commit adds a new IFLA_STATS64 attribute that exports them in full 64 bit. References: http://lkml.indiana.edu/hypermail/linux/kernel/0307.3/0215.html Signed-off-by: Jan Engelhardt <jengelh@medozas.de> --- include/linux/if_link.h | 33 +++++++++++++++++++++++++++++++++ net/core/rtnetlink.c | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletions(-)