diff mbox series

[ethtool,v2] ethtool: add support show/set-time-stamping

Message ID 20200903140714.1781654-1-yyd@google.com
State Changes Requested
Delegated to: Michal Kubecek
Headers show
Series [ethtool,v2] ethtool: add support show/set-time-stamping | expand

Commit Message

Kevin(Yudong) Yang Sept. 3, 2020, 2:07 p.m. UTC
Before this patch, ethtool has -T/--show-time-stamping that only
shows the device's time stamping capabilities but not the time
stamping policy that is used by the device.

This patch adds support to set/get device time stamping policy at
the driver level by calling ioctl(SIOCSHWTSTAMP).

Tested: ran following cmds on a Mellanox NIC with mlx4_en driver:
./ethtool -T eth1
...
Hardware Transmit Timestamp Modes:
        off                   (HWTSTAMP_TX_OFF)
        on                    (HWTSTAMP_TX_ON)
Hardware Receive Filter Modes:
        none                  (HWTSTAMP_FILTER_NONE)
        all                   (HWTSTAMP_FILTER_ALL)
Hardware Timestamping Policy:
        Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
        Tx type 0, off                   (HWTSTAMP_TX_OFF)

./ethtool --set-time-stamping eth1 rx 1; ./ethtool -T eth1;
...
Hardware Timestamping Policy:
	Rx filter 1, all                   (HWTSTAMP_FILTER_ALL)
	Tx type 0, off                   (HWTSTAMP_TX_OFF)

./ethtool --set-time-stamping eth1 rx 1 tx 1; ./ethtool -T eth1;
rx unmodified, ignoring
...
Hardware Timestamping Policy:
	Rx filter 1, all                   (HWTSTAMP_FILTER_ALL)
	Tx type 1, on                    (HWTSTAMP_TX_ON)

./ethtool --set-time-stamping eth1 rx 0; ./ethtool -T eth1;
...
Hardware Timestamping Policy:
	Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
	Tx type 1, on                    (HWTSTAMP_TX_ON)

./ethtool --set-time-stamping eth1 tx 0; ./ethtool -T eth1
...
Hardware Timestamping Policy:
	Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
	Tx type 0, off                   (HWTSTAMP_TX_OFF)

./ethtool --set-time-stamping eth1 rx 123 tx 456
rx should be in [0..15], tx should be in [0..2]

Signed-off-by: Kevin Yang <yyd@google.com>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
---
 ethtool.8.in | 10 +++++-
 ethtool.c    | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 94 insertions(+), 2 deletions(-)

Comments

Michal Kubecek Sept. 7, 2020, 12:53 p.m. UTC | #1
On Thu, Sep 03, 2020 at 10:07:14AM -0400, Kevin(Yudong) Yang wrote:
> Before this patch, ethtool has -T/--show-time-stamping that only
> shows the device's time stamping capabilities but not the time
> stamping policy that is used by the device.
> 
> This patch adds support to set/get device time stamping policy at
> the driver level by calling ioctl(SIOCSHWTSTAMP).
> 
> Tested: ran following cmds on a Mellanox NIC with mlx4_en driver:
> ./ethtool -T eth1
> ...
> Hardware Transmit Timestamp Modes:
>         off                   (HWTSTAMP_TX_OFF)
>         on                    (HWTSTAMP_TX_ON)
> Hardware Receive Filter Modes:
>         none                  (HWTSTAMP_FILTER_NONE)
>         all                   (HWTSTAMP_FILTER_ALL)
> Hardware Timestamping Policy:
>         Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
>         Tx type 0, off                   (HWTSTAMP_TX_OFF)
> 
> ./ethtool --set-time-stamping eth1 rx 1; ./ethtool -T eth1;
> ...
> Hardware Timestamping Policy:
> 	Rx filter 1, all                   (HWTSTAMP_FILTER_ALL)
> 	Tx type 0, off                   (HWTSTAMP_TX_OFF)
> 
> ./ethtool --set-time-stamping eth1 rx 1 tx 1; ./ethtool -T eth1;
> rx unmodified, ignoring
> ...
> Hardware Timestamping Policy:
> 	Rx filter 1, all                   (HWTSTAMP_FILTER_ALL)
> 	Tx type 1, on                    (HWTSTAMP_TX_ON)
> 
> ./ethtool --set-time-stamping eth1 rx 0; ./ethtool -T eth1;
> ...
> Hardware Timestamping Policy:
> 	Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
> 	Tx type 1, on                    (HWTSTAMP_TX_ON)
> 
> ./ethtool --set-time-stamping eth1 tx 0; ./ethtool -T eth1
> ...
> Hardware Timestamping Policy:
> 	Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
> 	Tx type 0, off                   (HWTSTAMP_TX_OFF)
> 
> ./ethtool --set-time-stamping eth1 rx 123 tx 456
> rx should be in [0..15], tx should be in [0..2]
> 
> Signed-off-by: Kevin Yang <yyd@google.com>
> Reviewed-by: Neal Cardwell <ncardwell@google.com>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
> ---

As I said in response to v1 patch, I don't like the idea of adding a new
ioctl interface to ethool when we are working on replacing and
deprecating the existing ones. Is there a strong reason why this feature
shouldn't be implemented using netlink?

Michal

>  ethtool.8.in | 10 +++++-
>  ethtool.c    | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in
> index a50a476..9930804 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -315,6 +315,11 @@ ethtool \- query or control network driver and hardware settings
>  .B ethtool \-T|\-\-show\-time\-stamping
>  .I devname
>  .HP
> +.B ethtool \-\-set\-time\-stamping
> +.I devname
> +.BN rx
> +.BN tx
> +.HP
>  .B ethtool \-x|\-\-show\-rxfh\-indir|\-\-show\-rxfh
>  .I devname
>  .HP
> @@ -1023,9 +1028,12 @@ is indicated, then ethtool fetches the dump data and directs it to a
>  Sets the dump flag for the device.
>  .TP
>  .B \-T \-\-show\-time\-stamping
> -Show the device's time stamping capabilities and associated PTP
> +Show the device's time stamping capabilities and policy and associated PTP
>  hardware clock.
>  .TP
> +.B \-\-set\-time\-stamping
> +Set the device's time stamping policy at the driver level.
> +.TP
>  .B \-x \-\-show\-rxfh\-indir \-\-show\-rxfh
>  Retrieves the receive flow hash indirection table and/or RSS hash key.
>  .TP
> diff --git a/ethtool.c b/ethtool.c
> index 606af3e..039b9a9 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -1618,6 +1618,24 @@ static int dump_tsinfo(const struct ethtool_ts_info *info)
>  	return 0;
>  }
>  
> +static int dump_hwtstamp_config(const struct hwtstamp_config *cfg)
> +{
> +	fprintf(stdout, "Hardware Timestamping Policy:\n");
> +	fprintf(stdout, "\tRx filter %d", cfg->rx_filter);
> +	if (cfg->rx_filter < N_RX_FILTERS)
> +		fprintf(stdout, ", %s\n", rx_filter_labels[cfg->rx_filter]);
> +	else
> +		fprintf(stdout, "\n");
> +
> +	fprintf(stdout, "\tTx type %d", cfg->tx_type);
> +	if (cfg->tx_type < N_TX_TYPES)
> +		fprintf(stdout, ", %s\n", tx_type_labels[cfg->tx_type]);
> +	else
> +		fprintf(stdout, "\n");
> +
> +	return 0;
> +}
> +
>  static struct ethtool_gstrings *
>  get_stringset(struct cmd_context *ctx, enum ethtool_stringset set_id,
>  	      ptrdiff_t drvinfo_offset, int null_terminate)
> @@ -4705,6 +4723,7 @@ err:
>  static int do_tsinfo(struct cmd_context *ctx)
>  {
>  	struct ethtool_ts_info info;
> +	struct hwtstamp_config cfg = { 0 };
>  
>  	if (ctx->argc != 0)
>  		exit_bad_args();
> @@ -4716,6 +4735,64 @@ static int do_tsinfo(struct cmd_context *ctx)
>  		return -1;
>  	}
>  	dump_tsinfo(&info);
> +
> +	ctx->ifr.ifr_data = (void *)&cfg;
> +	if (ioctl(ctx->fd, SIOCGHWTSTAMP, &ctx->ifr)) {
> +		perror("Cannot get device time stamping policy");
> +		return -1;
> +	}
> +	dump_hwtstamp_config(&cfg);
> +
> +	return 0;
> +}
> +
> +static int do_shwtstamp(struct cmd_context *ctx)
> +{
> +	struct hwtstamp_config cfg = { .rx_filter = -1, .tx_type = -1 },
> +			       pre_cfg;
> +	int changed = 0;
> +	struct cmdline_info cmdline_config[] = {
> +		{
> +			.name		= "rx",
> +			.type		= CMDL_S32,
> +			.wanted_val	= &cfg.rx_filter,
> +			.ioctl_val	= &pre_cfg.rx_filter,
> +		},
> +		{
> +			.name		= "tx",
> +			.type		= CMDL_S32,
> +			.wanted_val	= &cfg.tx_type,
> +			.ioctl_val	= &pre_cfg.tx_type,
> +		},
> +	};
> +
> +	parse_generic_cmdline(ctx, &changed,
> +			      cmdline_config, ARRAY_SIZE(cmdline_config));
> +
> +	ctx->ifr.ifr_data = (void *)&pre_cfg;
> +	if (ioctl(ctx->fd, SIOCGHWTSTAMP, &ctx->ifr)) {
> +		perror("Cannot get device time stamping settings");
> +		return -1;
> +	}
> +
> +	changed = 0;
> +	do_generic_set(cmdline_config, ARRAY_SIZE(cmdline_config), &changed);
> +	if (!changed) {
> +		fprintf(stderr, "no time-stamping policy changed.\n");
> +		return 0;
> +	}
> +
> +	if (cfg.tx_type >= N_TX_TYPES || cfg.rx_filter >= N_RX_FILTERS) {
> +		fprintf(stderr,
> +			"rx should be in [0..%d], tx should be in [0..%d]\n",
> +			N_RX_FILTERS - 1, N_TX_TYPES - 1);
> +		return -1;
> +	}
> +
> +	if (ioctl(ctx->fd, SIOCSHWTSTAMP, &ctx->ifr)) {
> +		perror("Cannot set device time stamping settings");
> +		return -1;
> +	}
>  	return 0;
>  }
>  
> @@ -5733,7 +5810,14 @@ static const struct option args[] = {
>  		.opts	= "-T|--show-time-stamping",
>  		.func	= do_tsinfo,
>  		.nlfunc	= nl_tsinfo,
> -		.help	= "Show time stamping capabilities"
> +		.help	= "Show time stamping capabilities and policy"
> +	},
> +	{
> +		.opts	= "--set-time-stamping",
> +		.func	= do_shwtstamp,
> +		.help	= "Set time stamping policy at the driver level",
> +		.xhelp	= "		[ rx N ]\n"
> +			  "		[ tx N ]\n"
>  	},
>  	{
>  		.opts	= "-x|--show-rxfh-indir|--show-rxfh",
> -- 
> 2.28.0.402.g5ffc5be6b7-goog
>
Eric Dumazet Sept. 7, 2020, 4:56 p.m. UTC | #2
On Mon, Sep 7, 2020 at 2:53 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Thu, Sep 03, 2020 at 10:07:14AM -0400, Kevin(Yudong) Yang wrote:
> > Before this patch, ethtool has -T/--show-time-stamping that only
> > shows the device's time stamping capabilities but not the time
> > stamping policy that is used by the device.
> >
> > This patch adds support to set/get device time stamping policy at
> > the driver level by calling ioctl(SIOCSHWTSTAMP).
> >
> > Tested: ran following cmds on a Mellanox NIC with mlx4_en driver:
> > ./ethtool -T eth1
> > ...
> > Hardware Transmit Timestamp Modes:
> >         off                   (HWTSTAMP_TX_OFF)
> >         on                    (HWTSTAMP_TX_ON)
> > Hardware Receive Filter Modes:
> >         none                  (HWTSTAMP_FILTER_NONE)
> >         all                   (HWTSTAMP_FILTER_ALL)
> > Hardware Timestamping Policy:
> >         Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
> >         Tx type 0, off                   (HWTSTAMP_TX_OFF)
> >
> > ./ethtool --set-time-stamping eth1 rx 1; ./ethtool -T eth1;
> > ...
> > Hardware Timestamping Policy:
> >       Rx filter 1, all                   (HWTSTAMP_FILTER_ALL)
> >       Tx type 0, off                   (HWTSTAMP_TX_OFF)
> >
> > ./ethtool --set-time-stamping eth1 rx 1 tx 1; ./ethtool -T eth1;
> > rx unmodified, ignoring
> > ...
> > Hardware Timestamping Policy:
> >       Rx filter 1, all                   (HWTSTAMP_FILTER_ALL)
> >       Tx type 1, on                    (HWTSTAMP_TX_ON)
> >
> > ./ethtool --set-time-stamping eth1 rx 0; ./ethtool -T eth1;
> > ...
> > Hardware Timestamping Policy:
> >       Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
> >       Tx type 1, on                    (HWTSTAMP_TX_ON)
> >
> > ./ethtool --set-time-stamping eth1 tx 0; ./ethtool -T eth1
> > ...
> > Hardware Timestamping Policy:
> >       Rx filter 0, none                  (HWTSTAMP_FILTER_NONE)
> >       Tx type 0, off                   (HWTSTAMP_TX_OFF)
> >
> > ./ethtool --set-time-stamping eth1 rx 123 tx 456
> > rx should be in [0..15], tx should be in [0..2]
> >
> > Signed-off-by: Kevin Yang <yyd@google.com>
> > Reviewed-by: Neal Cardwell <ncardwell@google.com>
> > Reviewed-by: Eric Dumazet <edumazet@google.com>
> > ---
>
> As I said in response to v1 patch, I don't like the idea of adding a new
> ioctl interface to ethool when we are working on replacing and
> deprecating the existing ones. Is there a strong reason why this feature
> shouldn't be implemented using netlink?

I do not think this is a fair request.

All known kernels support the ioctl(), none of them support netlink so far.

Are you working on the netlink interface, or are you requesting us to
implement it ?

The ioctl has been added years ago, and Kevin patch is reasonable enough.

Thank you.
Michal Kubecek Sept. 7, 2020, 9:25 p.m. UTC | #3
On Mon, Sep 07, 2020 at 06:56:20PM +0200, Eric Dumazet wrote:
> On Mon, Sep 7, 2020 at 2:53 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > As I said in response to v1 patch, I don't like the idea of adding a new
> > ioctl interface to ethool when we are working on replacing and
> > deprecating the existing ones. Is there a strong reason why this feature
> > shouldn't be implemented using netlink?
> 
> I do not think this is a fair request.
> 
> All known kernels support the ioctl(), none of them support netlink so far.

Several years ago, exactly the same was true for bonding, bridge or vlan
configuration: all known kernels supported ioctl() or sysfs interfaces
for them, none supported netlink at that point. By your logic, the right
course of action would have been using ioctl() and sysfs for iproute2
support. Instead, rtnetlink interfaces were implemented and used by
iproute2. I believe it was the right choice.

> Are you working on the netlink interface, or are you requesting us to
> implement it ?

If it helps, I'm willing to write the kernel side. Or both, if
necessary, just to avoid adding another ioctl monument that would have
to be kept and maintained for many years, maybe forever.

> The ioctl has been added years ago, and Kevin patch is reasonable enough.

And there is a utility using the ioctl, as Andrew pointed out. Just like
there were brctl and vconfig and ioctl they were using. The existence of
those ioctl was not considered sufficient reason to use them when bridge
and vlan support was added to iproute2. I don't believe today's
situation with ethtool is different.

Michal
Eric Dumazet Sept. 8, 2020, 5:35 a.m. UTC | #4
On Mon, Sep 7, 2020 at 11:25 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Mon, Sep 07, 2020 at 06:56:20PM +0200, Eric Dumazet wrote:
> > On Mon, Sep 7, 2020 at 2:53 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> > >
> > > As I said in response to v1 patch, I don't like the idea of adding a new
> > > ioctl interface to ethool when we are working on replacing and
> > > deprecating the existing ones. Is there a strong reason why this feature
> > > shouldn't be implemented using netlink?
> >
> > I do not think this is a fair request.
> >
> > All known kernels support the ioctl(), none of them support netlink so far.
>
> Several years ago, exactly the same was true for bonding, bridge or vlan
> configuration: all known kernels supported ioctl() or sysfs interfaces
> for them, none supported netlink at that point. By your logic, the right
> course of action would have been using ioctl() and sysfs for iproute2
> support. Instead, rtnetlink interfaces were implemented and used by
> iproute2. I believe it was the right choice.

Sure, but netlink does not yet provide the needed functionality for
our use case.

netlink was a medium/long term plan, for the kernel side at least.
I would totally understand and support a new iocl() in the kernel being blocked.
(In fact I have blocked Kevin from adding a sysfs and advised to use
existing ioctl())

Here we are not changing the kernel, we let ethtool use existing ABI
and old kernels.

I think you are mixing your own long term plans with simply letting ethtool
to meet existing kernel functionality.

>
> > Are you working on the netlink interface, or are you requesting us to
> > implement it ?
>
> If it helps, I'm willing to write the kernel side.

Yes please, that would help, but will still require months of
deployments at Google scale.


Or both, if
> necessary, just to avoid adding another ioctl monument that would have
> to be kept and maintained for many years, maybe forever.

The kernel part is there, and lack of equivalent  netlink support
means we have to keep it for ten years at least.

>
> > The ioctl has been added years ago, and Kevin patch is reasonable enough.
>
> And there is a utility using the ioctl, as Andrew pointed out. Just like
> there were brctl and vconfig and ioctl they were using. The existence of
> those ioctl was not considered sufficient reason to use them when bridge
> and vlan support was added to iproute2. I don't believe today's
> situation with ethtool is different.

I suspect Richard Cochran wrote the 190 lines of code outside of
ethtool because it was easier to not have to convince the ethtool
maintainer at that time :)

We do not have hwstamp_ctl deployed at this very moment, and for us it
is simply much faster to deploy a new ethtool version than having to
get security teams
approval to install a new binary.

Honestly, if this was an option, we would not have even bothered
writing ethtool support.

Now, you want netlink support instead of ioctl(), that is a very
different scope and amount of work.

Thanks.
Michal Kubecek Sept. 8, 2020, 10:37 a.m. UTC | #5
On Tue, Sep 08, 2020 at 07:35:58AM +0200, Eric Dumazet wrote:
> On Mon, Sep 7, 2020 at 11:25 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> > On Mon, Sep 07, 2020 at 06:56:20PM +0200, Eric Dumazet wrote:
> > > On Mon, Sep 7, 2020 at 2:53 PM Michal Kubecek <mkubecek@suse.cz> wrote:
> > > >
> > > > As I said in response to v1 patch, I don't like the idea of
> > > > adding a new ioctl interface to ethool when we are working on
> > > > replacing and deprecating the existing ones. Is there a strong
> > > > reason why this feature shouldn't be implemented using netlink?
> > >
> > > I do not think this is a fair request.
> > >
> > > All known kernels support the ioctl(), none of them support
> > > netlink so far.
> >
> > Several years ago, exactly the same was true for bonding, bridge or
> > vlan configuration: all known kernels supported ioctl() or sysfs
> > interfaces for them, none supported netlink at that point. By your
> > logic, the right course of action would have been using ioctl() and
> > sysfs for iproute2 support. Instead, rtnetlink interfaces were
> > implemented and used by iproute2. I believe it was the right choice.
> 
> Sure, but netlink does not yet provide the needed functionality for
> our use case.
> 
> netlink was a medium/long term plan, for the kernel side at least. I
> would totally understand and support a new iocl() in the kernel being
> blocked. (In fact I have blocked Kevin from adding a sysfs and advised
> to use existing ioctl())
> 
> Here we are not changing the kernel, we let ethtool use existing ABI
> and old kernels.
> 
> I think you are mixing your own long term plans with simply letting
> ethtool to meet existing kernel functionality.

In other words, the situation is exactly as it was with bridge
configuration back in 2014 or vlan configuration in 2007. There was
existing ioctl interface to configure them and no netlink interface. It
was also the same for bonding, except bonding was using sysfs and module
parameters, not ioctl.

But rather than using these existing interfaces for iproute2 support,
(rt)netlink interface was implemented and used by iproute2 instead.
I believe it was the right choice then and I believe it would be the
right choice now. That's why I'm asking for a strong reason not to add
and use a netlink based interface.

> > > Are you working on the netlink interface, or are you requesting us to
> > > implement it ?
> >
> > If it helps, I'm willing to write the kernel side.
> 
> Yes please, that would help, but will still require months of
> deployments at Google scale.
[...] 
> We do not have hwstamp_ctl deployed at this very moment, and for us it
> is simply much faster to deploy a new ethtool version than having to
> get security teams
> approval to install a new binary.
> 
> Honestly, if this was an option, we would not have even bothered
> writing ethtool support.
> 
> Now, you want netlink support instead of ioctl(), that is a very
> different scope and amount of work.

All this sounds as if the actual reason why you want this in ethtool -
and implemented using existing ioctl - were to provide a workaround for
your internal company processes which make it way harder to add a small
utility than to embed essentially the same code into another which has
been approved already. I understand that company processes sometimes
work like that (we have a customer who once asked us to patch kernel for
something that could be easily achieved by setting one sysctl on boot
becuse it was easier for them to deploy an updated kernel than to edit
a config file in their image) but I don't think this is a convincing
argument for upstream code inclusion.

> > Or both, if necessary, just to avoid adding another ioctl monument
> > that would have to be kept and maintained for many years, maybe
> > forever.
> 
> The kernel part is there, and lack of equivalent  netlink support
> means we have to keep it for ten years at least.

I meant the (proposed) userspace part. The kernel part is already there
and we cannot stop providing the SIOC[GS]HWTSTAMP ioctl any time soon.
But we don't have to use it in ethtool utility and I believe that rather
than using it, we should implement the feature via netlink from the
start to get all related benefits (extensibility, notifications, altname
support, dump support etc.).

BtW, I realized now that the way the patch is written, it would not
show the new information on systems with recent kernel supporting
ETHTOOL_MSG_TSINFO_GET netlink request (5.7 and newer) because then
ethtool >= 5.7 would use nl_tsinfo() rather than do_tsinfo().

Michal
Eric Dumazet Sept. 8, 2020, 11:17 a.m. UTC | #6
On Tue, Sep 8, 2020 at 12:37 PM Michal Kubecek <mkubecek@suse.cz> wrote:

>
> All this sounds as if the actual reason why you want this in ethtool -
> and implemented using existing ioctl - were to provide a workaround for
> your internal company processes which make it way harder to add a small
> utility than to embed essentially the same code into another which has
> been approved already. I understand that company processes sometimes
> work like that (we have a customer who once asked us to patch kernel for
> something that could be easily achieved by setting one sysctl on boot
> becuse it was easier for them to deploy an updated kernel than to edit
> a config file in their image) but I don't think this is a convincing
> argument for upstream code inclusion.
>

OK, we will carry this internally then.

We are not going to fight against some trivial change.
diff mbox series

Patch

diff --git a/ethtool.8.in b/ethtool.8.in
index a50a476..9930804 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -315,6 +315,11 @@  ethtool \- query or control network driver and hardware settings
 .B ethtool \-T|\-\-show\-time\-stamping
 .I devname
 .HP
+.B ethtool \-\-set\-time\-stamping
+.I devname
+.BN rx
+.BN tx
+.HP
 .B ethtool \-x|\-\-show\-rxfh\-indir|\-\-show\-rxfh
 .I devname
 .HP
@@ -1023,9 +1028,12 @@  is indicated, then ethtool fetches the dump data and directs it to a
 Sets the dump flag for the device.
 .TP
 .B \-T \-\-show\-time\-stamping
-Show the device's time stamping capabilities and associated PTP
+Show the device's time stamping capabilities and policy and associated PTP
 hardware clock.
 .TP
+.B \-\-set\-time\-stamping
+Set the device's time stamping policy at the driver level.
+.TP
 .B \-x \-\-show\-rxfh\-indir \-\-show\-rxfh
 Retrieves the receive flow hash indirection table and/or RSS hash key.
 .TP
diff --git a/ethtool.c b/ethtool.c
index 606af3e..039b9a9 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1618,6 +1618,24 @@  static int dump_tsinfo(const struct ethtool_ts_info *info)
 	return 0;
 }
 
+static int dump_hwtstamp_config(const struct hwtstamp_config *cfg)
+{
+	fprintf(stdout, "Hardware Timestamping Policy:\n");
+	fprintf(stdout, "\tRx filter %d", cfg->rx_filter);
+	if (cfg->rx_filter < N_RX_FILTERS)
+		fprintf(stdout, ", %s\n", rx_filter_labels[cfg->rx_filter]);
+	else
+		fprintf(stdout, "\n");
+
+	fprintf(stdout, "\tTx type %d", cfg->tx_type);
+	if (cfg->tx_type < N_TX_TYPES)
+		fprintf(stdout, ", %s\n", tx_type_labels[cfg->tx_type]);
+	else
+		fprintf(stdout, "\n");
+
+	return 0;
+}
+
 static struct ethtool_gstrings *
 get_stringset(struct cmd_context *ctx, enum ethtool_stringset set_id,
 	      ptrdiff_t drvinfo_offset, int null_terminate)
@@ -4705,6 +4723,7 @@  err:
 static int do_tsinfo(struct cmd_context *ctx)
 {
 	struct ethtool_ts_info info;
+	struct hwtstamp_config cfg = { 0 };
 
 	if (ctx->argc != 0)
 		exit_bad_args();
@@ -4716,6 +4735,64 @@  static int do_tsinfo(struct cmd_context *ctx)
 		return -1;
 	}
 	dump_tsinfo(&info);
+
+	ctx->ifr.ifr_data = (void *)&cfg;
+	if (ioctl(ctx->fd, SIOCGHWTSTAMP, &ctx->ifr)) {
+		perror("Cannot get device time stamping policy");
+		return -1;
+	}
+	dump_hwtstamp_config(&cfg);
+
+	return 0;
+}
+
+static int do_shwtstamp(struct cmd_context *ctx)
+{
+	struct hwtstamp_config cfg = { .rx_filter = -1, .tx_type = -1 },
+			       pre_cfg;
+	int changed = 0;
+	struct cmdline_info cmdline_config[] = {
+		{
+			.name		= "rx",
+			.type		= CMDL_S32,
+			.wanted_val	= &cfg.rx_filter,
+			.ioctl_val	= &pre_cfg.rx_filter,
+		},
+		{
+			.name		= "tx",
+			.type		= CMDL_S32,
+			.wanted_val	= &cfg.tx_type,
+			.ioctl_val	= &pre_cfg.tx_type,
+		},
+	};
+
+	parse_generic_cmdline(ctx, &changed,
+			      cmdline_config, ARRAY_SIZE(cmdline_config));
+
+	ctx->ifr.ifr_data = (void *)&pre_cfg;
+	if (ioctl(ctx->fd, SIOCGHWTSTAMP, &ctx->ifr)) {
+		perror("Cannot get device time stamping settings");
+		return -1;
+	}
+
+	changed = 0;
+	do_generic_set(cmdline_config, ARRAY_SIZE(cmdline_config), &changed);
+	if (!changed) {
+		fprintf(stderr, "no time-stamping policy changed.\n");
+		return 0;
+	}
+
+	if (cfg.tx_type >= N_TX_TYPES || cfg.rx_filter >= N_RX_FILTERS) {
+		fprintf(stderr,
+			"rx should be in [0..%d], tx should be in [0..%d]\n",
+			N_RX_FILTERS - 1, N_TX_TYPES - 1);
+		return -1;
+	}
+
+	if (ioctl(ctx->fd, SIOCSHWTSTAMP, &ctx->ifr)) {
+		perror("Cannot set device time stamping settings");
+		return -1;
+	}
 	return 0;
 }
 
@@ -5733,7 +5810,14 @@  static const struct option args[] = {
 		.opts	= "-T|--show-time-stamping",
 		.func	= do_tsinfo,
 		.nlfunc	= nl_tsinfo,
-		.help	= "Show time stamping capabilities"
+		.help	= "Show time stamping capabilities and policy"
+	},
+	{
+		.opts	= "--set-time-stamping",
+		.func	= do_shwtstamp,
+		.help	= "Set time stamping policy at the driver level",
+		.xhelp	= "		[ rx N ]\n"
+			  "		[ tx N ]\n"
 	},
 	{
 		.opts	= "-x|--show-rxfh-indir|--show-rxfh",