| Message ID | 1333293808-7729-1-git-send-email-richardcochran@gmail.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sun, 2012-04-01 at 17:23 +0200, Richard Cochran wrote: [...] > --- a/ethtool.c > +++ b/ethtool.c > @@ -38,6 +38,7 @@ > #include <netinet/in.h> > #include <arpa/inet.h> > > +#include <linux/net_tstamp.h> ethtool is meant to be compilable on quite old systems, so we can't unconditionally include this header. Perhaps we should carry a copy of it, like ethtool-copy.h? > #include <linux/sockios.h> > > #ifndef MAX_ADDR_LEN > @@ -1115,6 +1116,91 @@ static int dump_rxfhash(int fhash, u64 val) > return 0; > } > > +static int dump_tsinfo(const struct ethtool_ts_info *info) > +{ > + if (info->so_timestamping & SOF_TIMESTAMPING_TX_HARDWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_TX_HARDWARE\n"); > + > + if (info->so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_TX_SOFTWARE\n"); > + > + if (info->so_timestamping & SOF_TIMESTAMPING_RX_HARDWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_RX_HARDWARE\n"); > + > + if (info->so_timestamping & SOF_TIMESTAMPING_RX_SOFTWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_RX_SOFTWARE\n"); > + > + if (info->so_timestamping & SOF_TIMESTAMPING_SOFTWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_SOFTWARE\n"); > + > + if (info->so_timestamping & SOF_TIMESTAMPING_SYS_HARDWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_SYS_HARDWARE\n"); > + > + if (info->so_timestamping & SOF_TIMESTAMPING_RAW_HARDWARE) > + fprintf(stdout, "SOF_TIMESTAMPING_RAW_HARDWARE\n"); I would prefer for these to be shown as more human-understandable capabilities. Include the flag name in parentheses if you like. > + if (info->phc_index < 0) > + fprintf(stdout, "No PTP Hardware Clock\n"); > + else > + fprintf(stdout, "PTP Hardware Clock %d\n", info->phc_index); ethtool generally reports settings as 'name: value' so how about "PTP Hardware Clock: none" and "PTP Hardware Clock: %d"? > + if (info->tx_types & (1 << HWTSTAMP_TX_OFF)) > + fprintf(stdout, "HWTSTAMP_TX_OFF\n"); > + > + if (info->tx_types & (1 << HWTSTAMP_TX_ON)) > + fprintf(stdout, "HWTSTAMP_TX_ON\n"); > + > + if (info->tx_types & (1 << HWTSTAMP_TX_ONESTEP_SYNC)) > + fprintf(stdout, "HWTSTAMP_TX_ONESTEP_SYNC\n"); If there is no hardware timestamping available then shouldn't we see HWTSTAMP_TX_OFF reported here? But that's going to be the case. Not sure whether the kernel or the ethtool utility should deal with that, but don't assume the ethtool utility is the only consumer. > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_NONE)) > + fprintf(stdout, "HWTSTAMP_FILTER_NONE\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_ALL)) > + fprintf(stdout, "HWTSTAMP_FILTER_ALL\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_SOME)) > + fprintf(stdout, "HWTSTAMP_FILTER_SOME\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V1_L4_EVENT)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V1_L4_EVENT\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V1_L4_SYNC\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L4_EVENT)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L4_EVENT\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L4_SYNC)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L4_SYNC\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L2_EVENT\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L2_SYNC)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L2_SYNC\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_EVENT)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_EVENT\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_SYNC)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_SYNC\n"); > + > + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ)) > + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_DELAY_REQ\n"); Similarly, shouldn't we see HWTSTAMP_FILTER_NONE reported here? I think each of the capability lists should have its own heading. Combining all of my suggestions, I think you should see for example: Capabilities: receive-stack (SOF_TIMESTAMPING_RX_SOFTWARE) transmit-stack (SOF_TIMESTAMPING_SOFTWARE) transmit-driver (SOF_TIMESTAMPING_TX_SOFTWARE) PTP Hardware Clock: none Hardware Transmit Modes: off (HWTSTAMP_TX_OFF) Hardware Receive Filter Modes: none (HWTSTAMP_FILTER_NONE) or: Capabilities: software-transmit (SOF_TIMESTAMPING_TX_SOFTWARE) software-receive (SOF_TIMESTAMPING_RX_SOFTWARE) software-system-clock (SOF_TIMESTAMPING_SOFTWARE) hardware-transmit (SOF_TIMESTAMPING_TX_HARDWARE) hardware-receive (SOF_TIMESTAMPING_RX_HARDWARE) hardware-system-clock (SOF_TIMESTAMPING_SYS_HARDWARE) PTP Hardware Clock: 1 Hardware Transmit Modes: off (HWTSTAMP_TX_OFF) on (HWTSTAMP_TX_ON) one-step (HWTSTAMP_TX_ONESTEP_SYNC) Hardware Receive Filter Modes: none (HWTSTAMP_FILTER_NONE) ptpv2-l4-event (HWTSTAMP_FILTER_PTP_V2_L4_EVENT) ptpv2-l4-sync (HWTSTAMP_FILTER_PTP_V2_L4_SYNC) (Maybe you can think of better names.) > + return 0; > +} > + > static struct ethtool_gstrings * > get_stringset(struct cmd_context *ctx, enum ethtool_stringset set_id, > ptrdiff_t drvinfo_offset) > @@ -3077,6 +3163,23 @@ static int do_sprivflags(struct cmd_context *ctx) > return 0; > } > > +static int do_tsinfo(struct cmd_context *ctx) > +{ > + struct ethtool_ts_info info; > + > + if (ctx->argc != 0) > + exit_bad_args(); > + > + fprintf(stdout, "Time stamping parameters for %s:\n", ctx->devname); > + info.cmd = ETHTOOL_GET_TS_INFO; > + if (send_ioctl(ctx, &info)) { > + perror("Cannot get device time stamping settings"); > + return -1; > + } > + dump_tsinfo(&info); > + return 0; > +} > + > int send_ioctl(struct cmd_context *ctx, void *cmd) > { > #ifndef TEST_ETHTOOL > @@ -3206,6 +3309,7 @@ static const struct option { > " [ action %d ]\n" > " [ loc %d]] |\n" > " delete %d\n" }, > + { "-T|--timestamping", 1, do_tsinfo, "Show time stamping options" }, Surely 'capabilities'. You need to add this new option to the manual page and unit tests (test-cmdline.c) as well. Ben. > { "-x|--show-rxfh-indir", 1, do_grxfhindir, > "Show Rx flow hash indirection" }, > { "-X|--set-rxfh-indir", 1, do_srxfhindir,
On Mon, 2012-04-02 at 20:25 +0100, Ben Hutchings wrote: > On Sun, 2012-04-01 at 17:23 +0200, Richard Cochran wrote: [...] > > + if (info->tx_types & (1 << HWTSTAMP_TX_OFF)) > > + fprintf(stdout, "HWTSTAMP_TX_OFF\n"); > > + > > + if (info->tx_types & (1 << HWTSTAMP_TX_ON)) > > + fprintf(stdout, "HWTSTAMP_TX_ON\n"); > > + > > + if (info->tx_types & (1 << HWTSTAMP_TX_ONESTEP_SYNC)) > > + fprintf(stdout, "HWTSTAMP_TX_ONESTEP_SYNC\n"); > > If there is no hardware timestamping available then shouldn't we see > HWTSTAMP_TX_OFF reported here? But that's going to be the case. Not +not > sure whether the kernel or the ethtool utility should deal with that, > but don't assume the ethtool utility is the only consumer. [...]
On Mon, Apr 02, 2012 at 08:25:58PM +0100, Ben Hutchings wrote: > On Sun, 2012-04-01 at 17:23 +0200, Richard Cochran wrote: > > + if (info->tx_types & (1 << HWTSTAMP_TX_OFF)) > > + fprintf(stdout, "HWTSTAMP_TX_OFF\n"); > > + > > + if (info->tx_types & (1 << HWTSTAMP_TX_ON)) > > + fprintf(stdout, "HWTSTAMP_TX_ON\n"); > > + > > + if (info->tx_types & (1 << HWTSTAMP_TX_ONESTEP_SYNC)) > > + fprintf(stdout, "HWTSTAMP_TX_ONESTEP_SYNC\n"); > > If there is no hardware timestamping available then shouldn't we see > HWTSTAMP_TX_OFF reported here? But that's going to be the case. Not > sure whether the kernel or the ethtool utility should deal with that, > but don't assume the ethtool utility is the only consumer. Actually, if the device does not support hardware time stamping at all, then ioctl SIOCSHWTSTAMP returns EOPNOTSUPP. So, HWTSTAMP_TX_ON means you can turn Tx time stamping on, and HWTSTAMP_TX_OFF means you can turn it off again. It might not seem too logical, but that is how the interface works. So (tx_types == 0) means no hardware timestamping available. We can print that as a special case, if you think it best. > Hardware Receive Filter Modes: > none (HWTSTAMP_FILTER_NONE) > ptpv2-l4-event (HWTSTAMP_FILTER_PTP_V2_L4_EVENT) > ptpv2-l4-sync (HWTSTAMP_FILTER_PTP_V2_L4_SYNC) > > (Maybe you can think of better names.) Okay, I get the idea. Thanks, Richard -- 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 Tue, 2012-04-03 at 07:37 +0200, Richard Cochran wrote: > On Mon, Apr 02, 2012 at 08:25:58PM +0100, Ben Hutchings wrote: > > On Sun, 2012-04-01 at 17:23 +0200, Richard Cochran wrote: > > > > + if (info->tx_types & (1 << HWTSTAMP_TX_OFF)) > > > + fprintf(stdout, "HWTSTAMP_TX_OFF\n"); > > > + > > > + if (info->tx_types & (1 << HWTSTAMP_TX_ON)) > > > + fprintf(stdout, "HWTSTAMP_TX_ON\n"); > > > + > > > + if (info->tx_types & (1 << HWTSTAMP_TX_ONESTEP_SYNC)) > > > + fprintf(stdout, "HWTSTAMP_TX_ONESTEP_SYNC\n"); > > > > If there is no hardware timestamping available then shouldn't we see > > HWTSTAMP_TX_OFF reported here? But that's going to be the case. Not > > sure whether the kernel or the ethtool utility should deal with that, > > but don't assume the ethtool utility is the only consumer. > > Actually, if the device does not support hardware time stamping at > all, then ioctl SIOCSHWTSTAMP returns EOPNOTSUPP. So, HWTSTAMP_TX_ON > means you can turn Tx time stamping on, and HWTSTAMP_TX_OFF means you > can turn it off again. It might not seem too logical, but that is how > the interface works. OK, this makes sense. > So (tx_types == 0) means no hardware timestamping available. We can > print that as a special case, if you think it best. [...] I think that from an administrator's point of view it would make more sense to see that hardware timestamping is always off. But from a developer's point of view I can see it would make no sense to show a mode/filter name which cannot actually be used. So you could show tx_types == 0 or rx_filter == 0 using (1) no heading and no list or (2) heading and a dummy entry (e.g. 'N/A'). Or maybe you can think of something better. Ben.
diff --git a/ethtool-copy.h b/ethtool-copy.h index d904c1a..dd93cc5 100644 --- a/ethtool-copy.h +++ b/ethtool-copy.h @@ -680,6 +680,24 @@ struct ethtool_sfeatures { struct ethtool_set_features_block features[0]; }; +/** + * struct ethtool_ts_info - holds a device's timestamping and PHC association + * @cmd: command number = %ETHTOOL_GET_TS_INFO + * @so_timestamping: bit mask of SO_TIMESTAMPING modes supported by the device + * @phc_index: device index of the associated PHC, or -1 if there is none + * @tx_types: bit mask of hwtstamp_tx_types modes supported by the device + * @rx_filters: bit mask of hwtstamp_rx_filters modes supported by the device + */ +struct ethtool_ts_info { + __u32 cmd; + __u32 so_timestamping; + __s32 phc_index; + __u32 tx_types; + __u32 tx_reserved[3]; + __u32 rx_filters; + __u32 rx_reserved[3]; +}; + /* * %ETHTOOL_SFEATURES changes features present in features[].valid to the * values of corresponding bits in features[].requested. Bits in .requested @@ -786,6 +804,7 @@ enum ethtool_sfeatures_retval_bits { #define ETHTOOL_SET_DUMP 0x0000003e /* Set dump settings */ #define ETHTOOL_GET_DUMP_FLAG 0x0000003f /* Get dump settings */ #define ETHTOOL_GET_DUMP_DATA 0x00000040 /* Get dump data */ +#define ETHTOOL_GET_TS_INFO 0x00000041 /* Get time stamping and PHC info */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET diff --git a/ethtool.c b/ethtool.c index e80b38b..bef8aa7 100644 --- a/ethtool.c +++ b/ethtool.c @@ -38,6 +38,7 @@ #include <netinet/in.h> #include <arpa/inet.h> +#include <linux/net_tstamp.h> #include <linux/sockios.h> #ifndef MAX_ADDR_LEN @@ -1115,6 +1116,91 @@ static int dump_rxfhash(int fhash, u64 val) return 0; } +static int dump_tsinfo(const struct ethtool_ts_info *info) +{ + if (info->so_timestamping & SOF_TIMESTAMPING_TX_HARDWARE) + fprintf(stdout, "SOF_TIMESTAMPING_TX_HARDWARE\n"); + + if (info->so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE) + fprintf(stdout, "SOF_TIMESTAMPING_TX_SOFTWARE\n"); + + if (info->so_timestamping & SOF_TIMESTAMPING_RX_HARDWARE) + fprintf(stdout, "SOF_TIMESTAMPING_RX_HARDWARE\n"); + + if (info->so_timestamping & SOF_TIMESTAMPING_RX_SOFTWARE) + fprintf(stdout, "SOF_TIMESTAMPING_RX_SOFTWARE\n"); + + if (info->so_timestamping & SOF_TIMESTAMPING_SOFTWARE) + fprintf(stdout, "SOF_TIMESTAMPING_SOFTWARE\n"); + + if (info->so_timestamping & SOF_TIMESTAMPING_SYS_HARDWARE) + fprintf(stdout, "SOF_TIMESTAMPING_SYS_HARDWARE\n"); + + if (info->so_timestamping & SOF_TIMESTAMPING_RAW_HARDWARE) + fprintf(stdout, "SOF_TIMESTAMPING_RAW_HARDWARE\n"); + + if (info->phc_index < 0) + fprintf(stdout, "No PTP Hardware Clock\n"); + else + fprintf(stdout, "PTP Hardware Clock %d\n", info->phc_index); + + if (info->tx_types & (1 << HWTSTAMP_TX_OFF)) + fprintf(stdout, "HWTSTAMP_TX_OFF\n"); + + if (info->tx_types & (1 << HWTSTAMP_TX_ON)) + fprintf(stdout, "HWTSTAMP_TX_ON\n"); + + if (info->tx_types & (1 << HWTSTAMP_TX_ONESTEP_SYNC)) + fprintf(stdout, "HWTSTAMP_TX_ONESTEP_SYNC\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_NONE)) + fprintf(stdout, "HWTSTAMP_FILTER_NONE\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_ALL)) + fprintf(stdout, "HWTSTAMP_FILTER_ALL\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_SOME)) + fprintf(stdout, "HWTSTAMP_FILTER_SOME\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V1_L4_EVENT)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V1_L4_EVENT\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V1_L4_SYNC)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V1_L4_SYNC\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L4_EVENT)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L4_EVENT\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L4_SYNC)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L4_SYNC\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L2_EVENT)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L2_EVENT\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L2_SYNC)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L2_SYNC\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_EVENT)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_EVENT\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_SYNC)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_SYNC\n"); + + if (info->rx_filters & (1 << HWTSTAMP_FILTER_PTP_V2_DELAY_REQ)) + fprintf(stdout, "HWTSTAMP_FILTER_PTP_V2_DELAY_REQ\n"); + + return 0; +} + static struct ethtool_gstrings * get_stringset(struct cmd_context *ctx, enum ethtool_stringset set_id, ptrdiff_t drvinfo_offset) @@ -3077,6 +3163,23 @@ static int do_sprivflags(struct cmd_context *ctx) return 0; } +static int do_tsinfo(struct cmd_context *ctx) +{ + struct ethtool_ts_info info; + + if (ctx->argc != 0) + exit_bad_args(); + + fprintf(stdout, "Time stamping parameters for %s:\n", ctx->devname); + info.cmd = ETHTOOL_GET_TS_INFO; + if (send_ioctl(ctx, &info)) { + perror("Cannot get device time stamping settings"); + return -1; + } + dump_tsinfo(&info); + return 0; +} + int send_ioctl(struct cmd_context *ctx, void *cmd) { #ifndef TEST_ETHTOOL @@ -3206,6 +3309,7 @@ static const struct option { " [ action %d ]\n" " [ loc %d]] |\n" " delete %d\n" }, + { "-T|--timestamping", 1, do_tsinfo, "Show time stamping options" }, { "-x|--show-rxfh-indir", 1, do_grxfhindir, "Show Rx flow hash indirection" }, { "-X|--set-rxfh-indir", 1, do_srxfhindir,
Signed-off-by: Richard Cochran <richardcochran@gmail.com> --- ethtool-copy.h | 19 ++++++++++ ethtool.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 0 deletions(-)