| 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
| ||
|
Message-ID: <ZC6uXW5WMcnAet7F@localhost.localdomain>
Date: Thu, 6 Apr 2023 13:34:53 +0200
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
wojciech.drewek@...el.com, piotr.raczynski@...el.com,
pmenzel@...gen.mpg.de
Subject: Re: [PATCH net-next v3 4/5] ice: specify field names in ice_prot_ext
init
On Wed, Apr 05, 2023 at 03:25:53PM +0200, Alexander Lobakin wrote:
> From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Date: Wed, 5 Apr 2023 09:51:12 +0200
>
> > Anonymous initializers are now discouraged. Define ICE_PROTCOL_ENTRY
> > macro to rewrite anonymous initializers to named one. No functional
> > changes here.
> >
> > Suggested-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > ---
> > drivers/net/ethernet/intel/ice/ice_switch.c | 68 +++++++++++----------
> > 1 file changed, 36 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
> > index b55cdb9a009f..8872e26d1368 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_switch.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_switch.c
> > @@ -4540,6 +4540,11 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
> > return status;
> > }
> >
> > +#define ICE_PROTOCOL_ENTRY(id, ...) { \
> > + .prot_type = id, \
> > + .offs = {__VA_ARGS__}, \
>
> Minor: please use one tab in between field name and `=` sign (you have
> spaces there for now).
>
> > +}
> > +
> > /* This is mapping table entry that maps every word within a given protocol
> > * structure to the real byte offset as per the specification of that
> > * protocol header.
> > @@ -4550,38 +4555,37 @@ ice_free_res_cntr(struct ice_hw *hw, u8 type, u8 alloc_shared, u16 num_items,
> > * structure is added to that union.
> > */
> > static const struct ice_prot_ext_tbl_entry ice_prot_ext[ICE_PROTOCOL_LAST] = {
> > - { ICE_MAC_OFOS, { 0, 2, 4, 6, 8, 10, 12 } },
> > - { ICE_MAC_IL, { 0, 2, 4, 6, 8, 10, 12 } },
> > - { ICE_ETYPE_OL, { 0 } },
> > - { ICE_ETYPE_IL, { 0 } },
> > - { ICE_VLAN_OFOS, { 2, 0 } },
> > - { ICE_IPV4_OFOS, { 0, 2, 4, 6, 8, 10, 12, 14, 16, 18 } },
> > - { ICE_IPV4_IL, { 0, 2, 4, 6, 8, 10, 12, 14, 16, 18 } },
> > - { ICE_IPV6_OFOS, { 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24,
> > - 26, 28, 30, 32, 34, 36, 38 } },
> > - { ICE_IPV6_IL, { 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24,
> > - 26, 28, 30, 32, 34, 36, 38 } },
> > - { ICE_TCP_IL, { 0, 2 } },
> > - { ICE_UDP_OF, { 0, 2 } },
> > - { ICE_UDP_ILOS, { 0, 2 } },
> > - { ICE_VXLAN, { 8, 10, 12, 14 } },
> > - { ICE_GENEVE, { 8, 10, 12, 14 } },
> > - { ICE_NVGRE, { 0, 2, 4, 6 } },
> > - { ICE_GTP, { 8, 10, 12, 14, 16, 18, 20, 22 } },
> > - { ICE_GTP_NO_PAY, { 8, 10, 12, 14 } },
> > - { ICE_PPPOE, { 0, 2, 4, 6 } },
> > - { ICE_L2TPV3, { 0, 2, 4, 6, 8, 10 } },
> > - { ICE_VLAN_EX, { 2, 0 } },
> > - { ICE_VLAN_IN, { 2, 0 } },
> > - { ICE_HW_METADATA, { ICE_SOURCE_PORT_MDID_OFFSET,
> > - ICE_PTYPE_MDID_OFFSET,
> > - ICE_PACKET_LENGTH_MDID_OFFSET,
> > - ICE_SOURCE_VSI_MDID_OFFSET,
> > - ICE_PKT_VLAN_MDID_OFFSET,
> > - ICE_PKT_TUNNEL_MDID_OFFSET,
> > - ICE_PKT_TCP_MDID_OFFSET,
> > - ICE_PKT_ERROR_MDID_OFFSET,
> > - }},
> > + ICE_PROTOCOL_ENTRY(ICE_MAC_OFOS, 0, 2, 4, 6, 8, 10, 12),
> > + ICE_PROTOCOL_ENTRY(ICE_MAC_IL, 0, 2, 4, 6, 8, 10, 12),
> > + ICE_PROTOCOL_ENTRY(ICE_ETYPE_OL, 0),
> > + ICE_PROTOCOL_ENTRY(ICE_ETYPE_IL, 0),
>
> BTW, as offset arguments go into the array declaration, you can even
> omit such single-zero-element declarations. I.e., if I'm not mistaken,
> these two equal to just:
>
> ICE_PROTOCOL_ENTRY(ICE_ETYPE_OL),
> ICE_PROTOCOL_ENTRY(ICE_ETYPE_IL),
>
> But: 1) better to recheck; 2) up to you, maybe it's better to explicitly
> mention zero offsets here.
>
Good to know, will recheck.
As 0 offset is valid (0 means 0 offset not protocol without offset) I
preffere to mention 0 here.
> > + ICE_PROTOCOL_ENTRY(ICE_VLAN_OFOS, 2, 0),
> > + ICE_PROTOCOL_ENTRY(ICE_IPV4_OFOS, 0, 2, 4, 6, 8, 10, 12, 14, 16, 18),
> > + ICE_PROTOCOL_ENTRY(ICE_IPV4_IL, 0, 2, 4, 6, 8, 10, 12, 14, 16, 18),
> > + ICE_PROTOCOL_ENTRY(ICE_IPV6_OFOS, 0, 2, 4, 6, 8, 10, 12, 14, 16, 18,
> > + 20, 22, 24, 26, 28, 30, 32, 34, 36, 38),
> > + ICE_PROTOCOL_ENTRY(ICE_IPV6_IL, 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20,
> > + 22, 24, 26, 28, 30, 32, 34, 36, 38),
> > + ICE_PROTOCOL_ENTRY(ICE_TCP_IL, 0, 2),
> > + ICE_PROTOCOL_ENTRY(ICE_UDP_OF, 0, 2),
> > + ICE_PROTOCOL_ENTRY(ICE_UDP_ILOS, 0, 2),
> > + ICE_PROTOCOL_ENTRY(ICE_VXLAN, 8, 10, 12, 14),
> > + ICE_PROTOCOL_ENTRY(ICE_GENEVE, 8, 10, 12, 14),
> > + ICE_PROTOCOL_ENTRY(ICE_NVGRE, 0, 2, 4, 6),
> > + ICE_PROTOCOL_ENTRY(ICE_GTP, 8, 10, 12, 14, 16, 18, 20, 22),
> > + ICE_PROTOCOL_ENTRY(ICE_GTP_NO_PAY, 8, 10, 12, 14),
> > + ICE_PROTOCOL_ENTRY(ICE_PPPOE, 0, 2, 4, 6),
> > + ICE_PROTOCOL_ENTRY(ICE_L2TPV3, 0, 2, 4, 6, 8, 10),
> > + ICE_PROTOCOL_ENTRY(ICE_VLAN_EX, 2, 0),
> > + ICE_PROTOCOL_ENTRY(ICE_VLAN_IN, 2, 0),
> > + ICE_PROTOCOL_ENTRY(ICE_HW_METADATA, ICE_SOURCE_PORT_MDID_OFFSET,
>
> Nit: I think here's the exceptional case when you can specify this
> second argument on the next line, i.e. break the line even though it
> fits into 80 chars. This looks a bit off to me :D
>
Right, will move it
> > + ICE_PTYPE_MDID_OFFSET,
> > + ICE_PACKET_LENGTH_MDID_OFFSET,
> > + ICE_SOURCE_VSI_MDID_OFFSET,
> > + ICE_PKT_VLAN_MDID_OFFSET,
> > + ICE_PKT_TUNNEL_MDID_OFFSET,
> > + ICE_PKT_TCP_MDID_OFFSET,
> > + ICE_PKT_ERROR_MDID_OFFSET),
>
> Hmm, could this patch go as 3/5, i.e. before this last element is
> introduced, so that there'll be 16 lines less in diffstat?
>
Sure, I will rebase
Thanks,
Michal
> > };
> >
> > static struct ice_protocol_entry ice_prot_id_tbl[ICE_PROTOCOL_LAST] = {
>
> Thanks,
> Olek
Powered by blists - more mailing lists