Re: [Intel-wired-lan] [PATCH net-next] ice: Fix implicit cast u32 to u16

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Paul Menzel <pmenzel@molgen.mpg.de>
Date: Tue, 28 Feb 2023 16:44:17 +0100

> Dear Alexander,
> 
> 
> Am 28.02.23 um 16:21 schrieb Alexander Lobakin:
>> From: Paul Menzel <pmenzel@molgen.mpg.de>
>> Date: Tue, 28 Feb 2023 11:01:00 +0100
> 
>>> Am 28.02.23 um 09:49 schrieb Kalyan Kodamagula:
>>>> From: Marcin Szycik <marcin.szycik@intel.com>
>>>>
>>>> Fix implicit cast by changing argument types of two functions to
>>>> correct
>>>> types.
>>>>
>>>> Signed-off-by: Marcin Szycik <marcin.szycik@intel.com>
>>>> Signed-off-by: Kalyan Kodamagula <kalyan.kodamagula@intel.com>
>>>> ---
>>>>    drivers/net/ethernet/intel/ice/ice_ddp.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ice/ice_ddp.c
>>>> b/drivers/net/ethernet/intel/ice/ice_ddp.c
>>>> index d71ed210f9c4..830fa53b5e0a 100644
>>>> --- a/drivers/net/ethernet/intel/ice/ice_ddp.c
>>>> +++ b/drivers/net/ethernet/intel/ice/ice_ddp.c
>>>> @@ -701,14 +701,14 @@ struct ice_buf_build *ice_pkg_buf_alloc(struct
>>>> ice_hw *hw)
>>>>        return bld;
>>>>    }
>>>>    -static bool ice_is_gtp_u_profile(u16 prof_idx)
>>>> +static bool ice_is_gtp_u_profile(u32 prof_idx)
>>>>    {
>>>>        return (prof_idx >= ICE_PROFID_IPV6_GTPU_TEID &&
>>>>            prof_idx <= ICE_PROFID_IPV6_GTPU_IPV6_TCP_INNER) ||
>>>>               prof_idx == ICE_PROFID_IPV4_GTPU_TEID;
>>>>    }
>>>>    -static bool ice_is_gtp_c_profile(u16 prof_idx)
>>>> +static bool ice_is_gtp_c_profile(u32 prof_idx)
>>>>    {
>>>>        switch (prof_idx) {
>>>>        case ICE_PROFID_IPV4_GTPC_TEID:
>>>
>>> Is there a reason to limit the length or could `unsigned int` be used?
>>
>> You mean the string length? But what's the point of using `unsigned int`
>> if we have shorter and more elegant `u32`, which at the same time
>> explicitly states its width? :)
>> I've been encouraging lots o' folks to prefer the "shorties" where
>> possible (I basically only use {,unsigned} long from the "basic" types)
>> and now this :p I'm not saying any opinion is correct or incorrect here,
>> since it's a matter of taste mostly I believe, just curious.
> 
> If in future architectures, the smallest native size is 64 bit, than
> unnecessarily truncating the length, would create not optimal code.

So one day u32 will start forcing CPU to mask the results of operations?
O_________O
I thought HW will always be like "32 bit is fine" :s
But wait, does it mean `unsigned int` will become 8 byte there? This
would break a lot of stuff in the kernel I believe...

> Judging from the defined macros, `prof_idx` also does not need to be 32
> bit wide. (But it’s all microoptimization.)
> 
> 
> Kind regards,
> 
> Paul
Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan