| From c202050a456a70d021d7fc9b15a94daf84e17162 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Tue, 29 Nov 2022 17:48:13 +0100 |
| Subject: hsr: Synchronize sequence number updates. |
| |
| From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> |
| |
| [ Upstream commit 5c7aa13210c3abdd34fd421f62347665ec6eb551 ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/hsr/hsr_framereg.c | 9 ++++++++- |
| net/hsr/hsr_framereg.h | 2 ++ |
| 2 files changed, 10 insertions(+), 1 deletion(-) |
| |
| diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c |
| index 440788e5b3e2..414bf4d3d3c9 100644 |
| --- a/net/hsr/hsr_framereg.c |
| +++ b/net/hsr/hsr_framereg.c |
| @@ -159,6 +159,7 @@ static struct hsr_node *hsr_add_node(struct hsr_priv *hsr, |
| return NULL; |
| |
| ether_addr_copy(new_node->macaddress_A, addr); |
| + spin_lock_init(&new_node->seq_out_lock); |
| |
| /* We are only interested in time diffs here, so use current jiffies |
| * as initialization. (0 could trigger an spurious ring error warning). |
| @@ -313,6 +314,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame) |
| goto done; |
| |
| ether_addr_copy(node_real->macaddress_B, ethhdr->h_source); |
| + spin_lock_bh(&node_real->seq_out_lock); |
| for (i = 0; i < HSR_PT_PORTS; i++) { |
| if (!node_curr->time_in_stale[i] && |
| time_after(node_curr->time_in[i], node_real->time_in[i])) { |
| @@ -323,6 +325,7 @@ void hsr_handle_sup_frame(struct hsr_frame_info *frame) |
| if (seq_nr_after(node_curr->seq_out[i], node_real->seq_out[i])) |
| node_real->seq_out[i] = node_curr->seq_out[i]; |
| } |
| + spin_unlock_bh(&node_real->seq_out_lock); |
| node_real->addr_B_port = port_rcv->type; |
| |
| spin_lock_bh(&hsr->list_lock); |
| @@ -419,13 +422,17 @@ void hsr_register_frame_in(struct hsr_node *node, struct hsr_port *port, |
| int hsr_register_frame_out(struct hsr_port *port, struct hsr_node *node, |
| u16 sequence_nr) |
| { |
| + spin_lock_bh(&node->seq_out_lock); |
| if (seq_nr_before_or_eq(sequence_nr, node->seq_out[port->type]) && |
| time_is_after_jiffies(node->time_out[port->type] + |
| - msecs_to_jiffies(HSR_ENTRY_FORGET_TIME))) |
| + msecs_to_jiffies(HSR_ENTRY_FORGET_TIME))) { |
| + spin_unlock_bh(&node->seq_out_lock); |
| return 1; |
| + } |
| |
| node->time_out[port->type] = jiffies; |
| node->seq_out[port->type] = sequence_nr; |
| + spin_unlock_bh(&node->seq_out_lock); |
| return 0; |
| } |
| |
| diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h |
| index f9c83dc02ca5..48990166e4c4 100644 |
| --- a/net/hsr/hsr_framereg.h |
| +++ b/net/hsr/hsr_framereg.h |
| @@ -69,6 +69,8 @@ void prp_update_san_info(struct hsr_node *node, bool is_sup); |
| |
| struct hsr_node { |
| struct list_head mac_list; |
| + /* Protect R/W access to seq_out */ |
| + spinlock_t seq_out_lock; |
| unsigned char macaddress_A[ETH_ALEN]; |
| unsigned char macaddress_B[ETH_ALEN]; |
| /* Local slave through which AddrB frames are received from this node */ |
| -- |
| 2.35.1 |
| |