> From: Harry van Haaren <harry.van.haa...@intel.com>
> 
> Partial hardware offload is implemented in a very similar way to the
> scalar dpif.
> 
> Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com>

Thanks for the patch Harry/Cian.

Given its size I wonder would it make sense to merge with patch 4 of the 
series? Was the intention to do this eventually but just separate it here for 
reviewing purposes?

I'd like to get a test run on this also going forward on my side so not fully 
finished on review.

@Finn, Emma would you be able to give this a run when you have a chance?

Minor comments below.


> ---
>  lib/dpif-netdev-avx512.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/dpif-netdev-avx512.c b/lib/dpif-netdev-avx512.c
> index 91f51c479..98638de15 100644
> --- a/lib/dpif-netdev-avx512.c
> +++ b/lib/dpif-netdev-avx512.c
> @@ -27,6 +27,7 @@
>  #include "dpif-netdev-private-dpcls.h"
>  #include "dpif-netdev-private-flow.h"
>  #include "dpif-netdev-private-thread.h"
> +#include "dpif-netdev-private-hwol.h"
> 
>  #include "dp-packet.h"
>  #include "netdev.h"
> @@ -112,9 +113,32 @@ dp_netdev_input_outer_avx512(struct
> dp_netdev_pmd_thread *pmd,
>          uint32_t i = __builtin_ctz(iter);
>          iter = _blsr_u64(iter);
> 
> -        /* Initialize packet md and do miniflow extract. */
> +        /* Get packet pointer from bitmask and packet md. */
>          struct dp_packet *packet = packets->packets[i];
>          pkt_metadata_init(&packet->md, in_port);
> +
> +        struct dp_netdev_flow *f = NULL;
> +
> +        /* Check for partial hardware offload mark. */
> +        uint32_t mark;
> +        if (dp_packet_has_flow_mark(packet, &mark)) {
> +            f = mark_to_flow_find(pmd, mark);
> +            if (f) {
> +                rules[i] = &f->cr;
> +
> +                /* This is nasty - instead of using the HWOL provided flow,
> +                 * parse the packet data anyway to find the location of the 
> TCP
> +                 * header to extract the TCP flags for the rule.
> +                 */

Is it a case that this info is just not available in the HWOL rule provided?

This overhead would have an effect on performance  but only for the AVX512 DIPF 
implantation correct? No effect on existing HWOL and scalar?

BR
Ian
> +                pkt_meta[i].tcp_flags = parse_tcp_flags(packet);
> +
> +                pkt_meta[i].bytes = dp_packet_size(packet);
> +                hwol_emc_smc_hitmask |= (1 << i);
> +                continue;
> +            }
> +        }
> +
> +        /* Do miniflow extract into keys. */
>          struct netdev_flow_key *key = &keys[i];
>          miniflow_extract(packet, &key->mf);
> 
> @@ -125,8 +149,6 @@ dp_netdev_input_outer_avx512(struct
> dp_netdev_pmd_thread *pmd,
>          key->len = netdev_flow_key_size(miniflow_n_values(&key->mf));
>          key->hash = dpif_netdev_packet_get_rss_hash_orig_pkt(packet, &key-
> >mf);
> 
> -        struct dp_netdev_flow *f = NULL;
> -
>          if (emc_enabled) {
>              f = emc_lookup(&cache->emc_cache, key);
> 
> --
> 2.31.1
> 
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to