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: <93ae54eb-5287-8d63-5109-973bfacc6b74@gmail.com> Date: Thu, 6 Apr 2023 06:07:45 -0700 From: Florian Fainelli <f.fainelli@...il.com> To: Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org> Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch> Subject: Re: [PATCH v2 net-next] net: dsa: replace NETDEV_PRE_CHANGE_HWTSTAMP notifier with a stub On 4/6/2023 4:42 AM, Vladimir Oltean wrote: > There was a sort of rush surrounding commit 88c0a6b503b7 ("net: create a > netdev notifier for DSA to reject PTP on DSA master"), due to a desire > to convert DSA's attempt to deny TX timestamping on a DSA master to > something that doesn't block the kernel-wide API conversion from > ndo_eth_ioctl() to ndo_hwtstamp_set(). > > What was required was a mechanism that did not depend on ndo_eth_ioctl(), > and what was provided was a mechanism that did not depend on > ndo_eth_ioctl(), while at the same time introducing something that > wasn't absolutely necessary - a new netdev notifier. > > There have been objections from Jakub Kicinski that using notifiers in > general when they are not absolutely necessary creates complications to > the control flow and difficulties to maintainers who look at the code. > So there is a desire to not use notifiers. Jakub is there a general desire to move away from notifiers? If so, do you have a list of things that may no longer belong there? > > In addition to that, the notifier chain gets called even if there is no > DSA in the system and no one is interested in applying any restriction. > > Take the model of udp_tunnel_nic_ops and introduce a stub mechanism, > through which net/core/dev_ioctl.c can call into DSA even when > CONFIG_NET_DSA=m. > > Compared to the code that existed prior to the notifier conversion, aka > what was added in commits: > - 4cfab3566710 ("net: dsa: Add wrappers for overloaded ndo_ops") > - 3369afba1e46 ("net: Call into DSA netdevice_ops wrappers") > > this is different because we are not overloading any struct > net_device_ops of the DSA master anymore, but rather, we are exposing a > rather specific functionality which is orthogonal to which API is used > to enable it - ndo_eth_ioctl() or ndo_hwtstamp_set(). > > Also, what is similar is that both approaches use function pointers to > get from built-in code to DSA. > > There is no point in replicating the function pointers towards > __dsa_master_hwtstamp_validate() once for every CPU port (dev->dsa_ptr). > Instead, it is sufficient to introduce a singleton struct dsa_stubs, > built into the kernel, which contains a single function pointer to > __dsa_master_hwtstamp_validate(). > > I find this approach preferable to what we had originally, because > dev->dsa_ptr->netdev_ops->ndo_do_ioctl() used to require going through > struct dsa_port (dev->dsa_ptr), and so, this was incompatible with any > attempts to add any data encapsulation and hide DSA data structures from > the outside world. > > Link: https://lore.kernel.org/netdev/20230403083019.120b72fd@kernel.org/ > Signed-off-by: Vladimir Oltean <vladimir.oltean@....com> Reviewed-by: Florian Fainelli <f.fainelli@...il.com> -- Florian
Powered by blists - more mailing lists