From c283069c71adc49c182a1ac569a05e2dca949eda Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sun, 23 Jan 2011 20:01:30 -0800 Subject: [PATCH] datapath: Change vport type from string to integer enumeration. I plan to make the vport type part of the standard header stuck on each Netlink message related to a vport. As such, it is more convenient to use an integer than a string. In addition, by being fundamentally different from strings, using an integer may reduce the confusion we've had in the past over the differences in userspace and kernel names for network device and vport types. Signed-off-by: Ben Pfaff Acked-by: Jesse Gross --- datapath/datapath.c | 5 +- datapath/vport-capwap.c | 4 +- datapath/vport-gre.c | 4 +- datapath/vport-internal_dev.c | 4 +- datapath/vport-netdev.c | 2 +- datapath/vport-patch.c | 2 +- datapath/vport.c | 6 +- datapath/vport.h | 9 ++- include/openvswitch/datapath-protocol.h | 15 ++++- lib/dpif-linux.c | 75 ++++++++++++++++--------- lib/netdev-vport.c | 18 ++++-- 11 files changed, 91 insertions(+), 53 deletions(-) diff --git a/datapath/datapath.c b/datapath/datapath.c index 3f108c09..baf7940b 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -266,7 +266,7 @@ static int create_dp(int dp_idx, const char __user *devnamep) /* Set up our datapath device. */ BUILD_BUG_ON(sizeof(internal_dev_port.devname) != sizeof(devname)); strcpy(internal_dev_port.devname, devname); - strcpy(internal_dev_port.type, "internal"); + internal_dev_port.type = ODP_VPORT_TYPE_INTERNAL; err = new_vport(dp, &internal_dev_port, ODPP_LOCAL); if (err) { if (err == -EBUSY) @@ -393,7 +393,6 @@ static int attach_port(int dp_idx, struct odp_port __user *portp) if (copy_from_user(&port, portp, sizeof(port))) goto out; port.devname[IFNAMSIZ - 1] = '\0'; - port.type[VPORT_TYPE_SIZE - 1] = '\0'; rtnl_lock(); dp = get_dp_locked(dp_idx); @@ -1353,7 +1352,7 @@ static void compose_odp_port(const struct vport *vport, struct odp_port *odp_por { rcu_read_lock(); strncpy(odp_port->devname, vport_get_name(vport), sizeof(odp_port->devname)); - strncpy(odp_port->type, vport_get_type(vport), sizeof(odp_port->type)); + odp_port->type = vport_get_type(vport); vport_get_config(vport, odp_port->config); odp_port->port = vport->port_no; odp_port->dp_idx = vport->dp->dp_idx; diff --git a/datapath/vport-capwap.c b/datapath/vport-capwap.c index fc2ea8e5..a674b614 100644 --- a/datapath/vport-capwap.c +++ b/datapath/vport-capwap.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 Nicira Networks. + * Copyright (c) 2010, 2011 Nicira Networks. * Distributed under the terms of the GNU GPL version 2. * * Significant portions of this file may be copied from parts of the Linux @@ -645,7 +645,7 @@ static void capwap_frag_expire(unsigned long ifq) } const struct vport_ops capwap_vport_ops = { - .type = "capwap", + .type = ODP_VPORT_TYPE_CAPWAP, .flags = VPORT_F_GEN_STATS, .init = capwap_init, .exit = capwap_exit, diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c index bf8179b2..426b6cec 100644 --- a/datapath/vport-gre.c +++ b/datapath/vport-gre.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010 Nicira Networks. + * Copyright (c) 2010, 2011 Nicira Networks. * Distributed under the terms of the GNU GPL version 2. * * Significant portions of this file may be copied from parts of the Linux @@ -387,7 +387,7 @@ static void gre_exit(void) } const struct vport_ops gre_vport_ops = { - .type = "gre", + .type = ODP_VPORT_TYPE_GRE, .flags = VPORT_F_GEN_STATS | VPORT_F_TUN_ID, .init = gre_init, .exit = gre_exit, diff --git a/datapath/vport-internal_dev.c b/datapath/vport-internal_dev.c index b9e54ab1..b5406c73 100644 --- a/datapath/vport-internal_dev.c +++ b/datapath/vport-internal_dev.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010 Nicira Networks. + * Copyright (c) 2009, 2010, 2011 Nicira Networks. * Distributed under the terms of the GNU GPL version 2. * * Significant portions of this file may be copied from parts of the Linux @@ -245,7 +245,7 @@ static int internal_dev_recv(struct vport *vport, struct sk_buff *skb) } const struct vport_ops internal_vport_ops = { - .type = "internal", + .type = ODP_VPORT_TYPE_INTERNAL, .flags = VPORT_F_REQUIRED | VPORT_F_GEN_STATS | VPORT_F_FLOW, .create = internal_dev_create, .destroy = internal_dev_destroy, diff --git a/datapath/vport-netdev.c b/datapath/vport-netdev.c index 363651bd..e45e22fb 100644 --- a/datapath/vport-netdev.c +++ b/datapath/vport-netdev.c @@ -286,7 +286,7 @@ struct vport *netdev_get_vport(struct net_device *dev) } const struct vport_ops netdev_vport_ops = { - .type = "netdev", + .type = ODP_VPORT_TYPE_NETDEV, .flags = (VPORT_F_REQUIRED | (USE_VPORT_STATS ? VPORT_F_GEN_STATS : 0)), .init = netdev_init, diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c index 74d10f6d..1c32c5a4 100644 --- a/datapath/vport-patch.c +++ b/datapath/vport-patch.c @@ -297,7 +297,7 @@ static int patch_send(struct vport *vport, struct sk_buff *skb) } const struct vport_ops patch_vport_ops = { - .type = "patch", + .type = ODP_VPORT_TYPE_PATCH, .flags = VPORT_F_GEN_STATS, .init = patch_init, .exit = patch_exit, diff --git a/datapath/vport.c b/datapath/vport.c index acb480bb..2d18f285 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -569,7 +569,7 @@ struct vport *vport_add(const struct vport_parms *parms) ASSERT_VPORT(); for (i = 0; i < n_vport_types; i++) { - if (!strcmp(vport_ops_list[i]->type, parms->type)) { + if (vport_ops_list[i]->type == parms->type) { vport = vport_ops_list[i]->create(parms); if (IS_ERR(vport)) { err = PTR_ERR(vport); @@ -722,9 +722,9 @@ const char *vport_get_name(const struct vport *vport) * @vport: vport from which to retrieve the type. * * Retrieves the type of the given device. Either RTNL lock or rcu_read_lock - * must be held for the entire duration that the type is in use. + * must be held. */ -const char *vport_get_type(const struct vport *vport) +enum odp_vport_type vport_get_type(const struct vport *vport) { return vport->ops->type; } diff --git a/datapath/vport.h b/datapath/vport.h index 5f6e7452..5e75f565 100644 --- a/datapath/vport.h +++ b/datapath/vport.h @@ -49,7 +49,7 @@ int vport_set_addr(struct vport *, const unsigned char *); int vport_set_stats(struct vport *, struct rtnl_link_stats64 *); const char *vport_get_name(const struct vport *); -const char *vport_get_type(const struct vport *); +enum odp_vport_type vport_get_type(const struct vport *); const unsigned char *vport_get_addr(const struct vport *); struct kobject *vport_get_kobj(const struct vport *); @@ -140,7 +140,7 @@ struct vport { */ struct vport_parms { const char *name; - const char *type; + enum odp_vport_type type; const void *config; /* For vport_alloc(). */ @@ -151,8 +151,7 @@ struct vport_parms { /** * struct vport_ops - definition of a type of virtual port * - * @type: Name of port type, such as "netdev" or "internal" to be matched - * against the device type when a new port needs to be created. + * @type: %ODP_VPORT_TYPE_* value for this type of virtual port. * @flags: Flags of type VPORT_F_* that influence how the generic vport layer * handles this vport. * @init: Called at module initialization. If VPORT_F_REQUIRED is set then the @@ -186,7 +185,7 @@ struct vport_parms { * @send: Send a packet on the device. Returns the length of the packet sent. */ struct vport_ops { - const char *type; + enum odp_vport_type type; u32 flags; /* Called at module init and exit respectively. */ diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h index 25f8e132..1d68445e 100644 --- a/include/openvswitch/datapath-protocol.h +++ b/include/openvswitch/datapath-protocol.h @@ -176,17 +176,28 @@ struct odp_packet { uint32_t len; }; -#define VPORT_TYPE_SIZE 16 #define VPORT_CONFIG_SIZE 32 struct odp_port { char devname[16]; /* IFNAMSIZ */ - char type[VPORT_TYPE_SIZE]; + uint32_t type; /* One of ODP_VPORT_TYPE_*. */ uint16_t port; uint16_t dp_idx; uint32_t reserved2; __aligned_u64 config[VPORT_CONFIG_SIZE / 8]; /* type-specific */ }; +enum odp_vport_type { + ODP_VPORT_TYPE_UNSPEC, + ODP_VPORT_TYPE_NETDEV, /* network device */ + ODP_VPORT_TYPE_INTERNAL, /* network device implemented by datapath */ + ODP_VPORT_TYPE_PATCH, /* virtual tunnel connecting two vports */ + ODP_VPORT_TYPE_GRE, /* GRE tunnel */ + ODP_VPORT_TYPE_CAPWAP, /* CAPWAP tunnel */ + __ODP_VPORT_TYPE_MAX +}; + +#define ODP_VPORT_TYPE_MAX (__ODP_VPORT_TYPE_MAX - 1) + /** * struct odp_vport_dump - ODP_VPORT_DUMP argument. * @port: Points to port structure to fill in. diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index 6883605f..cf3c9b32 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -232,33 +232,50 @@ dpif_linux_set_drop_frags(struct dpif *dpif_, bool drop_frags) return do_ioctl(dpif_, ODP_SET_DROP_FRAGS, &drop_frags_int); } -static void -translate_vport_type_to_netdev_type(struct odp_port *port) +static const char * +vport_type_to_netdev_type(const struct odp_port *odp_port) { - char *type = port->type; + struct tnl_port_config tnl_config; - if (!strcmp(type, "netdev")) { - ovs_strlcpy(type, "system", sizeof port->type); - } else if (!strcmp(type, "gre")) { - const struct tnl_port_config *config; + switch (odp_port->type) { + case ODP_VPORT_TYPE_UNSPEC: + break; - config = (struct tnl_port_config *)port->config; - if (config->flags & TNL_F_IPSEC) { - ovs_strlcpy(type, "ipsec_gre", sizeof port->type); - } + case ODP_VPORT_TYPE_NETDEV: + return "system"; + + case ODP_VPORT_TYPE_INTERNAL: + return "internal"; + + case ODP_VPORT_TYPE_PATCH: + return "patch"; + + case ODP_VPORT_TYPE_GRE: + memcpy(&tnl_config, odp_port->config, sizeof tnl_config); + return tnl_config.flags & TNL_F_IPSEC ? "ipsec_gre" : "gre"; + + case ODP_VPORT_TYPE_CAPWAP: + return "capwap"; + + case __ODP_VPORT_TYPE_MAX: + break; } + + VLOG_WARN_RL(&error_rl, "dp%d: port `%s' has unsupported type %"PRIu32, + odp_port->dp_idx, odp_port->devname, odp_port->type); + return "unknown"; } -static void -translate_netdev_type_to_vport_type(struct odp_port *port) +static enum odp_vport_type +netdev_type_to_vport_type(const char *type) { - char *type = port->type; - - if (!strcmp(type, "system")) { - ovs_strlcpy(type, "netdev", sizeof port->type); - } else if (!strcmp(type, "ipsec_gre")) { - ovs_strlcpy(type, "gre", sizeof port->type); - } + return (!strcmp(type, "system") ? ODP_VPORT_TYPE_NETDEV + : !strcmp(type, "internal") ? ODP_VPORT_TYPE_INTERNAL + : !strcmp(type, "patch") ? ODP_VPORT_TYPE_PATCH + : (!strcmp(type, "gre") + || !strcmp(type, "ipsec_gre")) ? ODP_VPORT_TYPE_GRE + : !strcmp(type, "capwap") ? ODP_VPORT_TYPE_CAPWAP + : ODP_VPORT_TYPE_UNSPEC); } static int @@ -272,9 +289,15 @@ dpif_linux_port_add(struct dpif *dpif, struct netdev *netdev, memset(&port, 0, sizeof port); strncpy(port.devname, name, sizeof port.devname); - strncpy(port.type, type, sizeof port.type); netdev_vport_get_config(netdev, port.config); - translate_netdev_type_to_vport_type(&port); + + port.type = netdev_type_to_vport_type(type); + if (port.type == ODP_VPORT_TYPE_UNSPEC) { + VLOG_WARN_RL(&error_rl, "%s: cannot create port `%s' because it has " + "unsupported type `%s'", + dpif_name(dpif), name, type); + return EINVAL; + } error = do_ioctl(dpif, ODP_VPORT_ATTACH, &port); if (!error) { @@ -309,9 +332,8 @@ dpif_linux_port_query__(const struct dpif *dpif, uint32_t port_no, /* A vport named 'port_name' exists but in some other datapath. */ return ENOENT; } else { - translate_vport_type_to_netdev_type(&odp_port); dpif_port->name = xstrdup(odp_port.devname); - dpif_port->type = xstrdup(odp_port.type); + dpif_port->type = xstrdup(vport_type_to_netdev_type(&odp_port)); dpif_port->port_no = odp_port.port; return 0; } @@ -360,9 +382,8 @@ dpif_linux_port_dump_next(const struct dpif *dpif, void *state, } else if (odp_port->devname[0] == '\0') { return EOF; } else { - translate_vport_type_to_netdev_type(odp_port); dpif_port->name = odp_port->devname; - dpif_port->type = odp_port->type; + dpif_port->type = (char *) vport_type_to_netdev_type(odp_port); dpif_port->port_no = odp_port->port; odp_port->port++; return 0; @@ -681,7 +702,7 @@ lookup_internal_device(const char *name, int *dp_idx, int *port_no) name, strerror(errno)); } return errno; - } else if (!strcmp(odp_port.type, "internal")) { + } else if (odp_port.type == ODP_VPORT_TYPE_INTERNAL) { *dp_idx = odp_port.dp_idx; *port_no = odp_port.port; return 0; diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 550059dd..e930e1be 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -71,6 +71,7 @@ struct netdev_vport { }; struct vport_class { + enum odp_vport_type type; struct netdev_class netdev_class; int (*parse_config)(const char *name, const char *type, const struct shash *args, void *config); @@ -243,7 +244,7 @@ netdev_vport_set_config(struct netdev_dev *dev_, const struct shash *args) memset(&port, 0, sizeof port); strncpy(port.devname, netdev_dev_get_name(dev_), sizeof port.devname); - strncpy(port.type, netdev_dev_get_type(dev_), sizeof port.type); + port.type = vport_class->type; error = vport_class->parse_config(netdev_dev_get_name(dev_), netdev_dev_get_type(dev_), args, port.config); @@ -979,13 +980,20 @@ void netdev_vport_register(void) { static const struct vport_class vport_classes[] = { - { { "gre", VPORT_FUNCTIONS(netdev_vport_get_status) }, + { ODP_VPORT_TYPE_GRE, + { "gre", VPORT_FUNCTIONS(netdev_vport_get_status) }, parse_tunnel_config, unparse_tunnel_config }, - { { "ipsec_gre", VPORT_FUNCTIONS(netdev_vport_get_status) }, + + { ODP_VPORT_TYPE_GRE, + { "ipsec_gre", VPORT_FUNCTIONS(netdev_vport_get_status) }, parse_tunnel_config, unparse_tunnel_config }, - { { "capwap", VPORT_FUNCTIONS(netdev_vport_get_status) }, + + { ODP_VPORT_TYPE_CAPWAP, + { "capwap", VPORT_FUNCTIONS(netdev_vport_get_status) }, parse_tunnel_config, unparse_tunnel_config }, - { { "patch", VPORT_FUNCTIONS(NULL) }, + + { ODP_VPORT_TYPE_PATCH, + { "patch", VPORT_FUNCTIONS(NULL) }, parse_patch_config, unparse_patch_config } }; -- 2.30.2