diff mbox series

[ovs-dev,v4] netdev-vport: Use the dst_port in tunnel netdev name

Message ID 20190413080937.32478-1-chrism@mellanox.com
State Accepted
Commit 6998788197e23c409a6b6cecaa30867ff6d40928
Headers show
Series [ovs-dev,v4] netdev-vport: Use the dst_port in tunnel netdev name | expand

Commit Message

Chris Mi April 13, 2019, 8:09 a.m. UTC 
If tunnel device dst_port is not the default one, "ovs-dpctl dump-flows"
will fail. The error message for vxlan is:

netdev_linux|INFO|ioctl(SIOCGIFINDEX) on vxlan_sys_4789 device failed: No such device

That's because when calling netdev_vport_construct() for netdev
vxlan_sys_xxxx, the default dst_port is used. Actually, the dst_port
value is in the netdev name. Use it to avoid the error.

Signed-off-by: Chris Mi <chrism@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---

v1
==

Any comment about this patch? We are not sure if it is correct
to verify the port from the name. If it is correct, is it applicable
for other tunnels? Thanks!

v2
==

Apply the same fix to other tunnel types according to Flavio Leitner's
comment.

v3
==

Addressed Ben Pfaff's comment to deal with the string correctly.

v4
==

Fix errors reported by AddressSanitizer.

 lib/netdev-vport.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

Comments

Ben Pfaff April 15, 2019, 7:47 p.m. UTC | #1 
On Sat, Apr 13, 2019 at 04:09:37PM +0800, Chris Mi wrote:
> If tunnel device dst_port is not the default one, "ovs-dpctl dump-flows"
> will fail. The error message for vxlan is:
> 
> netdev_linux|INFO|ioctl(SIOCGIFINDEX) on vxlan_sys_4789 device failed: No such device
> 
> That's because when calling netdev_vport_construct() for netdev
> vxlan_sys_xxxx, the default dst_port is used. Actually, the dst_port
> value is in the netdev name. Use it to avoid the error.
> 
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>

Thanks, applied to master.
diff mbox series

Patch

diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 808a43f99..ab591667f 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -189,22 +189,34 @@  netdev_vport_alloc(void)
 int
 netdev_vport_construct(struct netdev *netdev_)
 {
+    const struct netdev_class *class = netdev_get_class(netdev_);
+    const char *dpif_port = netdev_vport_class_get_dpif_port(class);
     struct netdev_vport *dev = netdev_vport_cast(netdev_);
+    const char *p, *name = netdev_get_name(netdev_);
     const char *type = netdev_get_type(netdev_);
+    uint16_t port = 0;
 
     ovs_mutex_init(&dev->mutex);
     eth_addr_random(&dev->etheraddr);
 
-    /* Add a default destination port for tunnel ports if none specified. */
+    if (name && dpif_port && (strlen(name) > strlen(dpif_port) + 1) &&
+        (!strncmp(name, dpif_port, strlen(dpif_port)))) {
+        p = name + strlen(dpif_port) + 1;
+        port = atoi(p);
+    }
+
+    /* If a destination port for tunnel ports is specified in the netdev
+     * name, use it instead of the default one. Otherwise, use the default
+     * destination port */
     if (!strcmp(type, "geneve")) {
-        dev->tnl_cfg.dst_port = htons(GENEVE_DST_PORT);
+        dev->tnl_cfg.dst_port = port ? htons(port) : htons(GENEVE_DST_PORT);
     } else if (!strcmp(type, "vxlan")) {
-        dev->tnl_cfg.dst_port = htons(VXLAN_DST_PORT);
+        dev->tnl_cfg.dst_port = port ? htons(port) : htons(VXLAN_DST_PORT);
         update_vxlan_global_cfg(netdev_, NULL, &dev->tnl_cfg);
     } else if (!strcmp(type, "lisp")) {
-        dev->tnl_cfg.dst_port = htons(LISP_DST_PORT);
+        dev->tnl_cfg.dst_port = port ? htons(port) : htons(LISP_DST_PORT);
     } else if (!strcmp(type, "stt")) {
-        dev->tnl_cfg.dst_port = htons(STT_DST_PORT);
+        dev->tnl_cfg.dst_port = port ? htons(port) : htons(STT_DST_PORT);
     }
 
     dev->tnl_cfg.dont_fragment = true;