netdev - Re: [PATCH net-next v1 04/11] vxlan: vxlan_rcv(): Extract vxlan

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: <87ldwuie1s.fsf@nvidia.com>
Date: Thu, 5 Dec 2024 11:44:25 +0100
From: Petr Machata <petrm@...dia.com>
To: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
CC: Petr Machata <petrm@...dia.com>, "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>, Simon Horman
	<horms@...nel.org>, Ido Schimmel <idosch@...dia.com>, <mlxsw@...dia.com>,
	Andrew Lunn <andrew+netdev@...n.ch>, Menglong Dong
	<menglong8.dong@...il.com>, Guillaume Nault <gnault@...hat.com>, "Alexander
 Lobakin" <aleksander.lobakin@...el.com>, Breno Leitao <leitao@...ian.org>
Subject: Re: [PATCH net-next v1 04/11] vxlan: vxlan_rcv(): Extract
 vxlan_hdr(skb) to a named variable


Mateusz Polchlopek <mateusz.polchlopek@...el.com> writes:

> On 12/3/2024 3:30 PM, Petr Machata wrote:
>
>> @@ -1713,7 +1714,7 @@ static int vxlan_rcv(struct sock *sk, struct sk_buff *skb)
>>   	 * used by VXLAN extensions if explicitly requested.
>>   	 */
>>   	if (vxlan->cfg.flags & VXLAN_F_GPE) {
>> -		if (!vxlan_parse_gpe_proto(vxlan_hdr(skb), &protocol))
>> +		if (!vxlan_parse_gpe_proto(vh, &protocol))
>>   			goto drop;
>>   		unparsed.vx_flags &= ~VXLAN_GPE_USED_BITS;
>>   		raw_proto = true;
>
> Overall that's cool refactor but I wonder - couldn't it be somehow
> merged with patch03? You touch vxlan_rcv function and the same
> pieces of code in both patches, so maybe you can do that there?
> Squash those two patches into one? It seems that in this patch you
> change something you already changed in prev patch - maybe
> it should be done in patch03? Or do I miss something?

Look, I'm juggling various bits back and forth and honestly it's all
much of a muchness. There's nothing obviously better whichever way you
package it. First changing to open-coded vxlan_hdr in 03 makes sense,
because it's already open-coded like that several times. Then we have a
clean 04 that replaces all the existing open-coded sites, including the
new one, thus everything is done in one go.

I'd just leave it as is, largely because I don't want to touch something
that works for frankly cosmetic reasons when the end result is the same.