[ovs-dev] [PATCHv1 1/4] ovn-northd: Support Logical_Port.addresses to store multiple ips in each set

[Ben Pfaff]

On Fri, Jan 29, 2016 at 09:27:33PM +0530, Numan Siddique wrote:
> If a logical port has two ipv4 addresses and one ipv6 address
> it will be stored as ["MAC IPv41 IPv42 IPv61"] instead of
> ["MAC IPv41", "MAC IPv42", "MAC IPv61"].
> 
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>

This appears at first to add a lot of duplicate code in packets.c.  Can,
perhaps, the existing parsing functions be written in terms of the new
functions?  It would be better to avoid having a lot of cut-and-paste
code if we can.

extract_lport_addresses() appears to just silently ignore errors.  It
would be better to log them (rate-limited) so that administrators have a
chance to catch misconfiguration.

The description in ovn-nb.xml implies that IPv4 and IPv6 addresses can
be mixed together, but the implementation in extract_lport_addresses()
requires IPv4 addresses to precede IPv6 addresses.  It would be better
to allow the freer form.

The code appears to parse IPv6 addresses and then completely ignore
them.

Code of the form
        if (p) {
            free(p);
        }
can be simply written
        free(p);

extract_lport_addresses() does a lot of malloc()s.  At the very least,
the malloc of struct lport_addresses itself is unnecessary: the caller
can simply provide one on the stack, by pointer.

Thanks,

Ben.