1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next v2 00/36] BPF path manager
  4. View patch

[PATCH mptcp-next v2 02/36] mptcp: use __lookup_addr in pm_netlink

Geliang Tang posted 36 patches 5 months, 4 weeks ago
[PATCH mptcp-next v2 02/36] mptcp: use __lookup_addr in pm_netlink
Posted by Geliang Tang 5 months, 4 weeks ago 
From: Geliang Tang <tanggeliang@kylinos.cn>

The helper __lookup_addr() can be used in mptcp_pm_nl_get_local_id()
and mptcp_pm_nl_is_backup() to simplify the code if using
list_for_each_entry_rcu() instead of list_for_each_entry() in it.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_netlink.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 618289aac0ab..a60a6fc04bf4 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -524,7 +524,7 @@ __lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info)
 {
 	struct mptcp_pm_addr_entry *entry;
 
-	list_for_each_entry(entry, &pernet->local_addr_list, list) {
+	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
 		if (mptcp_addresses_equal(&entry->addr, info, entry->addr.port))
 			return entry;
 	}
@@ -1146,12 +1146,9 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
 	pernet = pm_nl_get_pernet_from_msk(msk);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
-		if (mptcp_addresses_equal(&entry->addr, skc, entry->addr.port)) {
-			ret = entry->addr.id;
-			break;
-		}
-	}
+	entry = __lookup_addr(pernet, skc);
+	if (entry)
+		ret = entry->addr.id;
 	rcu_read_unlock();
 	if (ret >= 0)
 		return ret;
@@ -1181,12 +1178,9 @@ bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
 	bool backup = false;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
-		if (mptcp_addresses_equal(&entry->addr, skc, entry->addr.port)) {
-			backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
-			break;
-		}
-	}
+	entry = __lookup_addr(pernet, skc);
+	if (entry)
+		backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
 	rcu_read_unlock();
 
 	return backup;
-- 
2.45.2
Re: [PATCH mptcp-next v2 02/36] mptcp: use __lookup_addr in pm_netlink
Posted by Matthieu Baerts 5 months, 4 weeks ago 
Hi Geliang,

On 22/10/2024 11:14, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The helper __lookup_addr() can be used in mptcp_pm_nl_get_local_id()
> and mptcp_pm_nl_is_backup() to simplify the code if using
> list_for_each_entry_rcu() instead of list_for_each_entry() in it.

Mmh, please justify why it is OK to use the _rcu() variant without
having to modify the caller.

Did you check everything was OK when running the tests with these kconfig:

  CONFIG_RCU_EXPERT=y
  CONFIG_PROVE_RCU_LIST=y

I guess you will get new issues, no?

We might need to have __lookup_addr() and __lookup_addr_rcu() if you
want to avoid duplicated code.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v2 02/36] mptcp: use __lookup_addr in pm_netlink
Posted by Geliang Tang 5 months, 4 weeks ago 
On Tue, 2024-10-22 at 19:09 +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 22/10/2024 11:14, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > The helper __lookup_addr() can be used in
> > mptcp_pm_nl_get_local_id()
> > and mptcp_pm_nl_is_backup() to simplify the code if using
> > list_for_each_entry_rcu() instead of list_for_each_entry() in it.
> 
> Mmh, please justify why it is OK to use the _rcu() variant without
> having to modify the caller.
> 
> Did you check everything was OK when running the tests with these
> kconfig:
> 
>   CONFIG_RCU_EXPERT=y
>   CONFIG_PROVE_RCU_LIST=y
> 
> I guess you will get new issues, no?

Indeed.

> 
> We might need to have __lookup_addr() and __lookup_addr_rcu() if you
> want to avoid duplicated code.

Also remove this patch from this series, it has nothing to do with the
entire BPF path manager set, and other paths have no dependencies on
it.

I will release a v2 later separately.

Thanks,
-Geliang

> 
> Cheers,
> Matt
Re: [PATCH mptcp-next v2 02/36] mptcp: use __lookup_addr in pm_netlink
Posted by Matthieu Baerts 5 months, 4 weeks ago 
On 23/10/2024 11:59, Geliang Tang wrote:
> On Tue, 2024-10-22 at 19:09 +0200, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 22/10/2024 11:14, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> The helper __lookup_addr() can be used in
>>> mptcp_pm_nl_get_local_id()
>>> and mptcp_pm_nl_is_backup() to simplify the code if using
>>> list_for_each_entry_rcu() instead of list_for_each_entry() in it.
>>
>> Mmh, please justify why it is OK to use the _rcu() variant without
>> having to modify the caller.
>>
>> Did you check everything was OK when running the tests with these
>> kconfig:
>>
>>   CONFIG_RCU_EXPERT=y
>>   CONFIG_PROVE_RCU_LIST=y
>>
>> I guess you will get new issues, no?
> 
> Indeed.
> 
>>
>> We might need to have __lookup_addr() and __lookup_addr_rcu() if you
>> want to avoid duplicated code.
> 
> Also remove this patch from this series, it has nothing to do with the
> entire BPF path manager set, and other paths have no dependencies on
> it.

Will do!

Note that you could use __lookup_addr_rcu() that is being added with the
following patch (if it is accepted):

https://patchwork.kernel.org/project/mptcp/patch/20241022-mptcp-pm-lookup_addr_rcu-v1-1-19d45f26c872@kernel.org/

> I will release a v2 later separately.

Please wait for the v3, I'm still looking at the series (... doing that
slowly, when I have time :-/)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-next v2 02/36] mptcp: use __lookup_addr in pm_netlink
Posted by Geliang Tang 5 months, 4 weeks ago 
On Wed, 2024-10-23 at 12:03 +0200, Matthieu Baerts wrote:
> On 23/10/2024 11:59, Geliang Tang wrote:
> > On Tue, 2024-10-22 at 19:09 +0200, Matthieu Baerts wrote:
> > > Hi Geliang,
> > > 
> > > On 22/10/2024 11:14, Geliang Tang wrote:
> > > > From: Geliang Tang <tanggeliang@kylinos.cn>
> > > > 
> > > > The helper __lookup_addr() can be used in
> > > > mptcp_pm_nl_get_local_id()
> > > > and mptcp_pm_nl_is_backup() to simplify the code if using
> > > > list_for_each_entry_rcu() instead of list_for_each_entry() in
> > > > it.
> > > 
> > > Mmh, please justify why it is OK to use the _rcu() variant
> > > without
> > > having to modify the caller.
> > > 
> > > Did you check everything was OK when running the tests with these
> > > kconfig:
> > > 
> > >   CONFIG_RCU_EXPERT=y
> > >   CONFIG_PROVE_RCU_LIST=y
> > > 
> > > I guess you will get new issues, no?
> > 
> > Indeed.
> > 
> > > 
> > > We might need to have __lookup_addr() and __lookup_addr_rcu() if
> > > you
> > > want to avoid duplicated code.
> > 
> > Also remove this patch from this series, it has nothing to do with
> > the
> > entire BPF path manager set, and other paths have no dependencies
> > on
> > it.
> 
> Will do!
> 
> Note that you could use __lookup_addr_rcu() that is being added with
> the
> following patch (if it is accepted):
> 
> https://patchwork.kernel.org/project/mptcp/patch/20241022-mptcp-pm-lookup_addr_rcu-v1-1-19d45f26c872@kernel.org/
> 
> > I will release a v2 later separately.
> 
> Please wait for the v3, I'm still looking at the series (... doing
> that
> slowly, when I have time :-/)

Great, I appreciate it.

> 
> Cheers,
> Matt

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.