Skip to content

Commit

Permalink
net: bonding: replace dev_trans_start() with the jiffies of the last …
Browse files Browse the repository at this point in the history 
…ARP/NS

The bonding driver piggybacks on time stamps kept by the network stack
for the purpose of the netdev TX watchdog, and this is problematic
because it does not work with NETIF_F_LLTX devices.

It is hard to say why the driver looks at dev_trans_start() of the
slave->dev, considering that this is updated even by non-ARP/NS probes
sent by us, and even by traffic not sent by us at all (for example PTP
on physical slave devices). ARP monitoring in active-backup mode appears
to still work even if we track only the last TX time of actual ARP
probes.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Vladimir Oltean authored and Jakub Kicinski committed Aug 4, 2022
1 parent f86d1fb commit 06799a9
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 16 deletions.
35 changes: 20 additions & 15 deletions drivers/net/bonding/bond_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2001,6 +2001,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
new_slave->target_last_arp_rx[i] = new_slave->last_rx;

new_slave->last_tx = new_slave->last_rx;

if (bond->params.miimon && !bond->params.use_carrier) {
link_reporting = bond_check_dev_link(bond, slave_dev, 1);

Expand Down Expand Up @@ -2884,8 +2886,11 @@ static void bond_arp_send(struct slave *slave, int arp_op, __be32 dest_ip,
return;
}

if (bond_handle_vlan(slave, tags, skb))
if (bond_handle_vlan(slave, tags, skb)) {
slave_update_last_tx(slave);
arp_xmit(skb);
}

return;
}

Expand Down Expand Up @@ -3074,8 +3079,7 @@ static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
curr_active_slave->last_link_up))
bond_validate_arp(bond, slave, tip, sip);
else if (curr_arp_slave && (arp->ar_op == htons(ARPOP_REPLY)) &&
bond_time_in_interval(bond,
dev_trans_start(curr_arp_slave->dev), 1))
bond_time_in_interval(bond, slave_last_tx(curr_arp_slave), 1))
bond_validate_arp(bond, slave, sip, tip);

out_unlock:
Expand Down Expand Up @@ -3103,8 +3107,10 @@ static void bond_ns_send(struct slave *slave, const struct in6_addr *daddr,
}

addrconf_addr_solict_mult(daddr, &mcaddr);
if (bond_handle_vlan(slave, tags, skb))
if (bond_handle_vlan(slave, tags, skb)) {
slave_update_last_tx(slave);
ndisc_send_skb(skb, &mcaddr, saddr);
}
}

static void bond_ns_send_all(struct bonding *bond, struct slave *slave)
Expand Down Expand Up @@ -3246,8 +3252,7 @@ static int bond_na_rcv(const struct sk_buff *skb, struct bonding *bond,
curr_active_slave->last_link_up))
bond_validate_ns(bond, slave, saddr, daddr);
else if (curr_arp_slave &&
bond_time_in_interval(bond,
dev_trans_start(curr_arp_slave->dev), 1))
bond_time_in_interval(bond, slave_last_tx(curr_arp_slave), 1))
bond_validate_ns(bond, slave, saddr, daddr);

out:
Expand Down Expand Up @@ -3335,12 +3340,12 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
* so it can wait
*/
bond_for_each_slave_rcu(bond, slave, iter) {
unsigned long trans_start = dev_trans_start(slave->dev);
unsigned long last_tx = slave_last_tx(slave);

bond_propose_link_state(slave, BOND_LINK_NOCHANGE);

if (slave->link != BOND_LINK_UP) {
if (bond_time_in_interval(bond, trans_start, 1) &&
if (bond_time_in_interval(bond, last_tx, 1) &&
bond_time_in_interval(bond, slave->last_rx, 1)) {

bond_propose_link_state(slave, BOND_LINK_UP);
Expand All @@ -3365,7 +3370,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
* when the source ip is 0, so don't take the link down
* if we don't know our ip yet
*/
if (!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
if (!bond_time_in_interval(bond, last_tx, bond->params.missed_max) ||
!bond_time_in_interval(bond, slave->last_rx, bond->params.missed_max)) {

bond_propose_link_state(slave, BOND_LINK_DOWN);
Expand Down Expand Up @@ -3431,7 +3436,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
*/
static int bond_ab_arp_inspect(struct bonding *bond)
{
unsigned long trans_start, last_rx;
unsigned long last_tx, last_rx;
struct list_head *iter;
struct slave *slave;
int commit = 0;
Expand Down Expand Up @@ -3482,9 +3487,9 @@ static int bond_ab_arp_inspect(struct bonding *bond)
* - (more than missed_max*delta since receive AND
* the bond has an IP address)
*/
trans_start = dev_trans_start(slave->dev);
last_tx = slave_last_tx(slave);
if (bond_is_active_slave(slave) &&
(!bond_time_in_interval(bond, trans_start, bond->params.missed_max) ||
(!bond_time_in_interval(bond, last_tx, bond->params.missed_max) ||
!bond_time_in_interval(bond, last_rx, bond->params.missed_max))) {
bond_propose_link_state(slave, BOND_LINK_DOWN);
commit++;
Expand All @@ -3501,8 +3506,8 @@ static int bond_ab_arp_inspect(struct bonding *bond)
*/
static void bond_ab_arp_commit(struct bonding *bond)
{
unsigned long trans_start;
struct list_head *iter;
unsigned long last_tx;
struct slave *slave;

bond_for_each_slave(bond, slave, iter) {
Expand All @@ -3511,10 +3516,10 @@ static void bond_ab_arp_commit(struct bonding *bond)
continue;

case BOND_LINK_UP:
trans_start = dev_trans_start(slave->dev);
last_tx = slave_last_tx(slave);
if (rtnl_dereference(bond->curr_active_slave) != slave ||
(!rtnl_dereference(bond->curr_active_slave) &&
bond_time_in_interval(bond, trans_start, 1))) {
bond_time_in_interval(bond, last_tx, 1))) {
struct slave *current_arp_slave;

current_arp_slave = rtnl_dereference(bond->current_arp_slave);
Expand Down
13 changes: 12 additions & 1 deletion include/net/bonding.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,9 @@ struct slave {
struct net_device *dev; /* first - useful for panic debug */
struct bonding *bond; /* our master */
int delay;
/* all three in jiffies */
/* all 4 in jiffies */
unsigned long last_link_up;
unsigned long last_tx;
unsigned long last_rx;
unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
s8 link; /* one of BOND_LINK_XXXX */
Expand Down Expand Up @@ -540,6 +541,16 @@ static inline unsigned long slave_last_rx(struct bonding *bond,
return slave->last_rx;
}

static inline void slave_update_last_tx(struct slave *slave)
{
WRITE_ONCE(slave->last_tx, jiffies);
}

static inline unsigned long slave_last_tx(struct slave *slave)
{
return READ_ONCE(slave->last_tx);
}

#ifdef CONFIG_NET_POLL_CONTROLLER
static inline netdev_tx_t bond_netpoll_send_skb(const struct slave *slave,
struct sk_buff *skb)
Expand Down

0 comments on commit 06799a9

Please sign in to comment.