Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-07-01 Thread Van Haaren, Harry
> -Original Message-
> From: dev  On Behalf Of Amber, Kumar
> Sent: Thursday, July 1, 2021 9:43 AM
> To: Flavio Leitner 
> Cc: d...@openvswitch.org; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic
> profiles
> 
> Hi Flavio,



> > > +static void
> > > +mfex_handle_tcp_flags(const struct tcp_header *tcp, uint64_t *block)
> > > +{
> > > +uint16_t ctl = (OVS_FORCE uint16_t) TCP_FLAGS_BE16(tcp->tcp_ctl);
> >
> > Why casting to uint16_t?

OVS marks the tcp header as a big-endian (BE) variable. Compiler warns (fails 
with -Werror)
to read that value as a little-endian (LE) value.

As the BE value must be pushed into the miniflow as BE, we need to cast it to a
little-endian value, to be able to store it without a byte-swap. The value from 
the
TCP header cannot be read directly, that's what the cast and the OVS_FORCE()
is there for. The LE uint16_t is then pushed into the miniflow, and the compiler
is happy to do that for us.



Regards, -Harry
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-07-01 Thread Amber, Kumar
Hi Flavio,

Pls find the replies inline,



> > +mfex_vlan_pcp(const uint8_t vlan_pcp, uint64_t *block) {
> > +/* Bitwise-OR in the CFI flag, keeping other data the same. */
> > +uint8_t *cfi_byte = (uint8_t *) block;
> > +cfi_byte[2] = 0x10 | vlan_pcp;
> 
> Trying to reduce the magic numbers around, can we use OVS's VLAN_CFI instead
> of 0x10?
> 

The values are not same for the Macro.

> > +}
> > +
> > +/* Process TCP flags using known LE endian-ness as this is AVX512
> > +code. */ #define TCP_FLAGS_BE32(tcp_ctl) ((OVS_FORCE ovs_be32)
> > +TCP_FLAGS_BE16(tcp_ctl))
> 
> The above is not used, right?
> 

Removed in v5.
> > +
> > +static void
> > +mfex_handle_tcp_flags(const struct tcp_header *tcp, uint64_t *block)
> > +{
> > +uint16_t ctl = (OVS_FORCE uint16_t) TCP_FLAGS_BE16(tcp->tcp_ctl);
> 
> Why casting to uint16_t?
> 
> > +uint64_t ctl_u64 = ctl;
> > +*block = ctl_u64 << 32;
> > +}
> > +
> >  /* Generic loop to process any mfex profile. This code is specialized into
> >   * multiple actual MFEX implementation functions. Its marked
> ALWAYS_INLINE
> >   * to ensure the compiler specializes each instance. The code is marked 
> > "hot"
> > @@ -321,6 +436,43 @@ mfex_avx512_process(struct dp_packet_batch
> *packets,
> >  ovs_assert(0); /* avoid compiler warning on missing ENUM */
> >  break;
> >
> > +case PROFILE_ETH_VLAN_IPV4_TCP: {
> > +mfex_vlan_pcp(pkt[14], &keys[i].buf[4]);
> 
> Maybe cast pkt[2*ETH_ADDR_LEN] to struct flow_vlan_hdr and pass to that,
> then use a flow_vlan_hdr->tci to improve readability?
> 

With Ipv6 patch following up maybe I can refactor the code a bit like you 
mentioned and create macros.
> > +
> > +uint32_t size_from_ipv4 = size - VLAN_ETH_HEADER_LEN;
> > +struct ip_header *nh = (void *)&pkt[VLAN_ETH_HEADER_LEN];
> > +if (mfex_ipv4_set_l2_pad_size(packet, nh, size_from_ipv4)) 
> > {
> > +continue;
> > +}
> > +
> > +/* Process TCP flags, and store to blocks. */
> > +const struct tcp_header *tcp = (void *)&pkt[38];
> > +mfex_handle_tcp_flags(tcp, &blocks[7]);
> > +} break;
> > +
> > +case PROFILE_ETH_VLAN_IPV4_UDP: {
> > +mfex_vlan_pcp(pkt[14], &keys[i].buf[4]);
> > +
> > +uint32_t size_from_ipv4 = size - VLAN_ETH_HEADER_LEN;
> > +struct ip_header *nh = (void *)&pkt[VLAN_ETH_HEADER_LEN];
> > +if (mfex_ipv4_set_l2_pad_size(packet, nh, size_from_ipv4)) 
> > {
> > +continue;
> > +}
> > +} break;
> > +
> > +case PROFILE_ETH_IPV4_TCP: {
> > +/* Process TCP flags, and store to blocks. */
> > +const struct tcp_header *tcp = (void *)&pkt[34];
> > +mfex_handle_tcp_flags(tcp, &blocks[6]);
> > +
> > +/* Handle dynamic l2_pad_size. */
> > +uint32_t size_from_ipv4 = size - sizeof(struct eth_header);
> > +struct ip_header *nh = (void *)&pkt[sizeof(struct 
> > eth_header)];
> > +if (mfex_ipv4_set_l2_pad_size(packet, nh, size_from_ipv4)) 
> > {
> > +continue;
> > +}
> > +} break;
> > +
> >  case PROFILE_ETH_IPV4_UDP: {
> >  /* Handle dynamic l2_pad_size. */
> >  uint32_t size_from_ipv4 = size - sizeof(struct eth_header);
> > @@ -370,6 +522,9 @@ mfex_avx512_##name(struct dp_packet_batch
> *packets, \
> >   * as required.
> >   */
> >  DECLARE_MFEX_FUNC(ip_udp,PROFILE_ETH_IPV4_UDP)
> > +DECLARE_MFEX_FUNC(ip_tcp,PROFILE_ETH_IPV4_TCP)
> > +DECLARE_MFEX_FUNC(dot1q_ip_udp,PROFILE_ETH_VLAN_IPV4_UDP)
> > +DECLARE_MFEX_FUNC(dot1q_ip_tcp,PROFILE_ETH_VLAN_IPV4_TCP)
> 
> I forgot to mention this in the previous patch, but please add a space after 
> the
> comma.
> 

Fixed.
> >
> >
> >  static int32_t
> > diff --git a/lib/dpif-netdev-private-extract.c
> > b/lib/dpif-netdev-private-extract.c
> > index 106a83867..65072eb38 100644
> > --- a/lib/dpif-netdev-private-extract.c
> > +++ b/lib/dpif-netdev-private-extract.c
> > @@ -60,6 +60,37 @@ static struct dpif_miniflow_extract_impl mfex_impls[] =
> {
> >  .extract_func = mfex_avx512_ip_udp,
> >  .name = "avx512_ipv4_udp",
> >  },
> > +{
> > +.probe = mfex_avx512_vbmi_probe,
> > +.extract_func = mfex_avx512_vbmi_ip_tcp,
> > +.name = "avx512_vbmi_ipv4_tcp",
> > +},
> > +{
> > +.probe = mfex_avx512_probe,
> > +.extract_func = mfex_avx512_ip_tcp,
> > +.name = "avx512_ipv4_tcp",
> > +},
> > +
> > +{
> > +.probe = mfex_avx512_vbmi_probe,
> > +.extract_func = mfex_avx512_vbmi_dot1q_ip_udp,
> > +.name = "avx512_vbmi_dot1q_ipv4_udp",
> > +},
> > +{
> > +.probe = mfex_avx512_probe,
> > +   

Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-06-30 Thread Eelco Chaudron


On 30 Jun 2021, at 19:01, Van Haaren, Harry wrote:

>> -Original Message-
>> From: Amber, Kumar 
>> Sent: Wednesday, June 30, 2021 4:10 PM
>> To: Eelco Chaudron ; Van Haaren, Harry
>> 
>> Cc: d...@openvswitch.org; i.maxim...@ovn.org; Flavio Leitner 
>> ;
>> Stokes, Ian 
>> Subject: RE: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic 
>> profiles
>>
>> Hi Eelco,
>>
>> Pls find my comments inline.
>
> Snip away all except the "callbacks" topic.
>
>>> NIT: As we might continue to add variants, would a callback in the profile 
>>> be
>>> cleaner? Not sure what arguments to pass? Just a thought…
>>
>> Nice thought we have patch for IPv6 coming up we can surely explore the idea 
>> 😊
>
> Callbacks can be more difficult for the compiler to inline.
> A direct function call is always taken, and compilers know this.
>
> A function pointer requires a little more "compiler knowledge"
> to successfully inline the actual function. (GCC can do it, its been
> tested before).
>
> I'm not seeing a better solution overall however, as that function pointer
> will still need to be set based on each profile... so we still have a switch()
> with all profiles inside it.

The function pointer is assigned statically at compile time in the profile 
structure, so no case statement is needed.

But I agree we can leave it for now. It was more a suggestion to make the 
function look more generic and avoid the switch growth over time.

> We could abstract each specific profile away to a profile-specific helper 
> function,
> however that would cause an explosion of functions, instead of some code 
> inside
> a switch statement.
>
> For now, lets leave as is, no need for churn at this point. If the list 
> really gets
> unmanageable, we can review again in future when adding more impls.
>
> Thanks for feedback, -Harry

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-06-30 Thread Flavio Leitner
Hi,

On Thu, Jun 17, 2021 at 09:57:53PM +0530, Kumar Amber wrote:
> From: Harry van Haaren 
> 
> This commit adds 3 new traffic profile implementations to the
> existing avx512 miniflow extract infrastructure. The profiles added are:
> - Ether()/IP()/TCP()
> - Ether()/Dot1Q()/IP()/UDP()
> - Ether()/Dot1Q()/IP()/TCP()
> 
> The design of the avx512 code here is for scalability to add more
> traffic profiles, as well as enabling CPU ISA. Note that an implementation
> is primarily adding static const data, which the compiler then specializes
> away when the profile specific function is declared below.
> 
> As a result, the code is relatively maintainable, and scalable for new
> traffic profiles as well as new ISA, and does not lower performance
> compared with manually written code for each profile/ISA.
> 
> Note that confidence in the correctness of each implementation is
> achieved through autovalidation, unit tests with known packets, and
> fuzz tested packets.
> 
> Signed-off-by: Harry van Haaren 
> 
> ---
> 
> Hi Readers,
> 
> If you have a traffic profile you'd like to see accelerated using
> avx512 code, please send me an email and we can collaborate on adding
> support for it!
> 
> Regards, -Harry
> ---
>  lib/dpif-netdev-extract-avx512.c  | 155 ++
>  lib/dpif-netdev-private-extract.c |  31 ++
>  lib/dpif-netdev-private-extract.h |   4 +
>  3 files changed, 190 insertions(+)
> 
> diff --git a/lib/dpif-netdev-extract-avx512.c 
> b/lib/dpif-netdev-extract-avx512.c
> index 1145ac8a9..0e0f6e295 100644
> --- a/lib/dpif-netdev-extract-avx512.c
> +++ b/lib/dpif-netdev-extract-avx512.c
> @@ -117,6 +117,13 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
> __m512i idx, __m512i a)
>  
>  #define PATTERN_ETHERTYPE_MASK PATTERN_ETHERTYPE_GEN(0xFF, 0xFF)
>  #define PATTERN_ETHERTYPE_IPV4 PATTERN_ETHERTYPE_GEN(0x08, 0x00)
> +#define PATTERN_ETHERTYPE_DT1Q PATTERN_ETHERTYPE_GEN(0x81, 0x00)
> +
> +/* VLAN (Dot1Q) patterns and masks. */
> +#define PATTERN_DT1Q_MASK   \
> +  0x00, 0x00, 0xFF, 0xFF,
> +#define PATTERN_DT1Q_IPV4   \
> +  0x00, 0x00, 0x08, 0x00,
>  
>  /* Generator for checking IPv4 ver, ihl, and proto */
>  #define PATTERN_IPV4_GEN(VER_IHL, FLAG_OFF_B0, FLAG_OFF_B1, PROTO) \
> @@ -142,6 +149,29 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
> __m512i idx, __m512i a)
>34, 35, 36, 37, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* UDP */  
>  \
>NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. 
> */
>  
> +/* TCP shuffle: tcp_ctl bits require mask/processing, not included here. */
> +#define PATTERN_IPV4_TCP_SHUFFLE \
> +   0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, NU, NU, /* Ether 
> */ \
> +  26, 27, 28, 29, 30, 31, 32, 33, NU, NU, NU, NU, 20, 15, 22, 23, /* IPv4 */ 
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, 34, 35, 36, 37, NU, NU, NU, NU, /* TCP */  
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. 
> */
> +
> +#define PATTERN_DT1Q_IPV4_UDP_SHUFFLE
>  \
> +  /* Ether (2 blocks): Note that *VLAN* type is written here. */ 
>  \
> +  0,  1,  2,  3,  4,  5,  6,  7, 8,  9, 10, 11, 16, 17,  0,  0,  
>  \
> +  /* VLAN (1 block): Note that the *EtherHdr->Type* is written here. */  
>  \
> +  12, 13, 14, 15, 0, 0, 0, 0,
>  \
> +  30, 31, 32, 33, 34, 35, 36, 37, 0, 0, 0, 0, 24, 19, 26, 27, /* IPv4 */ 
>  \
> +  38, 39, 40, 41, NU, NU, NU, NU, /* UDP */
> +
> +#define PATTERN_DT1Q_IPV4_TCP_SHUFFLE
>  \
> +  /* Ether (2 blocks): Note that *VLAN* type is written here. */ 
>  \
> +  0,  1,  2,  3,  4,  5,  6,  7, 8,  9, 10, 11, 16, 17,  0,  0,  
>  \
> +  /* VLAN (1 block): Note that the *EtherHdr->Type* is written here. */  
>  \
> +  12, 13, 14, 15, 0, 0, 0, 0,
>  \
> +  30, 31, 32, 33, 34, 35, 36, 37, 0, 0, 0, 0, 24, 19, 26, 27, /* IPv4 */ 
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, 38, 39, 40, 41, NU, NU, NU, NU, /* TCP */  
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. */
>  
>  /* Generation of K-mask bitmask values, to zero out data in result. Note that
>   * these correspond 1:1 to the above "*_SHUFFLE" values, and bit used must be
> @@ -151,12 +181,22 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
> __m512i idx, __m512i a)
>   * Note the ULL suffix allows shifting by 32 or more without integer 
> overflow.
>   */
>  #define KMASK_ETHER 0x1FFFULL
> +#define KMASK_DT1Q  0x000FULL
>  #define KMASK_IPV4  0xF0FFULL
>  #define KMASK_UDP   0x000FULL
> +#define KMASK_TCP   0x0F00ULL
>  
>  #define PATTERN_IPV4_UDP_KMASK \
>  (KMASK_ETHER | (KMASK_IPV4 << 16) | (KMASK_UDP << 32))
>  
> +#define PATTERN_IPV4_TCP_KMASK \
> +   

Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-06-30 Thread Van Haaren, Harry
> -Original Message-
> From: Amber, Kumar 
> Sent: Wednesday, June 30, 2021 4:10 PM
> To: Eelco Chaudron ; Van Haaren, Harry
> 
> Cc: d...@openvswitch.org; i.maxim...@ovn.org; Flavio Leitner 
> ;
> Stokes, Ian 
> Subject: RE: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic 
> profiles
> 
> Hi Eelco,
> 
> Pls find my comments inline.

Snip away all except the "callbacks" topic.

> > NIT: As we might continue to add variants, would a callback in the profile 
> > be
> > cleaner? Not sure what arguments to pass? Just a thought…
> 
> Nice thought we have patch for IPv6 coming up we can surely explore the idea 😊

Callbacks can be more difficult for the compiler to inline.
A direct function call is always taken, and compilers know this.

A function pointer requires a little more "compiler knowledge"
to successfully inline the actual function. (GCC can do it, its been
tested before).

I'm not seeing a better solution overall however, as that function pointer
will still need to be set based on each profile... so we still have a switch()
with all profiles inside it.

We could abstract each specific profile away to a profile-specific helper 
function,
however that would cause an explosion of functions, instead of some code inside
a switch statement.

For now, lets leave as is, no need for churn at this point. If the list really 
gets
unmanageable, we can review again in future when adding more impls.

Thanks for feedback, -Harry
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-06-30 Thread Amber, Kumar
Hi Eelco,

Pls find my comments inline.



> >  #define KMASK_ETHER 0x1FFFULL
> > +#define KMASK_DT1Q  0x000FULL
> 
> This was messing me up, as this suggests this is a 16-byte mask, but this is 
> only 8,
> so maybe we should indicate it by removing the two leading zeros?
> 
>#define KMASK_DT1Q0x0FULL
> 

Fixed in v5.

> >  #define KMASK_IPV4  0xF0FFULL
> >  #define KMASK_UDP   0x000FULL
> > +#define KMASK_TCP   0x0F00ULL
> >
> > @@ -233,6 +326,28 @@ mfex_ipv4_set_l2_pad_size(struct dp_packet *pkt,
> > +}
> > +
> > +/* Process TCP flags using known LE endian-ness as this is AVX512
> > +code. */ #define TCP_FLAGS_BE32(tcp_ctl) ((OVS_FORCE ovs_be32)
> > +TCP_FLAGS_BE16(tcp_ctl))
> > +
> 
> Looks like the TCP_FLAGS_BE32() macro is not used in this code.
> 

Cleared in v5.

> > +static void
> > +mfex_handle_tcp_flags(const struct tcp_header *tcp, uint64_t *block)
> > +{
> > +uint16_t ctl = (OVS_FORCE uint16_t) TCP_FLAGS_BE16(tcp->tcp_ctl);
> > +uint64_t ctl_u64 = ctl;
> > +*block = ctl_u64 << 32;
> > +}
> > +
> >  /* Generic loop to process any mfex profile. This code is specialized into
> >   * multiple actual MFEX implementation functions. Its marked
> ALWAYS_INLINE
> >   * to ensure the compiler specializes each instance. The code is marked 
> > "hot"
> > @@ -321,6 +436,43 @@ mfex_avx512_process(struct dp_packet_batch
> *packets,
> >  ovs_assert(0); /* avoid compiler warning on missing ENUM */
> >  break;
> >
> 
> NIT: As we might continue to add variants, would a callback in the profile be
> cleaner? Not sure what arguments to pass? Just a thought…
> 
> 

Nice thought we have patch for IPv6 coming up we can surely explore the idea 😊
> >
> >
> > --
> > 2.25.1
> >
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-06-30 Thread Eelco Chaudron


On 17 Jun 2021, at 18:27, Kumar Amber wrote:

> From: Harry van Haaren 
>
> This commit adds 3 new traffic profile implementations to the
> existing avx512 miniflow extract infrastructure. The profiles added are:
> - Ether()/IP()/TCP()
> - Ether()/Dot1Q()/IP()/UDP()
> - Ether()/Dot1Q()/IP()/TCP()
>
> The design of the avx512 code here is for scalability to add more
> traffic profiles, as well as enabling CPU ISA. Note that an implementation
> is primarily adding static const data, which the compiler then specializes
> away when the profile specific function is declared below.
>
> As a result, the code is relatively maintainable, and scalable for new
> traffic profiles as well as new ISA, and does not lower performance
> compared with manually written code for each profile/ISA.
>
> Note that confidence in the correctness of each implementation is
> achieved through autovalidation, unit tests with known packets, and
> fuzz tested packets.
>
> Signed-off-by: Harry van Haaren 
>
> ---
>
> Hi Readers,
>
> If you have a traffic profile you'd like to see accelerated using
> avx512 code, please send me an email and we can collaborate on adding
> support for it!
>
> Regards, -Harry
> ---
>  lib/dpif-netdev-extract-avx512.c  | 155 ++
>  lib/dpif-netdev-private-extract.c |  31 ++
>  lib/dpif-netdev-private-extract.h |   4 +
>  3 files changed, 190 insertions(+)
>
> diff --git a/lib/dpif-netdev-extract-avx512.c 
> b/lib/dpif-netdev-extract-avx512.c
> index 1145ac8a9..0e0f6e295 100644
> --- a/lib/dpif-netdev-extract-avx512.c
> +++ b/lib/dpif-netdev-extract-avx512.c
> @@ -117,6 +117,13 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
> __m512i idx, __m512i a)
>
>  #define PATTERN_ETHERTYPE_MASK PATTERN_ETHERTYPE_GEN(0xFF, 0xFF)
>  #define PATTERN_ETHERTYPE_IPV4 PATTERN_ETHERTYPE_GEN(0x08, 0x00)
> +#define PATTERN_ETHERTYPE_DT1Q PATTERN_ETHERTYPE_GEN(0x81, 0x00)
> +
> +/* VLAN (Dot1Q) patterns and masks. */
> +#define PATTERN_DT1Q_MASK   \
> +  0x00, 0x00, 0xFF, 0xFF,
> +#define PATTERN_DT1Q_IPV4   \
> +  0x00, 0x00, 0x08, 0x00,
>
>  /* Generator for checking IPv4 ver, ihl, and proto */
>  #define PATTERN_IPV4_GEN(VER_IHL, FLAG_OFF_B0, FLAG_OFF_B1, PROTO) \
> @@ -142,6 +149,29 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
> __m512i idx, __m512i a)
>34, 35, 36, 37, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* UDP */  
>  \
>NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. 
> */
>
> +/* TCP shuffle: tcp_ctl bits require mask/processing, not included here. */
> +#define PATTERN_IPV4_TCP_SHUFFLE \
> +   0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, NU, NU, /* Ether 
> */ \
> +  26, 27, 28, 29, 30, 31, 32, 33, NU, NU, NU, NU, 20, 15, 22, 23, /* IPv4 */ 
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, 34, 35, 36, 37, NU, NU, NU, NU, /* TCP */  
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. 
> */
> +
> +#define PATTERN_DT1Q_IPV4_UDP_SHUFFLE
>  \
> +  /* Ether (2 blocks): Note that *VLAN* type is written here. */ 
>  \
> +  0,  1,  2,  3,  4,  5,  6,  7, 8,  9, 10, 11, 16, 17,  0,  0,  
>  \
> +  /* VLAN (1 block): Note that the *EtherHdr->Type* is written here. */  
>  \
> +  12, 13, 14, 15, 0, 0, 0, 0,
>  \
> +  30, 31, 32, 33, 34, 35, 36, 37, 0, 0, 0, 0, 24, 19, 26, 27, /* IPv4 */ 
>  \
> +  38, 39, 40, 41, NU, NU, NU, NU, /* UDP */
> +
> +#define PATTERN_DT1Q_IPV4_TCP_SHUFFLE
>  \
> +  /* Ether (2 blocks): Note that *VLAN* type is written here. */ 
>  \
> +  0,  1,  2,  3,  4,  5,  6,  7, 8,  9, 10, 11, 16, 17,  0,  0,  
>  \
> +  /* VLAN (1 block): Note that the *EtherHdr->Type* is written here. */  
>  \
> +  12, 13, 14, 15, 0, 0, 0, 0,
>  \
> +  30, 31, 32, 33, 34, 35, 36, 37, 0, 0, 0, 0, 24, 19, 26, 27, /* IPv4 */ 
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, 38, 39, 40, 41, NU, NU, NU, NU, /* TCP */  
>  \
> +  NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. */
>
>  /* Generation of K-mask bitmask values, to zero out data in result. Note that
>   * these correspond 1:1 to the above "*_SHUFFLE" values, and bit used must be
> @@ -151,12 +181,22 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
> __m512i idx, __m512i a)
>   * Note the ULL suffix allows shifting by 32 or more without integer 
> overflow.
>   */
>  #define KMASK_ETHER 0x1FFFULL
> +#define KMASK_DT1Q  0x000FULL

This was messing me up, as this suggests this is a 16-byte mask, but this is 
only 8, so maybe we should indicate it by removing the two leading zeros?

   #define KMASK_DT1Q0x0FULL

>  #define KMASK_IPV4  0xF0FFULL
>  #define KMASK_UDP   0x000FULL
> +#define KMASK_TCP   0x

[ovs-dev] [v4 11/12] dpif-netdev/mfex: add more AVX512 traffic profiles

2021-06-17 Thread Kumar Amber
From: Harry van Haaren 

This commit adds 3 new traffic profile implementations to the
existing avx512 miniflow extract infrastructure. The profiles added are:
- Ether()/IP()/TCP()
- Ether()/Dot1Q()/IP()/UDP()
- Ether()/Dot1Q()/IP()/TCP()

The design of the avx512 code here is for scalability to add more
traffic profiles, as well as enabling CPU ISA. Note that an implementation
is primarily adding static const data, which the compiler then specializes
away when the profile specific function is declared below.

As a result, the code is relatively maintainable, and scalable for new
traffic profiles as well as new ISA, and does not lower performance
compared with manually written code for each profile/ISA.

Note that confidence in the correctness of each implementation is
achieved through autovalidation, unit tests with known packets, and
fuzz tested packets.

Signed-off-by: Harry van Haaren 

---

Hi Readers,

If you have a traffic profile you'd like to see accelerated using
avx512 code, please send me an email and we can collaborate on adding
support for it!

Regards, -Harry
---
 lib/dpif-netdev-extract-avx512.c  | 155 ++
 lib/dpif-netdev-private-extract.c |  31 ++
 lib/dpif-netdev-private-extract.h |   4 +
 3 files changed, 190 insertions(+)

diff --git a/lib/dpif-netdev-extract-avx512.c b/lib/dpif-netdev-extract-avx512.c
index 1145ac8a9..0e0f6e295 100644
--- a/lib/dpif-netdev-extract-avx512.c
+++ b/lib/dpif-netdev-extract-avx512.c
@@ -117,6 +117,13 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
__m512i idx, __m512i a)
 
 #define PATTERN_ETHERTYPE_MASK PATTERN_ETHERTYPE_GEN(0xFF, 0xFF)
 #define PATTERN_ETHERTYPE_IPV4 PATTERN_ETHERTYPE_GEN(0x08, 0x00)
+#define PATTERN_ETHERTYPE_DT1Q PATTERN_ETHERTYPE_GEN(0x81, 0x00)
+
+/* VLAN (Dot1Q) patterns and masks. */
+#define PATTERN_DT1Q_MASK   \
+  0x00, 0x00, 0xFF, 0xFF,
+#define PATTERN_DT1Q_IPV4   \
+  0x00, 0x00, 0x08, 0x00,
 
 /* Generator for checking IPv4 ver, ihl, and proto */
 #define PATTERN_IPV4_GEN(VER_IHL, FLAG_OFF_B0, FLAG_OFF_B1, PROTO) \
@@ -142,6 +149,29 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
__m512i idx, __m512i a)
   34, 35, 36, 37, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* UDP */   \
   NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. */
 
+/* TCP shuffle: tcp_ctl bits require mask/processing, not included here. */
+#define PATTERN_IPV4_TCP_SHUFFLE \
+   0,  1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, NU, NU, /* Ether */ \
+  26, 27, 28, 29, 30, 31, 32, 33, NU, NU, NU, NU, 20, 15, 22, 23, /* IPv4 */  \
+  NU, NU, NU, NU, NU, NU, NU, NU, 34, 35, 36, 37, NU, NU, NU, NU, /* TCP */   \
+  NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. */
+
+#define PATTERN_DT1Q_IPV4_UDP_SHUFFLE \
+  /* Ether (2 blocks): Note that *VLAN* type is written here. */  \
+  0,  1,  2,  3,  4,  5,  6,  7, 8,  9, 10, 11, 16, 17,  0,  0,   \
+  /* VLAN (1 block): Note that the *EtherHdr->Type* is written here. */   \
+  12, 13, 14, 15, 0, 0, 0, 0, \
+  30, 31, 32, 33, 34, 35, 36, 37, 0, 0, 0, 0, 24, 19, 26, 27, /* IPv4 */  \
+  38, 39, 40, 41, NU, NU, NU, NU, /* UDP */
+
+#define PATTERN_DT1Q_IPV4_TCP_SHUFFLE \
+  /* Ether (2 blocks): Note that *VLAN* type is written here. */  \
+  0,  1,  2,  3,  4,  5,  6,  7, 8,  9, 10, 11, 16, 17,  0,  0,   \
+  /* VLAN (1 block): Note that the *EtherHdr->Type* is written here. */   \
+  12, 13, 14, 15, 0, 0, 0, 0, \
+  30, 31, 32, 33, 34, 35, 36, 37, 0, 0, 0, 0, 24, 19, 26, 27, /* IPv4 */  \
+  NU, NU, NU, NU, NU, NU, NU, NU, 38, 39, 40, 41, NU, NU, NU, NU, /* TCP */   \
+  NU, NU, NU, NU, NU, NU, NU, NU, /* Unused. */
 
 /* Generation of K-mask bitmask values, to zero out data in result. Note that
  * these correspond 1:1 to the above "*_SHUFFLE" values, and bit used must be
@@ -151,12 +181,22 @@ _mm512_maskz_permutexvar_epi8_wrap(__mmask64 kmask, 
__m512i idx, __m512i a)
  * Note the ULL suffix allows shifting by 32 or more without integer overflow.
  */
 #define KMASK_ETHER 0x1FFFULL
+#define KMASK_DT1Q  0x000FULL
 #define KMASK_IPV4  0xF0FFULL
 #define KMASK_UDP   0x000FULL
+#define KMASK_TCP   0x0F00ULL
 
 #define PATTERN_IPV4_UDP_KMASK \
 (KMASK_ETHER | (KMASK_IPV4 << 16) | (KMASK_UDP << 32))
 
+#define PATTERN_IPV4_TCP_KMASK \
+(KMASK_ETHER | (KMASK_IPV4 << 16) | (KMASK_TCP << 32))
+
+#define PATTERN_DT1Q_IPV4_UDP_KMASK \
+(KMASK_ETHER | (KMASK_DT1Q << 16) | (KMASK_IPV4 << 24) | (KMASK_UDP << 40))
+
+#define PATTERN_DT1Q_IPV4_TCP_KMASK \
+(KMASK_ETHER | (KMASK_DT1Q << 16) | (KMASK_IPV4 << 24) | (KMASK_TCP << 40))
 
 /* This union allows initializin