[ovs-dev] [PATCH 1/4] ovn-controller: decouple localnet_port update from patch_run

Han Zhou zhouhan at gmail.com
Tue Jun 6 23:59:57 UTC 2017


On Tue, Jun 6, 2017 at 12:31 PM, Ben Pfaff <blp at ovn.org> wrote:
>
> On Thu, May 25, 2017 at 05:26:44PM -0700, Han Zhou wrote:
> > We figure out local datapaths in binding_run() but update the field
> > localnet_port for each local datapath that has localnet port in
> > patch_run(). This patch updates the localnet_port field in binding_run
> > directly and removes the logic in patch_run(), since the logic is
> > more about port-binding processing, and patch_run() is focusing on
> > patch port creation only.
> >
> > In a future patch binding_run() will be used in a new thread for
> > pinctrl, but patch_run() will not.
> >
> > Signed-off-by: Han Zhou <zhouhan at gmail.com>
>
> Thanks for working on this!  I'm looking forward to the improvements to
> ovn-controller from this series.
>
> I like this change even on its own.  I think that it will make the code
> clearer.
>
> I don't think that localnet_ports is being used as a hash table here.
> It is really being used as a list or an array, because the only
> operations on it are insertion and iteration.  Perhaps a dynamic array
> of "struct ovsrec_binding *" would be a better way?  Another way, which
> is even simpler, would be to simply add a second iteration over the
> sbrec_port_binding that looks only at localnet ports.  This would be
> slower, but it would not increase the big-O for binding_run() because it
> already has a similar loop.

Agree.

>
> I think that there is a memory leak here because I don't see anything
> that frees the "struct localnet_port"s (only the hash table that holds
> them).

Sorry, I will come up with next patch just iterate port-bindings again.


More information about the dev mailing list