> -----Original Message-----
> From: Finn, Emma <[email protected]>
> Sent: Wednesday, January 5, 2022 4:54 PM
> To: [email protected]; Van Haaren, Harry <[email protected]>;
> Amber, Kumar <[email protected]>
> Cc: Finn, Emma <[email protected]>
> Subject: [PATCH v4 8/9] odp-execute: Add ISA implementation of pop_vlan
> action.
>
> 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]>
Some comments below for variable renaming & clarity.
> diff --git a/NEWS b/NEWS
> index f13722ab7..f5032bdd0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,7 @@ Post-v2.16.0
> * Add build time configure command to enable auto-validator as default
> actions implementation at build time.
> * Add AVX512 implementation of actions.
> + * Add support for an AVX512 optimized version of pop_vlan action.
I'm not sure how verbose we like NEWS files, I think the "pop vlan" action is a
bit
too detailed, the above "add avx512 implementation of actions".
<snip>
> +static inline void ALWAYS_INLINE
> +avx512_dp_packet_resize_l2(struct dp_packet *b, int increment)
> +{
"increment" here is not a great variable name, as we reduce the size by
increment.
Really, it is the size to change the L2 that this parameter communicates. Rename
to "size" for a generic description, or perhaps "resize_by_bytes" for more
detail?
This is an int variable as negative increments are used to "pop" data out of
the packet.
This function gets reworked to be more generic in the last patch, so I'll
review more
in detail there.
<snip remainder of resize l2 func>
> +static inline void ALWAYS_INLINE
> +avx512_eth_pop_vlan(struct dp_packet *packet)
> +{
> + struct vlan_eth_header *veh = dp_packet_eth(packet);
> +
> + if (veh && dp_packet_size(packet) >= sizeof *veh &&
> + eth_type_vlan(veh->veth_type)) {
Verified that the checks here are the same as those on scalar path.
<snip>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev