From patchwork Thu Jun 2 10:18:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 629166 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3rL39g11P4z9t8Q for ; Thu, 2 Jun 2016 20:19:18 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id C5E90106C7; Thu, 2 Jun 2016 03:19:09 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 282AF106C5 for ; Thu, 2 Jun 2016 03:19:08 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id A1A8B162323 for ; Thu, 2 Jun 2016 04:19:07 -0600 (MDT) X-ASG-Debug-ID: 1464862745-0b32373c90ff8d0001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar6.cudamail.com with ESMTP id YJCKzouQho13zdlA (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 02 Jun 2016 04:19:05 -0600 (MDT) X-Barracuda-Envelope-From: cascardo@redhat.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO mx1.redhat.com) (209.132.183.28) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 2 Jun 2016 10:19:05 -0000 Received-SPF: pass (mx1-pf2.cudamail.com: SPF record at _spf1.redhat.com designates 209.132.183.28 as permitted sender) X-Barracuda-Apparent-Source-IP: 209.132.183.28 X-Barracuda-RBL-IP: 209.132.183.28 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EF011AA0BB for ; Thu, 2 Jun 2016 10:19:02 +0000 (UTC) Received: from indiana.gru.redhat.com (ovpn-116-146.phx2.redhat.com [10.3.116.146]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u52AIxjr001864 for ; Thu, 2 Jun 2016 06:19:02 -0400 X-CudaMail-Envelope-Sender: cascardo@redhat.com From: Thadeu Lima de Souza Cascardo To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E2-601005484 X-CudaMail-DTE: 060216 X-CudaMail-Originating-IP: 209.132.183.28 Date: Thu, 2 Jun 2016 07:18:48 -0300 X-ASG-Orig-Subj: [##CM-E2-601005484##][PATCH 2/3] netdev-vport: reject concomitant incompatible tunnels Message-Id: <1464862729-31563-2-git-send-email-cascardo@redhat.com> In-Reply-To: <1464862729-31563-1-git-send-email-cascardo@redhat.com> References: <1464862729-31563-1-git-send-email-cascardo@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 02 Jun 2016 10:19:02 +0000 (UTC) X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1464862745 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH 2/3] netdev-vport: reject concomitant incompatible tunnels X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" If VXLAN-GBP and VXLAN are set on the same port, only one of those tunnel types is going to be created. As other tunnel flags are added, like VXLAN-GPE, and they are incompatible, reject any configuration that put incompatible tunnels on the same port. A manual test has been done as well and the port didn't get an ofport. During a restart, the other tunnel type was rejected and the right flags were on the dpif_port. Signed-off-by: Thadeu Lima de Souza Cascardo --- lib/netdev-vport.c | 78 ++++++++++++++++++++++++++++++++++++++++++------------ tests/tunnel.at | 29 ++++++++++++++++++++ 2 files changed, 90 insertions(+), 17 deletions(-) diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 6016fd8..aaf6bfc 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -38,6 +38,7 @@ #include "packets.h" #include "poll-loop.h" #include "route-table.h" +#include "simap.h" #include "smap.h" #include "socket-util.h" #include "unaligned.h" @@ -63,6 +64,7 @@ static bool tunnel_check_status_change__(struct netdev_vport *); struct vport_class { const char *dpif_port; struct netdev_class netdev_class; + struct simap dst_port_to_exts; }; bool @@ -71,7 +73,7 @@ netdev_vport_is_vport_class(const struct netdev_class *class) return is_vport_class(class); } -static const struct vport_class * +static struct vport_class * vport_class_cast(const struct netdev_class *class) { ovs_assert(is_vport_class(class)); @@ -404,6 +406,36 @@ parse_tunnel_ip(const char *value, bool accept_mcast, bool *flow, return 0; } +static bool +netdev_vport_may_add(struct netdev *netdev) +{ + char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; + const char *name; + struct simap_node *node; + struct vport_class *vclass; + const struct netdev_vport *vport; + const struct netdev_class *class = netdev_get_class(netdev); + + if (!is_vport_class(class)) { + return false; + } + + if (!netdev_vport_needs_dst_port(netdev)) { + return true; + } + + vport = netdev_vport_cast(netdev); + vclass = vport_class_cast(class); + name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); + + node = simap_find(&vclass->dst_port_to_exts, name); + if (!node) { + simap_put(&vclass->dst_port_to_exts, name, vport->tnl_cfg.exts); + return true; + } + return node->data == vport->tnl_cfg.exts; +} + static int set_tunnel_config(struct netdev *dev_, const struct smap *args) { @@ -615,6 +647,10 @@ set_tunnel_config(struct netdev *dev_, const struct smap *args) } ovs_mutex_unlock(&dev->mutex); + if (!netdev_vport_may_add(dev_)) { + return EEXIST; + } + return 0; } @@ -883,33 +919,40 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats) NULL, /* rx_drain */ -#define TUNNEL_CLASS(NAME, DPIF_PORT, BUILD_HEADER, PUSH_HEADER, POP_HEADER) \ - { DPIF_PORT, \ - { NAME, false, \ - VPORT_FUNCTIONS(get_tunnel_config, \ - set_tunnel_config, \ - get_netdev_tunnel_config, \ - tunnel_get_status, \ - BUILD_HEADER, PUSH_HEADER, POP_HEADER) }} +#define TUNNEL_CLASS(TUNNEL, NAME, DPIF_PORT, BUILD_HEADER, PUSH_HEADER, POP_HEADER) \ + { DPIF_PORT, \ + { NAME, false, \ + VPORT_FUNCTIONS(get_tunnel_config, \ + set_tunnel_config, \ + get_netdev_tunnel_config, \ + tunnel_get_status, \ + BUILD_HEADER, PUSH_HEADER, POP_HEADER) }, \ + SIMAP_INITIALIZER(&(TUNNEL)->dst_port_to_exts) } void netdev_vport_tunnel_register(void) { /* The name of the dpif_port should be short enough to accomodate adding * a port number to the end if one is necessary. */ - static const struct vport_class vport_classes[] = { - TUNNEL_CLASS("geneve", "genev_sys", netdev_geneve_build_header, + static struct vport_class vport_classes[] = { + TUNNEL_CLASS(&(vport_classes[0]), "geneve", "genev_sys", + netdev_geneve_build_header, netdev_tnl_push_udp_header, netdev_geneve_pop_header), - TUNNEL_CLASS("gre", "gre_sys", netdev_gre_build_header, + TUNNEL_CLASS(&(vport_classes[1]), "gre", "gre_sys", + netdev_gre_build_header, netdev_gre_push_header, netdev_gre_pop_header), - TUNNEL_CLASS("ipsec_gre", "gre_sys", NULL, NULL, NULL), - TUNNEL_CLASS("vxlan", "vxlan_sys", netdev_vxlan_build_header, + TUNNEL_CLASS(&(vport_classes[2]), "ipsec_gre", "gre_sys", + NULL, NULL, NULL), + TUNNEL_CLASS(&(vport_classes[3]),"vxlan", "vxlan_sys", + netdev_vxlan_build_header, netdev_tnl_push_udp_header, netdev_vxlan_pop_header), - TUNNEL_CLASS("lisp", "lisp_sys", NULL, NULL, NULL), - TUNNEL_CLASS("stt", "stt_sys", NULL, NULL, NULL), + TUNNEL_CLASS(&(vport_classes[4]), "lisp", "lisp_sys", + NULL, NULL, NULL), + TUNNEL_CLASS(&(vport_classes[5]), "stt", "stt_sys", + NULL, NULL, NULL), }; static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; @@ -936,6 +979,7 @@ netdev_vport_patch_register(void) VPORT_FUNCTIONS(get_patch_config, set_patch_config, NULL, - NULL, NULL, NULL, NULL) }}; + NULL, NULL, NULL, NULL)}, + SIMAP_INITIALIZER(&(&patch_class)->dst_port_to_exts)}; netdev_register_provider(&patch_class.netdev_class); } diff --git a/tests/tunnel.at b/tests/tunnel.at index 7f82785..97af317 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -548,6 +548,35 @@ AT_CHECK([tail -1 stdout], [0], [Datapath actions: set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,flags(df|key))),4789 ]) +OVS_VSWITCHD_STOP +AT_CLEANUP + +AT_SETUP([tunnel - concomitant incompatible tunnels on the same port]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \ + options:remote_ip=flow ofport_request=1]) + +AT_CHECK([ovs-vsctl add-port br0 p2 -- set Interface p2 type=vxlan \ + options:remote_ip=flow options:exts=gbp ofport_request=2], [0], + [], [ovs-vsctl: Error detected while setting up 'p2'. See ovs-vswitchd log for details. +]) + +AT_CHECK([grep p2 ovs-vswitchd.log | sed "s/^.*\(p2:.*\)$/\1/"], [0], + [p2: could not set configuration (File exists) +]) + +OVS_VSWITCHD_STOP(["/could not set configuration/d"]) +AT_CLEANUP + +AT_SETUP([tunnel - concomitant incompatible tunnels on different ports]) +OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \ + options:remote_ip=flow ofport_request=1]) + +AT_CHECK([ovs-vsctl add-port br0 p2 -- set Interface p2 type=vxlan options:dst_port=9000 \ + options:remote_ip=flow options:exts=gbp ofport_request=2]) + +AT_CHECK([grep p2 ovs-vswitchd.log | sed "s/^.*\(bridge br0:.*\)$/\1/"], [0], + [bridge br0: added interface p2 on port 2 +]) OVS_VSWITCHD_STOP AT_CLEANUP