On Sat, Aug 06, 2016 at 04:55:15AM +0800, Binbin Xu wrote:
> If a kernel space vxlan port was added first, and then we try to
> add a user space vxlan port. But unfortunate, the user space
> vxlan port can't be created.
>
> This commit separates kernel space with user space tunnel port,
> for example:
> kernel_space user_space
> vxlan vxlan_sys_4789 vxlan_usr_4789
> gre gre_sys gre_usr
> ......
>
This makes sense to me. You point out a real problem, and both tunnel types
should be able to coexist.
However, I am only guessing why this would fail. Can you point out some of the
details on why this fails?
Thanks.
Cascardo.
> Signed-off-by: Binbin Xu <xu.binb...@zte.com.cn>
> ---
> lib/dpif-netdev.c | 3 ++-
> lib/dpif-netlink.c | 2 +-
> lib/netdev-vport.c | 26 +++++++++++++++-----------
> lib/netdev-vport.h | 2 +-
> lib/netdev.c | 13 ++++++++++---
> lib/netdev.h | 2 +-
> ofproto/ofproto-dpif.c | 15 ++++++++++-----
> vswitchd/bridge.c | 2 +-
> 8 files changed, 41 insertions(+), 24 deletions(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index e39362e..3bdf204 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1343,7 +1343,8 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev
> *netdev,
> int error;
>
> ovs_mutex_lock(&dp->port_mutex);
> - dpif_port = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
> + dpif_port = netdev_vport_get_dpif_port(netdev, "netdev",
> + namebuf, sizeof namebuf);
> if (*port_nop != ODPP_NONE) {
> port_no = *port_nop;
> error = dp_netdev_lookup_port(dp, *port_nop) ? EBUSY : 0;
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index a39faa2..1b0cd2c 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -810,7 +810,7 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, struct
> netdev *netdev,
> {
> const struct netdev_tunnel_config *tnl_cfg;
> char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> - const char *name = netdev_vport_get_dpif_port(netdev,
> + const char *name = netdev_vport_get_dpif_port(netdev, "netlink",
> namebuf, sizeof namebuf);
> const char *type = netdev_get_type(netdev);
> struct dpif_netlink_vport request, reply;
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index f5eb76f..e374f9d 100755
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -119,16 +119,19 @@ netdev_vport_class_get_dpif_port(const struct
> netdev_class *class)
> }
>
> const char *
> -netdev_vport_get_dpif_port(const struct netdev *netdev,
> +netdev_vport_get_dpif_port(const struct netdev *netdev, const char *dp_type,
> char namebuf[], size_t bufsize)
> {
> const struct netdev_class *class = netdev_get_class(netdev);
> const char *dpif_port = netdev_vport_class_get_dpif_port(class);
> + char *type;
>
> if (!dpif_port) {
> return netdev_get_name(netdev);
> }
>
> + type = !strcmp(dp_type, "netdev") ? "usr" : "sys";
> +
> if (netdev_vport_needs_dst_port(netdev)) {
> const struct netdev_vport *vport = netdev_vport_cast(netdev);
>
> @@ -138,13 +141,14 @@ netdev_vport_get_dpif_port(const struct netdev *netdev,
> * port numbers but assert just in case.
> */
> BUILD_ASSERT(NETDEV_VPORT_NAME_BUFSIZE >= IFNAMSIZ);
> - ovs_assert(strlen(dpif_port) + 6 < IFNAMSIZ);
> - snprintf(namebuf, bufsize, "%s_%d", dpif_port,
> + ovs_assert(strlen(dpif_port) + 10 < IFNAMSIZ);
> + snprintf(namebuf, bufsize, "%s_%s_%d", dpif_port, type,
> ntohs(vport->tnl_cfg.dst_port));
> - return namebuf;
> } else {
> - return dpif_port;
> + snprintf(namebuf, NETDEV_VPORT_NAME_BUFSIZE, "%s_%s",
> + dpif_port, type);
> }
> + return namebuf;
> }
>
> /* Whenever the route-table change number is incremented,
> @@ -890,18 +894,18 @@ 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,
> + TUNNEL_CLASS("geneve", "genev", netdev_geneve_build_header,
> netdev_tnl_push_udp_header,
> netdev_geneve_pop_header),
> - TUNNEL_CLASS("gre", "gre_sys", netdev_gre_build_header,
> + TUNNEL_CLASS("gre", "gre", 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("ipsec_gre", "gre", NULL, NULL, NULL),
> + TUNNEL_CLASS("vxlan", "vxlan", 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("lisp", "lisp", NULL, NULL, NULL),
> + TUNNEL_CLASS("stt", "stt", NULL, NULL, NULL),
> };
> static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
>
> diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
> index 048aa6e..d4c168e 100755
> --- a/lib/netdev-vport.h
> +++ b/lib/netdev-vport.h
> @@ -48,7 +48,7 @@ enum { NETDEV_VPORT_NAME_BUFSIZE = 16 };
> #else
> enum { NETDEV_VPORT_NAME_BUFSIZE = 256 };
> #endif
> -const char *netdev_vport_get_dpif_port(const struct netdev *,
> +const char *netdev_vport_get_dpif_port(const struct netdev *, const char *,
> char namebuf[], size_t bufsize)
> OVS_WARN_UNUSED_RESULT;
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 75bf1cb..268185e 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -287,16 +287,23 @@ netdev_enumerate_types(struct sset *types)
> *
> * Returns true if there is a name conflict, false otherwise. */
> bool
> -netdev_is_reserved_name(const char *name)
> +netdev_is_reserved_name(const char *name, const char *dp_type)
> OVS_EXCLUDED(netdev_mutex)
> {
> + char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
> + char *type;
> +
> netdev_initialize();
>
> struct netdev_registered_class *rc;
> CMAP_FOR_EACH (rc, cmap_node, &netdev_classes) {
> const char *dpif_port = netdev_vport_class_get_dpif_port(rc->class);
> - if (dpif_port && !strncmp(name, dpif_port, strlen(dpif_port))) {
> - return true;
> + if (dpif_port) {
> + type = !strcmp(dp_type, "netdev") ? "usr" : "sys";
> + snprintf(namebuf, sizeof namebuf, "%s_%s", dpif_port, type);
> + if(!strncmp(name, namebuf, strlen(namebuf))) {
> + return true;
> + }
> }
> }
>
> diff --git a/lib/netdev.h b/lib/netdev.h
> index dc7ede8..26e9d1d 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -105,7 +105,7 @@ void netdev_run(void);
> void netdev_wait(void);
>
> void netdev_enumerate_types(struct sset *types);
> -bool netdev_is_reserved_name(const char *name);
> +bool netdev_is_reserved_name(const char *name, const char *dp_type);
>
> int netdev_n_txq(const struct netdev *netdev);
> int netdev_n_rxq(const struct netdev *netdev);
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index fd2c2bd..9d2f2b1 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -552,6 +552,7 @@ type_run(const char *type)
> }
>
> dp_port = netdev_vport_get_dpif_port(iter->up.netdev,
> + ofproto->up.type,
> namebuf, sizeof
> namebuf);
> node = simap_find(&tmp_backers, dp_port);
> if (node) {
> @@ -1771,7 +1772,8 @@ port_construct(struct ofport *port_)
> return 0;
> }
>
> - dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof
> namebuf);
> + dp_port_name = netdev_vport_get_dpif_port(netdev, port_->ofproto->type,
> + namebuf, sizeof namebuf);
> error = dpif_port_query_by_name(ofproto->backer->dpif, dp_port_name,
> &dpif_port);
> if (error) {
> @@ -1832,8 +1834,9 @@ port_destruct(struct ofport *port_, bool del)
> xlate_ofport_remove(port);
> xlate_txn_commit();
>
> - dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
> - sizeof namebuf);
> + dp_port_name = netdev_vport_get_dpif_port(port->up.netdev,
> + port_->ofproto->type,
> + namebuf, sizeof namebuf);
> if (del && dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
> /* The underlying device is still there, so delete it. This
> * happens when the ofproto is being destroyed, since the caller
> @@ -1904,7 +1907,8 @@ port_modified(struct ofport *port_)
> ofproto_dpif_monitor_port_update(port, port->bfd, port->cfm,
> port->lldp, &port->up.pp.hw_addr);
>
> - dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof
> namebuf);
> + dp_port_name = netdev_vport_get_dpif_port(netdev, port_->ofproto->type,
> + namebuf, sizeof namebuf);
>
> if (port->is_tunnel) {
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
> @@ -3485,7 +3489,8 @@ port_add(struct ofproto *ofproto_, struct netdev
> *netdev)
> return 0;
> }
>
> - dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof
> namebuf);
> + dp_port_name = netdev_vport_get_dpif_port(netdev, ofproto_->type,
> + namebuf, sizeof namebuf);
> if (!dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
> odp_port_t port_no = ODPP_NONE;
> int error;
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ddf1fe5..7961784 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1744,7 +1744,7 @@ iface_do_create(const struct bridge *br,
> int error;
> const char *type;
>
> - if (netdev_is_reserved_name(iface_cfg->name)) {
> + if (netdev_is_reserved_name(iface_cfg->name, br->type)) {
> VLOG_WARN("could not create interface %s, name is reserved",
> iface_cfg->name);
> error = EINVAL;
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
