diff mbox series

[RFC,net-next] net: dsa: replace NETDEV_PRE_CHANGE_HWTSTAMP notifier with a stub

Message ID 20230405165115.3744445-1-vladimir.oltean@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [RFC,net-next] net: dsa: replace NETDEV_PRE_CHANGE_HWTSTAMP notifier with a stub | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 4246 this patch: 4247
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang fail Errors and warnings before: 948 this patch: 959
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 4453 this patch: 4454
netdev/checkpatch warning CHECK: DEFINE_MUTEX definition without comment WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable WARNING: It's generally not useful to have the filename in the file WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 84 exceeds 80 columns WARNING: line length of 91 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Vladimir Oltean April 5, 2023, 4:51 p.m. UTC
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.

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.

Since the new functionality does not overload the NDO of any DSA master,
there is no point in replicating the function pointers towards
__dsa_master_hwtstamp_validate() once for every CPU port (dev->dsa_ptr).
But rather, it is fine to introduce a singleton struct dsa_stubs,
built-in to the kernel, which contains a single function pointer to
__dsa_master_hwtstamp_validate().

I find this approach rather 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@nxp.com>
---
 include/linux/netdevice.h |  6 -----
 include/net/dsa_stubs.h   | 49 +++++++++++++++++++++++++++++++++++++++
 net/Makefile              |  2 +-
 net/core/dev.c            |  2 +-
 net/core/dev_ioctl.c      | 12 ++--------
 net/dsa/Makefile          |  6 +++++
 net/dsa/dsa.c             | 27 +++++++++++++++++++++
 net/dsa/master.c          |  2 +-
 net/dsa/master.h          |  2 +-
 net/dsa/slave.c           | 10 --------
 net/dsa/stubs.c           | 13 +++++++++++
 11 files changed, 101 insertions(+), 30 deletions(-)
 create mode 100644 include/net/dsa_stubs.h
 create mode 100644 net/dsa/stubs.c

Comments

Vladimir Oltean April 5, 2023, 4:56 p.m. UTC | #1
On Wed, Apr 05, 2023 at 07:51:15PM +0300, Vladimir Oltean wrote:
> +#if IS_ENABLED(CONFIG_NET_DSA)
> +
> +static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
> +					       const struct kernel_hwtstamp_config *config,
> +					       struct netlink_ext_ack *extack)
> +{
> +}
> +
> +#else
> +
> +static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
> +					       const struct kernel_hwtstamp_config *config)
> +{
> +	return 0;
> +}
> +
> +#endif

This happened again, I need to do something about it... I ran
format-patch before I added this last hunk to the commit:

diff --git a/include/net/dsa_stubs.h b/include/net/dsa_stubs.h
index 27a1ad85c038..9556f9fb5a86 100644
--- a/include/net/dsa_stubs.h
+++ b/include/net/dsa_stubs.h
@@ -41,7 +41,8 @@ static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
 #else
 
 static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
-					       const struct kernel_hwtstamp_config *config)
+					       const struct kernel_hwtstamp_config *config,
+					       struct netlink_ext_ack *extack)
 {
 	return 0;
 }

so that the build with CONFIG_NET_DSA=n passes too (which I did test).

It's now fixed on my computer, but whoever is curious to test this will
have to fix it up manually; I won't resend the RFC just for this.
Simon Horman April 5, 2023, 8:05 p.m. UTC | #2
On Wed, Apr 05, 2023 at 07:51:15PM +0300, 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.
> 
> 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.
> 
> Since the new functionality does not overload the NDO of any DSA master,
> there is no point in replicating the function pointers towards
> __dsa_master_hwtstamp_validate() once for every CPU port (dev->dsa_ptr).
> But rather, it is fine to introduce a singleton struct dsa_stubs,
> built-in to the kernel, which contains a single function pointer to
> __dsa_master_hwtstamp_validate().
> 
> I find this approach rather 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@nxp.com>

...

> diff --git a/include/net/dsa_stubs.h b/include/net/dsa_stubs.h
> new file mode 100644
> index 000000000000..27a1ad85c038
> --- /dev/null
> +++ b/include/net/dsa_stubs.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * include/net/dsa_stubs.h - Stubs for the Distributed Switch Architecture framework
> + */
> +
> +#include <linux/mutex.h>
> +#include <linux/netdevice.h>
> +#include <linux/net_tstamp.h>
> +#include <net/dsa.h>
> +
> +#if IS_ENABLED(CONFIG_NET_DSA)
> +
> +extern const struct dsa_stubs *dsa_stubs;
> +extern struct mutex dsa_stubs_lock;
> +
> +struct dsa_stubs {
> +	int (*master_hwtstamp_validate)(struct net_device *dev,
> +					const struct kernel_hwtstamp_config *config,
> +					struct netlink_ext_ack *extack);
> +};
> +
> +static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
> +					       const struct kernel_hwtstamp_config *config,
> +					       struct netlink_ext_ack *extack)
> +{
> +	int err;
> +
> +	if (!netdev_uses_dsa(dev))
> +		return 0;
> +
> +	mutex_lock(&dsa_stubs_lock);
> +
> +	if (dsa_stubs)
> +		err = dsa_stubs->master_hwtstamp_validate(dev, config, extack);
> +
> +	mutex_unlock(&dsa_stubs_lock);

nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.

> +
> +	return err;
> +}
> +
> +#else

...

> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 8abc1658ac47..36bb7533ddd6 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -3419,16 +3419,6 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
>  
>  		return NOTIFY_OK;
>  	}
> -	case NETDEV_PRE_CHANGE_HWTSTAMP: {
> -		struct netdev_notifier_hwtstamp_info *info = ptr;
> -		int err;
> -
> -		if (!netdev_uses_dsa(dev))
> -			return NOTIFY_DONE;
> -
> -		err = dsa_master_pre_change_hwtstamp(dev, info->config, extack);

nit: clang-16 tell me that extack is now unused in this function.

> -		return notifier_from_errno(err);
> -	}
>  	default:
>  		break;
>  	}
Vladimir Oltean April 5, 2023, 8:07 p.m. UTC | #3
On Wed, Apr 05, 2023 at 10:05:15PM +0200, Simon Horman wrote:
> nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.
> nit: clang-16 tell me that extack is now unused in this function.

Thanks, all good points. Thank clang-16 on my behalf!
Vladimir Oltean April 5, 2023, 8:08 p.m. UTC | #4
On Wed, Apr 05, 2023 at 11:07:05PM +0300, Vladimir Oltean wrote:
> On Wed, Apr 05, 2023 at 10:05:15PM +0200, Simon Horman wrote:
> > nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.
> > nit: clang-16 tell me that extack is now unused in this function.
> 
> Thanks, all good points. Thank clang-16 on my behalf!

Perhaps I shouldn't have left it there. I'll start looking into setting
up a toolchain with that for my own builds.
Vladimir Oltean April 5, 2023, 8:31 p.m. UTC | #5
On Wed, Apr 05, 2023 at 10:05:15PM +0200, Simon Horman wrote:
> > +static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
> > +					       const struct kernel_hwtstamp_config *config,
> > +					       struct netlink_ext_ack *extack)
> > +{
> > +	int err;
> > +
> > +	if (!netdev_uses_dsa(dev))
> > +		return 0;
> > +
> > +	mutex_lock(&dsa_stubs_lock);
> > +
> > +	if (dsa_stubs)
> > +		err = dsa_stubs->master_hwtstamp_validate(dev, config, extack);
> > +
> > +	mutex_unlock(&dsa_stubs_lock);
> 
> nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.

In fact, clang-16 is saying something much smarter than that, because I
did test this code path and it did work reliably (not like an uninitialized
return value would).

It's saying that when netdev_uses_dsa() returns true, the DSA module has
surely been loaded, so the stubs have surely been registered, so the
mutex_lock() and the check for the NULL quality of dsa_stubs are
completely redundant and can be removed.

> > +
> > +	return err;
> > +}
> > +
> > +#else
Jakub Kicinski April 6, 2023, 2:08 a.m. UTC | #6
On Wed,  5 Apr 2023 19:51:15 +0300 Vladimir Oltean wrote:
> 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.

LGTM, thank you!
Simon Horman April 6, 2023, 7:07 a.m. UTC | #7
On Wed, Apr 05, 2023 at 11:08:52PM +0300, Vladimir Oltean wrote:
> On Wed, Apr 05, 2023 at 11:07:05PM +0300, Vladimir Oltean wrote:
> > On Wed, Apr 05, 2023 at 10:05:15PM +0200, Simon Horman wrote:
> > > nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.
> > > nit: clang-16 tell me that extack is now unused in this function.
> > 
> > Thanks, all good points. Thank clang-16 on my behalf!
> 
> Perhaps I shouldn't have left it there. I'll start looking into setting
> up a toolchain with that for my own builds.

I did this a few weeks back using the toolchain at the link below.
It was quite a simple process. And well worth the effort.

https://mirrors.edge.kernel.org/pub/tools/llvm/
Simon Horman April 6, 2023, 7:12 a.m. UTC | #8
On Wed, Apr 05, 2023 at 11:31:07PM +0300, Vladimir Oltean wrote:
> On Wed, Apr 05, 2023 at 10:05:15PM +0200, Simon Horman wrote:
> > > +static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
> > > +					       const struct kernel_hwtstamp_config *config,
> > > +					       struct netlink_ext_ack *extack)
> > > +{
> > > +	int err;
> > > +
> > > +	if (!netdev_uses_dsa(dev))
> > > +		return 0;
> > > +
> > > +	mutex_lock(&dsa_stubs_lock);
> > > +
> > > +	if (dsa_stubs)
> > > +		err = dsa_stubs->master_hwtstamp_validate(dev, config, extack);
> > > +
> > > +	mutex_unlock(&dsa_stubs_lock);
> > 
> > nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.
> 
> In fact, clang-16 is saying something much smarter than that, because I
> did test this code path and it did work reliably (not like an uninitialized
> return value would).
> 
> It's saying that when netdev_uses_dsa() returns true, the DSA module has
> surely been loaded, so the stubs have surely been registered, so the
> mutex_lock() and the check for the NULL quality of dsa_stubs are
> completely redundant and can be removed.

For the record, what it had to say, when used with W=1, was:

In file included from net/dsa/dsa.c:20:
./include/net/dsa_stubs.h:33:6: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
        if (dsa_stubs)
            ^~~~~~~~~
./include/net/dsa_stubs.h:38:9: note: uninitialized use occurs here
        return err;
               ^~~
./include/net/dsa_stubs.h:33:2: note: remove the 'if' if its condition is always true
        if (dsa_stubs)
        ^~~~~~~~~~~~~~
./include/net/dsa_stubs.h:26:9: note: initialize the variable 'err' to silence this warning
        int err;
               ^
                = 0
1 error generated.

--

In file included from net/dsa/stubs.c:7:
./include/net/dsa_stubs.h:33:6: error: variable 'err' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
        if (dsa_stubs)
            ^~~~~~~~~
./include/net/dsa_stubs.h:38:9: note: uninitialized use occurs here
        return err;
               ^~~
./include/net/dsa_stubs.h:33:2: note: remove the 'if' if its condition is always true
        if (dsa_stubs)
        ^~~~~~~~~~~~~~
./include/net/dsa_stubs.h:26:9: note: initialize the variable 'err' to silence this warning
        int err;
               ^
                = 0
1 error generated.
Vladimir Oltean April 6, 2023, 11:45 a.m. UTC | #9
On Thu, Apr 06, 2023 at 09:07:11AM +0200, Simon Horman wrote:
> On Wed, Apr 05, 2023 at 11:08:52PM +0300, Vladimir Oltean wrote:
> > On Wed, Apr 05, 2023 at 11:07:05PM +0300, Vladimir Oltean wrote:
> > > On Wed, Apr 05, 2023 at 10:05:15PM +0200, Simon Horman wrote:
> > > > nit: clang-16 tells me that err is uninitialised here if dsa_stubs is false.
> > > > nit: clang-16 tell me that extack is now unused in this function.
> > > 
> > > Thanks, all good points. Thank clang-16 on my behalf!
> > 
> > Perhaps I shouldn't have left it there. I'll start looking into setting
> > up a toolchain with that for my own builds.
> 
> I did this a few weeks back using the toolchain at the link below.
> It was quite a simple process. And well worth the effort.
> 
> https://mirrors.edge.kernel.org/pub/tools/llvm/

Thanks, I did set this up yesterday, using Documentation/kbuild/llvm.rst
as a guide.
diff mbox series

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a740be3bb911..1c25b39681b3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2878,7 +2878,6 @@  enum netdev_cmd {
 	NETDEV_OFFLOAD_XSTATS_REPORT_USED,
 	NETDEV_OFFLOAD_XSTATS_REPORT_DELTA,
 	NETDEV_XDP_FEAT_CHANGE,
-	NETDEV_PRE_CHANGE_HWTSTAMP,
 };
 const char *netdev_cmd_to_name(enum netdev_cmd cmd);
 
@@ -2929,11 +2928,6 @@  struct netdev_notifier_pre_changeaddr_info {
 	const unsigned char *dev_addr;
 };
 
-struct netdev_notifier_hwtstamp_info {
-	struct netdev_notifier_info info; /* must be first */
-	struct kernel_hwtstamp_config *config;
-};
-
 enum netdev_offload_xstats_type {
 	NETDEV_OFFLOAD_XSTATS_TYPE_L3 = 1,
 };
diff --git a/include/net/dsa_stubs.h b/include/net/dsa_stubs.h
new file mode 100644
index 000000000000..27a1ad85c038
--- /dev/null
+++ b/include/net/dsa_stubs.h
@@ -0,0 +1,49 @@ 
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * include/net/dsa_stubs.h - Stubs for the Distributed Switch Architecture framework
+ */
+
+#include <linux/mutex.h>
+#include <linux/netdevice.h>
+#include <linux/net_tstamp.h>
+#include <net/dsa.h>
+
+#if IS_ENABLED(CONFIG_NET_DSA)
+
+extern const struct dsa_stubs *dsa_stubs;
+extern struct mutex dsa_stubs_lock;
+
+struct dsa_stubs {
+	int (*master_hwtstamp_validate)(struct net_device *dev,
+					const struct kernel_hwtstamp_config *config,
+					struct netlink_ext_ack *extack);
+};
+
+static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
+					       const struct kernel_hwtstamp_config *config,
+					       struct netlink_ext_ack *extack)
+{
+	int err;
+
+	if (!netdev_uses_dsa(dev))
+		return 0;
+
+	mutex_lock(&dsa_stubs_lock);
+
+	if (dsa_stubs)
+		err = dsa_stubs->master_hwtstamp_validate(dev, config, extack);
+
+	mutex_unlock(&dsa_stubs_lock);
+
+	return err;
+}
+
+#else
+
+static inline int dsa_master_hwtstamp_validate(struct net_device *dev,
+					       const struct kernel_hwtstamp_config *config)
+{
+	return 0;
+}
+
+#endif
diff --git a/net/Makefile b/net/Makefile
index 0914bea9c335..87592009366f 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -24,7 +24,7 @@  obj-$(CONFIG_PACKET)		+= packet/
 obj-$(CONFIG_NET_KEY)		+= key/
 obj-$(CONFIG_BRIDGE)		+= bridge/
 obj-$(CONFIG_NET_DEVLINK)	+= devlink/
-obj-$(CONFIG_NET_DSA)		+= dsa/
+obj-y				+= dsa/
 obj-$(CONFIG_ATALK)		+= appletalk/
 obj-$(CONFIG_X25)		+= x25/
 obj-$(CONFIG_LAPB)		+= lapb/
diff --git a/net/core/dev.c b/net/core/dev.c
index 7ce5985be84b..480600a075ce 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1612,7 +1612,7 @@  const char *netdev_cmd_to_name(enum netdev_cmd cmd)
 	N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
 	N(PRE_CHANGEADDR) N(OFFLOAD_XSTATS_ENABLE) N(OFFLOAD_XSTATS_DISABLE)
 	N(OFFLOAD_XSTATS_REPORT_USED) N(OFFLOAD_XSTATS_REPORT_DELTA)
-	N(XDP_FEAT_CHANGE) N(PRE_CHANGE_HWTSTAMP)
+	N(XDP_FEAT_CHANGE)
 	}
 #undef N
 	return "UNKNOWN_NETDEV_EVENT";
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 6d772837eb3f..3730945ee294 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -7,7 +7,7 @@ 
 #include <linux/net_tstamp.h>
 #include <linux/wireless.h>
 #include <linux/if_bridge.h>
-#include <net/dsa.h>
+#include <net/dsa_stubs.h>
 #include <net/wext.h>
 
 #include "dev.h"
@@ -259,9 +259,6 @@  static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 
 static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 {
-	struct netdev_notifier_hwtstamp_info info = {
-		.info.dev = dev,
-	};
 	struct kernel_hwtstamp_config kernel_cfg;
 	struct netlink_ext_ack extack = {};
 	struct hwtstamp_config cfg;
@@ -276,12 +273,7 @@  static int dev_set_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 	if (err)
 		return err;
 
-	info.info.extack = &extack;
-	info.config = &kernel_cfg;
-
-	err = call_netdevice_notifiers_info(NETDEV_PRE_CHANGE_HWTSTAMP,
-					    &info.info);
-	err = notifier_to_errno(err);
+	err = dsa_master_hwtstamp_validate(dev, &kernel_cfg, &extack);
 	if (err) {
 		if (extack._msg)
 			netdev_err(dev, "%s\n", extack._msg);
diff --git a/net/dsa/Makefile b/net/dsa/Makefile
index cc7e93a562fe..3835de286116 100644
--- a/net/dsa/Makefile
+++ b/net/dsa/Makefile
@@ -1,4 +1,10 @@ 
 # SPDX-License-Identifier: GPL-2.0
+
+# the stubs are built-in whenever DSA is built-in or module
+ifdef CONFIG_NET_DSA
+obj-y := stubs.o
+endif
+
 # the core
 obj-$(CONFIG_NET_DSA) += dsa_core.o
 dsa_core-y += \
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index e5f156940c67..119d480b3d53 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -17,6 +17,7 @@ 
 #include <linux/of.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
+#include <net/dsa_stubs.h>
 #include <net/sch_generic.h>
 
 #include "devlink.h"
@@ -1702,6 +1703,28 @@  bool dsa_mdb_present_in_other_db(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL_GPL(dsa_mdb_present_in_other_db);
 
+static const struct dsa_stubs __dsa_stubs = {
+	.master_hwtstamp_validate = __dsa_master_hwtstamp_validate,
+};
+
+static void dsa_register_stubs(void)
+{
+	mutex_lock(&dsa_stubs_lock);
+
+	dsa_stubs = &__dsa_stubs;
+
+	mutex_unlock(&dsa_stubs_lock);
+}
+
+static void dsa_unregister_stubs(void)
+{
+	mutex_lock(&dsa_stubs_lock);
+
+	dsa_stubs = NULL;
+
+	mutex_unlock(&dsa_stubs_lock);
+}
+
 static int __init dsa_init_module(void)
 {
 	int rc;
@@ -1721,6 +1744,8 @@  static int __init dsa_init_module(void)
 	if (rc)
 		goto netlink_register_fail;
 
+	dsa_register_stubs();
+
 	return 0;
 
 netlink_register_fail:
@@ -1735,6 +1760,8 @@  module_init(dsa_init_module);
 
 static void __exit dsa_cleanup_module(void)
 {
+	dsa_unregister_stubs();
+
 	rtnl_link_unregister(&dsa_link_ops);
 
 	dsa_slave_unregister_notifier();
diff --git a/net/dsa/master.c b/net/dsa/master.c
index c2cabe6248b1..6be89ab0cc01 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -198,7 +198,7 @@  static void dsa_master_get_strings(struct net_device *dev, uint32_t stringset,
 /* Deny PTP operations on master if there is at least one switch in the tree
  * that is PTP capable.
  */
-int dsa_master_pre_change_hwtstamp(struct net_device *dev,
+int __dsa_master_hwtstamp_validate(struct net_device *dev,
 				   const struct kernel_hwtstamp_config *config,
 				   struct netlink_ext_ack *extack)
 {
diff --git a/net/dsa/master.h b/net/dsa/master.h
index 80842f4e27f7..76e39d3ec909 100644
--- a/net/dsa/master.h
+++ b/net/dsa/master.h
@@ -15,7 +15,7 @@  int dsa_master_lag_setup(struct net_device *lag_dev, struct dsa_port *cpu_dp,
 			 struct netlink_ext_ack *extack);
 void dsa_master_lag_teardown(struct net_device *lag_dev,
 			     struct dsa_port *cpu_dp);
-int dsa_master_pre_change_hwtstamp(struct net_device *dev,
+int __dsa_master_hwtstamp_validate(struct net_device *dev,
 				   const struct kernel_hwtstamp_config *config,
 				   struct netlink_ext_ack *extack);
 
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8abc1658ac47..36bb7533ddd6 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -3419,16 +3419,6 @@  static int dsa_slave_netdevice_event(struct notifier_block *nb,
 
 		return NOTIFY_OK;
 	}
-	case NETDEV_PRE_CHANGE_HWTSTAMP: {
-		struct netdev_notifier_hwtstamp_info *info = ptr;
-		int err;
-
-		if (!netdev_uses_dsa(dev))
-			return NOTIFY_DONE;
-
-		err = dsa_master_pre_change_hwtstamp(dev, info->config, extack);
-		return notifier_from_errno(err);
-	}
 	default:
 		break;
 	}
diff --git a/net/dsa/stubs.c b/net/dsa/stubs.c
new file mode 100644
index 000000000000..0b91f1d06028
--- /dev/null
+++ b/net/dsa/stubs.c
@@ -0,0 +1,13 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Stubs for DSA functionality called by the core network stack.
+ * These are necessary because CONFIG_NET_DSA can be a module, and built-in
+ * code cannot directly call symbols exported by modules.
+ */
+#include <net/dsa_stubs.h>
+
+DEFINE_MUTEX(dsa_stubs_lock);
+EXPORT_SYMBOL_GPL(dsa_stubs_lock);
+
+const struct dsa_stubs *dsa_stubs;
+EXPORT_SYMBOL_GPL(dsa_stubs);