Re: [PATCH net-next] bonding: Remove support for use_carrier = 0
From: Jay Vosburgh <jv@jvosburgh.net>
Date: 2025-06-17 17:56:59
Also in:
linux-doc
Nikolay Aleksandrov [off-list ref] wrote:
On 6/17/25 00:28, Jay Vosburgh wrote:quoted
Remove the ability to disable use_carrier in bonding, and remove all code related to the old link state check that utilizes ethtool or ioctl to determine the link state of an interface in a bond. To avoid acquiring RTNL many times per second, bonding's miimon link monitor inspects link state under RCU, but not under RTNL. However, ethtool implementations in drivers may sleep, and therefore the ethtool or ioctl strategy is unsuitable for use with calls into driver ethtool functions. The use_carrier option was introduced in 2003, to provide backwards compatibility for network device drivers that did not support the then-new netif_carrier_ok/on/off system. Today, device drivers are expected to support netif_carrier_*, and the use_carrier backwards compatibility logic is no longer necessary. Bonding now always behaves as if use_carrier=1, which relies on netif_carrier_ok() to determine the link state of interfaces. This has been the default setting for use_carrier since its introduction. For backwards compatibility, the option itself remains, but may only be set to 1, and queries will always return 1. Reported-by: syzbot+b8c48ea38ca27d150063@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=b8c48ea38ca27d150063 Link: https://lore.kernel.org/lkml/000000000000eb54bf061cfd666a@google.com/ (local) Link: https://lore.kernel.org/netdev/20240718122017.d2e33aaac43a.I10ab9c9ded97163aef4e4de10985cd8f7de60d28@changeid/ (local) Link: http://lore.kernel.org/netdev/aEt6LvBMwUMxmUyx@mini-arch (local) Signed-off-by: Jay Vosburgh <jv@jvosburgh.net> --- Documentation/networking/bonding.rst | 79 +++---------------- drivers/net/bonding/bond_main.c | 113 ++------------------------- drivers/net/bonding/bond_netlink.c | 11 +-- drivers/net/bonding/bond_options.c | 7 +- drivers/net/bonding/bond_sysfs.c | 6 +- include/net/bonding.h | 1 - 6 files changed, 25 insertions(+), 192 deletions(-)[snip]quoted
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c index ac5e402c34bc..98f9bef61474 100644 --- a/drivers/net/bonding/bond_netlink.c +++ b/drivers/net/bonding/bond_netlink.c@@ -258,13 +258,8 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[], return err; } if (data[IFLA_BOND_USE_CARRIER]) { - int use_carrier = nla_get_u8(data[IFLA_BOND_USE_CARRIER]); - - bond_opt_initval(&newval, use_carrier); - err = __bond_opt_set(bond, BOND_OPT_USE_CARRIER, &newval, - data[IFLA_BOND_USE_CARRIER], extack); - if (err) - return err; + if (nla_get_u8(data[IFLA_BOND_USE_CARRIER]) != 1)you can set extack to send back an error to the user that use_carrier is now obsolete
Fair point, will add that and repost. -J
quoted
+ return -EINVAL; } if (data[IFLA_BOND_ARP_INTERVAL]) { int arp_interval = nla_get_u32(data[IFLA_BOND_ARP_INTERVAL]);@@ -676,7 +671,7 @@ static int bond_fill_info(struct sk_buff *skb, bond->params.peer_notif_delay * bond->params.miimon)) goto nla_put_failure; - if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, bond->params.use_carrier)) + if (nla_put_u8(skb, IFLA_BOND_USE_CARRIER, 1)) goto nla_put_failure; if (nla_put_u32(skb, IFLA_BOND_ARP_INTERVAL, bond->params.arp_interval))
--- -Jay Vosburgh, jv@jvosburgh.net