netdev - Re: [PATCH] ipv4: remove useless arg

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: <cbd3f835-0d01-46fa-9125-a0fbd5f50919@redhat.com>
Date: Wed, 4 Dec 2024 10:16:26 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: tianyu2 <tianyu2@...nelsoft.com>
Cc: eric.dumazet@...il.com, Pablo Neira Ayuso <pablo@...filter.org>,
 netdev@...r.kernel.org
Subject: Re: [PATCH] ipv4: remove useless arg

On 12/4/24 04:16, tianyu2 wrote:
>> On 12/2/24 04:32, tianyu2 wrote:
>>> The "struct sock *sk" parameter in ip_rcv_finish_core is unused, which
>>> leads the compiler to optimize it out. As a result, the
>>> "struct sk_buff *skb" parameter is passed using x1. And this make kprobe
>>> hard to use.
>>>
>>> Signed-off-by: tianyu2 <tianyu2@...nelsoft.com>
>>
>> The patch code good, but the above does not look like a real name?!?
>>
>> If so, please re-submit, using your real full name and including the
>> target tree (net-next in this case) in the subj prefix.
>>
>> See:
>> https://elixir.bootlin.com/linux/v6.12.1/source/Documentation/process/submitting-patches.rst#L440
>> https://elixir.bootlin.com/linux/v6.12.1/source/Documentation/process/maintainer-netdev.rst#L12
>>
>> @Pablo: after this change will be merged, I *think* that a possible
>> follow-up could drop the 'sk' arg from NF_HOOK_LIST and ip_rcv_finish() too.
>>
>> Thanks!
>>
>> Paolo
> 
> Thank you for the reminder. I’ll adjust the patch format in the next version.
> 
> If ip_rcv_finish is modified,  NF_HOOK/NF_HOOK_LIST also needs to be adjusted. I noticed that many places use NF_HOOK. These modifications should be fine, right?

Ouch, I missed the NF_HOOK implication. Touching all NF_HOOK()
call-sites looks IMHO way to invasive to justify this change.

> However, I found that the ip_rcv_finish function doesn’t seem to be optimized by the compiler.
> (ARM64)( gcc version 8.5.0 20210514 (Red Hat 8.5.0-4) (GCC) )

FTR, that is quite an old one :) You should try with gcc 13.

Thanks,

Paolo