netdev - Re: [PATCH net-next RFC] Add NDOs for hardware timestamp get/set

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: <20230401122450.0fd88313@kernel.org>
Date:   Sat, 1 Apr 2023 12:24:50 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     Max Georgiev <glipus@...il.com>, kory.maincent@...tlin.com,
        netdev@...r.kernel.org, maxime.chevallier@...tlin.com
Subject: Re: [PATCH net-next RFC] Add NDOs for hardware timestamp get/set

On Sat, 1 Apr 2023 22:12:15 +0300 Vladimir Oltean wrote:
> Actually, and here is the problem, DSA will want to see the timestamping
> request with the new code path too, not just with the legacy one.
> But, in this form, the dsa_ndo_eth_ioctl() -> dsa_master_ioctl() code
> path wants to do one of two things: it either denies the configuration,
> or passes it further, unchanged, to the master's netdev_ops->ndo_eth_ioctl().
> 
> By being written around the legacy ndo_eth_ioctl(), dsa_ndo_eth_ioctl()
> places a requirement which conflicts with any attempt to convert any
> kernel driver to the new API, because basically any net device can serve
> as a DSA master, and simply put, DSA wants to see timestamping requests
> to the DSA master, old or new API.
> 
> The only "useful" piece of logic from dsa_master_ioctl() is to deny the
> hwtstamp_set operation in some cases, so it's clear that it's useless
> for dsa_master_ioctl() to have to call the master's netdev_ops->ndo_eth_ioctl()
> when dev_eth_ioctl() already would have done it anyway.

So the current patch can only convert drivers which can't be a DSA
master :( (realistically any big iron NIC for example)
It should be relatively easy to plumb both the ifr and the in-kernel
config thru all the DSA APIs and have it call the right helper, too,
tho? SMOC?

> I can make dsa_ndo_eth_ioctl() disappear and replace it with a netdev
> notifier as per this patch:
> https://lore.kernel.org/netdev/20220317225035.3475538-1-vladimir.oltean@nxp.com/
> 
> My understanding of Jakub's objection is that the scope of the
> NETDEV_ETH_IOCTL is too wide, and as such, it would need to change to
> something like NETDEV_HWTSTAMP_SET. I can make that change if that is
> the only objection, and resubmit that as preparation work for the
> ndo_hwtstamp_set() effort.

My objection to the IOCTL is that there's a lot of boilerplate that 
the drivers have to copy and that it makes it harder to do meaningful
work in the core.