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

[Simon Horman <horms@kernel.org>]

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