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

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

Message-ID: <f478c65a-d70d-4cf8-a742-e0456cf97fd4@intel.com>
Date: Thu, 5 Dec 2024 12:22:05 +0100
From: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
To: Petr Machata <petrm@...dia.com>
CC: "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



On 12/5/2024 11:44 AM, Petr Machata wrote:
> 
> 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.

Okay, you convinced me, so for this patch:

Reviewed-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>