> This commit adds the AVX512 implementation of the pop_vlan action.
> The implementation here is auto-validated by the miniflow
> extract autovalidator, hence its correctness can be easily
> tested and verified.
>
> Signed-off-by: Emma Finn <[email protected]>
Hi Emma, some comments below.
> ---
> lib/odp-execute-avx512.c | 77 ++++++++++++++++++++++++++++++++++++++-
> lib/odp-execute-private.c | 2 +-
> lib/odp-execute-private.h | 2 +-
> 3 files changed, 78 insertions(+), 3 deletions(-)
>
> diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
> index aa71faa1c..fcf27f070 100644
> --- a/lib/odp-execute-avx512.c
> +++ b/lib/odp-execute-avx512.c
> @@ -14,6 +14,11 @@
> * limitations under the License.
> */
>
> +#ifdef __x86_64__
> +/* Sparse cannot handle the AVX512 instructions. */
> +#if !defined(__CHECKER__)
> +
> +
> #include <config.h>
> #include <errno.h>
>
> @@ -25,6 +30,71 @@
>
> #include "immintrin.h"
>
> +VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
> +BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_5_ofs) +
> + MEMBER_SIZEOF(struct dp_packet, l2_5_ofs) ==
> + offsetof(struct dp_packet, l3_ofs));
> +
> +BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
> + MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
> + offsetof(struct dp_packet, l4_ofs));
> +
> +static inline void ALWAYS_INLINE
> +avx512_dp_packet_resize_l2(struct dp_packet *b, int resize_by_bytes)
> +{
> + /* update packet size/data pointers */
Minor, Capitalize start of comment, missing period (goes for a few of the
comments in the rest of this function also).
> + dp_packet_set_data(b, (char *) dp_packet_data(b) - resize_by_bytes);
> + dp_packet_set_size(b, dp_packet_size(b) + resize_by_bytes);
> +
> + /* Increment u16 packet offset values */
> + const __m128i v_zeros = _mm_setzero_si128();
> + const __m128i v_u16_max = _mm_cmpeq_epi16(v_zeros, v_zeros);
> +
> + /* Only these lanes can be incremented for push-VLAN action. */
> + const uint8_t k_lanes = 0b1110;
> + __m128i v_offset = _mm_set1_epi16(VLAN_HEADER_LEN);
Can you walk through the above logic, as this is the pop use cases, are you
saying you don't want to use these lanes as they should only be used for the
push vlan case?
> +
> + /* Load packet and compare with UINT16_MAX */
> + void *adjust_ptr = &b->l2_pad_size;
> + __m128i v_adjust_src = _mm_loadu_si128(adjust_ptr);
> + __mmask8 k_cmp = _mm_mask_cmpneq_epu16_mask(k_lanes,
> v_adjust_src,
> + v_u16_max);
> +
> + /* Add VLAN_HEADER_LEN using compare mask, store results. */
Again I'm confused here, if the operation being added is to pop_vlan why are we
adding a vlan header?
If you could give a general run down of the expected operations here and logic
it would be appreciated.
Thanks
Ian
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
