Hi Ben,
The first two patches in this series aren't necessary. ovn_datapaths are
allocated from scratch and then all destroyed during each loop of
ovn-northd. They never survive multiple loops. When entering
init_ipam_info_for_datapath(), you can assert that od->ipam_info == NULL
[1].
For patch 1, the unconditional allocation in the IPv6 case is not a
leak. The reason why the IPv4 case is conditional is because there may
be an IPv4 subnet and IPv6 prefix set. In that case, the ipam_info is
allocated in the IPv6 case, and so when the IPv4 case arises, it's
already allocated.
For patch 2, since the ipam_info is allocated in
init_ipam_info_for_datapath, and it is allocated using xzalloc, it is
redundant to zero out unset fields on the struct.
For patch 3, I'm commenting directly on the patch submission.
[1] The only way this assertion would fail is if we somehow managed to
allocate two logical switches with the same UUID.
On 06/15/2018 07:11 PM, Ben Pfaff wrote:
While reviewing https://patchwork.ozlabs.org/patch/924319/, I discovered
some bugs in IPAM that seem worth fixing. The first two patches below are
minimal so that they can be backported. The third is an improvement that
doesn't need backporting.
Ben Pfaff (3):
ovn-northd: Fix memory leak when IPv6 IPAM is in use.
ovn-northd: Make it possible to disable IPAM once enabled.
ovn-northd: Always allocate ipam_info for an ovn_datapath.
ovn/northd/ovn-northd.c | 78 +++++++++++++++++++++++--------------------------
1 file changed, 37 insertions(+), 41 deletions(-)
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
