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: <8b3eb0ea-3c8f-43ee-b756-21ea570c6ea0@kernel.org>
Date: Tue, 11 Feb 2025 11:21:34 +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

On 11/02/2025 11:13, Simon Horman wrote:
> On Tue, Feb 11, 2025 at 10:31:05AM +0100, Matthieu Baerts wrote:
>> 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
> 
> Hi Mat,
> 
> Thanks for the explanation. I see that in your process the Reviewed-by
> and Signed-off-by have distinct meanings. Which does make sense.
> 
> I'm ambivalent regarding which way to go (sorry that isn't very helpful).
> But I do suspect I won't be the last person to ask about this.

That's OK, now I have a canned reply ready to be sent for that :)

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