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

[Matthieu Baerts]

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@xxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxx>
>>>> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@xxxxxxxxxx>
>>>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@xxxxxxxxxx>
>>>
>>> Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
>>
>> 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.