> From: Emma Finn <[email protected]>
>
> This commit adds support for the AVX512 implementation of the
> ipv4_set_addrs action as well as an AVX512 implementation of
> updating the checksums.
>
> Signed-off-by: Emma Finn <[email protected]>
> ---
> lib/odp-execute-avx512.c | 208 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 208 insertions(+)
>
> diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
> index 8ecdaecf6..a0c97f312 100644
> --- a/lib/odp-execute-avx512.c
> +++ b/lib/odp-execute-avx512.c
> @@ -21,6 +21,7 @@
> #include <config.h>
> #include <errno.h>
>
> +#include "csum.h"
> #include "dp-packet.h"
> #include "immintrin.h"
> #include "odp-execute.h"
> @@ -58,6 +59,22 @@ BUILD_ASSERT_DECL(offsetof(struct ovs_key_ethernet,
> eth_src) +
> MEMBER_SIZEOF(struct ovs_key_ethernet, eth_src) ==
> offsetof(struct ovs_key_ethernet, eth_dst));
>
> +BUILD_ASSERT_DECL(offsetof(struct ovs_key_ipv4, ipv4_src) +
> + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_src) ==
> + offsetof(struct ovs_key_ipv4, ipv4_dst));
> +
> +BUILD_ASSERT_DECL(offsetof(struct ovs_key_ipv4, ipv4_dst) +
> + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_dst) ==
> + offsetof(struct ovs_key_ipv4, ipv4_proto));
> +
> +BUILD_ASSERT_DECL(offsetof(struct ovs_key_ipv4, ipv4_proto) +
> + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_proto) ==
> + offsetof(struct ovs_key_ipv4, ipv4_tos));
> +
> +BUILD_ASSERT_DECL(offsetof(struct ovs_key_ipv4, ipv4_tos) +
> + MEMBER_SIZEOF(struct ovs_key_ipv4, ipv4_tos) ==
> + offsetof(struct ovs_key_ipv4, ipv4_ttl));
> +
> /* Array of callback functions, one for each masked operation. */
> odp_execute_action_cb impl_set_masked_funcs[__OVS_KEY_ATTR_MAX];
>
> @@ -279,6 +296,196 @@ action_avx512_eth_set_addrs(struct dp_packet_batch
> *batch,
> }
> }
>
> +static inline uint16_t ALWAYS_INLINE
> +avx512_get_delta(__m256i old_header, __m256i res)
Can we renamed res to new_header, and also make sure we use the same order
in all three functions?
> +{
> + __m256i v_zeros = _mm256_setzero_si256();
> + uint16_t delta;
> +
> + /* These two shuffle masks, v_swap16a and v_swap16b, are to shuffle the
> + * old and new header to add padding after each 16-bit value for the
> + * following carry over addition. */
> + __m256i v_swap16a = _mm256_setr_epi16(0x0100, 0xFFFF, 0x0302, 0xFFFF,
> + 0x0504, 0xFFFF, 0x0706, 0xFFFF,
> + 0x0100, 0xFFFF, 0x0302, 0xFFFF,
> + 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF);
> + __m256i v_swap16b = _mm256_setr_epi16(0x0908, 0xFFFF, 0x0B0A, 0xFFFF,
> + 0x0D0C, 0xFFFF, 0x0F0E, 0xFFFF,
> + 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF,
> + 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF);
> + __m256i v_shuf_old1 = _mm256_shuffle_epi8(old_header, v_swap16a);
> + __m256i v_shuf_old2 = _mm256_shuffle_epi8(old_header, v_swap16b);
> + __m256i v_shuf_new1 = _mm256_shuffle_epi8(res, v_swap16a);
> + __m256i v_shuf_new2 = _mm256_shuffle_epi8(res, v_swap16b);
> +
> + /* Add each part of the old and new headers together. */
> + __m256i v_delta1 = _mm256_add_epi32(v_shuf_old1, v_shuf_new1);
> + __m256i v_delta2 = _mm256_add_epi32(v_shuf_old2, v_shuf_new2);
> +
> + /* Add old and new header. */
> + __m256i v_delta = _mm256_add_epi32(v_delta1, v_delta2);
> +
> + /* Perform horizontal add to go from 8x32-bits to 2x32-bits. */
> + v_delta = _mm256_hadd_epi32(v_delta, v_zeros);
> + v_delta = _mm256_hadd_epi32(v_delta, v_zeros);
> +
> + /* Shuffle 32-bit value from 3rd lane into first lane for final
> + * horizontal add. */
> + __m256i v_swap32a = _mm256_setr_epi32(0x0, 0x4, 0xF, 0xF,
> + 0xF, 0xF, 0xF, 0xF);
> + v_delta = _mm256_permutexvar_epi32(v_swap32a, v_delta);
> +
> + v_delta = _mm256_hadd_epi32(v_delta, v_zeros);
> + v_delta = _mm256_hadd_epi16(v_delta, v_zeros);
> +
> + /* Extract delta value. */
> + delta = _mm256_extract_epi16(v_delta, 0);
> +
> + return delta;
> +}
> +
> +static inline uint16_t ALWAYS_INLINE
> +avx512_l4_update_csum(__m256i old_header, __m256i res)
This function does not really update the ipv4 csum, so it should probably be
renamed to something like, avx512_ipv4_addr_csum_delta().
Also the "res" name does not make sense, maybe something like, new_header?
And some help before the function!?
/* This function will calculate the csum delta for the IPv4 addresses in the
* new_header and old_header, assuming the csum field on the new_header was
* updated. */
> +{
> + __m256i v_zeros = _mm256_setzero_si256();
> + uint16_t delta;
> +
> + /* Set the v_ones register to all one's. */
> + __m256i v_ones = _mm256_cmpeq_epi16(v_zeros, v_zeros);
> +
> + /* Combine the old and new header, i.e. adding in the new IP addresses
> + * in the old header (oh). This is done by using the 0x03C 16-bit mask,
> + * picking 16-bit word 7 till 10. */
> + __m256i v_blend_new = _mm256_mask_blend_epi16(0x03C0, old_header, res);
> +
> + /* Invert the old_header register. */
> + old_header =_mm256_andnot_si256(old_header, v_ones);
> +
> + /* Calculate the delta between the old and new header. */
> + delta = avx512_get_delta(old_header, v_blend_new);
> +
> + return delta;
> +
> +}
> +
> +static inline uint16_t ALWAYS_INLINE
> +avx512_ipv4_update_csum(__m256i res, __m256i old_header)
This function does not really update the ipv4 csum, so it should probably be
renamed to something like, avx512_ipv4_hdr_csum_delta().
Also the "res" name does not make sense, maybe something like, new_header?
And some help before the function!?
/* This function will calculate the csum delta between the new_header and
* old_header, assuming the csum field on the new_header was not yet updated
* or reset. It also assumes headers contain the first 20-bytes of the IPv4
* header data, and the rest is zeroed out. */
> +{
> + __m256i v_zeros = _mm256_setzero_si256();
> + uint16_t delta;
> +
These two should be reversed and an extra cr/lf.
+ uint16_t delta;
+
+ /* Set the v_zeros register to all zero's. */
+ __m256i v_zeros = _mm256_setzero_si256();
> + /* Set the v_ones register to all one's. */
> + __m256i v_ones = _mm256_cmpeq_epi16(v_zeros, v_zeros);
> +
> + /* Invert the old_header register. */
> + old_header =_mm256_andnot_si256(old_header, v_ones);
> +
> + /* Calculate the delta between the old and new header. */
> + delta = avx512_get_delta(old_header, res);
> +
> + return delta;
> +}
> +
> +/* This function performs the same operation on each packet in the batch as
> + * the scalar odp_set_ipv4() function. */
> +static void
> +action_avx512_ipv4_set_addrs(struct dp_packet_batch *batch,
> + const struct nlattr *a)
> +{
> + const struct ovs_key_ipv4 *key, *mask;
> + struct dp_packet *packet;
> + a = nl_attr_get(a);
> + key = nl_attr_get(a);
> + mask = odp_get_key_mask(a, struct ovs_key_ipv4);
> +
> + /* Read the content of the key(src) and mask in the respective registers.
> + * We only load the size of the actual structure, which is only 96-bits.
> */
> + __m256i v_key = _mm256_maskz_loadu_epi32(0x7, (void *) key);
> + __m256i v_mask = _mm256_maskz_loadu_epi32(0x7, (void *) mask);
Same question as on the previous patch:
"The second load, loads 128 bits of data, but there are only 12 bytes to load.
What happens if the memory at the remaining 6 bytes are not mapped in memory
(i.e. a page does not exist/can't be loaded)? Will we crash!?
Guess the key is fine, as we will read some bytes of the mask data."
> +
> + /* This two shuffle masks, v_shuf32, v_shuffle, are to shuffle key and
> + * mask to match the ip_header structure layout. */
> + static const uint8_t ip_shuffle_mask[32] = {
> + 0xFF, 0x05, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0x06, 0xFF, 0xFF, 0xFF, 0x00, 0x01, 0x02, 0x03,
> + 0x00, 0x01, 0x02, 0x03, 0xFF, 0xFF, 0xFF, 0xFF,
> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
> +
> + __m256i v_shuf32 = _mm256_setr_epi32(0x0, 0x2, 0xF, 0xF,
> + 0x1, 0xF, 0xF, 0xF);
> +
> + __m256i v_shuffle = _mm256_loadu_si256((void *) ip_shuffle_mask);
> +
> + /* Two shuffles are required for key and mask to match the layout of
> + * the ip_header struct. The _shuffle_epi8 only works within 128-bit
> + * lanes, so a permute is required to move src and dst into the correct
> + * lanes. And then a shuffle is used to move the fields into the right
> + * order.
> + */
> + __m256i v_key_shuf = _mm256_permutexvar_epi32(v_shuf32, v_key);
> + v_key_shuf = _mm256_shuffle_epi8(v_key_shuf, v_shuffle);
> +
> + __m256i v_mask_shuf = _mm256_permutexvar_epi32(v_shuf32, v_mask);
> + v_mask_shuf = _mm256_shuffle_epi8(v_mask_shuf, v_shuffle);
> +
> + DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
> + struct ip_header *nh = dp_packet_l3(packet);
> + ovs_be16 old_csum = ~nh->ip_csum;
> +
> + /* Load the 20 bytes of the IPv4 header. Without options, which is
> the
> + * most common case it's 20 bytes, but can be up to 60 bytes. */
> + __m256i v_packet = _mm256_maskz_loadu_epi32(0x1F, (void *) nh);
> +
> + /* AND the v_pkt_mask to the packet data (v_packet). */
> + __m256i v_pkt_masked = _mm256_andnot_si256(v_mask_shuf, v_packet);
> +
> + /* OR the new addresses (v_key_shuf) with the masked packet addresses
> + * (v_pkt_masked). */
> + __m256i v_res = _mm256_or_si256(v_key_shuf, v_pkt_masked);
> +
> + /* Update the IP checksum based on updated IP values. */
> + uint16_t delta = avx512_ipv4_update_csum(v_res, v_packet);
> + uint32_t new_csum = old_csum + delta;
> + delta = csum_finish(new_csum);
> +
> + /* Insert new checksum. */
> + v_res = _mm256_insert_epi16(v_res, delta, 5);
> +
> + /* If ip_src or ip_dst has been modified, L4 checksum needs to
> + * be updated too. */
> + if (mask->ipv4_src || mask->ipv4_dst) {
> +
> + uint16_t delta_checksum = avx512_l4_update_csum(v_packet, v_res);
> +
Wondering if all this AVX code being executed really is faster than
recalc_csum32(uh->udp_csum, old_addr, new_addr)?
> + if (nh->ip_proto == IPPROTO_UDP) {
> + /* New UDP checksum. */
> + struct udp_header *uh = dp_packet_l4(packet);
> + if (uh->udp_csum) {
> + uint16_t old_udp_checksum = ~uh->udp_csum;
> + uint32_t udp_checksum = old_udp_checksum +
> delta_checksum;
> + udp_checksum = csum_finish(udp_checksum);
> +
> + if (!udp_checksum) {
> + udp_checksum = htons(0xffff);
> + }
> + /* Insert new udp checksum. */
> + uh->udp_csum = udp_checksum;
> + }
> + } else if (nh->ip_proto == IPPROTO_TCP) {
> + /* New TCP checksum. */
> + struct tcp_header *th = dp_packet_l4(packet);
> + uint16_t old_tcp_checksum = ~th->tcp_csum;
> + uint32_t tcp_checksum = old_tcp_checksum + delta_checksum;
> + tcp_checksum = csum_finish(tcp_checksum);
> +
> + th->tcp_csum = tcp_checksum;
> + }
> + }
> + /* Write back the modified IPv4 addresses. */
> + _mm256_mask_storeu_epi32((void *) nh, 0x1F, v_res);
> + }
> +}
> +
> static void
> action_avx512_set_masked(struct dp_packet_batch *batch, const struct nlattr
> *a)
> {
> @@ -304,6 +511,7 @@ action_avx512_init(struct odp_execute_action_impl *self
> OVS_UNUSED)
> /* Set function pointers for the individual operations supported by the
> * SET_MASKED action. */
> impl_set_masked_funcs[OVS_KEY_ATTR_ETHERNET] =
> action_avx512_eth_set_addrs;
> + impl_set_masked_funcs[OVS_KEY_ATTR_IPV4] = action_avx512_ipv4_set_addrs;
>
> return 0;
> }
> --
> 2.32.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev