RE: [RFC PATCH] net/tipc: reduce tipc_node lock holding time in tipc_rcv

From: xu
Date: Thu Nov 23 2023 - 01:22:16 EST


>>diff --git a/net/tipc/node.c b/net/tipc/node.c index 3105abe97bb9..2a036b8a7da3 100644
>>--- a/net/tipc/node.c
>>+++ b/net/tipc/node.c
>>@@ -2154,14 +2154,15 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
>> /* Receive packet directly if conditions permit */
>> tipc_node_read_lock(n);
>> if (likely((n->state == SELF_UP_PEER_UP) && (usr != TUNNEL_PROTOCOL))) {
>>+ tipc_node_read_unlock(n);
>> spin_lock_bh(&le->lock);
>> if (le->link) {
>> rc = tipc_link_rcv(le->link, skb, &xmitq);
>> skb = NULL;
>> }
>> spin_unlock_bh(&le->lock);
>>- }
>>- tipc_node_read_unlock(n);
>>+ } else
>>+ tipc_node_read_unlock(n);
>>
>> /* Check/update node state before receiving */
>> if (unlikely(skb)) {
>>@@ -2169,12 +2170,13 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
>> goto out_node_put;
>> tipc_node_write_lock(n);
>> if (tipc_node_check_state(n, skb, bearer_id, &xmitq)) {
>>+ tipc_node_write_unlock(n);
>> if (le->link) {
>> rc = tipc_link_rcv(le->link, skb, &xmitq);
>> skb = NULL;
>> }
>>- }
>>- tipc_node_write_unlock(n);
>>+ } else
>>+ tipc_node_write_unlock(n);
>> }
>>
>> if (unlikely(rc & TIPC_LINK_UP_EVT))
>>--
>>2.15.2
>>
>>
>This patch is wrong. le->link and link status must be protected by node lock. See what happens if tipc_node_timeout() is called, and the link goes down:
>tipc_node_timeout()
> tipc_node_link_down()
> {
> struct tipc_link *l = le->link;
> ...
> if (delete) {
> kfree(l);
> le->link = NULL;
> }
> ...
> }

Happy to see your reply. But Why? 'delete' is false from tipc_node_timeout(). Refer to:
https://elixir.bootlin.com/linux/v6.7-rc2/source/net/tipc/node.c#L844