net: core: synchronize link-watch when carrier is queried

There are multiple ways to query for the carrier state: through
rtnetlink, sysfs, and (possibly) ethtool. Synchronize linkwatch
work before these operations so that we don't have a situation
where userspace queries the carrier state between the driver's
carrier off->on transition and linkwatch running and expects it
to work, when really (at least) TX cannot work until linkwatch
has run.

I previously posted a longer explanation of how this applies to
wireless [1] but with this wireless can simply query the state
before sending data, to ensure the kernel is ready for it.

[1] https://lore.kernel.org/all/346b21d87c69f817ea3c37caceb34f1f56255884.camel@sipsolutions.net/

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Link: https://lore.kernel.org/r/20231204214706.303c62768415.I1caedccae72ee5a45c9085c5eb49c145ce1c0dd5@changeid
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cb96aad6a6ee372c30378b5eac43e644ae1d1399..1b935ee341b42d44e471ff519b1dc302fadafca4 100644 (file)
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4229,6 +4229,15 @@ static inline void netdev_ref_replace(struct net_device *odev,
  */
 void linkwatch_fire_event(struct net_device *dev);
 
+/**
+ * linkwatch_sync_dev - sync linkwatch for the given device
+ * @dev: network device to sync linkwatch for
+ *
+ * Sync linkwatch for the given device, removing it from the
+ * pending work list (if queued).
+ */
+void linkwatch_sync_dev(struct net_device *dev);
+
 /**
  *     netif_carrier_ok - test if carrier present
  *     @dev: network device

diff --git a/net/core/dev.c b/net/core/dev.c
index c5679dfbaa70f7e00588acb889bfe17dbd118759..0432b04cf9b000628497345d9ec0e8a141a617a3 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10548,7 +10548,7 @@ void netdev_run_todo(void)
                write_lock(&dev_base_lock);
                dev->reg_state = NETREG_UNREGISTERED;
                write_unlock(&dev_base_lock);
-               linkwatch_forget_dev(dev);
+               linkwatch_sync_dev(dev);
        }
 
        while (!list_empty(&list)) {

diff --git a/net/core/dev.h b/net/core/dev.h
index 7795b8ad841d5e9c5041f07bf09d0c49648cdf84..cf93e188785ba7f0fd6e9428762bf02105eb3154 100644 (file)
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -30,7 +30,6 @@ int __init dev_proc_init(void);
 #endif
 
 void linkwatch_init_dev(struct net_device *dev);
-void linkwatch_forget_dev(struct net_device *dev);
 void linkwatch_run_queue(void);
 
 void dev_addr_flush(struct net_device *dev);

diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index c469d1c4db5d7a5d1fa3d4ae4a36f706c16bf25e..a19f21403339da3d5d737a7bf370d0b5ae38ef81 100644 (file)
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -245,7 +245,7 @@ static void __linkwatch_run_queue(int urgent_only)
        spin_unlock_irq(&lweventlist_lock);
 }
 
-void linkwatch_forget_dev(struct net_device *dev)
+void linkwatch_sync_dev(struct net_device *dev)
 {
        unsigned long flags;
        int clean = 0;

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index fccaa5bac0ed0a34a55bef1f4f6487a81285f286..d9b33e923b1873953a5234ec0c539171fe46972c 100644 (file)
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -194,8 +194,14 @@ static ssize_t carrier_show(struct device *dev,
 {
        struct net_device *netdev = to_net_dev(dev);
 
-       if (netif_running(netdev))
+       if (netif_running(netdev)) {
+               /* Synchronize carrier state with link watch,
+                * see also rtnl_getlink().
+                */
+               linkwatch_sync_dev(netdev);
+
                return sysfs_emit(buf, fmt_dec, !!netif_carrier_ok(netdev));
+       }
 
        return -EINVAL;
 }

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 592164c2a540413e69e89a8bd05c4f8de9eee9d7..5e0ab4c08f72ed857adb0fce1bdf9b086394c268 100644 (file)
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -3853,6 +3853,14 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
        if (nskb == NULL)
                goto out;
 
+       /* Synchronize the carrier state so we don't report a state
+        * that we're not actually going to honour immediately; if
+        * the driver just did a carrier off->on transition, we can
+        * only TX if link watch work has run, but without this we'd
+        * already report carrier on, even if it doesn't work yet.
+        */
+       linkwatch_sync_dev(dev);
+
        err = rtnl_fill_ifinfo(nskb, dev, net,
                               RTM_NEWLINK, NETLINK_CB(skb).portid,
                               nlh->nlmsg_seq, 0, 0, ext_filter_mask,

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 0b0ce4f81c017c50db62a00e90b84e7600254c0a..a977f8903467302f4cba1d93b74a4bf97d42a990 100644 (file)
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -58,6 +58,9 @@ static struct devlink *netdev_to_devlink_get(struct net_device *dev)
 
 u32 ethtool_op_get_link(struct net_device *dev)
 {
+       /* Synchronize carrier state with link watch, see also rtnl_getlink() */
+       linkwatch_sync_dev(dev);
+
        return netif_carrier_ok(dev) ? 1 : 0;
 }
 EXPORT_SYMBOL(ethtool_op_get_link);