[Intel-wired-lan] [PATCH net v2] ice: refactor to remove not needed packing

[Alexander Lobakin]

From: Brandeburg, Jesse <jesse.brandeburg at intel.com>
Date: Thu, 30 Mar 2023 12:20:49 -0700

> After the changes to the structures to make them flex array safe,
> packing is no longer necessary.
> 
> to reproduce:
> make EXTRA_CFLAGS=-Wpacked drivers/net/ethernet/intel/ice/ice.ko

When have we started to fix hypothetical custom compiler flags?
`-Wpacked` is enabled only on `W=3`[0], which's comment clearly says:
"lots of noise, you can safely ignore those"[1].

Anyway, see below.

> 
> In file included from drivers/net/ethernet/intel/ice/ice_controlq.h:7,
>                  from drivers/net/ethernet/intel/ice/ice_type.h:14,
>                  from drivers/net/ethernet/intel/ice/ice.h:59:
> drivers/net/ethernet/intel/ice/ice_adminq_cmd.h:618:1: warning: packed attribute is unnecessary for ‘ice_aqc_sw_rules_elem_hdr’ [-Wpacked]
>   618 | } __packed __aligned(sizeof(__le16));
>       | ^
> drivers/net/ethernet/intel/ice/ice_adminq_cmd.h:705:1: warning: packed attribute is unnecessary for ‘ice_sw_rule_lkup_rx_tx’ [-Wpacked]
>   705 | } __packed __aligned(sizeof(__le16));
>       | ^
> drivers/net/ethernet/intel/ice/ice_adminq_cmd.h:767:1: warning: packed attribute is unnecessary for ‘ice_sw_rule_lg_act’ [-Wpacked]
>   767 | } __packed __aligned(sizeof(__le16));
>       | ^
> drivers/net/ethernet/intel/ice/ice_adminq_cmd.h:779:1: warning: packed attribute is unnecessary for ‘ice_sw_rule_vsi_list’ [-Wpacked]
>   779 | } __packed __aligned(sizeof(__le16));
>       | ^

Packing here is completely unrelated to zero-fake-flex-arrays. It's
added because those structures are shared between HW and SW. In such
cases, you need 1:1 match, so you can't allow the compiler do whatever
it wants with paddings and stuff.
Each `__packed` is balanced here by `__aligned` to not blow up the
object code. The `-Wpacked` warnings come from the fact that there's
nothing to pack -- the layout are organized the way that each field has
natural alignment. It doesn't mean there's something wrong with the code
at all.
But if you have a really good reason to remove those, please add static
assertions to make sure each structure's size is precisely the one that
is described in the specs, otherwise you may face obscure bugs one day
on some weird arch/compiler setup. And provide bloat-o-meter stats,
which would clearly show that removing `__packed` improves codegen.

> 
> Fixes: 6e1ff618737a ("ice: fix access-beyond-end in the switch code")

Definitely incorrect, this change doesn't "fix" anything.

> Signed-off-by: Jesse Brandeburg <jesse.brandeburg at intel.com>
> ---
> v2: send to net instead of net-next
[...]

[0]
https://elixir.bootlin.com/linux/latest/source/scripts/Makefile.extrawarn#L99
[1]
https://elixir.bootlin.com/linux/latest/source/scripts/Makefile.extrawarn#L91

Thanks,
Olek