linux-kernel - Re: [PATCH v8 net-next 3/4] ipv4/udp: Add 4-tuple hash for connected socket

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] [day] [month] [year] [list]

Message-ID: <978cdfc7-f584-4ab5-a062-db19ed363691@redhat.com>
Date: Thu, 14 Nov 2024 10:12:58 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Philo Lu <lulie@...ux.alibaba.com>, netdev@...r.kernel.org
Cc: willemdebruijn.kernel@...il.com, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, dsahern@...nel.org, horms@...nel.org,
 antony.antony@...unet.com, steffen.klassert@...unet.com,
 linux-kernel@...r.kernel.org, dust.li@...ux.alibaba.com,
 jakub@...udflare.com, fred.cc@...baba-inc.com,
 yubing.qiuyubing@...baba-inc.com
Subject: Re: [PATCH v8 net-next 3/4] ipv4/udp: Add 4-tuple hash for connected
 socket

On 11/13/24 02:50, Philo Lu wrote:
> On 2024/11/12 22:58, Paolo Abeni wrote:
>> On 11/8/24 06:48, Philo Lu wrote:
>> [...]
>>> Signed-off-by: Philo Lu <lulie@...ux.alibaba.com>
>>> Signed-off-by: Cambda Zhu <cambda@...ux.alibaba.com>
>>> Signed-off-by: Fred Chen <fred.cc@...baba-inc.com>
>>> Signed-off-by: Yubing Qiu <yubing.qiuyubing@...baba-inc.com>
>>
>> [...]
>>> @@ -2937,7 +3128,7 @@ struct proto udp_prot = {
>>>   	.owner			= THIS_MODULE,
>>>   	.close			= udp_lib_close,
>>>   	.pre_connect		= udp_pre_connect,
>>> -	.connect		= ip4_datagram_connect,
>>> +	.connect		= udp_connect,
>>>   	.disconnect		= udp_disconnect,
>>>   	.ioctl			= udp_ioctl,
>>>   	.init			= udp_init_sock,
>>
>> 2 minor notes, possibly not needing a repost:
>>
>> - The SoB chain looks strange, do you mean co-developed-by actually?
> 
> Yes, we're all involved in the development. I think it could be 
> indicated by SoBs (and all of us agree with this). Please let me know if 
> I'm wrong :)
> 
> Or strictly as [1], it should be:
> 
> Co-developed-by: Cambda Zhu <cambda@...ux.alibaba.com>
> Signed-off-by: Cambda Zhu <cambda@...ux.alibaba.com>
> Co-developed-by: Fred Chen <fred.cc@...baba-inc.com>
> Signed-off-by: Fred Chen <fred.cc@...baba-inc.com>
> Co-developed-by: Yubing Qiu <yubing.qiuyubing@...baba-inc.com>
> Signed-off-by: Yubing Qiu <yubing.qiuyubing@...baba-inc.com>
> Signed-off-by: Philo Lu <lulie@...ux.alibaba.com>
> 
> [1]
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

The latter (with co-dev-by tags) I think would be better, it will give
full attribution to all of you. Since the scope of this work is relevant
I think it would be better a complete recognition.

>> - udplite is not touched. AFAICS should not be a problem - just the
>> feature will not be available for udplite. 
> 
> Agreed. Theoretically, the feature relies on udp4_hash4/udp6_hash4 when 
> connecting, and all other functions including lookup/unhash/rehash 
> always check "hashed4" firstly, and do nothing if it's false (which is 
> the case for udplite).
> 
> AFAICT, the effects to udplite include:
> - Additional memory consumption in udp_sock and udptable
> - Control path: udp_hashed4 checking when unhash/rehash
> - Data path: udp_has_hash4 checking when lookup
>               (like unconnected sks in udp)

Looks good. Please note explicitly in the cover letter that udplite is
intentionally excluded here, the reason why such choice is safe
(basically a synopsis of the above) and repost with the update tags
chain including my Acked-by tag
(you should retain Willem acked-by tag, too.)

Thanks!

Paolo