Re: [PATCH v4 net-next 5/5] ice: switch: convert packet template match code to rodata

[Alexander Lobakin]

From: Wojciech Drewek <wojciech.drewek@xxxxxxxxx>
Date: Tue, 29 Mar 2022 15:12:44 +0000

> Hi Alex,
> 
> > -----Original Message-----
> > From: Lobakin, Alexandr <alexandr.lobakin@xxxxxxxxx>
> > Sent: poniedzialek, 21 marca 2022 12:00
> > To: intel-wired-lan@xxxxxxxxxxxxxxxx
> > Cc: Lobakin, Alexandr <alexandr.lobakin@xxxxxxxxx>; Fijalkowski, Maciej <maciej.fijalkowski@xxxxxxxxx>; Michal Swiatkowski
> > <michal.swiatkowski@xxxxxxxxxxxxxxx>; Drewek, Wojciech <wojciech.drewek@xxxxxxxxx>; Marcin Szycik
> > <marcin.szycik@xxxxxxxxxxxxxxx>; Szapar-Mudlaw, Martyna <martyna.szapar-mudlaw@xxxxxxxxx>; David S. Miller
> > <davem@xxxxxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni <pabeni@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> > kernel@xxxxxxxxxxxxxxx
> > Subject: [PATCH v4 net-next 5/5] ice: switch: convert packet template match code to rodata
> >
> > Trade text size for rodata size and replace tons of nested if-elses
> > to the const mask match based structs. The almost entire
> > ice_find_dummy_packet() now becomes just one plain while-increment
> > loop. The order in ice_dummy_pkt_profiles[] should be same with the
> > if-elses order previously, as masks become less and less strict
> > through the array to follow the original code flow.
> > Apart from removing 80 locs of 4-level if-elses, it brings a solid
> > text size optimization:
> >
> > add/remove: 0/1 grow/shrink: 1/1 up/down: 2/-1058 (-1056)
> > Function                                     old     new   delta
> > ice_fill_adv_dummy_packet                    289     291      +2
> > ice_adv_add_update_vsi_list                  201       -    -201
> > ice_add_adv_rule                            2950    2093    -857
> > Total: Before=414512, After=413456, chg -0.25%
> > add/remove: 53/52 grow/shrink: 0/0 up/down: 4660/-3988 (672)
> > RO Data                                      old     new   delta
> > ice_dummy_pkt_profiles                         -     672    +672
> > Total: Before=37895, After=38567, chg +1.77%
> >
> > Signed-off-by: Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>
> > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@xxxxxxxxxxxxxxx>
> > Tested-by: Marcin Szycik <marcin.szycik@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/net/ethernet/intel/ice/ice_switch.c | 215 ++++++++++----------
> >  1 file changed, 108 insertions(+), 107 deletions(-)

--- 8< ---

> > +	ICE_PKT_PROFILE(vlan_udp, ICE_PKT_INNER_UDP | ICE_PKT_VLAN),
> > +	ICE_PKT_PROFILE(udp, ICE_PKT_INNER_UDP),
> > +	ICE_PKT_PROFILE(vlan_tcp_ipv6, ICE_PKT_INNER_IPV6 | ICE_PKT_VLAN),
> > +	ICE_PKT_PROFILE(tcp_ipv6, ICE_PKT_INNER_IPV6),
> 
> I think that in both "vlan_tcp_ipv6" and "tcp_ipv6" we should use ICE_PKT_OUTER_IPV6 instead
> of ICE_PKT_INNER_IPV6. We are not dealing with tunnels in those cases so inner addresses are 
> incorrect here.

Oh, some copy'n'paste braino indeed.
I'll send a fixup to Tony in a moment.

> 
> Thanks,
> Wojtek

Great catch, thanks for noticing!

> 
> > +	ICE_PKT_PROFILE(vlan_tcp, ICE_PKT_VLAN),
> > +	ICE_PKT_PROFILE(tcp, 0),
> > +};

--- 8< ---

> > --
> > 2.35.1

Al