Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

hsr: Synchronize sequence number updates.

hsr_register_frame_out() compares new sequence_nr vs the old one
recorded in hsr_node::seq_out and if the new sequence_nr is higher then
it will be written to hsr_node::seq_out as the new value.

This operation isn't locked so it is possible that two frames with the
same sequence number arrive (via the two slave devices) and are fed to
hsr_register_frame_out() at the same time. Both will pass the check and
update the sequence counter later to the same value. As a result the
content of the same packet is fed into the stack twice.

This was noticed by running ping and observing DUP being reported from
time to time.

Instead of using the hsr_priv::seqnr_lock for the whole receive path (as
it is for sending in the master node) add an additional lock that is only
used for sequence number checks and updates.

Add a per-node lock that is used during sequence number reads and
updates.

Fixes: f421436a591d3 ("net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Sebastian Andrzej Siewior and committed by
Jakub Kicinski
5c7aa132 06afd2c3

+10 -1
+8 -1
net/hsr/hsr_framereg.c
··· 157 157 return NULL; 158 158 159 159 ether_addr_copy(new_node->macaddress_A, addr); 160 + spin_lock_init(&new_node->seq_out_lock); 160 161 161 162 /* We are only interested in time diffs here, so use current jiffies 162 163 * as initialization. (0 could trigger an spurious ring error warning). ··· 354 353 } 355 354 356 355 ether_addr_copy(node_real->macaddress_B, ethhdr->h_source); 356 + spin_lock_bh(&node_real->seq_out_lock); 357 357 for (i = 0; i < HSR_PT_PORTS; i++) { 358 358 if (!node_curr->time_in_stale[i] && 359 359 time_after(node_curr->time_in[i], node_real->time_in[i])) { ··· 365 363 if (seq_nr_after(node_curr->seq_out[i], node_real->seq_out[i])) 366 364 node_real->seq_out[i] = node_curr->seq_out[i]; 367 365 } 366 + spin_unlock_bh(&node_real->seq_out_lock); 368 367 node_real->addr_B_port = port_rcv->type; 369 368 370 369 spin_lock_bh(&hsr->list_lock); ··· 459 456 int hsr_register_frame_out(struct hsr_port *port, struct hsr_node *node, 460 457 u16 sequence_nr) 461 458 { 459 + spin_lock_bh(&node->seq_out_lock); 462 460 if (seq_nr_before_or_eq(sequence_nr, node->seq_out[port->type]) && 463 461 time_is_after_jiffies(node->time_out[port->type] + 464 - msecs_to_jiffies(HSR_ENTRY_FORGET_TIME))) 462 + msecs_to_jiffies(HSR_ENTRY_FORGET_TIME))) { 463 + spin_unlock_bh(&node->seq_out_lock); 465 464 return 1; 465 + } 466 466 467 467 node->time_out[port->type] = jiffies; 468 468 node->seq_out[port->type] = sequence_nr; 469 + spin_unlock_bh(&node->seq_out_lock); 469 470 return 0; 470 471 } 471 472
+2
net/hsr/hsr_framereg.h
··· 69 69 70 70 struct hsr_node { 71 71 struct list_head mac_list; 72 + /* Protect R/W access to seq_out */ 73 + spinlock_t seq_out_lock; 72 74 unsigned char macaddress_A[ETH_ALEN]; 73 75 unsigned char macaddress_B[ETH_ALEN]; 74 76 /* Local slave through which AddrB frames are received from this node */