Commit facd15df authored by Johannes Berg's avatar Johannes Berg Committed by Jakub Kicinski

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: default avatarJohannes Berg <johannes.berg@intel.com>
Reviewed-by: default avatarJiri Pirko <jiri@nvidia.com>
Link: https://lore.kernel.org/r/20231204214706.303c62768415.I1caedccae72ee5a45c9085c5eb49c145ce1c0dd5@changeidSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent faf4cf74
...@@ -4229,6 +4229,15 @@ static inline void netdev_ref_replace(struct net_device *odev, ...@@ -4229,6 +4229,15 @@ static inline void netdev_ref_replace(struct net_device *odev,
*/ */
void linkwatch_fire_event(struct net_device *dev); 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 * netif_carrier_ok - test if carrier present
* @dev: network device * @dev: network device
......
...@@ -10548,7 +10548,7 @@ void netdev_run_todo(void) ...@@ -10548,7 +10548,7 @@ void netdev_run_todo(void)
write_lock(&dev_base_lock); write_lock(&dev_base_lock);
dev->reg_state = NETREG_UNREGISTERED; dev->reg_state = NETREG_UNREGISTERED;
write_unlock(&dev_base_lock); write_unlock(&dev_base_lock);
linkwatch_forget_dev(dev); linkwatch_sync_dev(dev);
} }
while (!list_empty(&list)) { while (!list_empty(&list)) {
......
...@@ -30,7 +30,6 @@ int __init dev_proc_init(void); ...@@ -30,7 +30,6 @@ int __init dev_proc_init(void);
#endif #endif
void linkwatch_init_dev(struct net_device *dev); void linkwatch_init_dev(struct net_device *dev);
void linkwatch_forget_dev(struct net_device *dev);
void linkwatch_run_queue(void); void linkwatch_run_queue(void);
void dev_addr_flush(struct net_device *dev); void dev_addr_flush(struct net_device *dev);
......
...@@ -245,7 +245,7 @@ static void __linkwatch_run_queue(int urgent_only) ...@@ -245,7 +245,7 @@ static void __linkwatch_run_queue(int urgent_only)
spin_unlock_irq(&lweventlist_lock); spin_unlock_irq(&lweventlist_lock);
} }
void linkwatch_forget_dev(struct net_device *dev) void linkwatch_sync_dev(struct net_device *dev)
{ {
unsigned long flags; unsigned long flags;
int clean = 0; int clean = 0;
......
...@@ -194,8 +194,14 @@ static ssize_t carrier_show(struct device *dev, ...@@ -194,8 +194,14 @@ static ssize_t carrier_show(struct device *dev,
{ {
struct net_device *netdev = to_net_dev(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 sysfs_emit(buf, fmt_dec, !!netif_carrier_ok(netdev));
}
return -EINVAL; return -EINVAL;
} }
......
...@@ -3853,6 +3853,14 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3853,6 +3853,14 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (nskb == NULL) if (nskb == NULL)
goto out; 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, err = rtnl_fill_ifinfo(nskb, dev, net,
RTM_NEWLINK, NETLINK_CB(skb).portid, RTM_NEWLINK, NETLINK_CB(skb).portid,
nlh->nlmsg_seq, 0, 0, ext_filter_mask, nlh->nlmsg_seq, 0, 0, ext_filter_mask,
......
...@@ -58,6 +58,9 @@ static struct devlink *netdev_to_devlink_get(struct net_device *dev) ...@@ -58,6 +58,9 @@ static struct devlink *netdev_to_devlink_get(struct net_device *dev)
u32 ethtool_op_get_link(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; return netif_carrier_ok(dev) ? 1 : 0;
} }
EXPORT_SYMBOL(ethtool_op_get_link); EXPORT_SYMBOL(ethtool_op_get_link);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment