[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20241217080944.3971820-1-buaajxlj@163.com>
Date: Tue, 17 Dec 2024 16:09:44 +0800
From: Liang Jie <buaajxlj@....com>
To: erdnetdev@...il.com
Cc: andrew+netdev@...n.ch,
anthony.l.nguyen@...el.com,
buaajxlj@....com,
davem@...emloft.net,
edumazet@...gle.com,
horms@...nel.org,
kuba@...nel.org,
liangjie@...iang.com,
linux-kernel@...r.kernel.org,
netdev@...r.kernel.org,
pabeni@...hat.com
Subject: Re: [PATCH] net: Refine key_len calculations in rhashtable_params
On Tue, 17 Dec 2024 08:33:57 +0100, ericnetdev dumazet wrote:
>>
>> From: Liang Jie <liangjie@...iang.com>
>>
>> This patch improves the calculation of key_len in the rhashtable_params
>> structures across the net driver modules by replacing hardcoded sizes
>> and previous calculations with appropriate macros like sizeof_field()
>> and offsetofend().
>>
>> Previously, key_len was set using hardcoded sizes like sizeof(u32) or
>> sizeof(unsigned long), or using offsetof() calculations. This patch
>> replaces these with sizeof_field() and correct use of offsetofend(),
>> making the code more robust, maintainable, and improving readability.
>>
>> Using sizeof_field() and offsetofend() provides several advantages:
>> - They explicitly specify the size of the field or the end offset of a
>> member being used as a key.
>> - They ensure that the key_len is accurate even if the structs change in
>> the future.
>> - They improve code readability by clearly indicating which fields are used
>> and how their sizes are determined, making the code easier to understand
>> and maintain.
>>
>> For example, instead of:
>> .key_len = sizeof(u32),
>> we now use:
>> .key_len = sizeof_field(struct mae_mport_desc, mport_id),
>>
>> And instead of:
>> .key_len = offsetof(struct efx_tc_encap_match, linkage),
>> we now use:
>> .key_len = offsetofend(struct efx_tc_encap_match, ip_tos_mask),
>>
>> These changes eliminate the risk of including unintended padding or extra
>> data in the key, ensuring the rhashtable functions correctly.
>
>I do not see how this patch can eliminate padding.
>
>If keys include holes or padding, something still needs to clear the
>holes/padding in objects and lookup keys.
>
>struct key {
> u8 first_component;
> u32 second_component;
>};
You are right, this patch can not eliminate padding in the case you mentioned.
This patch addresses the following cases present in the current code:
struct efx_tc_encap_match {
__be32 src_ip, dst_ip;
struct in6_addr src_ip6, dst_ip6;
__be16 udp_dport;
__be16 udp_sport, udp_sport_mask;
u8 ip_tos, ip_tos_mask;
<there may be padding gap here>
struct rhash_head linkage;
......
};
Instead of:
.key_len = offsetof(struct efx_tc_encap_match, linkage),
now use:
.key_len = offsetofend(struct efx_tc_encap_match, ip_tos_mask),
Powered by blists - more mailing lists