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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKZ19+AJOf5_5orPrUObYef+L-HrwF_Oay6o75ZbG7UhQ@mail.gmail.com>
Date:   Mon, 7 Sep 2020 18:56:20 +0200
From:   Eric Dumazet <edumazet@...gle.com>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     "Kevin(Yudong) Yang" <yyd@...gle.com>,
        netdev <netdev@...r.kernel.org>,
        Neal Cardwell <ncardwell@...gle.com>
Subject: Re: [PATCH ethtool,v2] ethtool: add support show/set-time-stamping

On Mon, Sep 7, 2020 at 2:53 PM Michal Kubecek <mkubecek@...e.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@...gle.com>
> > Reviewed-by: Neal Cardwell <ncardwell@...gle.com>
> > Reviewed-by: Eric Dumazet <edumazet@...gle.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.