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
| ||
|
Message-ID: <20230511134807.v4u3ofn6jvgphqco@skbuf> Date: Thu, 11 May 2023 16:48:07 +0300 From: Vladimir Oltean <vladimir.oltean@....com> To: Köry Maincent <kory.maincent@...tlin.com> Cc: netdev@...r.kernel.org, kuba@...nel.org, glipus@...il.com, maxime.chevallier@...tlin.com, vadim.fedorenko@...ux.dev, richardcochran@...il.com, gerhard@...leder-embedded.com, thomas.petazzoni@...tlin.com, krzysztof.kozlowski+dt@...aro.org, robh+dt@...nel.org, linux@...linux.org.uk Subject: Re: [PATCH net-next RFC v4 4/5] net: Let the active time stamping layer be selectable. On Tue, May 02, 2023 at 01:05:25PM +0200, Köry Maincent wrote: > On Sat, 29 Apr 2023 20:58:07 +0300 > Vladimir Oltean <vladimir.oltean@....com> wrote: > > Thanks for the review and sorry for the coding style issues, I forgot to run the > checkpatch script. > > > > > > > #if IS_ENABLED(CONFIG_MACSEC) > > > @@ -2879,6 +2885,7 @@ enum netdev_cmd { > > > NETDEV_OFFLOAD_XSTATS_REPORT_DELTA, > > > NETDEV_XDP_FEAT_CHANGE, > > > NETDEV_PRE_CHANGE_HWTSTAMP, > > > + NETDEV_CHANGE_HWTSTAMP, > > > > Don't create new netdev notifiers with no users. Also, > > NETDEV_PRE_CHANGE_HWTSTAMP has disappered. > > Ok, right you move it on to dsa stub. What do you think of our case, should we > continue with netdev notifier? I don't know. AFAIU, the plan forward with this patch set is that, if the active timestamping layer is the PHY, phy_mii_ioctl() gets called and the MAC driver does not get notified in any way of that. That is an issue because if it's a switch, it will want to trap PTP even if it doesn't timestamp it, and with this proposal it doesn't get a chance to do that. What is your need for this? Do you have this scenario? If not, just drop this part from the patch. Jakub, you said "nope" to netdev notifiers, what would you suggest here instead? ndo_change_ptp_traps()? > Just some thought: > Maybe we could create a PHC_ID 0xXY where X is the layer and Y the PHCs number > in this layer, but what will append if there is two MACs consecutively? > Also in case of several MACs or PHYs in serial > netdev->selected_timestamping_layer is inappropriate. > > Maybe using it like 0xABC where A is the MAC number, B the PHY number and C > the PHC number in the device. > For example if we select the second MAC and its third PHC, PHC_ID will be > 0x203. Or if we select the second PHC of the PHY it will be 0x012. I think the kernel has (or can have) enough information to deduce the timestamping layer from just the PHC number, everything else is redundant information (also, what's "MAC number"? what is "PHY number"? how can user space correlate them to anything?). If you want to design an UAPI where individual PHCs present in the timestamping list of a netdev can individually have timestamping turned on or off (for a future where a socket can communicate hardware timestamps from multiple PHCs to user space in an identifiable way), then that is fine by me and maybe even a good idea. Just be aware that the current kernel will have to deny more than a single active timestamping PHC for now, because of the lack of hwtstamp identification in the SO_TIMESTAMPING API.
Powered by blists - more mailing lists