[ovs-dev] [PATCH 0/3] ovn-northd IPAM fixes

[Mark Michelson]

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(-)
>