On 2/12/24 07:50, Mike Pattrick wrote:
> Include inner offsets in functions where l3 and l4 offsets are either
> modified or checked.
>
> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
> v2:
> - Prints out new offsets in autovalidator
> - Extends resize_l2 change to avx512
> v3:
> - Reordered fields in dp_packet_compare_offsets error print message
> - Updated and simplified comments in avx512_dp_packet_resize_l2()
> v4:
> - Removed comment about three asserts
> ---
> lib/dp-packet.c | 18 +++++++++++++-----
> lib/odp-execute-avx512.c | 31 ++++++++++++++++++++-----------
> 2 files changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c
> index 0e23c766e..305822293 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -507,6 +507,8 @@ dp_packet_resize_l2_5(struct dp_packet *b, int increment)
> /* Adjust layer offsets after l2_5. */
> dp_packet_adjust_layer_offset(&b->l3_ofs, increment);
> dp_packet_adjust_layer_offset(&b->l4_ofs, increment);
> + dp_packet_adjust_layer_offset(&b->inner_l3_ofs, increment);
> + dp_packet_adjust_layer_offset(&b->inner_l4_ofs, increment);
>
> return dp_packet_data(b);
> }
> @@ -529,17 +531,23 @@ dp_packet_compare_offsets(struct dp_packet *b1, struct
> dp_packet *b2,
> if ((b1->l2_pad_size != b2->l2_pad_size) ||
> (b1->l2_5_ofs != b2->l2_5_ofs) ||
> (b1->l3_ofs != b2->l3_ofs) ||
> - (b1->l4_ofs != b2->l4_ofs)) {
> + (b1->l4_ofs != b2->l4_ofs) ||
> + (b1->inner_l3_ofs != b2->inner_l3_ofs) ||
> + (b1->inner_l4_ofs != b2->inner_l4_ofs)) {
> if (err_str) {
> ds_put_format(err_str, "Packet offset comparison failed\n");
> ds_put_format(err_str, "Buffer 1 offsets: l2_pad_size %u,"
> - " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> + " l2_5_ofs : %u l3_ofs %u, l4_ofs %u,"
> + " inner_l3_ofs %u, inner_l4_ofs %u\n",
> b1->l2_pad_size, b1->l2_5_ofs,
> - b1->l3_ofs, b1->l4_ofs);
> + b1->l3_ofs, b1->l4_ofs,
> + b1->inner_l3_ofs, b1->inner_l4_ofs);
> ds_put_format(err_str, "Buffer 2 offsets: l2_pad_size %u,"
> - " l2_5_ofs : %u l3_ofs %u, l4_ofs %u\n",
> + " l2_5_ofs : %u l3_ofs %u, l4_ofs %u,"
> + " inner_l3_ofs %u, inner_l4_ofs %u\n",
> b2->l2_pad_size, b2->l2_5_ofs,
> - b2->l3_ofs, b2->l4_ofs);
> + b2->l3_ofs, b2->l4_ofs,
> + b2->inner_l3_ofs, b2->inner_l4_ofs);
> }
> return false;
> }
> diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
> index 747e04014..103ff2d0d 100644
> --- a/lib/odp-execute-avx512.c
> +++ b/lib/odp-execute-avx512.c
> @@ -35,10 +35,10 @@
>
> VLOG_DEFINE_THIS_MODULE(odp_execute_avx512);
>
> -/* The below three build asserts make sure that l2_5_ofs, l3_ofs, and l4_ofs
> - * fields remain in the same order and offset to l2_padd_size. This is needed
> - * as the avx512_dp_packet_resize_l2() function will manipulate those fields
> at
> - * a fixed memory index based on the l2_padd_size offset. */
> +/* The below build asserts make sure that the below fields remain in the same
> + * order and offset to l2_pad_size. This is needed as the
> + * avx512_dp_packet_resize_l2() function will manipulate those fields at a
> + * fixed memory index based on the l2_pad_size offset. */
> BUILD_ASSERT_DECL(offsetof(struct dp_packet, l2_pad_size) +
> MEMBER_SIZEOF(struct dp_packet, l2_pad_size) ==
> offsetof(struct dp_packet, l2_5_ofs));
> @@ -51,6 +51,14 @@ BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
> MEMBER_SIZEOF(struct dp_packet, l3_ofs) ==
> offsetof(struct dp_packet, l4_ofs));
>
> +BUILD_ASSERT_DECL(offsetof(struct dp_packet, l4_ofs) +
> + MEMBER_SIZEOF(struct dp_packet, l4_ofs) ==
> + offsetof(struct dp_packet, inner_l3_ofs));
> +
> +BUILD_ASSERT_DECL(offsetof(struct dp_packet, inner_l3_ofs) +
> + MEMBER_SIZEOF(struct dp_packet, inner_l3_ofs) ==
> + offsetof(struct dp_packet, inner_l4_ofs));
> +
> /* The below build assert makes sure it's safe to read/write 128-bits
> starting
> * at the l2_pad_size location. */
> BUILD_ASSERT_DECL(sizeof(struct dp_packet) -
> @@ -112,7 +120,7 @@ avx512_dp_packet_resize_l2(struct dp_packet *b, int
> resize_by_bytes)
> dp_packet_pull(b, -resize_by_bytes);
> }
>
> - /* The next step is to update the l2_5_ofs, l3_ofs and l4_ofs fields
> which
> + /* The next step is to update the l2_5_ofs to inner_l4_ofs fields which
> * the scalar implementation does with the
> dp_packet_adjust_layer_offset()
> * function. */
>
> @@ -122,13 +130,14 @@ avx512_dp_packet_resize_l2(struct dp_packet *b, int
> resize_by_bytes)
> /* Set the v_u16_max register to all one's. */
> const __m128i v_u16_max = _mm_cmpeq_epi16(v_zeros, v_zeros);
>
> - /* Each lane represents 16 bits in a 12-bit register. In this case the
> - * first three 16-bit values, which will map to the l2_5_ofs, l3_ofs and
> - * l4_ofs fields. */
> - const uint8_t k_lanes = 0b1110;
> + /* Each lane represents 16 bits in a 12-bit register. Here the bitmask
Not an issue of this patch, but there should be 128-bit register, not 12.
I fixed that before applying the set since we're touching this line anyway.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
