Re: [PATCH mptcp-net v2 3/3] mptcp: pm: avoid code duplication to lookup endp

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

Thank you for the review!

On 25/10/2024 12:37, Geliang Tang wrote:
> On Fri, 2024-10-25 at 11:32 +0200, Matthieu Baerts (NGI0) wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> The helper __lookup_addr_rcu() can be used in
>> mptcp_pm_nl_get_local_id()
>> and mptcp_pm_nl_is_backup() to simplify the code, and avoid code
>> duplication.

(...)

>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index
>> f38e1ccd34e95cd88b179a8b50e6965731542871..7c6e664b236d1659a554d003c78
>> c72ec91895ba5 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c

(...)

>> @@ -1191,15 +1187,11 @@ bool mptcp_pm_nl_is_backup(struct mptcp_sock
>> *msk, struct mptcp_addr_info *skc)
>>  {
>>  	struct pm_nl_pernet *pernet =
>> pm_nl_get_pernet_from_msk(msk);
>>  	struct mptcp_pm_addr_entry *entry;
>> -	bool backup = false;
>> +	bool backup;
>>  
>>  	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_rcu(pernet, skc);
>> +	backup = entry && !!(entry->flags &
>> MPTCP_PM_ADDR_FLAG_BACKUP);
> 
> I think we should check whether entry is NULL here too. No?

Yes, but that's what I did, no?

  backup = entry && (entry->flags & BACKUP)

"backup" is set to "true" if entry is not NULL and the backup flag is set.

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