On Wed, Sep 13, 2017 at 05:20:58AM +0000, Darrell Ball wrote:
> 
> 
> On 9/5/17, 2:23 AM, "Yuanhan Liu" <[email protected]> wrote:
> 
>     So that we could skip the heavy emc processing, notably, the
>     miniflow_extract function. A simple PHY-PHY forwarding testing
>     shows 53% performance improvement.
>     
>     Note that though the heavy miniflow_extract is skipped
> 
> [Darrell] How much of the increased performance is due to skipping 
>                miniflow_extract ?

For the PHY-PHY case, following are the performance gains for each case.

with_miniflow_extract 22%                       
with_parse_tcp_falgs  54%
without both:         70%

[...]
>     +        if (dp_packet_has_flow_mark(packet, &flow_mark)) {
>     +            flow = dp_netdev_pmd_find_flow_by_mark(pmd, flow_mark);
>     +            if (flow) {
>     +                tcp_flags = parse_tcp_flags(packet);
>     +                dp_netdev_queue_batches(packet, flow, tcp_flags, batches,
>     +                                        n_batches);
>     +                continue;
>     +            }
>     +        }
>     +
> 
> [Darrell] As mentioned, you would move dp_netdev_pmd_find_flow_by_mark()  to 
> following code; 
>               also, maybe, you can get rid of parse_tcp_flags() as a result.

I doubt it, as you see from above data, the performance impact due to
miniflow_extract is huge.

> 
>              if (i != size - 1) {
>                  struct dp_packet **packets = packets_->packets;
>                  /* Prefetch next packet data and metadata. */
>     @@ -4989,7 +5001,8 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>              /* If EMC is disabled skip emc_lookup */
>              flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
>              if (OVS_LIKELY(flow)) {
>     -            dp_netdev_queue_batches(packet, flow, &key->mf, batches,
>     +            tcp_flags = miniflow_get_tcp_flags(&key->mf);
>     +            dp_netdev_queue_batches(packet, flow, tcp_flags, batches,
>                                          n_batches);
>              } else {
>                  /* Exact match cache missed. Group missed packets together at
>     @@ -5166,7 +5179,9 @@ fast_path_processing(struct dp_netdev_pmd_thread 
> *pmd,
>              flow = dp_netdev_flow_cast(rules[i]);
>      
>              emc_probabilistic_insert(pmd, &keys[i], flow);
>     -        dp_netdev_queue_batches(packet, flow, &keys[i].mf, batches, 
> n_batches);
>     +        dp_netdev_queue_batches(packet, flow,
>     +                                miniflow_get_tcp_flags(&keys[i].mf),
>     +                                batches, n_batches);
>          }
>      
>          dp_netdev_count_packet(pmd, DP_STAT_MASKED_HIT, cnt - miss_cnt);
>     diff --git a/lib/flow.c b/lib/flow.c
>     index b2b10aa..912c538 100644
>     --- a/lib/flow.c
>     +++ b/lib/flow.c
>     @@ -991,6 +991,84 @@ parse_dl_type(const struct eth_header *data_, size_t 
> size)
>          return parse_ethertype(&data, &size);
>      }
>      
> 
> [Darrell] Hopefully we can get rid of the following function which removes 
> the changes to flow.c/.h
>                 If not, we would have to splice out common code.

To let parse_tcp_flags() and miniflow_extract() share more common code?

        --yliu
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to