[ovs-dev] [PATCH] netdev: Prevent using reserved names
Ben Pfaff
blp at nicira.com
Thu May 16 21:04:08 UTC 2013
On Thu, May 16, 2013 at 02:11:51PM -0700, Alex Wang wrote:
> This commit adds a function to lib/netdev.c to check that the interface name
> is not the same as any of the registered vport providers' dpif_port name
> (e.g. gre_system) or the datapath's internal port name (e.g. ovs-system).
>
> Bug #15077
>
> Signed-off-by: Alex Wang <alexw at nicira.com>
Thanks!
I folded in the following changes which fix minor style complaints and
Jesse's comments. I'll apply this to master soon.
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index a5bf79d..38fc996 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -117,8 +117,7 @@ netdev_vport_needs_dst_port(const struct netdev *dev)
const char *
netdev_vport_class_get_dpif_port(const struct netdev_class *class)
{
- return is_vport_class(class)
- ? vport_class_cast(class)->dpif_port : NULL;
+ return is_vport_class(class) ? vport_class_cast(class)->dpif_port : NULL;
}
const char *
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index b98b83d..94fe937 100644
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2011 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -39,4 +39,5 @@ void netdev_vport_inc_tx(const struct netdev *,
const char *netdev_vport_get_dpif_port(const struct netdev *);
const char *netdev_vport_class_get_dpif_port(const struct netdev_class *);
+
#endif /* netdev-vport.h */
diff --git a/lib/netdev.c b/lib/netdev.c
index 74adba2..5c2e9f5 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -205,18 +205,14 @@ netdev_enumerate_types(struct sset *types)
* vport providers' dpif_port name (dpif_port is NULL if the vport provider
* does not define it) or the datapath internal port name (e.g. ovs-system).
*
- * false is returned if there is no naming conflict.
- * true is returned otherwise.
- */
+ * Returns true if there is a name conflict, false otherwise. */
bool
netdev_is_reserved_name(const char *name)
{
struct shash_node *node;
- struct sset types;
- const char *type;
netdev_initialize();
- SHASH_FOR_EACH(node, &netdev_classes) {
+ SHASH_FOR_EACH (node, &netdev_classes) {
const char *dpif_port;
dpif_port = netdev_vport_class_get_dpif_port(node->data);
if (dpif_port && !strcmp(dpif_port, name)) {
@@ -225,6 +221,9 @@ netdev_is_reserved_name(const char *name)
}
if (!strncmp(name, "ovs-", 4)) {
+ struct sset types;
+ const char *type;
+
sset_init(&types);
dp_enumerate_types(&types);
SSET_FOR_EACH (type, &types) {
@@ -235,6 +234,7 @@ netdev_is_reserved_name(const char *name)
}
sset_destroy(&types);
}
+
return false;
}
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index ffc4057..df28df3 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -30,10 +30,6 @@
#include "tunnel.h"
#include "vlog.h"
-/* XXX:
- *
- * Disallow netdevs with names like "gre64_system" to prevent collisions. */
-
VLOG_DEFINE_THIS_MODULE(tunnel);
struct tnl_match {
----------------------------------------------------------------------
Here's the full commit that I will apply:
--8<--------------------------cut here-------------------------->8--
From: Alex Wang <alexw at nicira.com>
Date: Thu, 16 May 2013 14:11:51 -0700
Subject: [PATCH] netdev: Prevent using reserved names
This commit adds a function to lib/netdev.c to check that the interface name
is not the same as any of the registered vport providers' dpif_port name
(e.g. gre_system) or the datapath's internal port name (e.g. ovs-system).
Bug #15077.
Signed-off-by: Alex Wang <alexw at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
lib/netdev-vport.c | 10 ++++++--
lib/netdev-vport.h | 4 ++-
lib/netdev.c | 38 +++++++++++++++++++++++++++++++
lib/netdev.h | 1 +
ofproto/tunnel.c | 4 ---
tests/ovs-vsctl.at | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
vswitchd/bridge.c | 7 ++++++
7 files changed, 118 insertions(+), 8 deletions(-)
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 699ed71..38fc996 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -115,6 +115,12 @@ netdev_vport_needs_dst_port(const struct netdev *dev)
}
const char *
+netdev_vport_class_get_dpif_port(const struct netdev_class *class)
+{
+ return is_vport_class(class) ? vport_class_cast(class)->dpif_port : NULL;
+}
+
+const char *
netdev_vport_get_dpif_port(const struct netdev *netdev)
{
const char *dpif_port;
@@ -136,9 +142,7 @@ netdev_vport_get_dpif_port(const struct netdev *netdev)
return dpif_port_combined;
} else {
const struct netdev_class *class = netdev_get_class(netdev);
- dpif_port = (is_vport_class(class)
- ? vport_class_cast(class)->dpif_port
- : NULL);
+ dpif_port = netdev_vport_class_get_dpif_port(class);
}
return dpif_port ? dpif_port : netdev_get_name(netdev);
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index c907b0c..94fe937 100644
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2011 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
struct dpif_linux_vport;
struct dpif_flow_stats;
struct netdev;
+struct netdev_class;
struct netdev_stats;
void netdev_vport_tunnel_register(void);
@@ -37,5 +38,6 @@ void netdev_vport_inc_tx(const struct netdev *,
const struct dpif_flow_stats *);
const char *netdev_vport_get_dpif_port(const struct netdev *);
+const char *netdev_vport_class_get_dpif_port(const struct netdev_class *);
#endif /* netdev-vport.h */
diff --git a/lib/netdev.c b/lib/netdev.c
index d93526a..5c2e9f5 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -25,6 +25,7 @@
#include <unistd.h>
#include "coverage.h"
+#include "dpif.h"
#include "dynamic-string.h"
#include "fatal-signal.h"
#include "hash.h"
@@ -200,6 +201,43 @@ netdev_enumerate_types(struct sset *types)
}
}
+/* Check that the network device name is not the same as any of the registered
+ * vport providers' dpif_port name (dpif_port is NULL if the vport provider
+ * does not define it) or the datapath internal port name (e.g. ovs-system).
+ *
+ * Returns true if there is a name conflict, false otherwise. */
+bool
+netdev_is_reserved_name(const char *name)
+{
+ struct shash_node *node;
+
+ netdev_initialize();
+ SHASH_FOR_EACH (node, &netdev_classes) {
+ const char *dpif_port;
+ dpif_port = netdev_vport_class_get_dpif_port(node->data);
+ if (dpif_port && !strcmp(dpif_port, name)) {
+ return true;
+ }
+ }
+
+ if (!strncmp(name, "ovs-", 4)) {
+ struct sset types;
+ const char *type;
+
+ sset_init(&types);
+ dp_enumerate_types(&types);
+ SSET_FOR_EACH (type, &types) {
+ if (!strcmp(name+4, type)) {
+ sset_destroy(&types);
+ return true;
+ }
+ }
+ sset_destroy(&types);
+ }
+
+ return false;
+}
+
/* Opens the network device named 'name' (e.g. "eth0") of the specified 'type'
* (e.g. "system") and returns zero if successful, otherwise a positive errno
* value. On success, sets '*netdevp' to the new network device, otherwise to
diff --git a/lib/netdev.h b/lib/netdev.h
index 852b75d..c7f3c1d 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -106,6 +106,7 @@ void netdev_run(void);
void netdev_wait(void);
void netdev_enumerate_types(struct sset *types);
+bool netdev_is_reserved_name(const char *name);
/* Open and close. */
int netdev_open(const char *name, const char *type, struct netdev **);
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index ffc4057..df28df3 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -30,10 +30,6 @@
#include "tunnel.h"
#include "vlog.h"
-/* XXX:
- *
- * Disallow netdevs with names like "gre64_system" to prevent collisions. */
-
VLOG_DEFINE_THIS_MODULE(tunnel);
struct tnl_match {
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 56d9bae..ec08cd4 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1185,3 +1185,65 @@ AT_CHECK([RUN_OVS_VSCTL(
[-- list Queue])], [0], [], [], [OVS_VSCTL_CLEANUP])
OVS_VSCTL_CLEANUP
AT_CLEANUP
+
+dnl ----------------------------------------------------------------------
+AT_BANNER([ovs-vsctl add-port -- reserved port names])
+
+AT_SETUP([add-port -- reserved names 1])
+OVS_VSWITCHD_START
+
+# Test creating all reserved port names
+m4_foreach(
+[reserved_name],
+[[ovs-netdev],
+[ovs-dummy],
+[gre_system],
+[gre64_system],
+[lisp_system],
+[vxlan_system]],
+[
+# Try creating the port
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [])
+# Detect the warning log message
+AT_CHECK([sed -n "s/^.*\(|bridge|WARN|.*\)$/\1/p" ovs-vswitchd.log], [0], [dnl
+|bridge|WARN|could not create interface reserved_name, name is reserved
+])
+# Delete the warning log message
+AT_CHECK([sed "/|bridge|WARN|/d" ovs-vswitchd.log > ovs-vswitchd.log], [0], [], [])
+# Delete the port
+AT_CHECK([ovs-vsctl del-port br0 reserved_name], [0], [], [])])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([add-port -- reserved names 2])
+# Creates all type of tunnel ports
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=gre \
+ options:remote_ip=1.1.1.1 ofport_request=1\
+ -- add-port br0 p2 -- set Interface p2 type=gre64 \
+ options:local_ip=2.2.2.2 options:remote_ip=1.1.1.1 \
+ ofport_request=2 \
+ -- add-port br0 p3 -- set Interface p3 type=lisp \
+ options:remote_ip=2.2.2.2 ofport_request=3 \
+ -- add-port br0 p4 -- set Interface p4 type=vxlan \
+ options:remote_ip=2.2.2.2 ofport_request=4])
+
+# Test creating all reserved tunnel port names
+m4_foreach(
+[reserved_name],
+[[gre_system],
+[gre64_system],
+[lisp_system],
+[vxlan_system]],
+[
+# Try creating the port
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [])
+# Detect the warning log message
+AT_CHECK([sed -n "s/^.*\(|bridge|WARN|.*\)$/\1/p" ovs-vswitchd.log], [0], [dnl
+|bridge|WARN|could not create interface reserved_name, name is reserved
+])
+# Delete the warning log message
+AT_CHECK([sed "/|bridge|WARN|/d" ovs-vswitchd.log > ovs-vswitchd.log], [0], [], [])
+# Delete the port
+AT_CHECK([ovs-vsctl del-port br0 reserved_name], [0], [], [])])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
\ No newline at end of file
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index cf26e87..28e306e 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1418,6 +1418,13 @@ iface_do_create(const struct bridge *br,
struct netdev *netdev;
int error;
+ if (netdev_is_reserved_name(iface_cfg->name)) {
+ VLOG_WARN("could not create interface %s, name is reserved",
+ iface_cfg->name);
+ error = EINVAL;
+ goto error;
+ }
+
error = netdev_open(iface_cfg->name,
iface_get_type(iface_cfg, br->cfg), &netdev);
if (error) {
--
1.7.2.5
More information about the dev
mailing list