[PATCH net-next v2 1/3] net: move the netdev_ops checks into a helper

[PATCH net-next v2 1/3] net: move the netdev_ops checks into a helper

[Nicolai Buchwitz]

register_netdevice() already does two ops sanity checks and is about to
get a third. Collect them in netdev_check_ops().

The call stays after ndo_init() because drivers set features from there.

Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 net/core/dev.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 5ac08da8b9d7..a44bcfc29d54 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11451,6 +11451,24 @@ static void netdev_free_phy_link_topology(struct net_device *dev)
 	}
 }
 
+static int netdev_check_ops(struct net_device *dev)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (((dev->hw_features | dev->features) &
+	     NETIF_F_HW_VLAN_CTAG_FILTER) &&
+	    (!ops->ndo_vlan_rx_add_vid || !ops->ndo_vlan_rx_kill_vid)) {
+		netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
+		return -EINVAL;
+	}
+
+	if (netdev_need_ops_lock(dev) && ops->ndo_set_rx_mode &&
+	    !ops->ndo_set_rx_mode_async)
+		netdev_WARN(dev, "ops-locked drivers should use ndo_set_rx_mode_async\n");
+
+	return 0;
+}
+
 /**
  * register_netdevice() - register a network device
  * @dev: device to register
@@ -11506,19 +11524,9 @@ int register_netdevice(struct net_device *dev)
 		}
 	}
 
-	if (((dev->hw_features | dev->features) &
-	     NETIF_F_HW_VLAN_CTAG_FILTER) &&
-	    (!dev->netdev_ops->ndo_vlan_rx_add_vid ||
-	     !dev->netdev_ops->ndo_vlan_rx_kill_vid)) {
-		netdev_WARN(dev, "Buggy VLAN acceleration in driver!\n");
-		ret = -EINVAL;
+	ret = netdev_check_ops(dev);
+	if (ret)
 		goto err_uninit;
-	}
-
-	if (netdev_need_ops_lock(dev) &&
-	    dev->netdev_ops->ndo_set_rx_mode &&
-	    !dev->netdev_ops->ndo_set_rx_mode_async)
-		netdev_WARN(dev, "ops-locked drivers should use ndo_set_rx_mode_async\n");
 
 	ret = netdev_do_alloc_pcpu_stats(dev);
 	if (ret)

base-commit: 071876fd50482a68603a9460d80dd6dd58827ee1
-- 
2.53.0

[PATCH net-next v2 2/3] net: reject ops-locked drivers without ndo_set_rx_mode_async

[Nicolai Buchwitz]

ndo_set_rx_mode cannot sleep and runs with netif_addr_lock_bh held, so an
ops-locked driver cannot take the instance lock from it.
ndo_set_rx_mode_async is the replacement and runs in process context.

Registration has only warned about this since v7.1, long enough to find
the offenders. Make it an error.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
I grepped for potential offenders in the tree using

  for f in $(git grep -lE 'request_ops_lock|queue_mgmt_ops|net_shaper_ops' \
                          -- drivers/net); do
          d=$(dirname $f)
          git grep -q '\.ndo_set_rx_mode\s*=' -- $d &&
          ! git grep -q 'ndo_set_rx_mode_async' -- $d && echo "$d"
  done

but haven't found any. net-next testing will show if I missed something.

 net/core/dev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index a44bcfc29d54..ca7f4844cc40 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11463,8 +11463,10 @@ static int netdev_check_ops(struct net_device *dev)
 	}
 
 	if (netdev_need_ops_lock(dev) && ops->ndo_set_rx_mode &&
-	    !ops->ndo_set_rx_mode_async)
+	    !ops->ndo_set_rx_mode_async) {
 		netdev_WARN(dev, "ops-locked drivers should use ndo_set_rx_mode_async\n");
+		return -EINVAL;
+	}
 
 	return 0;
 }
-- 
2.53.0

[PATCH net-next v2 3/3] net: reject a netdev that implements only one hwtstamp NDO

[Nicolai Buchwitz]

ndo_hwtstamp_get and ndo_hwtstamp_set only make sense as a pair. A
driver providing just one gets no complaint today, and the mismatch
turns up in user space, where a set can succeed while the matching get
fails.

Catch it at registration instead.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
---
 net/core/dev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index ca7f4844cc40..db765cb5811e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11462,6 +11462,11 @@ static int netdev_check_ops(struct net_device *dev)
 		return -EINVAL;
 	}
 
+	if (!ops->ndo_hwtstamp_get != !ops->ndo_hwtstamp_set) {
+		netdev_WARN(dev, "driver implements only one hwtstamp NDO\n");
+		return -EINVAL;
+	}
+
 	if (netdev_need_ops_lock(dev) && ops->ndo_set_rx_mode &&
 	    !ops->ndo_set_rx_mode_async) {
 		netdev_WARN(dev, "ops-locked drivers should use ndo_set_rx_mode_async\n");
-- 
2.53.0