> -----Original Message-----
> From: Amber, Kumar <[email protected]>
> Sent: Thursday, May 26, 2022 11:09 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; Ferriter, Cian
> <[email protected]>; Stokes, Ian <[email protected]>;
> [email protected]; [email protected]; Van Haaren, Harry
> <[email protected]>; Amber, Kumar <[email protected]>
> Subject: [PATCH v8 1/4] mfex_avx512: Calculate pkt offsets at compile time.
>
> The patch removes magic numbers pkt offsets and
> minimum packet lenght and instead calculate it at
> compile time.
>
> Signed-off-by: Kumar Amber <[email protected]>
>
> ---
> v8:
> - Rename offset defines.
Thanks, the names are much more readable now; I'm testing the commit, see
inline comments below;
The MFEX Autovalidator fails with this patch applied, and passes on master, so
there is work to be done here.
Can you confirm/reproduce the issue, and respin a v9 if required?
> lib/dpif-netdev-extract-avx512.c | 29 +++++++++++++++++++++--------
> 1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/lib/dpif-netdev-extract-avx512.c
> b/lib/dpif-netdev-extract-avx512.c
> index 6b6fe07db..211c4cbe4 100644
> --- a/lib/dpif-netdev-extract-avx512.c
> +++ b/lib/dpif-netdev-extract-avx512.c
> @@ -246,6 +246,17 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64
> kmask, __m512i idx, __m512i a)
> NC, NC, NC, NC, 0xBF, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, \
> NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC, NC
>
> +#define PKT_OFFSET_L2_PAD_SIZE (ETH_HEADER_LEN)
> +#define PKT_OFFSET_L3 (ETH_HEADER_LEN)
Initially I wanted to comment on the same value being used 2x here, but its
actually useful to keep the two named for their use-case below.
> +#define PKT_OFFSET_VLAN_L3 (ETH_HEADER_LEN + VLAN_HEADER_LEN)
> +#define PKT_OFFSET_IPV4_L4 (ETH_HEADER_LEN + IP_HEADER_LEN)
> +#define PKT_OFFSET_VLAN_IPV4_L4 (PKT_OFFSET_IPV4_L4 + VLAN_HEADER_LEN)
By breaking the above #defines, (using +1), and autovalidation is failing (as
expected, due to
invalid code changes introduced).
> +#define PKT_MIN_ETH_IPV4_UDP (PKT_OFFSET_IPV4_L4 + UDP_HEADER_LEN)
> +#define PKT_MIN_ETH_VLAN_IPV4_UDP (PKT_OFFSET_VLAN_IPV4_L4 +
> UDP_HEADER_LEN)
> +#define PKT_MIN_ETH_IPV4_TCP (PKT_OFFSET_IPV4_L4 + TCP_HEADER_LEN)
> +#define PKT_MIN_ETH_VLAN_IPV4_TCP (PKT_OFFSET_VLAN_IPV4_L4 +
> TCP_HEADER_LEN)
> +
> /* This union allows initializing static data as u8, but easily loading it
> * into AVX512 registers too. The union ensures proper alignment for the zmm.
> */
> @@ -345,9 +356,9 @@ static const struct mfex_profile
> mfex_profiles[PROFILE_COUNT] =
>
> .mf_bits = { 0x18a0000000000000, 0x0000000000040401},
> .dp_pkt_offs = {
> - 0, UINT16_MAX, 14, 34,
> + 0, UINT16_MAX, PKT_OFFSET_L3, PKT_OFFSET_IPV4_L4,
> },
> - .dp_pkt_min_size = 42,
> + .dp_pkt_min_size = PKT_MIN_ETH_IPV4_UDP,
> },
>
> [PROFILE_ETH_IPV4_TCP] = {
> @@ -368,9 +379,9 @@ static const struct mfex_profile
> mfex_profiles[PROFILE_COUNT] =
>
> .mf_bits = { 0x18a0000000000000, 0x0000000000044401},
> .dp_pkt_offs = {
> - 0, UINT16_MAX, 14, 34,
> + 0, UINT16_MAX, PKT_OFFSET_L3, PKT_OFFSET_IPV4_L4,
> },
> - .dp_pkt_min_size = 54,
> + .dp_pkt_min_size = PKT_MIN_ETH_IPV4_TCP,
> },
>
> [PROFILE_ETH_VLAN_IPV4_UDP] = {
> @@ -387,9 +398,10 @@ static const struct mfex_profile
> mfex_profiles[PROFILE_COUNT] =
>
> .mf_bits = { 0x38a0000000000000, 0x0000000000040401},
> .dp_pkt_offs = {
> - 14, UINT16_MAX, 18, 38,
> + PKT_OFFSET_L2_PAD_SIZE, UINT16_MAX, PKT_OFFSET_VLAN_L3,
> + PKT_OFFSET_VLAN_IPV4_L4,
> },
> - .dp_pkt_min_size = 46,
> + .dp_pkt_min_size = PKT_MIN_ETH_VLAN_IPV4_UDP,
> },
>
> [PROFILE_ETH_VLAN_IPV4_TCP] = {
> @@ -412,9 +424,10 @@ static const struct mfex_profile
> mfex_profiles[PROFILE_COUNT] =
>
> .mf_bits = { 0x38a0000000000000, 0x0000000000044401},
> .dp_pkt_offs = {
> - 14, UINT16_MAX, 18, 38,
> + PKT_OFFSET_L2_PAD_SIZE, UINT16_MAX, PKT_OFFSET_VLAN_L3,
> + PKT_OFFSET_VLAN_IPV4_L4,
> },
> - .dp_pkt_min_size = 58,
> + .dp_pkt_min_size = PKT_MIN_ETH_VLAN_IPV4_TCP,
> },
> };
Generally the code changes look fine, however as autovalidation fails, it needs
re-checking.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
