[ovs-dev] [PATCH] dpif-netdev: Avoid copying netdev_flow_key in emc_processing().

Ben Pfaff blp at ovn.org
Mon Feb 1 17:29:54 UTC 2016


On Thu, Jan 28, 2016 at 01:14:59AM +0000, Daniele Di Proietto wrote:
> 
> 
> On 26/01/2016 08:23, "Ben Pfaff" <blp at ovn.org> wrote:
> 
> >On Tue, Jan 26, 2016 at 03:28:47AM +0000, Daniele Di Proietto wrote:
> >> 
> >> 
> >> On 25/01/2016 18:20, "Andy Zhou" <azhou at ovn.org> wrote:
> >> 
> >> >On Sun, Jan 24, 2016 at 8:32 AM, Ben Pfaff <blp at ovn.org> wrote:
> >> >
> >> >> Before this commit, emc_processing() copied a netdev_flow_key if
> >>there
> >> >>was
> >> >> no exact-match cache (EMC) hit.  This commit eliminates the copy by
> >> >> constructing the netdev_flow_key in the place it would be copied.
> >> >>
> >> >> Found by inspection.
> >> >>
> >> >> Shahbaz (CCed) reports that this reduces the cost of an EMC miss by
> >>72
> >> >> cycles in his test case in which the EMC is disabled.  Presumably
> >>this
> >> >> is similarly valuable in cases where the EMC merely has few hits.
> >> >>
> >> >> CC: Muhammad Shahbaz <mshahbaz at cs.princeton.edu>
> >> >> Signed-off-by: Ben Pfaff <blp at ovn.org>
> >> >>
> >> >
> >> >Logic looks good to me.
> >> >
> >> >Acked-by: Andy Zhou <azhou at ovn.org>
> >> >
> >> >I think Daniele is testing on performance impact on this patch. so it
> >>is
> >> >may be worthwhile
> >> >to wait for his input before pushing the change.
> >> 
> >> If I disable the EMC I observe an increase in throughput when this patch
> >> is applied (from ~4.8 Mpps to ~5.4 Mpps).
> >> 
> >> If the EMC is enabled, instead, I see a drop in throughput
> >> (from ~12.4 Mpps to ~12.0 Mpps) with this patch.
> >> 
> >> At a first glance it looks like this patch is preventing my compiler
> >> (GCC 4.8) to perform some optimization.
> >
> >Does the "restrict" keyword make any difference?
> >e.g. in the function prototype, changing
> >    struct netdev_flow_key *keys
> >to
> >    struct netdev_flow_key *restrict keys
> >
> >Just a thought, not sure it'll help.
> 
> I tried adding "restrict", but I didn't notice any differences.
> 
> It seems that GCC decides to inline a different set of functions with
> this patch applied.
> 
> Given the numbers, and given that we can restore the performance of the
> emc path in other ways, I'm in favour of this patch.
> 
> Acked-by: Daniele Di Proietto <diproiettod at vmware.com>

Thanks.  I applied this to master.



More information about the dev mailing list