[ovs-dev] [PATCH v2 2/5] dpif-netdev: Avoid reading RSS hash when EMC is disabled.

O Mahony, Billy billy.o.mahony at intel.com
Mon Jul 31 16:22:05 UTC 2017


Hi Antonio,

This is patch is definitely simpler than the original. 

However on the original patch I suggested: 

"If so it would be less disturbing to the existing code to just add a bool arg to dpif_netdev_packet_get_rss_hash() called do_not_check_recirc_depth and use that to return early (before the if (recirc_depth) check). Also in that case the patch would require none of the  conditional logic changes (neither the original or that suggested in this email) and should be able to just set the proposed do_not_check_recirc_depth based on md_is_valid."

I know you checked this and reported the performance gain was lower than with the v1 patch. We surmised that it was related to introducing a branch in the dpif_netdev_packet_get_rss_hash(). However there are many branches in this patch also.

Can you give details of how you are testing? 
* What is the traffic
* the flows/rules and 
* how are you measuring the performance difference  (ie. cycles per packet or packet throughput or some other measure).

Apologies for going on about this but if we can't get the same effect with a two or three line change than a 20line change I think it'll be worth it.

One other comment below

Thanks,
Billy.


> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-
> bounces at openvswitch.org] On Behalf Of antonio.fischetti at intel.com
> Sent: Wednesday, July 19, 2017 5:05 PM
> To: dev at openvswitch.org
> Subject: [ovs-dev] [PATCH v2 2/5] dpif-netdev: Avoid reading RSS hash when
> EMC is disabled.
> 
> When EMC is disabled the reading of RSS hash is skipped.

[[BO'M]] I think this is already the case with the existing code?  Just addition of OVS_UNLIKELY on the check. 

> For packets that are not recirculated it retrieves the hash value without
> considering the recirc id.
> 
> This is mostly a preliminary change for the next patch in this series.
> 
> Signed-off-by: Antonio Fischetti <antonio.fischetti at intel.com>
> ---
>  lib/dpif-netdev.c | 42 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 123e04a..9562827
> 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4472,6 +4472,22 @@ dp_netdev_upcall(struct dp_netdev_pmd_thread
> *pmd, struct dp_packet *packet_,  }
> 
>  static inline uint32_t
> +dpif_netdev_packet_get_rss_hash_orig_pkt(struct dp_packet *packet,
> +                                const struct miniflow *mf) {
> +    uint32_t hash;
> +
> +    if (OVS_LIKELY(dp_packet_rss_valid(packet))) {
> +        hash = dp_packet_get_rss_hash(packet);
> +    } else {
> +        hash = miniflow_hash_5tuple(mf, 0);
> +        dp_packet_set_rss_hash(packet, hash);
> +    }
> +
> +    return hash;
> +}
> +
> +static inline uint32_t
>  dpif_netdev_packet_get_rss_hash(struct dp_packet *packet,
>                                  const struct miniflow *mf)  { @@ -4572,7 +4588,8 @@ static
> inline size_t  emc_processing(struct dp_netdev_pmd_thread *pmd,
>                 struct dp_packet_batch *packets_,
>                 struct netdev_flow_key *keys,
> -               struct packet_batch_per_flow batches[], size_t *n_batches)
> +               struct packet_batch_per_flow batches[], size_t *n_batches,
> +               bool md_is_valid)
>  {
>      struct emc_cache *flow_cache = &pmd->flow_cache;
>      struct netdev_flow_key *key = &keys[0]; @@ -4602,10 +4619,19 @@
> emc_processing(struct dp_netdev_pmd_thread *pmd,
> 
>          miniflow_extract(packet, &key->mf);
>          key->len = 0; /* Not computed yet. */
> -        key->hash = dpif_netdev_packet_get_rss_hash(packet, &key->mf);
> 
> -        /* If EMC is disabled skip emc_lookup */
> -        flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
> +        /* If EMC is disabled skip hash computation and emc_lookup */
> +        if (OVS_LIKELY(cur_min)) {
> +            if (!md_is_valid) {
> +                key->hash = dpif_netdev_packet_get_rss_hash_orig_pkt(packet,
> +                        &key->mf);
> +            } else {
> +                key->hash = dpif_netdev_packet_get_rss_hash(packet, &key-
> >mf);
> +            }
> +            flow = emc_lookup(flow_cache, key);
> +        } else {
> +            flow = NULL;
> +        }
>          if (OVS_LIKELY(flow)) {
>              dp_netdev_queue_batches(packet, flow, &key->mf, batches,
>                                      n_batches); @@ -4801,7 +4827,7 @@
> fast_path_processing(struct dp_netdev_pmd_thread *pmd,
>   * valid, 'md_is_valid' must be true and 'port_no' will be ignored. */  static
> void  dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
> -                  struct dp_packet_batch *packets)
> +                  struct dp_packet_batch *packets, bool md_is_valid)
>  {
>      int cnt = packets->count;
>  #if !defined(__CHECKER__) && !defined(_WIN32) @@ -4818,7 +4844,7 @@
> dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
>      odp_port_t in_port;
> 
>      n_batches = 0;
> -    emc_processing(pmd, packets, keys, batches, &n_batches);
> +    emc_processing(pmd, packets, keys, batches, &n_batches,
> + md_is_valid);
>      if (!dp_packet_batch_is_empty(packets)) {
>          /* Get ingress port from first packet's metadata. */
>          in_port = packets->packets[0]->md.in_port.odp_port;
> @@ -4855,14 +4881,14 @@ dp_netdev_input(struct
> dp_netdev_pmd_thread *pmd,
>          pkt_metadata_init(&packet->md, port_no);
>      }
> 
> -    dp_netdev_input__(pmd, packets);
> +    dp_netdev_input__(pmd, packets, false);
>  }
> 
>  static void
>  dp_netdev_recirculate(struct dp_netdev_pmd_thread *pmd,
>                        struct dp_packet_batch *packets)  {
> -    dp_netdev_input__(pmd, packets);
> +    dp_netdev_input__(pmd, packets, true);
>  }
> 
>  struct dp_netdev_execute_aux {
> --
> 2.4.11
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list