On Fri, Feb 23, 2018 at 01:15:38PM -0600, Mark Michelson wrote:
> The ofport_is_internal_or_patch() function called
> ofproto_port_open_type() twice. This resulted in the ofproto_class being
> looked up twice, when we only needed to look it up once.
> This patch alters ofport_is_internal_or_patch() to look up the
> ofproto_class in-line, and then use the looked up class's port_open_type
> callback twice.
> Signed-off-by: Mark Michelson <mmich...@redhat.com>
> Tested-by: Aliasgar Ginwala <aginw...@ebay.com>
> This patch arises as a result of testing done by Ali Ginwala and Han
> Zhou. Their test showed that commit 2d4beba resulted in slower
> performance of ovs-vswitchd than was seen in previous versions of OVS.
> With this patch, Ali retested and reported that this patch had nearly
> the same effect on performance as reverting 2d4beba.
> The test was to create 10000 logical switch ports using 20 farm nodes,
> each running 50 HV sandboxes. "Base" in the tests below is OVS master
> with Han Zhou's ovn-controller incremental processing patch applied.
> base: Test completed in 8 hours
> base with 2d4beba reverted: Test completed in 5 hours 28 minutes
> base with this patch: Test completed in 5 hours 30 minutes
Thanks for finding and fixing the performance problem.
Taking a look at what ofproto_port_open_type() is doing, it's badly
written for the job it has to do in context. Its caller has a
particular ofproto, from which one can find the ofproto_class and
therefore the port_open_type function by just following two pointers.
But it was actually throwing away that information and going to some
trouble to find it again via ofproto_class_find__(), which is super slow
and not really meant for inner loops.
We can solve the problem better by changing ofproto_port_open_type()'s
parameter from a datapath type name to an ofproto and then passing that
in. It requires a little effort in a couple of places in the tree to
find the ofproto, but it's not exactly difficult.
So I think that we can do even better than this patch and probably
obtain an additional performance boost. I posted a patch that does
What do you think?
dev mailing list