Hi Flavio,
Pls find the replies inline,
<Snip>
> > +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,
> > + .extract_func = mfex_avx512_dot1q_ip_udp,
> > + .name = "avx512_dot1q_ipv4_udp",
> > + },
> > + {
> > + .probe = mfex_avx512_vbmi_probe,
> > + .extract_func = mfex_avx512_vbmi_dot1q_ip_tcp,
> > + .name = "avx512_vbmi_dot1q_ipv4_tcp",
> > + },
> > + {
> > + .probe = mfex_avx512_probe,
> > + .extract_func = mfex_avx512_dot1q_ip_tcp,
> > + .name = "avx512_dot1q_ipv4_tcp",
> > + },
> > #endif
> > };
> >
> > diff --git a/lib/dpif-netdev-private-extract.h
> > b/lib/dpif-netdev-private-extract.h
> > index f32be202a..b9a59c5a0 100644
> > --- a/lib/dpif-netdev-private-extract.h
> > +++ b/lib/dpif-netdev-private-extract.h
> > @@ -152,6 +152,10 @@ int32_t mfex_avx512_vbmi_probe(void);
> > odp_port_t in_port, void *pmd_handle);
> >
> > DECLARE_AVX512_MFEX_PROTOTYPE(ip_udp);
> > +DECLARE_AVX512_MFEX_PROTOTYPE(ip_tcp);
> > +DECLARE_AVX512_MFEX_PROTOTYPE(dot1q_ip_udp);
> > +DECLARE_AVX512_MFEX_PROTOTYPE(dot1q_ip_tcp);
> > +
> > #endif /* __x86_64__ */
> >
> >
> > --
> > 2.25.1
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> --
> fbl
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
