Hi Eelco,
Pls find my comments inline.
<Snip>
> > #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_DT1Q 0x0FULL
>
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
