* [PATCH 0/2] ethtool(1) support for reading Phy stats @ 2015-12-23 11:58 Andrew Lunn 2015-12-23 11:58 ` [PATCH 1/2] ethtool-copy.h: sync with net Andrew Lunn 2015-12-23 11:58 ` [PATCH 2/2] ethtool: Add PHY statistics support Andrew Lunn 0 siblings, 2 replies; 9+ messages in thread From: Andrew Lunn @ 2015-12-23 11:58 UTC (permalink / raw) To: Ben Hutchings; +Cc: Florian Fainelli, netdev, Andrew Lunn Add support to ethertool(1) for reading phy statistics. The mechanism is the same for reading MAC stats. Andrew Lunn (2): ethtool-copy.h: sync with net ethtool: Add PHY statistics support ethtool-copy.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++------ ethtool.8.in | 6 ++++++ ethtool.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 6 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] ethtool-copy.h: sync with net 2015-12-23 11:58 [PATCH 0/2] ethtool(1) support for reading Phy stats Andrew Lunn @ 2015-12-23 11:58 ` Andrew Lunn 2015-12-23 11:58 ` [PATCH 2/2] ethtool: Add PHY statistics support Andrew Lunn 1 sibling, 0 replies; 9+ messages in thread From: Andrew Lunn @ 2015-12-23 11:58 UTC (permalink / raw) To: Ben Hutchings; +Cc: Florian Fainelli, netdev, Andrew Lunn This covers kernel changes up to: commit eff3cddc222c88943ff515ae9335687c9e2cbaf6 Author: Jacob Keller <jacob.e.keller@intel.com> Date: Wed Apr 22 14:40:30 2015 -0700 clarify implementation of ethtool's get_ts_info op Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- ethtool-copy.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/ethtool-copy.h b/ethtool-copy.h index d23ffc4..57fa390 100644 --- a/ethtool-copy.h +++ b/ethtool-copy.h @@ -10,8 +10,8 @@ * Portions Copyright (C) Sun Microsystems 2008 */ -#ifndef _LINUX_ETHTOOL_H -#define _LINUX_ETHTOOL_H +#ifndef _UAPI_LINUX_ETHTOOL_H +#define _UAPI_LINUX_ETHTOOL_H #include <linux/types.h> #include <linux/if_ether.h> @@ -110,7 +110,7 @@ struct ethtool_cmd { __u32 reserved[2]; }; -static __inline__ void ethtool_cmd_speed_set(struct ethtool_cmd *ep, +static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep, __u32 speed) { @@ -118,7 +118,7 @@ static __inline__ void ethtool_cmd_speed_set(struct ethtool_cmd *ep, ep->speed_hi = (__u16)(speed >> 16); } -static __inline__ __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep) +static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep) { return (ep->speed_hi << 16) | ep->speed; } @@ -215,6 +215,11 @@ enum tunable_id { ETHTOOL_ID_UNSPEC, ETHTOOL_RX_COPYBREAK, ETHTOOL_TX_COPYBREAK, + /* + * Add your fresh new tubale attribute above and remember to update + * tunable_strings[] in net/core/ethtool.c + */ + __ETHTOOL_TUNABLE_COUNT, }; enum tunable_type_id { @@ -537,6 +542,7 @@ struct ethtool_pauseparam { * now deprecated * @ETH_SS_FEATURES: Device feature names * @ETH_SS_RSS_HASH_FUNCS: RSS hush function names + * @ETH_SS_PHY_STATS: Statistic names, for use with %ETHTOOL_GPHYSTATS */ enum ethtool_stringset { ETH_SS_TEST = 0, @@ -545,6 +551,8 @@ enum ethtool_stringset { ETH_SS_NTUPLE_FILTERS, ETH_SS_FEATURES, ETH_SS_RSS_HASH_FUNCS, + ETH_SS_TUNABLES, + ETH_SS_PHY_STATS, }; /** @@ -796,6 +804,31 @@ struct ethtool_rx_flow_spec { __u32 location; }; +/* How rings are layed out when accessing virtual functions or + * offloaded queues is device specific. To allow users to do flow + * steering and specify these queues the ring cookie is partitioned + * into a 32bit queue index with an 8 bit virtual function id. + * This also leaves the 3bytes for further specifiers. It is possible + * future devices may support more than 256 virtual functions if + * devices start supporting PCIe w/ARI. However at the moment I + * do not know of any devices that support this so I do not reserve + * space for this at this time. If a future patch consumes the next + * byte it should be aware of this possiblity. + */ +#define ETHTOOL_RX_FLOW_SPEC_RING 0x00000000FFFFFFFFLL +#define ETHTOOL_RX_FLOW_SPEC_RING_VF 0x000000FF00000000LL +#define ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF 32 +static inline __u64 ethtool_get_flow_spec_ring(__u64 ring_cookie) +{ + return ETHTOOL_RX_FLOW_SPEC_RING & ring_cookie; +}; + +static inline __u64 ethtool_get_flow_spec_ring_vf(__u64 ring_cookie) +{ + return (ETHTOOL_RX_FLOW_SPEC_RING_VF & ring_cookie) >> + ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF; +}; + /** * struct ethtool_rxnfc - command to get or set RX flow classification rules * @cmd: Specific command number - %ETHTOOL_GRXFH, %ETHTOOL_SRXFH, @@ -1062,6 +1095,11 @@ struct ethtool_sfeatures { * the 'hwtstamp_tx_types' and 'hwtstamp_rx_filters' enumeration values, * respectively. For example, if the device supports HWTSTAMP_TX_ON, * then (1 << HWTSTAMP_TX_ON) in 'tx_types' will be set. + * + * Drivers should only report the filters they actually support without + * upscaling in the SIOCSHWTSTAMP ioctl. If the SIOCSHWSTAMP request for + * HWTSTAMP_FILTER_V1_SYNC is supported by HWTSTAMP_FILTER_V1_EVENT, then the + * driver should only report HWTSTAMP_FILTER_V1_EVENT in this op. */ struct ethtool_ts_info { __u32 cmd; @@ -1189,6 +1227,7 @@ enum ethtool_sfeatures_retval_bits { #define ETHTOOL_SRSSH 0x00000047 /* Set RX flow hash configuration */ #define ETHTOOL_GTUNABLE 0x00000048 /* Get tunable configuration */ #define ETHTOOL_STUNABLE 0x00000049 /* Set tunable configuration */ +#define ETHTOOL_GPHYSTATS 0x0000004a /* get PHY-specific statistics */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET @@ -1264,15 +1303,19 @@ enum ethtool_sfeatures_retval_bits { * it was forced up into this mode or autonegotiated. */ -/* The forced speed, 10Mb, 100Mb, gigabit, [2.5|10|20|40|56]GbE. */ +/* The forced speed, 10Mb, 100Mb, gigabit, [2.5|5|10|20|25|40|50|56|100]GbE. */ #define SPEED_10 10 #define SPEED_100 100 #define SPEED_1000 1000 #define SPEED_2500 2500 +#define SPEED_5000 5000 #define SPEED_10000 10000 #define SPEED_20000 20000 +#define SPEED_25000 25000 #define SPEED_40000 40000 +#define SPEED_50000 50000 #define SPEED_56000 56000 +#define SPEED_100000 100000 #define SPEED_UNKNOWN -1 @@ -1398,4 +1441,4 @@ enum ethtool_reset_flags { }; #define ETH_RESET_SHARED_SHIFT 16 -#endif /* _LINUX_ETHTOOL_H */ +#endif /* _UAPI_LINUX_ETHTOOL_H */ -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] ethtool: Add PHY statistics support 2015-12-23 11:58 [PATCH 0/2] ethtool(1) support for reading Phy stats Andrew Lunn 2015-12-23 11:58 ` [PATCH 1/2] ethtool-copy.h: sync with net Andrew Lunn @ 2015-12-23 11:58 ` Andrew Lunn 2016-03-13 15:50 ` Ben Hutchings ` (2 more replies) 1 sibling, 3 replies; 9+ messages in thread From: Andrew Lunn @ 2015-12-23 11:58 UTC (permalink / raw) To: Ben Hutchings; +Cc: Florian Fainelli, netdev, Andrew Lunn This adds support for printing statistics from the network devices PHY. Signed-off-by: Andrew Lunn <andrew@lunn.ch> --- ethtool.8.in | 6 ++++++ ethtool.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/ethtool.8.in b/ethtool.8.in index eeffa70..2316556 100644 --- a/ethtool.8.in +++ b/ethtool.8.in @@ -220,6 +220,9 @@ ethtool \- query or control network driver and hardware settings .B ethtool \-S|\-\-statistics .I devname .HP +.B ethtool \-I|\-\-phy-statistics +.I devname +.HP .B ethtool \-t|\-\-test .I devname .RI [\*(SD] @@ -492,6 +495,9 @@ auto-negotiation is enabled. Queries the specified network device for NIC- and driver-specific statistics. .TP +.B \-I \-\-phy\-statistics +Queries the specified network device for PHY specific statistics. +.TP .B \-t \-\-test Executes adapter selftest on the specified network device. Possible test modes are: .TP diff --git a/ethtool.c b/ethtool.c index 92c40b8..480c14c 100644 --- a/ethtool.c +++ b/ethtool.c @@ -2995,6 +2995,64 @@ static int do_gstats(struct cmd_context *ctx) return 0; } +static int do_gphystats(struct cmd_context *ctx) +{ + struct ethtool_gstrings *strings; + struct ethtool_stats *stats; + unsigned int n_stats, sz_stats, i; + int err; + + if (ctx->argc != 0) + exit_bad_args(); + + strings = get_stringset(ctx, ETH_SS_PHY_STATS, + offsetof(struct ethtool_drvinfo, n_stats), + 0); + if (!strings) { + perror("Cannot get stats strings information"); + return 96; + } + + n_stats = strings->len; + if (n_stats < 1) { + fprintf(stderr, "no stats available\n"); + free(strings); + return 94; + } + + sz_stats = n_stats * sizeof(u64); + + stats = calloc(1, sz_stats + sizeof(struct ethtool_stats)); + if (!stats) { + fprintf(stderr, "no memory available\n"); + free(strings); + return 95; + } + + stats->cmd = ETHTOOL_GPHYSTATS; + stats->n_stats = n_stats; + err = send_ioctl(ctx, stats); + if (err < 0) { + perror("Cannot get stats information"); + free(strings); + free(stats); + return 97; + } + + /* todo - pretty-print the strings per-driver */ + fprintf(stdout, "PHY statistics:\n"); + for (i = 0; i < n_stats; i++) { + fprintf(stdout, " %.*s: %llu\n", + ETH_GSTRING_LEN, + &strings->data[i * ETH_GSTRING_LEN], + stats->data[i]); + } + free(strings); + free(stats); + + return 0; +} + static int do_srxntuple(struct cmd_context *ctx, struct ethtool_rx_flow_spec *rx_rule_fs); @@ -4078,6 +4136,8 @@ static const struct option { { "-t|--test", 1, do_test, "Execute adapter self test", " [ online | offline | external_lb ]\n" }, { "-S|--statistics", 1, do_gstats, "Show adapter statistics" }, + { "-I|--phy-statistics", 1, do_gphystats, + "Show phy statistics" }, { "-n|-u|--show-nfc|--show-ntuple", 1, do_grxclass, "Show Rx network flow classification options or rules", " [ rx-flow-hash tcp4|udp4|ah4|esp4|sctp4|" -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ethtool: Add PHY statistics support 2015-12-23 11:58 ` [PATCH 2/2] ethtool: Add PHY statistics support Andrew Lunn @ 2016-03-13 15:50 ` Ben Hutchings 2016-03-13 16:08 ` Andrew Lunn 2016-03-13 16:01 ` [PATCH ethtool 1/2] Remove short option -I for PHY statistics Ben Hutchings 2016-03-13 16:01 ` [PATCH ethtool 2/2] Refactor do_gstats() and do_gphystats() to avoid code duplication Ben Hutchings 2 siblings, 1 reply; 9+ messages in thread From: Ben Hutchings @ 2016-03-13 15:50 UTC (permalink / raw) To: Andrew Lunn; +Cc: Florian Fainelli, netdev [-- Attachment #1: Type: text/plain, Size: 3777 bytes --] On Wed, 2015-12-23 at 12:58 +0100, Andrew Lunn wrote: > This adds support for printing statistics from the network devices PHY. Sorry it's taken me so long to respond to this. There are a few issues with it but I'll apply it anyway then fix them up. > Signed-off-by: Andrew Lunn <andrew@lunn.ch> > --- > ethtool.8.in | 6 ++++++ > ethtool.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 66 insertions(+) > > diff --git a/ethtool.8.in b/ethtool.8.in > index eeffa70..2316556 100644 > --- a/ethtool.8.in > +++ b/ethtool.8.in > @@ -220,6 +220,9 @@ ethtool \- query or control network driver and hardware settings > .B ethtool \-S|\-\-statistics > .I devname > .HP > +.B ethtool \-I|\-\-phy-statistics > +.I devname > +.HP -I isn't a useful mnemonic, so I will drop the short option altogether. > .B ethtool \-t|\-\-test > .I devname > .RI [\*(SD] > @@ -492,6 +495,9 @@ auto-negotiation is enabled. > Queries the specified network device for NIC- and driver-specific > statistics. > .TP > +.B \-I \-\-phy\-statistics > +Queries the specified network device for PHY specific statistics. > +.TP > .B \-t \-\-test > Executes adapter selftest on the specified network device. Possible test modes are: > .TP > diff --git a/ethtool.c b/ethtool.c > index 92c40b8..480c14c 100644 > --- a/ethtool.c > +++ b/ethtool.c > @@ -2995,6 +2995,64 @@ static int do_gstats(struct cmd_context *ctx) > return 0; > } > > +static int do_gphystats(struct cmd_context *ctx) > +{ > + struct ethtool_gstrings *strings; > + struct ethtool_stats *stats; > + unsigned int n_stats, sz_stats, i; > + int err; > + > + if (ctx->argc != 0) > + exit_bad_args(); > + > + strings = get_stringset(ctx, ETH_SS_PHY_STATS, > + offsetof(struct ethtool_drvinfo, n_stats), > + 0); > + if (!strings) { > + perror("Cannot get stats strings information"); > + return 96; > + } > + > + n_stats = strings->len; > + if (n_stats < 1) { > + fprintf(stderr, "no stats available\n"); > + free(strings); > + return 94; > + } > + > + sz_stats = n_stats * sizeof(u64); > + > + stats = calloc(1, sz_stats + sizeof(struct ethtool_stats)); > + if (!stats) { > + fprintf(stderr, "no memory available\n"); > + free(strings); > + return 95; > + } > + > + stats->cmd = ETHTOOL_GPHYSTATS; > + stats->n_stats = n_stats; > + err = send_ioctl(ctx, stats); > + if (err < 0) { > + perror("Cannot get stats information"); > + free(strings); > + free(stats); > + return 97; > + } > + > + /* todo - pretty-print the strings per-driver */ > + fprintf(stdout, "PHY statistics:\n"); > + for (i = 0; i < n_stats; i++) { > + fprintf(stdout, " %.*s: %llu\n", > + ETH_GSTRING_LEN, > + &strings->data[i * ETH_GSTRING_LEN], > + stats->data[i]); > + } > + free(strings); > + free(stats); > + > + return 0; > +} This is basically a copy-paste of do_gstats() so they should be merged into one function. Ben. > static int do_srxntuple(struct cmd_context *ctx, > struct ethtool_rx_flow_spec *rx_rule_fs); > > @@ -4078,6 +4136,8 @@ static const struct option { > { "-t|--test", 1, do_test, "Execute adapter self test", > " [ online | offline | external_lb ]\n" }, > { "-S|--statistics", 1, do_gstats, "Show adapter statistics" }, > + { "-I|--phy-statistics", 1, do_gphystats, > + "Show phy statistics" }, > { "-n|-u|--show-nfc|--show-ntuple", 1, do_grxclass, > "Show Rx network flow classification options or rules", > " [ rx-flow-hash tcp4|udp4|ah4|esp4|sctp4|" -- Ben Hutchings If at first you don't succeed, you're doing about average. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ethtool: Add PHY statistics support 2016-03-13 15:50 ` Ben Hutchings @ 2016-03-13 16:08 ` Andrew Lunn 0 siblings, 0 replies; 9+ messages in thread From: Andrew Lunn @ 2016-03-13 16:08 UTC (permalink / raw) To: Ben Hutchings; +Cc: Florian Fainelli, netdev > > +.B ethtool \-I|\-\-phy-statistics > > +.I devname > > +.HP > > -I isn't a useful mnemonic, so I will drop the short option altogether. It was the best i could come up with. -P would of been nice, since we have -S for MAC statistics, but that is taken. PHY sounds a bit like I, so why not. But i'm O.K. for -I to be dropped. > > + /* todo - pretty-print the strings per-driver */ > > + fprintf(stdout, "PHY statistics:\n"); > > + for (i = 0; i < n_stats; i++) { > > + fprintf(stdout, " %.*s: %llu\n", > > + ETH_GSTRING_LEN, > > + &strings->data[i * ETH_GSTRING_LEN], > > + stats->data[i]); > > + } > > + free(strings); > > + free(stats); > > + > > + return 0; > > +} > > This is basically a copy-paste of do_gstats() so they should be merged > into one function. O.K, i can do that. Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH ethtool 1/2] Remove short option -I for PHY statistics 2015-12-23 11:58 ` [PATCH 2/2] ethtool: Add PHY statistics support Andrew Lunn 2016-03-13 15:50 ` Ben Hutchings @ 2016-03-13 16:01 ` Ben Hutchings 2016-03-13 16:09 ` Andrew Lunn 2016-03-13 16:01 ` [PATCH ethtool 2/2] Refactor do_gstats() and do_gphystats() to avoid code duplication Ben Hutchings 2 siblings, 1 reply; 9+ messages in thread From: Ben Hutchings @ 2016-03-13 16:01 UTC (permalink / raw) To: netdev; +Cc: Andrew Lunn It's not mnemonic and there's no requirement to have short options for every command. Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- ethtool.8.in | 4 ++-- ethtool.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ethtool.8.in b/ethtool.8.in index 2316556d65de..e44db99dc5d6 100644 --- a/ethtool.8.in +++ b/ethtool.8.in @@ -220,7 +220,7 @@ ethtool \- query or control network driver and hardware settings .B ethtool \-S|\-\-statistics .I devname .HP -.B ethtool \-I|\-\-phy-statistics +.B \-\-phy\-statistics .I devname .HP .B ethtool \-t|\-\-test @@ -495,7 +495,7 @@ auto-negotiation is enabled. Queries the specified network device for NIC- and driver-specific statistics. .TP -.B \-I \-\-phy\-statistics +.B \-\-phy\-statistics Queries the specified network device for PHY specific statistics. .TP .B \-t \-\-test diff --git a/ethtool.c b/ethtool.c index 480c14c8d30c..1c988f7d8a9d 100644 --- a/ethtool.c +++ b/ethtool.c @@ -4136,7 +4136,7 @@ static const struct option { { "-t|--test", 1, do_test, "Execute adapter self test", " [ online | offline | external_lb ]\n" }, { "-S|--statistics", 1, do_gstats, "Show adapter statistics" }, - { "-I|--phy-statistics", 1, do_gphystats, + { "--phy-statistics", 1, do_gphystats, "Show phy statistics" }, { "-n|-u|--show-nfc|--show-ntuple", 1, do_grxclass, "Show Rx network flow classification options or rules", ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH ethtool 1/2] Remove short option -I for PHY statistics 2016-03-13 16:01 ` [PATCH ethtool 1/2] Remove short option -I for PHY statistics Ben Hutchings @ 2016-03-13 16:09 ` Andrew Lunn 0 siblings, 0 replies; 9+ messages in thread From: Andrew Lunn @ 2016-03-13 16:09 UTC (permalink / raw) To: Ben Hutchings; +Cc: netdev On Sun, Mar 13, 2016 at 04:01:20PM +0000, Ben Hutchings wrote: > It's not mnemonic and there's no requirement to have short options > for every command. > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Thanks Andrew > --- > ethtool.8.in | 4 ++-- > ethtool.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/ethtool.8.in b/ethtool.8.in > index 2316556d65de..e44db99dc5d6 100644 > --- a/ethtool.8.in > +++ b/ethtool.8.in > @@ -220,7 +220,7 @@ ethtool \- query or control network driver and hardware settings > .B ethtool \-S|\-\-statistics > .I devname > .HP > -.B ethtool \-I|\-\-phy-statistics > +.B \-\-phy\-statistics > .I devname > .HP > .B ethtool \-t|\-\-test > @@ -495,7 +495,7 @@ auto-negotiation is enabled. > Queries the specified network device for NIC- and driver-specific > statistics. > .TP > -.B \-I \-\-phy\-statistics > +.B \-\-phy\-statistics > Queries the specified network device for PHY specific statistics. > .TP > .B \-t \-\-test > diff --git a/ethtool.c b/ethtool.c > index 480c14c8d30c..1c988f7d8a9d 100644 > --- a/ethtool.c > +++ b/ethtool.c > @@ -4136,7 +4136,7 @@ static const struct option { > { "-t|--test", 1, do_test, "Execute adapter self test", > " [ online | offline | external_lb ]\n" }, > { "-S|--statistics", 1, do_gstats, "Show adapter statistics" }, > - { "-I|--phy-statistics", 1, do_gphystats, > + { "--phy-statistics", 1, do_gphystats, > "Show phy statistics" }, > { "-n|-u|--show-nfc|--show-ntuple", 1, do_grxclass, > "Show Rx network flow classification options or rules", > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH ethtool 2/2] Refactor do_gstats() and do_gphystats() to avoid code duplication 2015-12-23 11:58 ` [PATCH 2/2] ethtool: Add PHY statistics support Andrew Lunn 2016-03-13 15:50 ` Ben Hutchings 2016-03-13 16:01 ` [PATCH ethtool 1/2] Remove short option -I for PHY statistics Ben Hutchings @ 2016-03-13 16:01 ` Ben Hutchings 2016-03-13 16:12 ` Andrew Lunn 2 siblings, 1 reply; 9+ messages in thread From: Ben Hutchings @ 2016-03-13 16:01 UTC (permalink / raw) To: netdev; +Cc: Andrew Lunn The new do_gphystats() function is almost exactly the same as do_gstats(), which is silly. * Add parameters to do_gstats() for the command number, string set number and heading * Introduce do_gnicstats() as a wrapper for do_gstats() that does what do_gstats() used to * Change do_gphystats() into a wrapper for do_gstats() Signed-off-by: Ben Hutchings <ben@decadent.org.uk> --- ethtool.c | 71 +++++++++++---------------------------------------------------- 1 file changed, 12 insertions(+), 59 deletions(-) diff --git a/ethtool.c b/ethtool.c index 1c988f7d8a9d..4f69a825849a 100644 --- a/ethtool.c +++ b/ethtool.c @@ -2937,7 +2937,8 @@ static int do_phys_id(struct cmd_context *ctx) return err; } -static int do_gstats(struct cmd_context *ctx) +static int do_gstats(struct cmd_context *ctx, int cmd, int stringset, + const char *name) { struct ethtool_gstrings *strings; struct ethtool_stats *stats; @@ -2947,7 +2948,7 @@ static int do_gstats(struct cmd_context *ctx) if (ctx->argc != 0) exit_bad_args(); - strings = get_stringset(ctx, ETH_SS_STATS, + strings = get_stringset(ctx, stringset, offsetof(struct ethtool_drvinfo, n_stats), 0); if (!strings) { @@ -2971,7 +2972,7 @@ static int do_gstats(struct cmd_context *ctx) return 95; } - stats->cmd = ETHTOOL_GSTATS; + stats->cmd = cmd; stats->n_stats = n_stats; err = send_ioctl(ctx, stats); if (err < 0) { @@ -2982,7 +2983,7 @@ static int do_gstats(struct cmd_context *ctx) } /* todo - pretty-print the strings per-driver */ - fprintf(stdout, "NIC statistics:\n"); + fprintf(stdout, "%s statistics:\n", name); for (i = 0; i < n_stats; i++) { fprintf(stdout, " %.*s: %llu\n", ETH_GSTRING_LEN, @@ -2995,62 +2996,14 @@ static int do_gstats(struct cmd_context *ctx) return 0; } -static int do_gphystats(struct cmd_context *ctx) +static int do_gnicstats(struct cmd_context *ctx) { - struct ethtool_gstrings *strings; - struct ethtool_stats *stats; - unsigned int n_stats, sz_stats, i; - int err; - - if (ctx->argc != 0) - exit_bad_args(); - - strings = get_stringset(ctx, ETH_SS_PHY_STATS, - offsetof(struct ethtool_drvinfo, n_stats), - 0); - if (!strings) { - perror("Cannot get stats strings information"); - return 96; - } - - n_stats = strings->len; - if (n_stats < 1) { - fprintf(stderr, "no stats available\n"); - free(strings); - return 94; - } - - sz_stats = n_stats * sizeof(u64); - - stats = calloc(1, sz_stats + sizeof(struct ethtool_stats)); - if (!stats) { - fprintf(stderr, "no memory available\n"); - free(strings); - return 95; - } - - stats->cmd = ETHTOOL_GPHYSTATS; - stats->n_stats = n_stats; - err = send_ioctl(ctx, stats); - if (err < 0) { - perror("Cannot get stats information"); - free(strings); - free(stats); - return 97; - } - - /* todo - pretty-print the strings per-driver */ - fprintf(stdout, "PHY statistics:\n"); - for (i = 0; i < n_stats; i++) { - fprintf(stdout, " %.*s: %llu\n", - ETH_GSTRING_LEN, - &strings->data[i * ETH_GSTRING_LEN], - stats->data[i]); - } - free(strings); - free(stats); + return do_gstats(ctx, ETHTOOL_GSTATS, ETH_SS_STATS, "NIC"); +} - return 0; +static int do_gphystats(struct cmd_context *ctx) +{ + return do_gstats(ctx, ETHTOOL_GPHYSTATS, ETH_SS_PHY_STATS, "PHY"); } static int do_srxntuple(struct cmd_context *ctx, @@ -4135,7 +4088,7 @@ static const struct option { " [ TIME-IN-SECONDS ]\n" }, { "-t|--test", 1, do_test, "Execute adapter self test", " [ online | offline | external_lb ]\n" }, - { "-S|--statistics", 1, do_gstats, "Show adapter statistics" }, + { "-S|--statistics", 1, do_gnicstats, "Show adapter statistics" }, { "--phy-statistics", 1, do_gphystats, "Show phy statistics" }, { "-n|-u|--show-nfc|--show-ntuple", 1, do_grxclass, ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH ethtool 2/2] Refactor do_gstats() and do_gphystats() to avoid code duplication 2016-03-13 16:01 ` [PATCH ethtool 2/2] Refactor do_gstats() and do_gphystats() to avoid code duplication Ben Hutchings @ 2016-03-13 16:12 ` Andrew Lunn 0 siblings, 0 replies; 9+ messages in thread From: Andrew Lunn @ 2016-03-13 16:12 UTC (permalink / raw) To: Ben Hutchings; +Cc: netdev On Sun, Mar 13, 2016 at 04:01:49PM +0000, Ben Hutchings wrote: > The new do_gphystats() function is almost exactly the same as > do_gstats(), which is silly. > > * Add parameters to do_gstats() for the command number, string set > number and heading > * Introduce do_gnicstats() as a wrapper for do_gstats() that does > what do_gstats() used to > * Change do_gphystats() into a wrapper for do_gstats() > > Signed-off-by: Ben Hutchings <ben@decadent.org.uk> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Thanks Andrew > --- > ethtool.c | 71 +++++++++++---------------------------------------------------- > 1 file changed, 12 insertions(+), 59 deletions(-) > > diff --git a/ethtool.c b/ethtool.c > index 1c988f7d8a9d..4f69a825849a 100644 > --- a/ethtool.c > +++ b/ethtool.c > @@ -2937,7 +2937,8 @@ static int do_phys_id(struct cmd_context *ctx) > return err; > } > > -static int do_gstats(struct cmd_context *ctx) > +static int do_gstats(struct cmd_context *ctx, int cmd, int stringset, > + const char *name) > { > struct ethtool_gstrings *strings; > struct ethtool_stats *stats; > @@ -2947,7 +2948,7 @@ static int do_gstats(struct cmd_context *ctx) > if (ctx->argc != 0) > exit_bad_args(); > > - strings = get_stringset(ctx, ETH_SS_STATS, > + strings = get_stringset(ctx, stringset, > offsetof(struct ethtool_drvinfo, n_stats), > 0); > if (!strings) { > @@ -2971,7 +2972,7 @@ static int do_gstats(struct cmd_context *ctx) > return 95; > } > > - stats->cmd = ETHTOOL_GSTATS; > + stats->cmd = cmd; > stats->n_stats = n_stats; > err = send_ioctl(ctx, stats); > if (err < 0) { > @@ -2982,7 +2983,7 @@ static int do_gstats(struct cmd_context *ctx) > } > > /* todo - pretty-print the strings per-driver */ > - fprintf(stdout, "NIC statistics:\n"); > + fprintf(stdout, "%s statistics:\n", name); > for (i = 0; i < n_stats; i++) { > fprintf(stdout, " %.*s: %llu\n", > ETH_GSTRING_LEN, > @@ -2995,62 +2996,14 @@ static int do_gstats(struct cmd_context *ctx) > return 0; > } > > -static int do_gphystats(struct cmd_context *ctx) > +static int do_gnicstats(struct cmd_context *ctx) > { > - struct ethtool_gstrings *strings; > - struct ethtool_stats *stats; > - unsigned int n_stats, sz_stats, i; > - int err; > - > - if (ctx->argc != 0) > - exit_bad_args(); > - > - strings = get_stringset(ctx, ETH_SS_PHY_STATS, > - offsetof(struct ethtool_drvinfo, n_stats), > - 0); > - if (!strings) { > - perror("Cannot get stats strings information"); > - return 96; > - } > - > - n_stats = strings->len; > - if (n_stats < 1) { > - fprintf(stderr, "no stats available\n"); > - free(strings); > - return 94; > - } > - > - sz_stats = n_stats * sizeof(u64); > - > - stats = calloc(1, sz_stats + sizeof(struct ethtool_stats)); > - if (!stats) { > - fprintf(stderr, "no memory available\n"); > - free(strings); > - return 95; > - } > - > - stats->cmd = ETHTOOL_GPHYSTATS; > - stats->n_stats = n_stats; > - err = send_ioctl(ctx, stats); > - if (err < 0) { > - perror("Cannot get stats information"); > - free(strings); > - free(stats); > - return 97; > - } > - > - /* todo - pretty-print the strings per-driver */ > - fprintf(stdout, "PHY statistics:\n"); > - for (i = 0; i < n_stats; i++) { > - fprintf(stdout, " %.*s: %llu\n", > - ETH_GSTRING_LEN, > - &strings->data[i * ETH_GSTRING_LEN], > - stats->data[i]); > - } > - free(strings); > - free(stats); > + return do_gstats(ctx, ETHTOOL_GSTATS, ETH_SS_STATS, "NIC"); > +} > > - return 0; > +static int do_gphystats(struct cmd_context *ctx) > +{ > + return do_gstats(ctx, ETHTOOL_GPHYSTATS, ETH_SS_PHY_STATS, "PHY"); > } > > static int do_srxntuple(struct cmd_context *ctx, > @@ -4135,7 +4088,7 @@ static const struct option { > " [ TIME-IN-SECONDS ]\n" }, > { "-t|--test", 1, do_test, "Execute adapter self test", > " [ online | offline | external_lb ]\n" }, > - { "-S|--statistics", 1, do_gstats, "Show adapter statistics" }, > + { "-S|--statistics", 1, do_gnicstats, "Show adapter statistics" }, > { "--phy-statistics", 1, do_gphystats, > "Show phy statistics" }, > { "-n|-u|--show-nfc|--show-ntuple", 1, do_grxclass, ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-03-13 16:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-23 11:58 [PATCH 0/2] ethtool(1) support for reading Phy stats Andrew Lunn 2015-12-23 11:58 ` [PATCH 1/2] ethtool-copy.h: sync with net Andrew Lunn 2015-12-23 11:58 ` [PATCH 2/2] ethtool: Add PHY statistics support Andrew Lunn 2016-03-13 15:50 ` Ben Hutchings 2016-03-13 16:08 ` Andrew Lunn 2016-03-13 16:01 ` [PATCH ethtool 1/2] Remove short option -I for PHY statistics Ben Hutchings 2016-03-13 16:09 ` Andrew Lunn 2016-03-13 16:01 ` [PATCH ethtool 2/2] Refactor do_gstats() and do_gphystats() to avoid code duplication Ben Hutchings 2016-03-13 16:12 ` Andrew Lunn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).