When looking at something else, I noticed that the local endpoint
entries list was iterated under rcu_read_lock, but using
list_for_each_entry() instead of the _rcu variant. That's what patch 1
is fixing.

At the previous meeting, Mat and Christoph mentioned we could use the
RCU variant elsewhere. That's what is done in patch 2, for -next then.

Patch 3 is a simple change to remove duplicated code.

Note: I see that we are using spin_lock_bh(), but the RCU read "locks"
are always used without the _bh() variant. Is that OK here, or did we
miss something?

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v3:
- Use list_for_each_entry_rcu() with a 4th parameter: lockdep_is_held()
- See individual changelog in the different patches.
- Link to v2: https://lore.kernel.org/r/20241025-mptcp-pm-lookup_addr_rcu-v2-0-1478f6c4b205@kernel.org

Changes in v2:
- Add patch 2 and 3
- Patch 1: avoid > 80 chars per line in __lookup_addr_rcu() + update
  commit message.
- Link to v1: https://lore.kernel.org/r/20241022-mptcp-pm-lookup_addr_rcu-v1-1-19d45f26c872@kernel.org

---
Geliang Tang (1):
      mptcp: pm: avoid code duplication to lookup endp

Matthieu Baerts (NGI0) (2):
      mptcp: pm: use _rcu variant under rcu_read_lock
      mptcp: pm: lockless list traversal to dump endpoints

 net/mptcp/pm_netlink.c | 36 +++++++++++++++---------------------
 1 file changed, 15 insertions(+), 21 deletions(-)
---
base-commit: ca26062fcd85c61922f543674c5dd0382e2059cd
change-id: 20241022-mptcp-pm-lookup_addr_rcu-01833ea95155

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

Hello,

On 07/11/2024 10:04, Matthieu Baerts (NGI0) wrote:
> When looking at something else, I noticed that the local endpoint
> entries list was iterated under rcu_read_lock, but using
> list_for_each_entry() instead of the _rcu variant. That's what patch 1
> is fixing.
> 
> At the previous meeting, Mat and Christoph mentioned we could use the
> RCU variant elsewhere. That's what is done in patch 2, for -next then.
> 
> Patch 3 is a simple change to remove duplicated code.

Now in our tree (fixes for -net, and feat. for -next):

New patches for t/upstream-net and t/upstream:
- 562b779666e2: mptcp: pm: use _rcu variant under rcu_read_lock
- Results: 41c594936cc0..d67f9238deda (export-net)

New patches for t/upstream:
- bd3a08dc4788: mptcp: pm: lockless list traversal to dump endpoints
- (7045c531565f: tg:msg: title under < 50)
- 26fc0949dd21: mptcp: pm: avoid code duplication to lookup endp
- Results: 19f842ec6498..17e34effcf7b (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/5855ee9f27ce230c846422d4106358907c313f09/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/9a258dc7072ac24b37a8c6a8d03533981747ba6a/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

On 11/7/24 10:04, Matthieu Baerts (NGI0) wrote:
> When looking at something else, I noticed that the local endpoint
> entries list was iterated under rcu_read_lock, but using
> list_for_each_entry() instead of the _rcu variant. That's what patch 1
> is fixing.
> 
> At the previous meeting, Mat and Christoph mentioned we could use the
> RCU variant elsewhere. That's what is done in patch 2, for -next then.
> 
> Patch 3 is a simple change to remove duplicated code.
> 
> Note: I see that we are using spin_lock_bh(), but the RCU read "locks"
> are always used without the _bh() variant. Is that OK here, or did we
> miss something?
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
> Changes in v3:
> - Use list_for_each_entry_rcu() with a 4th parameter: lockdep_is_held()
> - See individual changelog in the different patches.
> - Link to v2: https://lore.kernel.org/r/20241025-mptcp-pm-lookup_addr_rcu-v2-0-1478f6c4b205@kernel.org
> 
> Changes in v2:
> - Add patch 2 and 3
> - Patch 1: avoid > 80 chars per line in __lookup_addr_rcu() + update
>   commit message.
> - Link to v1: https://lore.kernel.org/r/20241022-mptcp-pm-lookup_addr_rcu-v1-1-19d45f26c872@kernel.org
> 

FWIW, LGTM, too.

I don't add my explicit formal ack just to avoid possibly weird-looking
tag areas after merge on netdev ;)

/P