Re: [ovs-dev] [RFC PATCH 03/13] tunneling: add ipv6 fields to netdev_tunnel_config

[Ben Pfaff]

On Thu, May 14, 2015 at 08:12:34PM +0200, Jiri Benc wrote:
> Allow configuration of ipv6 tunnel endpoints.
> 
> Signed-off-by: Jiri Benc 

The OVS coding style, for better or worse, calls for {} to be used even
when there's only a single statement, so there are some missing {} in
parse_tunnel_ip().

Now that I see its use, I think I would rather see smap_add_ipv6() as a
helper within netdev-vport.c, because I don't expect it to see wide use
outside of that file.

Documentation for the new tunnel support is missing from
vswitch/vswitch.xml and it is not yet mentioned in NEWS (perhaps those
are in an upcoming patch)?

Thanks,

Ben.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [RFC PATCH 03/13] tunneling: add ipv6 fields to netdev_tunnel_config

[Jiri Benc]

Allow configuration of ipv6 tunnel endpoints.

Signed-off-by: Jiri Benc 
---
 lib/dpif-netlink.c |  6 +++-
 lib/netdev-vport.c | 88 --
 lib/netdev.h   |  3 ++
 3 files changed, 74 insertions(+), 23 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 6838def7d9ad..73ad0cb046e2 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -854,12 +854,16 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, struct 
netdev *netdev,
 }
 
 tnl_cfg = netdev_get_tunnel_config(netdev);
-if (tnl_cfg && (tnl_cfg->dst_port != 0 || tnl_cfg->exts)) {
+if (tnl_cfg && (tnl_cfg->dst_port != 0 || tnl_cfg->protocol != 0
+ || tnl_cfg->exts)) {
 ofpbuf_use_stack(&options, options_stub, sizeof options_stub);
 if (tnl_cfg->dst_port) {
 nl_msg_put_u16(&options, OVS_TUNNEL_ATTR_DST_PORT,
ntohs(tnl_cfg->dst_port));
 }
+if (tnl_cfg->protocol == ETH_TYPE_IPV6) {
+nl_msg_put_flag(&options, OVS_TUNNEL_ATTR_OVER_IPV6);
+}
 if (tnl_cfg->exts) {
 size_t ext_ofs;
 int i;
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index ea9abf9e7a78..6bcb844608a4 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -427,12 +427,43 @@ parse_key(const struct smap *args, const char *name,
 }
 
 static int
+parse_tunnel_ip(const char *value, bool accept_mcast, bool *flow,
+ovs_be32 *ip, struct in6_addr *ipv6, uint16_t *protocol)
+{
+if (!strcmp(value, "flow")) {
+*flow = true;
+*protocol = ETH_TYPE_IP;
+return 0;
+}
+if (!strcmp(value, "flow6")) {
+*flow = true;
+*protocol = ETH_TYPE_IPV6;
+return 0;
+}
+if (addr_is_ipv6(value)) {
+if (lookup_ipv6(value, ipv6))
+return ENOENT;
+if (!accept_mcast && ipv6_addr_is_multicast(ipv6))
+return EINVAL;
+*protocol = ETH_TYPE_IPV6;
+} else {
+if (lookup_ip(value, (struct in_addr *)ip))
+return ENOENT;
+if (!accept_mcast && ip_is_multicast(*ip))
+return EINVAL;
+*protocol = ETH_TYPE_IP;
+}
+return 0;
+}
+
+static int
 set_tunnel_config(struct netdev *dev_, const struct smap *args)
 {
 struct netdev_vport *dev = netdev_vport_cast(dev_);
 const char *name = netdev_get_name(dev_);
 const char *type = netdev_get_type(dev_);
 bool ipsec_mech_set, needs_dst_port, has_csum;
+uint16_t dst_proto = 0, src_proto = 0;
 struct netdev_tunnel_config tnl_cfg;
 struct smap_node *node;
 
@@ -464,28 +495,28 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args)
 
 SMAP_FOR_EACH (node, args) {
 if (!strcmp(node->key, "remote_ip")) {
-struct in_addr in_addr;
-if (!strcmp(node->value, "flow")) {
-tnl_cfg.ip_dst_flow = true;
-tnl_cfg.ip_dst = htonl(0);
-} else if (lookup_ip(node->value, &in_addr)) {
+int err;
+err = parse_tunnel_ip(node->value, false, &tnl_cfg.ip_dst_flow,
+  &tnl_cfg.ip_dst, &tnl_cfg.ipv6_dst,
+  &dst_proto);
+switch (err) {
+case ENOENT:
 VLOG_WARN("%s: bad %s 'remote_ip'", name, type);
-} else if (ip_is_multicast(in_addr.s_addr)) {
-VLOG_WARN("%s: multicast remote_ip="IP_FMT" not allowed",
-  name, IP_ARGS(in_addr.s_addr));
+break;
+case EINVAL:
+VLOG_WARN("%s: multicast remote_ip=%s not allowed",
+  name, node->value);
 return EINVAL;
-} else {
-tnl_cfg.ip_dst = in_addr.s_addr;
 }
 } else if (!strcmp(node->key, "local_ip")) {
-struct in_addr in_addr;
-if (!strcmp(node->value, "flow")) {
-tnl_cfg.ip_src_flow = true;
-tnl_cfg.ip_src = htonl(0);
-} else if (lookup_ip(node->value, &in_addr)) {
+int err;
+err = parse_tunnel_ip(node->value, true, &tnl_cfg.ip_src_flow,
+  &tnl_cfg.ip_src, &tnl_cfg.ipv6_src,
+  &src_proto);
+switch (err) {
+case ENOENT:
 VLOG_WARN("%s: bad %s 'local_ip'", name, type);
-} else {
-tnl_cfg.ip_src = in_addr.s_addr;
+break;
 }
 } else if (!strcmp(node->key, "tos")) {
 if (!strcmp(node->value, "inherit")) {
@@ -603,16 +634,25 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
*args)
 }
 }
 
-if (!tnl_cfg.ip_dst && !tnl_cfg.ip_dst_flow) {
+if (!tnl_cfg.ip_dst && !ipv6_addr_is_set(&tnl_cfg.ipv6_dst) &&
+!tnl_cfg.ip_dst_flow) {