[ovs-dev] [ovn-ipv6 14/26] ovn: Support multiple addresses on a single logical router port.

Ben Pfaff blp at ovn.org
Wed Jul 13 03:44:18 UTC 2016


On Mon, Jul 11, 2016 at 11:56:44PM -0700, Justin Pettit wrote:
> Supporting multiple addresses is only mildly interesting for IPv4.
> However, it is a requirement for IPv6, which will arrive in a future
> commit.
> 
> This commit introduces the extract_lrp_networks() function to ovn-util.[ch].
> 
> Signed-off-by: Justin Pettit <jpettit at ovn.org>

"sparse" complains.  Here is the fix:

diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c
index 3e0c138..c3cb8e4 100644
--- a/ovn/lib/ovn-util.c
+++ b/ovn/lib/ovn-util.c
@@ -20,7 +20,7 @@
 VLOG_DEFINE_THIS_MODULE(ovn_util);
 
 static void
-add_ipv4_netaddr(struct lport_addresses *laddrs, uint32_t addr,
+add_ipv4_netaddr(struct lport_addresses *laddrs, ovs_be32 addr,
                  unsigned int plen)
 {
     laddrs->n_ipv4_addrs++;

The documentation for the lrp-add command is confusing for a few
reasons.  First, it talks about an optional argument "peer" that isn't
in the synopsis.  I guess that this is a special case of the ability to
set arbitrary key/value pairs, but this may not be obvious to the casual
reader.  It's also somewhat weird that --may-exist verifies that this
doesn't change but not other key-value pairs.  Second, it's probably not
easy to a casual reader to tell how the command distinguishes "network"
parameters from key-values.  An example might help.

The usage message for ovn-nbctl mentions the peer=PEER special case but
not the general case.  If that's intentional, I guess it's OK--a usage
message cannot be complete, otherwise it would be a manpage.

In ovn-nbctl, I think that the minimum valid number of arguments is
really 4 here (since 3 will always yield an error):
+    { "lrp-add", 3, INT_MAX,
+      "ROUTER PORT MAC NETWORK... [COLUMN[:KEY]=VALUE]...",

In add_ipv4_netaddr() and add_ipv6_netaddr(), usually we write "sizeof
*object" instead of "sizeof(type)".

In add_ipv6_netaddr(), it seems odd to bother xmallocing() a fixed size
instead of just including an equivalent fixed-sized buffer in struct
ipv6_netaddr.

Acked-by: Ben Pfaff <blp at ovn.org>



More information about the dev mailing list