Subject: Re: [PATCH net-next v1 04/11] vxlan: vxlan_rcv(): Extract vxlan_hdr(skb) to a named variable

From: Petr Machata <[email-masked]>
Date: Thu, 5 Dec 2024 11:44:25 +0100

Mateusz Polchlopek <[email-masked]> 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.