netdev - Re: [PATCH net-next v3 01/15] mptcp: pm: drop info of userspace_pm_remove_id_zero

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <adeff1d6-f80b-4a2c-b4bb-da44ecd5b747@kernel.org>
Date: Tue, 11 Feb 2025 10:31:05 +0100
From: Matthieu Baerts <matttbe@...nel.org>
To: Simon Horman <horms@...nel.org>
Cc: mptcp@...ts.linux.dev, Mat Martineau <martineau@...nel.org>,
 Geliang Tang <geliang@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v3 01/15] mptcp: pm: drop info of
 userspace_pm_remove_id_zero_address

Hi Simon,

On 10/02/2025 20:49, Simon Horman wrote:
> On Fri, Feb 07, 2025 at 02:59:19PM +0100, Matthieu Baerts (NGI0) wrote:
>> From: Geliang Tang <tanggeliang@...inos.cn>
>>
>> The only use of 'info' parameter of userspace_pm_remove_id_zero_address()
>> is to set an error message into it.
>>
>> Plus, this helper will only fail when it cannot find any subflows with a
>> local address ID 0.
>>
>> This patch drops this parameter and sets the error message where this
>> function is called in mptcp_pm_nl_remove_doit().
>>
>> Signed-off-by: Geliang Tang <tanggeliang@...inos.cn>
>> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@...nel.org>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@...nel.org>
> 
> Reviewed-by: Simon Horman <horms@...nel.org>

Thank you for the review, and this message!

> A minor nit, perhaps it has been discussed before:
> 
> I'm not sure that your Reviewed-by is needed if you also provide
> your Signed-off-by. Because it I think that the latter implies the former.

This has been discussed a while ago, but only on the MPTCP list I think.
To be honest, we didn't find a precise answer in the doc [1], and maybe
we are doing it wrong for all this time :)

Technically, when someone shares a patch on the MPTCP ML, someone else
does the review, sent the "Reviewed-by" tag, then the patch is queued,
and the one sending the patch to the netdev ML adds a "Signed-off-by"
tag. With this patch here, I did both.

Before, we were removing the RvB tag when it was the same as the SoB
one, but we stopped doing that because we thought that was not correct
and / or not needed. We can re-introduce this if preferred. My
understanding is that the SoB tag is for the authors and the
intermediate maintainers -- who might have not done a full review --
while the RvB one seems to indicate that a "proper" review has been
done. If someone else does a review on a patch, I can add my SoB tag
when "forwarding" the patch, trusting the review done by someone else.

Do you think it is better to remove the RvB tag if there is a SoB one
for the same person?

[1] https://docs.kernel.org/process/submitting-patches.html

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