> From: Emma Finn <[email protected]>
>
> This commit includes infrastructure changes for enabling set_masked_X
> actions and also adds support for the AVX512 implementation of the
> eth_set_addrs action.
>
> Signed-off-by: Emma Finn <[email protected]>
> ---
> lib/odp-execute-avx512.c | 90 +++++++++++++++++++++++++++++++++++++++
> lib/odp-execute-private.c | 14 ++++++
> lib/odp-execute-private.h | 3 ++
> lib/odp-execute.c | 49 +++++++++++----------
> lib/odp-execute.h | 3 ++
> 5 files changed, 137 insertions(+), 22 deletions(-)
>
> diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c
> index 3449acff7..8ecdaecf6 100644
> --- a/lib/odp-execute-avx512.c
> +++ b/lib/odp-execute-avx512.c
> @@ -23,6 +23,7 @@
>
> #include "dp-packet.h"
> #include "immintrin.h"
> +#include "odp-execute.h"
> #include "odp-execute-private.h"
> #include "odp-netlink.h"
> #include "openvswitch/vlog.h"
> @@ -50,6 +51,16 @@ BUILD_ASSERT_DECL(offsetof(struct dp_packet, l3_ofs) +
> BUILD_ASSERT_DECL(sizeof(struct dp_packet) -
> offsetof(struct dp_packet, l2_pad_size) >=
> sizeof(__m128i));
>
> +/* The below build assert makes sure the order of the fields needed by
> + * the set masked functions shuffle operations do not change. This should not
> + * happen as these are defined under the Linux uapi. */
> +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));
> +
> +/* Array of callback functions, one for each masked operation. */
> +odp_execute_action_cb impl_set_masked_funcs[__OVS_KEY_ATTR_MAX];
> +
> static inline void ALWAYS_INLINE
> avx512_dp_packet_resize_l2(struct dp_packet *b, int resize_by_bytes)
> {
> @@ -207,6 +218,80 @@ action_avx512_push_vlan(struct dp_packet_batch *batch,
> const struct nlattr *a)
> }
> }
>
> +/* This function performs the same operation on each packet in the batch as
> + * the scalar odp_eth_set_addrs() function. */
> +static void
> +action_avx512_eth_set_addrs(struct dp_packet_batch *batch,
> + const struct nlattr *a)
> +{
> + const struct ovs_key_ethernet *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_ethernet);
> +
> + /* Read the content of the key(src) and mask in the respective registers.
> + * We only load the src and dest addresses, which is only 96-bits and not
> + * 128-bits. */
> + __m128i v_src = _mm_maskz_loadu_epi32(0x7,(void *) key);
> + __m128i v_mask = _mm_maskz_loadu_epi32(0x7, (void *) mask);
One question here I asked throughout the various revisions but got not answered:
"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."
<SNIP>
> +void
> +odp_execute_scalar_action(struct dp_packet_batch *batch,
> + const struct nlattr *action)
> +{
> + enum ovs_action_attr type = nl_attr_type(action);
> +
> + if (action_impls[ACTION_IMPL_SCALAR].funcs[type] &&
> + type <= OVS_ACTION_ATTR_MAX) {
Guess the two checks above need to be reversed, i.e. the type <=
OVS_ACTION_ATTR_MAX should be first.
> +
> + action_impls[ACTION_IMPL_SCALAR].funcs[type](batch, action);
> + }
> +}
<SNIP>
> +static void
> +action_set_masked(struct dp_packet_batch *batch, const struct nlattr *a)
> +{
> + const struct nlattr *key = nl_attr_get(a);
> + struct dp_packet *packet;
> +
> + DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
> + odp_execute_masked_set_action(packet, key);
> + }
Indentation is off here.
<SNIP>
The rest of the patch looks good to me.
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
