On 02/20/2016 08:07 AM, Ben Pfaff wrote:
> 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 <[email protected]>
> 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.
Sure. I will do it.
>
> 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.
Done.
>
> 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.
Sure. I will do it that way.
>
> 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.
Sure. I will do it as per your comments.
Thanks for reviewing.
> Thanks,
>
> Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
