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 <[email protected]>
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. 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). _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
