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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250211101315.GJ554665@kernel.org>
Date: Tue, 11 Feb 2025 10:13:15 +0000
From: Simon Horman <horms@...nel.org>
To: Matthieu Baerts <matttbe@...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 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.