From: Ben Pfaff Date: Fri, 3 Dec 2010 22:41:38 +0000 (-0800) Subject: datapath: Make adding and attaching a vport a single step. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c3827f619a38d3d202020838e1f92860046a3dbe;p=openvswitch datapath: Make adding and attaching a vport a single step. For some time now, Open vSwitch datapaths have internally made a distinction between adding a vport and attaching it to a datapath. Adding a vport just means to create it, as an entity detached from any datapath. Attaching it gives it a port number and a datapath. Similarly, a vport could be detached and deleted separately. After some study, I think I understand why this distinction exists. It is because ovs-vswitchd tries to open all the datapath ports before it tries to create them. However, changing it to create them before it tries to open them is not difficult, so this commit does this. The bulk of this commit, however, changes the datapath interface to one that always creates a vport and attaches it to a datapath in a single step, and similarly detaches a vport and deletes it in a single step. Signed-off-by: Ben Pfaff Acked-by: Jesse Gross --- diff --git a/datapath/datapath.c b/datapath/datapath.c index 529d4969..dc91feca 100644 --- a/datapath/datapath.c +++ b/datapath/datapath.c @@ -252,7 +252,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); - internal_dev_port.flags = ODP_PORT_INTERNAL; + strcpy(internal_dev_port.type, "internal"); err = new_dp_port(dp, &internal_dev_port, ODPP_LOCAL); if (err) { if (err == -EBUSY) @@ -275,7 +275,7 @@ static int create_dp(int dp_idx, const char __user *devnamep) return 0; err_destroy_local_port: - dp_detach_port(dp->ports[ODPP_LOCAL], 1); + dp_detach_port(dp->ports[ODPP_LOCAL]); err_destroy_table: tbl_destroy(dp->table, NULL); err_free_dp: @@ -296,13 +296,13 @@ static void do_destroy_dp(struct datapath *dp) list_for_each_entry_safe (p, n, &dp->port_list, node) if (p->port_no != ODPP_LOCAL) - dp_detach_port(p, 1); + dp_detach_port(p); dp_sysfs_del_dp(dp); rcu_assign_pointer(dps[dp->dp_idx], NULL); - dp_detach_port(dp->ports[ODPP_LOCAL], 1); + dp_detach_port(dp->ports[ODPP_LOCAL]); tbl_destroy(dp->table, flow_free_tbl); @@ -350,25 +350,21 @@ static struct kobj_type brport_ktype = { /* Called with RTNL lock and dp_mutex. */ static int new_dp_port(struct datapath *dp, struct odp_port *odp_port, int port_no) { + struct vport_parms parms; struct vport *vport; struct dp_port *p; int err; - vport = vport_locate(odp_port->devname); - if (!vport) { - struct vport_parms parms; + parms.name = odp_port->devname; + parms.type = odp_port->type; + parms.config = odp_port->config; - parms.name = odp_port->devname; - parms.type = odp_port->flags & ODP_PORT_INTERNAL ? "internal" : "netdev"; - parms.config = NULL; + vport_lock(); + vport = vport_add(&parms); + vport_unlock(); - vport_lock(); - vport = vport_add(&parms); - vport_unlock(); - - if (IS_ERR(vport)) - return PTR_ERR(vport); - } + if (IS_ERR(vport)) + return PTR_ERR(vport); p = kzalloc(sizeof(*p), GFP_KERNEL); if (!p) @@ -410,6 +406,7 @@ 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); @@ -441,7 +438,7 @@ out: return err; } -int dp_detach_port(struct dp_port *p, int may_delete) +int dp_detach_port(struct dp_port *p) { struct vport *vport = p->vport; int err; @@ -464,15 +461,9 @@ int dp_detach_port(struct dp_port *p, int may_delete) /* Then wait until no one is still using it, and destroy it. */ synchronize_rcu(); - if (may_delete) { - const char *port_type = vport_get_type(vport); - - if (!strcmp(port_type, "netdev") || !strcmp(port_type, "internal")) { - vport_lock(); - vport_del(vport); - vport_unlock(); - } - } + vport_lock(); + vport_del(vport); + vport_unlock(); kobject_put(&p->kobj); @@ -500,7 +491,7 @@ static int detach_port(int dp_idx, int port_no) if (!p) goto out_unlock_dp; - err = dp_detach_port(p, 1); + err = dp_detach_port(p); out_unlock_dp: mutex_unlock(&dp->mutex); @@ -1437,10 +1428,10 @@ static int put_port(const struct dp_port *p, struct odp_port __user *uop) rcu_read_lock(); strncpy(op.devname, vport_get_name(p->vport), sizeof op.devname); + strncpy(op.type, vport_get_type(p->vport), sizeof op.type); rcu_read_unlock(); op.port = p->port_no; - op.flags = is_internal_vport(p->vport) ? ODP_PORT_INTERNAL : 0; return copy_to_user(uop, &op, sizeof op) ? -EFAULT : 0; } @@ -1553,26 +1544,18 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd, err = destroy_dp(dp_idx); goto exit; - case ODP_PORT_ATTACH: + case ODP_VPORT_ATTACH: err = attach_port(dp_idx, (struct odp_port __user *)argp); goto exit; - case ODP_PORT_DETACH: + case ODP_VPORT_DETACH: err = get_user(port_no, (int __user *)argp); if (!err) err = detach_port(dp_idx, port_no); goto exit; - case ODP_VPORT_ADD: - err = vport_user_add((struct odp_vport_add __user *)argp); - goto exit; - case ODP_VPORT_MOD: - err = vport_user_mod((struct odp_vport_mod __user *)argp); - goto exit; - - case ODP_VPORT_DEL: - err = vport_user_del((char __user *)argp); + err = vport_user_mod((struct odp_port __user *)argp); goto exit; case ODP_VPORT_STATS_GET: @@ -1650,11 +1633,11 @@ static long openvswitch_ioctl(struct file *f, unsigned int cmd, dp->sflow_probability = sflow_probability; break; - case ODP_PORT_QUERY: + case ODP_VPORT_QUERY: err = query_port(dp, (struct odp_port __user *)argp); break; - case ODP_PORT_LIST: + case ODP_VPORT_LIST: err = list_ports(dp, (struct odp_portvec __user *)argp); break; @@ -1910,9 +1893,9 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned return openvswitch_ioctl(f, cmd, argp); case ODP_DP_CREATE: - case ODP_PORT_ATTACH: - case ODP_PORT_DETACH: - case ODP_VPORT_DEL: + case ODP_VPORT_ATTACH: + case ODP_VPORT_DETACH: + case ODP_VPORT_MOD: case ODP_VPORT_MTU_SET: case ODP_VPORT_MTU_GET: case ODP_VPORT_ETHER_SET: @@ -1926,15 +1909,9 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned case ODP_GET_LISTEN_MASK: case ODP_SET_SFLOW_PROBABILITY: case ODP_GET_SFLOW_PROBABILITY: - case ODP_PORT_QUERY: + case ODP_VPORT_QUERY: /* Ioctls that just need their pointer argument extended. */ return openvswitch_ioctl(f, cmd, (unsigned long)compat_ptr(argp)); - - case ODP_VPORT_ADD32: - return compat_vport_user_add(compat_ptr(argp)); - - case ODP_VPORT_MOD32: - return compat_vport_user_mod(compat_ptr(argp)); } dp = get_dp_locked(dp_idx); @@ -1943,7 +1920,7 @@ static long openvswitch_compat_ioctl(struct file *f, unsigned int cmd, unsigned goto exit; switch (cmd) { - case ODP_PORT_LIST32: + case ODP_VPORT_LIST32: err = compat_list_ports(dp, compat_ptr(argp)); break; diff --git a/datapath/datapath.h b/datapath/datapath.h index 3a38235f..36accfdf 100644 --- a/datapath/datapath.h +++ b/datapath/datapath.h @@ -153,7 +153,7 @@ extern struct notifier_block dp_device_notifier; extern int (*dp_ioctl_hook)(struct net_device *dev, struct ifreq *rq, int cmd); void dp_process_received_packet(struct dp_port *, struct sk_buff *); -int dp_detach_port(struct dp_port *, int may_delete); +int dp_detach_port(struct dp_port *); int dp_output_control(struct datapath *, struct sk_buff *, int, u32 arg); int dp_min_mtu(const struct datapath *dp); void set_internal_devs_mtu(const struct datapath *dp); diff --git a/datapath/dp_notify.c b/datapath/dp_notify.c index b6bb90ec..1843d756 100644 --- a/datapath/dp_notify.c +++ b/datapath/dp_notify.c @@ -39,7 +39,7 @@ static int dp_device_event(struct notifier_block *unused, unsigned long event, switch (event) { case NETDEV_UNREGISTER: mutex_lock(&dp->mutex); - dp_detach_port(p, 1); + dp_detach_port(p); mutex_unlock(&dp->mutex); break; diff --git a/datapath/odp-compat.h b/datapath/odp-compat.h index 669a5216..d30b05f6 100644 --- a/datapath/odp-compat.h +++ b/datapath/odp-compat.h @@ -22,8 +22,6 @@ #define ODP_FLOW_DEL32 _IOWR('O', 17, struct compat_odp_flow) #define ODP_EXECUTE32 _IOR('O', 18, struct compat_odp_execute) #define ODP_FLOW_DEL32 _IOWR('O', 17, struct compat_odp_flow) -#define ODP_VPORT_ADD32 _IOR('O', 21, struct compat_odp_vport_add) -#define ODP_VPORT_MOD32 _IOR('O', 22, struct compat_odp_vport_mod) struct compat_odp_portvec { compat_uptr_t ports; @@ -59,17 +57,6 @@ struct compat_odp_execute { compat_uptr_t data; u32 length; }; - -struct compat_odp_vport_add { - char port_type[VPORT_TYPE_SIZE]; - char devname[16]; /* IFNAMSIZ */ - compat_uptr_t config; -}; - -struct compat_odp_vport_mod { - char devname[16]; /* IFNAMSIZ */ - compat_uptr_t config; -}; #endif /* CONFIG_COMPAT */ #endif /* odp-compat.h */ diff --git a/datapath/tunnel.c b/datapath/tunnel.c index cdbb4f45..b6bd6f65 100644 --- a/datapath/tunnel.c +++ b/datapath/tunnel.c @@ -1300,15 +1300,14 @@ out: return sent_len; } -static int set_config(const void __user *uconfig, const struct tnl_ops *tnl_ops, +static int set_config(const void *config, const struct tnl_ops *tnl_ops, const struct vport *cur_vport, struct tnl_mutable_config *mutable) { const struct vport *old_vport; const struct tnl_mutable_config *old_mutable; - if (copy_from_user(&mutable->port_config, uconfig, sizeof(struct tnl_port_config))) - return -EFAULT; + mutable->port_config = *(struct tnl_port_config *)config; if (mutable->port_config.daddr == 0) return -EINVAL; @@ -1401,7 +1400,7 @@ error: return ERR_PTR(err); } -int tnl_modify(struct vport *vport, const void __user *config) +int tnl_modify(struct vport *vport, struct odp_port *port) { struct tnl_vport *tnl_vport = tnl_vport_priv(vport); struct tnl_mutable_config *mutable; @@ -1413,7 +1412,7 @@ int tnl_modify(struct vport *vport, const void __user *config) goto error; } - err = set_config(config, tnl_vport->tnl_ops, vport, mutable); + err = set_config(port->config, tnl_vport->tnl_ops, vport, mutable); if (err) goto error_free; diff --git a/datapath/tunnel.h b/datapath/tunnel.h index 63f7dd42..3ed7ef37 100644 --- a/datapath/tunnel.h +++ b/datapath/tunnel.h @@ -186,7 +186,7 @@ struct tnl_vport { struct vport *tnl_create(const struct vport_parms *, const struct vport_ops *, const struct tnl_ops *); -int tnl_modify(struct vport *, const void __user *config); +int tnl_modify(struct vport *, struct odp_port *); int tnl_destroy(struct vport *); int tnl_set_mtu(struct vport *vport, int mtu); int tnl_set_addr(struct vport *vport, const unsigned char *addr); diff --git a/datapath/vport-patch.c b/datapath/vport-patch.c index eb391580..b5276b17 100644 --- a/datapath/vport-patch.c +++ b/datapath/vport-patch.c @@ -83,17 +83,12 @@ static void patch_exit(void) kfree(peer_table); } -static int set_config(struct vport *vport, const void __user *uconfig) +static int set_config(struct vport *vport, const void *config) { struct patch_vport *patch_vport = patch_vport_priv(vport); char peer_name[IFNAMSIZ]; - int retval; - retval = strncpy_from_user(peer_name, uconfig, IFNAMSIZ); - if (retval < 0) - return -EFAULT; - else if (retval >= IFNAMSIZ) - return -ENAMETOOLONG; + strlcpy(peer_name, config, IFNAMSIZ); if (!strcmp(patch_vport->name, peer_name)) return -EINVAL; @@ -149,9 +144,9 @@ error: return ERR_PTR(err); } -static int patch_modify(struct vport *vport, const void __user *config) +static int patch_modify(struct vport *vport, struct odp_port *port) { - return set_config(vport, config); + return set_config(vport, port->config); } static int patch_destroy(struct vport *vport) diff --git a/datapath/vport.c b/datapath/vport.c index ffdd5216..27232443 100644 --- a/datapath/vport.c +++ b/datapath/vport.c @@ -177,189 +177,36 @@ void vport_exit(void) kfree(dev_table); } -static int do_vport_add(struct odp_vport_add *vport_config) -{ - struct vport_parms parms; - struct vport *vport; - int err = 0; - - vport_config->port_type[VPORT_TYPE_SIZE - 1] = '\0'; - vport_config->devname[IFNAMSIZ - 1] = '\0'; - - rtnl_lock(); - - vport = vport_locate(vport_config->devname); - if (vport) { - err = -EBUSY; - goto out; - } - - parms.name = vport_config->devname; - parms.type = vport_config->port_type; - parms.config = vport_config->config; - - vport_lock(); - vport = vport_add(&parms); - vport_unlock(); - - if (IS_ERR(vport)) - err = PTR_ERR(vport); - -out: - rtnl_unlock(); - return err; -} - -/** - * vport_user_add - add vport device (for userspace callers) - * - * @uvport_config: New port configuration. - * - * Creates a new vport with the specified configuration (which is dependent - * on device type). This function is for userspace callers and assumes no - * locks are held. - */ -int vport_user_add(const struct odp_vport_add __user *uvport_config) -{ - struct odp_vport_add vport_config; - - if (copy_from_user(&vport_config, uvport_config, sizeof(struct odp_vport_add))) - return -EFAULT; - - return do_vport_add(&vport_config); -} - -#ifdef CONFIG_COMPAT -int compat_vport_user_add(struct compat_odp_vport_add *ucompat) -{ - struct compat_odp_vport_add compat; - struct odp_vport_add vport_config; - - if (copy_from_user(&compat, ucompat, sizeof(struct compat_odp_vport_add))) - return -EFAULT; - - memcpy(vport_config.port_type, compat.port_type, VPORT_TYPE_SIZE); - memcpy(vport_config.devname, compat.devname, IFNAMSIZ); - vport_config.config = compat_ptr(compat.config); - - return do_vport_add(&vport_config); -} -#endif - -static int do_vport_mod(struct odp_vport_mod *vport_config) -{ - struct vport *vport; - int err; - - vport_config->devname[IFNAMSIZ - 1] = '\0'; - - rtnl_lock(); - - vport = vport_locate(vport_config->devname); - if (!vport) { - err = -ENODEV; - goto out; - } - - vport_lock(); - err = vport_mod(vport, vport_config->config); - vport_unlock(); - -out: - rtnl_unlock(); - return err; -} - /** * vport_user_mod - modify existing vport device (for userspace callers) * - * @uvport_config: New configuration for vport + * @uport: New configuration for vport * * Modifies an existing device with the specified configuration (which is * dependent on device type). This function is for userspace callers and * assumes no locks are held. */ -int vport_user_mod(const struct odp_vport_mod __user *uvport_config) -{ - struct odp_vport_mod vport_config; - - if (copy_from_user(&vport_config, uvport_config, sizeof(struct odp_vport_mod))) - return -EFAULT; - - return do_vport_mod(&vport_config); -} - -#ifdef CONFIG_COMPAT -int compat_vport_user_mod(struct compat_odp_vport_mod *ucompat) -{ - struct compat_odp_vport_mod compat; - struct odp_vport_mod vport_config; - - if (copy_from_user(&compat, ucompat, sizeof(struct compat_odp_vport_mod))) - return -EFAULT; - - memcpy(vport_config.devname, compat.devname, IFNAMSIZ); - vport_config.config = compat_ptr(compat.config); - - return do_vport_mod(&vport_config); -} -#endif - -/** - * vport_user_del - delete existing vport device (for userspace callers) - * - * @udevname: Name of device to delete - * - * Deletes the specified device. Detaches the device from a datapath first - * if it is attached. Deleting the device will fail if it does not exist or it - * is the datapath local port. It is also possible to fail for less obvious - * reasons, such as lack of memory. This function is for userspace callers and - * assumes no locks are held. - */ -int vport_user_del(const char __user *udevname) +int vport_user_mod(const struct odp_port __user *uport) { - char devname[IFNAMSIZ]; + struct odp_port port; struct vport *vport; - struct dp_port *dp_port; - int err = 0; - int retval; + int err; - retval = strncpy_from_user(devname, udevname, IFNAMSIZ); - if (retval < 0) + if (copy_from_user(&port, uport, sizeof(port))) return -EFAULT; - else if (retval >= IFNAMSIZ) - return -ENAMETOOLONG; + + port.devname[IFNAMSIZ - 1] = '\0'; rtnl_lock(); - vport = vport_locate(devname); + vport = vport_locate(port.devname); if (!vport) { err = -ENODEV; goto out; } - dp_port = vport_get_dp_port(vport); - if (dp_port) { - struct datapath *dp = dp_port->dp; - - mutex_lock(&dp->mutex); - - if (!strcmp(dp_name(dp), devname)) { - err = -EINVAL; - goto dp_port_out; - } - - err = dp_detach_port(dp_port, 0); - -dp_port_out: - mutex_unlock(&dp->mutex); - - if (err) - goto out; - } - vport_lock(); - err = vport_del(vport); + err = vport_mod(vport, &port); vport_unlock(); out: @@ -744,18 +591,18 @@ out: * vport_mod - modify existing vport device (for kernel callers) * * @vport: vport to modify. - * @config: Device type specific configuration. Userspace pointer. + * @port: New configuration. * * Modifies an existing device with the specified configuration (which is * dependent on device type). Both RTNL and vport locks must be held. */ -int vport_mod(struct vport *vport, const void __user *config) +int vport_mod(struct vport *vport, struct odp_port *port) { ASSERT_RTNL(); ASSERT_VPORT(); if (vport->ops->modify) - return vport->ops->modify(vport, config); + return vport->ops->modify(vport, port); else return -EOPNOTSUPP; } diff --git a/datapath/vport.h b/datapath/vport.h index 5f3c7e9f..7251b4cd 100644 --- a/datapath/vport.h +++ b/datapath/vport.h @@ -24,14 +24,7 @@ struct dp_port; /* The following definitions are for users of the vport subsytem: */ -int vport_user_add(const struct odp_vport_add __user *); -int vport_user_mod(const struct odp_vport_mod __user *); -int vport_user_del(const char __user *udevname); - -#ifdef CONFIG_COMPAT -int compat_vport_user_add(struct compat_odp_vport_add __user *); -int compat_vport_user_mod(struct compat_odp_vport_mod __user *); -#endif +int vport_user_mod(const struct odp_port __user *); int vport_user_stats_get(struct odp_vport_stats_req __user *); int vport_user_stats_set(struct odp_vport_stats_req __user *); @@ -47,7 +40,7 @@ int vport_init(void); void vport_exit(void); struct vport *vport_add(const struct vport_parms *); -int vport_mod(struct vport *, const void __user *config); +int vport_mod(struct vport *, struct odp_port *); int vport_del(struct vport *); struct vport *vport_locate(const char *name); @@ -117,13 +110,13 @@ struct vport { * * @name: New vport's name. * @type: New vport's type. - * @config: New vport's configuration, as %NULL or a userspace pointer to an - * arbitrary type-specific structure. + * @config: Kernel copy of 'config' member of &struct odp_port describing + * configuration for new port. Exactly %VPORT_CONFIG_SIZE bytes. */ struct vport_parms { const char *name; const char *type; - const void __user *config; + const void *config; }; /** @@ -179,7 +172,7 @@ struct vport_ops { /* Called with RTNL lock. */ struct vport *(*create)(const struct vport_parms *); - int (*modify)(struct vport *, const void __user *config); + int (*modify)(struct vport *, struct odp_port *); int (*destroy)(struct vport *); int (*attach)(struct vport *); diff --git a/include/openvswitch/datapath-protocol.h b/include/openvswitch/datapath-protocol.h index 48859064..8e07b8bd 100644 --- a/include/openvswitch/datapath-protocol.h +++ b/include/openvswitch/datapath-protocol.h @@ -61,6 +61,10 @@ #include #endif +#ifndef __aligned_u64 +#define __aligned_u64 __u64 __attribute__((aligned(8))) +#endif + #include #define ODP_MAX 256 /* Maximum number of datapaths. */ @@ -75,10 +79,10 @@ #define ODP_GET_LISTEN_MASK _IOW('O', 5, int) #define ODP_SET_LISTEN_MASK _IOR('O', 6, int) -#define ODP_PORT_ATTACH _IOR('O', 7, struct odp_port) -#define ODP_PORT_DETACH _IOR('O', 8, int) -#define ODP_PORT_QUERY _IOWR('O', 9, struct odp_port) -#define ODP_PORT_LIST _IOWR('O', 10, struct odp_portvec) +#define ODP_VPORT_ATTACH _IOR('O', 7, struct odp_port) +#define ODP_VPORT_DETACH _IOR('O', 8, int) +#define ODP_VPORT_QUERY _IOWR('O', 9, struct odp_port) +#define ODP_VPORT_LIST _IOWR('O', 10, struct odp_portvec) #define ODP_FLOW_GET _IOWR('O', 13, struct odp_flow) #define ODP_FLOW_PUT _IOWR('O', 14, struct odp_flow) @@ -91,9 +95,7 @@ #define ODP_SET_SFLOW_PROBABILITY _IOR('O', 19, int) #define ODP_GET_SFLOW_PROBABILITY _IOW('O', 20, int) -#define ODP_VPORT_ADD _IOR('O', 21, struct odp_vport_add) -#define ODP_VPORT_MOD _IOR('O', 22, struct odp_vport_mod) -#define ODP_VPORT_DEL _IO('O', 23) +#define ODP_VPORT_MOD _IOR('O', 22, struct odp_port) #define ODP_VPORT_STATS_GET _IOWR('O', 24, struct odp_vport_stats_req) #define ODP_VPORT_ETHER_GET _IOWR('O', 25, struct odp_vport_ether) #define ODP_VPORT_ETHER_SET _IOW('O', 26, struct odp_vport_ether) @@ -181,12 +183,15 @@ struct odp_sflow_sample_header { uint32_t n_actions; }; -#define ODP_PORT_INTERNAL (1 << 0) /* This port is simulated. */ +#define VPORT_TYPE_SIZE 16 +#define VPORT_CONFIG_SIZE 32 struct odp_port { char devname[16]; /* IFNAMSIZ */ + char type[VPORT_TYPE_SIZE]; uint16_t port; - uint16_t flags; + uint16_t reserved1; uint32_t reserved2; + __aligned_u64 config[VPORT_CONFIG_SIZE / 8]; /* type-specific */ }; struct odp_portvec { diff --git a/include/openvswitch/tunnel.h b/include/openvswitch/tunnel.h index dd700d0d..e7d3fce7 100644 --- a/include/openvswitch/tunnel.h +++ b/include/openvswitch/tunnel.h @@ -41,6 +41,7 @@ #define OPENVSWITCH_TUNNEL_H 1 #include +#include "openvswitch/datapath-protocol.h" #define TNL_F_CSUM (1 << 1) /* Checksum packets. */ #define TNL_F_IN_KEY_MATCH (1 << 2) /* Store the key in tun_id to match in flow table. */ @@ -50,6 +51,7 @@ #define TNL_F_PMTUD (1 << 6) /* Enable path MTU discovery. */ #define TNL_F_HDR_CACHE (1 << 7) /* Enable tunnel header caching. */ +/* This goes in the "config" member of struct odp_port for tunnel vports. */ struct tnl_port_config { __u32 flags; __be32 saddr; diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c index cb218376..89cee1fe 100644 --- a/lib/dpif-linux.c +++ b/lib/dpif-linux.c @@ -35,6 +35,7 @@ #include "dpif-provider.h" #include "netdev.h" +#include "netdev-vport.h" #include "ofpbuf.h" #include "poll-loop.h" #include "rtnetlink.h" @@ -156,7 +157,7 @@ dpif_linux_open(const struct dpif_class *class OVS_UNUSED, const char *name, * getting the local port's name. */ memset(&port, 0, sizeof port); port.port = ODPP_LOCAL; - if (ioctl(dpif->fd, ODP_PORT_QUERY, &port)) { + if (ioctl(dpif->fd, ODP_VPORT_QUERY, &port)) { error = errno; if (error != ENODEV) { VLOG_WARN("%s: probe returned unexpected error: %s", @@ -195,28 +196,6 @@ dpif_linux_get_all_names(const struct dpif *dpif_, struct svec *all_names) static int dpif_linux_destroy(struct dpif *dpif_) { - struct odp_port *ports; - size_t n_ports; - int err; - int i; - - err = dpif_port_list(dpif_, &ports, &n_ports); - if (err) { - return err; - } - - for (i = 0; i < n_ports; i++) { - if (ports[i].port != ODPP_LOCAL) { - err = do_ioctl(dpif_, ODP_VPORT_DEL, ports[i].devname); - if (err) { - VLOG_WARN_RL(&error_rl, "%s: error deleting port %s (%s)", - dpif_name(dpif_), ports[i].devname, strerror(err)); - } - } - } - - free(ports); - return do_ioctl(dpif_, ODP_DP_DESTROY, NULL); } @@ -247,66 +226,78 @@ 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(char *type, size_t size) +{ + if (!strcmp(type, "netdev")) { + ovs_strlcpy(type, "system", size); + } +} + +static void +translate_netdev_type_to_vport_type(char *type, size_t size) +{ + if (!strcmp(type, "system")) { + ovs_strlcpy(type, "netdev", size); + } +} + static int -dpif_linux_port_add(struct dpif *dpif_, const char *devname, uint16_t flags, - uint16_t *port_no) +dpif_linux_port_add(struct dpif *dpif, struct netdev *netdev, + uint16_t *port_nop) { + const char *name = netdev_get_name(netdev); + const char *type = netdev_get_type(netdev); struct odp_port port; int error; memset(&port, 0, sizeof port); - strncpy(port.devname, devname, sizeof port.devname); - port.flags = flags; - error = do_ioctl(dpif_, ODP_PORT_ATTACH, &port); + strncpy(port.devname, name, sizeof port.devname); + strncpy(port.type, type, sizeof port.type); + translate_netdev_type_to_vport_type(port.type, sizeof port.type); + netdev_vport_get_config(netdev, port.config); + + error = do_ioctl(dpif, ODP_VPORT_ATTACH, &port); if (!error) { - *port_no = port.port; + *port_nop = port.port; } + return error; } static int -dpif_linux_port_del(struct dpif *dpif_, uint16_t port_no) +dpif_linux_port_del(struct dpif *dpif_, uint16_t port_no_) { - int tmp = port_no; - int err; - struct odp_port port; - - err = dpif_port_query_by_number(dpif_, port_no, &port); - if (err) { - return err; - } - - err = do_ioctl(dpif_, ODP_PORT_DETACH, &tmp); - if (err) { - return err; - } + int port_no = port_no_; /* Kernel expects an "int". */ + return do_ioctl(dpif_, ODP_VPORT_DETACH, &port_no); +} - if (!netdev_is_open(port.devname)) { - /* Try deleting the port if no one has it open. This shouldn't - * actually be necessary unless the config changed while we weren't - * running but it won't hurt anything if the port is already gone. */ - do_ioctl(dpif_, ODP_VPORT_DEL, port.devname); +static int +dpif_linux_port_query__(const struct dpif *dpif, struct odp_port *port) +{ + int error = do_ioctl(dpif, ODP_VPORT_QUERY, port); + if (!error) { + translate_vport_type_to_netdev_type(port->type, sizeof port->type); } - - return 0; + return error; } static int -dpif_linux_port_query_by_number(const struct dpif *dpif_, uint16_t port_no, - struct odp_port *port) +dpif_linux_port_query_by_number(const struct dpif *dpif, uint16_t port_no, + struct odp_port *port) { memset(port, 0, sizeof *port); port->port = port_no; - return do_ioctl(dpif_, ODP_PORT_QUERY, port); + return dpif_linux_port_query__(dpif, port); } static int -dpif_linux_port_query_by_name(const struct dpif *dpif_, const char *devname, +dpif_linux_port_query_by_name(const struct dpif *dpif, const char *devname, struct odp_port *port) { memset(port, 0, sizeof *port); strncpy(port->devname, devname, sizeof port->devname); - return do_ioctl(dpif_, ODP_PORT_QUERY, port); + return dpif_linux_port_query__(dpif, port); } static int @@ -319,12 +310,22 @@ static int dpif_linux_port_list(const struct dpif *dpif_, struct odp_port *ports, int n) { struct odp_portvec pv; + unsigned int i; int error; pv.ports = ports; pv.n_ports = n; - error = do_ioctl(dpif_, ODP_PORT_LIST, &pv); - return error ? -error : pv.n_ports; + error = do_ioctl(dpif_, ODP_VPORT_LIST, &pv); + if (error) { + return -error; + } + + for (i = 0; i < pv.n_ports; i++) { + struct odp_port *port = &pv.ports[i]; + + translate_vport_type_to_netdev_type(port->type, sizeof port->type); + } + return pv.n_ports; } static int diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 096656e6..9f78be44 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -91,7 +91,7 @@ struct dp_netdev_port { int port_no; /* Index into dp_netdev's 'ports'. */ struct list node; /* Element in dp_netdev's 'port_list'. */ struct netdev *netdev; - bool internal; /* Internal port (as ODP_PORT_INTERNAL)? */ + bool internal; /* Internal port? */ }; /* A flow in dp_netdev's 'flow_table'. */ @@ -130,8 +130,8 @@ static int get_port_by_name(struct dp_netdev *, const char *devname, struct dp_netdev_port **portp); static void dp_netdev_free(struct dp_netdev *); static void dp_netdev_flow_flush(struct dp_netdev *); -static int do_add_port(struct dp_netdev *, const char *devname, uint16_t flags, - uint16_t port_no); +static int do_add_port(struct dp_netdev *, const char *devname, + const char *type, uint16_t port_no); static int do_del_port(struct dp_netdev *, uint16_t port_no); static int dpif_netdev_open(const struct dpif_class *, const char *name, bool create, struct dpif **); @@ -191,7 +191,7 @@ create_dp_netdev(const char *name, const struct dpif_class *class, } hmap_init(&dp->flow_table); list_init(&dp->port_list); - error = do_add_port(dp, name, ODP_PORT_INTERNAL, ODPP_LOCAL); + error = do_add_port(dp, name, "internal", ODPP_LOCAL); if (error) { dp_netdev_free(dp); return error; @@ -307,17 +307,25 @@ dpif_netdev_set_drop_frags(struct dpif *dpif, bool drop_frags) } static int -do_add_port(struct dp_netdev *dp, const char *devname, uint16_t flags, +do_add_port(struct dp_netdev *dp, const char *devname, const char *type, uint16_t port_no) { - bool internal = (flags & ODP_PORT_INTERNAL) != 0; struct dp_netdev_port *port; struct netdev_options netdev_options; struct netdev *netdev; + bool internal; int mtu; int error; /* XXX reject devices already in some dp_netdev. */ + if (type[0] == '\0' || !strcmp(type, "system")) { + internal = false; + } else if (!strcmp(type, "internal")) { + internal = true; + } else { + VLOG_WARN("%s: unsupported port type %s", devname, type); + return EINVAL; + } /* Open and validate network device. */ memset(&netdev_options, 0, sizeof netdev_options); @@ -361,7 +369,7 @@ do_add_port(struct dp_netdev *dp, const char *devname, uint16_t flags, } static int -dpif_netdev_port_add(struct dpif *dpif, const char *devname, uint16_t flags, +dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev, uint16_t *port_nop) { struct dp_netdev *dp = get_dp_netdev(dpif); @@ -370,7 +378,8 @@ dpif_netdev_port_add(struct dpif *dpif, const char *devname, uint16_t flags, for (port_no = 0; port_no < MAX_PORTS; port_no++) { if (!dp->ports[port_no]) { *port_nop = port_no; - return do_add_port(dp, devname, flags, port_no); + return do_add_port(dp, netdev_get_name(netdev), + netdev_get_type(netdev), port_no); } } return EFBIG; @@ -450,7 +459,7 @@ answer_port_query(const struct dp_netdev_port *port, struct odp_port *odp_port) ovs_strlcpy(odp_port->devname, netdev_get_name(port->netdev), sizeof odp_port->devname); odp_port->port = port->port_no; - odp_port->flags = port->internal ? ODP_PORT_INTERNAL : 0; + strcpy(odp_port->type, port->internal ? "internal" : "system"); } static int diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h index e1f10c47..26cd6b09 100644 --- a/lib/dpif-provider.h +++ b/lib/dpif-provider.h @@ -138,10 +138,9 @@ struct dpif_class { * meaning is the same as for the get_drop_frags member function. */ int (*set_drop_frags)(struct dpif *dpif, bool drop_frags); - /* Creates a new port in 'dpif' connected to network device 'devname'. - * 'flags' is a set of ODP_PORT_* flags. If successful, sets '*port_no' + /* Adds 'netdev' as a new port in 'dpif'. If successful, sets '*port_no' * to the new port's port number. */ - int (*port_add)(struct dpif *dpif, const char *devname, uint16_t flags, + int (*port_add)(struct dpif *dpif, struct netdev *netdev, uint16_t *port_no); /* Removes port numbered 'port_no' from 'dpif'. */ diff --git a/lib/dpif.c b/lib/dpif.c index 5402033d..952e1a2d 100644 --- a/lib/dpif.c +++ b/lib/dpif.c @@ -27,6 +27,7 @@ #include "coverage.h" #include "dynamic-string.h" #include "flow.h" +#include "netdev.h" #include "netlink.h" #include "odp-util.h" #include "ofp-print.h" @@ -443,27 +444,26 @@ dpif_set_drop_frags(struct dpif *dpif, bool drop_frags) return error; } -/* Attempts to add 'devname' as a port on 'dpif', given the combination of - * ODP_PORT_* flags in 'flags'. If successful, returns 0 and sets '*port_nop' - * to the new port's port number (if 'port_nop' is non-null). On failure, - * returns a positive errno value and sets '*port_nop' to UINT16_MAX (if - * 'port_nop' is non-null). */ +/* Attempts to add 'netdev' as a port on 'dpif'. If successful, returns 0 and + * sets '*port_nop' to the new port's port number (if 'port_nop' is non-null). + * On failure, returns a positive errno value and sets '*port_nop' to + * UINT16_MAX (if 'port_nop' is non-null). */ int -dpif_port_add(struct dpif *dpif, const char *devname, uint16_t flags, - uint16_t *port_nop) +dpif_port_add(struct dpif *dpif, struct netdev *netdev, uint16_t *port_nop) { + const char *netdev_name = netdev_get_name(netdev); uint16_t port_no; int error; COVERAGE_INC(dpif_port_add); - error = dpif->dpif_class->port_add(dpif, devname, flags, &port_no); + error = dpif->dpif_class->port_add(dpif, netdev, &port_no); if (!error) { VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu16, - dpif_name(dpif), devname, port_no); + dpif_name(dpif), netdev_name, port_no); } else { VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s", - dpif_name(dpif), devname, strerror(error)); + dpif_name(dpif), netdev_name, strerror(error)); port_no = UINT16_MAX; } if (port_nop) { diff --git a/lib/dpif.h b/lib/dpif.h index c844289c..927776cc 100644 --- a/lib/dpif.h +++ b/lib/dpif.h @@ -30,6 +30,7 @@ extern "C" { #endif struct dpif; +struct netdev; struct ofpbuf; struct svec; struct dpif_class; @@ -59,8 +60,7 @@ int dpif_get_dp_stats(const struct dpif *, struct odp_stats *); int dpif_get_drop_frags(const struct dpif *, bool *drop_frags); int dpif_set_drop_frags(struct dpif *, bool drop_frags); -int dpif_port_add(struct dpif *, const char *devname, uint16_t flags, - uint16_t *port_no); +int dpif_port_add(struct dpif *, struct netdev *, uint16_t *port_nop); int dpif_port_del(struct dpif *, uint16_t port_no); int dpif_port_query_by_number(const struct dpif *, uint16_t port_no, struct odp_port *); diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index bcd1633e..cbe4222c 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -495,9 +495,9 @@ netdev_linux_cache_cb(const struct rtnetlink_change *change, } } -/* Creates the netdev device of 'type' with 'name'. */ +/* Creates system and internal devices. */ static int -netdev_linux_create_system(const struct netdev_class *class OVS_UNUSED, +netdev_linux_create(const struct netdev_class *class, const char *name, const struct shash *args, struct netdev_dev **netdev_devp) { @@ -505,7 +505,8 @@ netdev_linux_create_system(const struct netdev_class *class OVS_UNUSED, int error; if (!shash_is_empty(args)) { - VLOG_WARN("%s: arguments for system devices should be empty", name); + VLOG_WARN("%s: arguments for %s devices should be empty", + name, class->type); } if (!cache_notifier_refcount) { @@ -518,7 +519,7 @@ netdev_linux_create_system(const struct netdev_class *class OVS_UNUSED, cache_notifier_refcount++; netdev_dev = xzalloc(sizeof *netdev_dev); - netdev_dev_init(&netdev_dev->netdev_dev, name, &netdev_linux_class); + netdev_dev_init(&netdev_dev->netdev_dev, name, class); *netdev_devp = &netdev_dev->netdev_dev; return 0; @@ -629,9 +630,19 @@ netdev_linux_open(struct netdev_dev *netdev_dev_, int ethertype, netdev->fd = -1; netdev_init(&netdev->netdev, netdev_dev_); - error = netdev_get_flags(&netdev->netdev, &flags); - if (error == ENODEV) { - goto error; + /* Verify that the device really exists, by attempting to read its flags. + * (The flags might be cached, in which case this won't actually do an + * ioctl.) + * + * Don't do this for "internal" netdevs, though, because those have to be + * created as netdev objects before they exist in the kernel, because + * creating them in the kernel happens by passing a netdev object to + * dpif_port_add(). */ + if (netdev_dev_get_class(netdev_dev_) != &netdev_internal_class) { + error = netdev_get_flags(&netdev->netdev, &flags); + if (error == ENODEV) { + goto error; + } } if (!strcmp(netdev_dev_get_type(netdev_dev_), "tap") && @@ -2107,125 +2118,87 @@ netdev_linux_poll_remove(struct netdev_notifier *notifier_) } } -const struct netdev_class netdev_linux_class = { - "system", - - netdev_linux_init, - netdev_linux_run, - netdev_linux_wait, - - netdev_linux_create_system, - netdev_linux_destroy, - NULL, /* reconfigure */ - - netdev_linux_open, - netdev_linux_close, - - netdev_linux_enumerate, - - netdev_linux_recv, - netdev_linux_recv_wait, - netdev_linux_drain, - - netdev_linux_send, - netdev_linux_send_wait, - - netdev_linux_set_etheraddr, - netdev_linux_get_etheraddr, - netdev_linux_get_mtu, - netdev_linux_get_ifindex, - netdev_linux_get_carrier, - netdev_linux_get_stats, - netdev_vport_set_stats, - - netdev_linux_get_features, - netdev_linux_set_advertisements, - netdev_linux_get_vlan_vid, - - netdev_linux_set_policing, - netdev_linux_get_qos_types, - netdev_linux_get_qos_capabilities, - netdev_linux_get_qos, - netdev_linux_set_qos, - netdev_linux_get_queue, - netdev_linux_set_queue, - netdev_linux_delete_queue, - netdev_linux_get_queue_stats, - netdev_linux_dump_queues, - netdev_linux_dump_queue_stats, - - netdev_linux_get_in4, - netdev_linux_set_in4, - netdev_linux_get_in6, - netdev_linux_add_router, - netdev_linux_get_next_hop, - netdev_linux_arp_lookup, - - netdev_linux_update_flags, - - netdev_linux_poll_add, - netdev_linux_poll_remove, -}; - -const struct netdev_class netdev_tap_class = { - "tap", - - netdev_linux_init, - netdev_linux_run, - netdev_linux_wait, - - netdev_linux_create_tap, - netdev_linux_destroy, - NULL, /* reconfigure */ - - netdev_linux_open, - netdev_linux_close, - - NULL, /* enumerate */ - - netdev_linux_recv, - netdev_linux_recv_wait, - netdev_linux_drain, - - netdev_linux_send, - netdev_linux_send_wait, - - netdev_linux_set_etheraddr, - netdev_linux_get_etheraddr, - netdev_linux_get_mtu, - netdev_linux_get_ifindex, - netdev_linux_get_carrier, - netdev_linux_get_stats, - NULL, /* set_stats */ - - netdev_linux_get_features, - netdev_linux_set_advertisements, - netdev_linux_get_vlan_vid, - - netdev_linux_set_policing, - netdev_linux_get_qos_types, - netdev_linux_get_qos_capabilities, - netdev_linux_get_qos, - netdev_linux_set_qos, - netdev_linux_get_queue, - netdev_linux_set_queue, - netdev_linux_delete_queue, - netdev_linux_get_queue_stats, - netdev_linux_dump_queues, - netdev_linux_dump_queue_stats, - - netdev_linux_get_in4, - netdev_linux_set_in4, - netdev_linux_get_in6, - netdev_linux_add_router, - netdev_linux_get_next_hop, - netdev_linux_arp_lookup, - - netdev_linux_update_flags, - - netdev_linux_poll_add, - netdev_linux_poll_remove, -}; +#define NETDEV_LINUX_CLASS(NAME, CREATE, ENUMERATE, SET_STATS) \ +{ \ + NAME, \ + \ + netdev_linux_init, \ + netdev_linux_run, \ + netdev_linux_wait, \ + \ + CREATE, \ + netdev_linux_destroy, \ + NULL, /* reconfigure */ \ + \ + netdev_linux_open, \ + netdev_linux_close, \ + \ + ENUMERATE, \ + \ + netdev_linux_recv, \ + netdev_linux_recv_wait, \ + netdev_linux_drain, \ + \ + netdev_linux_send, \ + netdev_linux_send_wait, \ + \ + netdev_linux_set_etheraddr, \ + netdev_linux_get_etheraddr, \ + netdev_linux_get_mtu, \ + netdev_linux_get_ifindex, \ + netdev_linux_get_carrier, \ + netdev_linux_get_stats, \ + SET_STATS, \ + \ + netdev_linux_get_features, \ + netdev_linux_set_advertisements, \ + netdev_linux_get_vlan_vid, \ + \ + netdev_linux_set_policing, \ + netdev_linux_get_qos_types, \ + netdev_linux_get_qos_capabilities, \ + netdev_linux_get_qos, \ + netdev_linux_set_qos, \ + netdev_linux_get_queue, \ + netdev_linux_set_queue, \ + netdev_linux_delete_queue, \ + netdev_linux_get_queue_stats, \ + netdev_linux_dump_queues, \ + netdev_linux_dump_queue_stats, \ + \ + netdev_linux_get_in4, \ + netdev_linux_set_in4, \ + netdev_linux_get_in6, \ + netdev_linux_add_router, \ + netdev_linux_get_next_hop, \ + netdev_linux_arp_lookup, \ + \ + netdev_linux_update_flags, \ + \ + netdev_linux_poll_add, \ + netdev_linux_poll_remove \ +} + +const struct netdev_class netdev_linux_class = + NETDEV_LINUX_CLASS( + "system", + netdev_linux_create, + netdev_linux_enumerate, + netdev_vport_set_stats); + +const struct netdev_class netdev_tap_class = + NETDEV_LINUX_CLASS( + "tap", + netdev_linux_create_tap, + NULL, /* enumerate */ + NULL); /* set_stats */ + +const struct netdev_class netdev_internal_class = + NETDEV_LINUX_CLASS( + "internal", + netdev_linux_create, + NULL, /* enumerate */ + netdev_vport_set_stats); /* HTB traffic control class. */ diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index 170136d4..d955bb17 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -552,8 +552,10 @@ struct netdev_class { int netdev_register_provider(const struct netdev_class *); int netdev_unregister_provider(const char *type); +const struct netdev_class *netdev_lookup_provider(const char *type); extern const struct netdev_class netdev_linux_class; +extern const struct netdev_class netdev_internal_class; extern const struct netdev_class netdev_tap_class; #ifdef __cplusplus diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 197e74bb..f0de376a 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -42,21 +42,17 @@ struct netdev_vport_notifier { struct netdev_dev_vport { struct netdev_dev netdev_dev; + uint64_t config[VPORT_CONFIG_SIZE / 8]; }; struct netdev_vport { struct netdev netdev; }; -struct vport_info { - const char *devname; - const char *type; - void *config; -}; - struct vport_class { - const struct netdev_class netdev_class; - int (*parse_config)(struct vport_info *port, const struct shash *args); + struct netdev_class netdev_class; + int (*parse_config)(const struct netdev_dev *, const struct shash *args, + void *config); }; static struct shash netdev_vport_notifiers = @@ -97,70 +93,39 @@ netdev_vport_cast(const struct netdev *netdev) return CONTAINER_OF(netdev, struct netdev_vport, netdev); } -static int -netdev_vport_parse_config(const struct netdev_class *netdev_class, - const char *name, const struct shash *args, - void **configp) +/* If 'netdev' is a vport netdev, copies its kernel configuration into + * 'config'. Otherwise leaves 'config' untouched. */ +void +netdev_vport_get_config(const struct netdev *netdev, void *config) { - const struct vport_class *c = vport_class_cast(netdev_class); - if (c->parse_config) { - struct vport_info info; - int error; - - info.devname = name; - info.type = netdev_class->type; - error = (c->parse_config)(&info, args); - *configp = error ? NULL : info.config; - return error; - } else { - if (!shash_is_empty(args)) { - VLOG_WARN("%s: arguments for %s vports should be empty", - name, netdev_class->type); - } - *configp = NULL; - return 0; + const struct netdev_dev *dev = netdev_get_dev(netdev); + + if (is_vport_class(netdev_dev_get_class(dev))) { + const struct netdev_dev_vport *vport = netdev_dev_vport_cast(dev); + memcpy(config, vport->config, VPORT_CONFIG_SIZE); } } static int -netdev_vport_create(const struct netdev_class *class, const char *name, - const struct shash *args, struct netdev_dev **netdev_devp) +netdev_vport_create(const struct netdev_class *netdev_class, const char *name, + const struct shash *args, + struct netdev_dev **netdev_devp) { - int err; - struct odp_vport_add ova; - struct netdev_dev_vport *netdev_dev; + const struct vport_class *vport_class = vport_class_cast(netdev_class); + struct netdev_dev_vport *dev; + int error; - ovs_strlcpy(ova.port_type, class->type, sizeof ova.port_type); - ovs_strlcpy(ova.devname, name, sizeof ova.devname); - err = netdev_vport_parse_config(class, name, args, &ova.config); - if (err) { - goto exit; - } - - err = netdev_vport_do_ioctl(ODP_VPORT_ADD, &ova); - - if (err == EBUSY) { - VLOG_WARN("%s: destroying existing device", name); + dev = xmalloc(sizeof *dev); + *netdev_devp = &dev->netdev_dev; + netdev_dev_init(&dev->netdev_dev, name, netdev_class); - err = netdev_vport_do_ioctl(ODP_VPORT_DEL, ova.devname); - if (err) { - goto exit; - } + memset(dev->config, 0, sizeof dev->config); + error = vport_class->parse_config(&dev->netdev_dev, args, dev->config); - err = netdev_vport_do_ioctl(ODP_VPORT_ADD, &ova); - } - if (err) { - goto exit; + if (error) { + netdev_dev_uninit(&dev->netdev_dev, true); } - - netdev_dev = xmalloc(sizeof *netdev_dev); - netdev_dev_init(&netdev_dev->netdev_dev, name, class); - - *netdev_devp = &netdev_dev->netdev_dev; - -exit: - free(ova.config); - return err; + return error; } static void @@ -168,8 +133,6 @@ netdev_vport_destroy(struct netdev_dev *netdev_dev_) { struct netdev_dev_vport *netdev_dev = netdev_dev_vport_cast(netdev_dev_); - netdev_vport_do_ioctl(ODP_VPORT_DEL, - (char *)netdev_dev_get_name(netdev_dev_)); free(netdev_dev); } @@ -194,23 +157,29 @@ netdev_vport_close(struct netdev *netdev_) } static int -netdev_vport_reconfigure(struct netdev_dev *netdev_dev, +netdev_vport_reconfigure(struct netdev_dev *dev_, const struct shash *args) { - const char *name = netdev_dev_get_name(netdev_dev); - struct odp_vport_mod ovm; - int err; - - ovs_strlcpy(ovm.devname, name, sizeof ovm.devname); - err = netdev_vport_parse_config(netdev_dev_get_class(netdev_dev), name, - args, &ovm.config); - if (err) { - return err; + const struct netdev_class *netdev_class = netdev_dev_get_class(dev_); + const struct vport_class *vport_class = vport_class_cast(netdev_class); + struct netdev_dev_vport *dev = netdev_dev_vport_cast(dev_); + struct odp_port port; + int error; + + 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); + error = vport_class->parse_config(dev_, args, port.config); + if (!error && memcmp(port.config, dev->config, sizeof dev->config)) { + error = netdev_vport_do_ioctl(ODP_VPORT_MOD, &port); + if (!error || error == ENODEV) { + /* Either reconfiguration succeeded or this vport is not installed + * in the kernel (e.g. it hasn't been added to a dpif yet with + * dpif_port_add()). */ + memcpy(dev->config, port.config, sizeof dev->config); + } } - - err = netdev_vport_do_ioctl(ODP_VPORT_MOD, &ovm); - free(ovm.config); - return err; + return error; } static int @@ -458,76 +427,77 @@ netdev_vport_poll_notify(const struct netdev *netdev) /* Code specific to individual vport types. */ static int -parse_tunnel_config(struct vport_info *port, const struct shash *args) +parse_tunnel_config(const struct netdev_dev *dev, const struct shash *args, + void *configp) { - const char *name = port->devname; - bool is_gre = !strcmp(port->type, "gre"); - struct tnl_port_config *config; + const char *name = netdev_dev_get_name(dev); + const char *type = netdev_dev_get_type(dev); + bool is_gre = !strcmp(type, "gre"); + struct tnl_port_config config; struct shash_node *node; bool ipsec_ip_set = false; bool ipsec_mech_set = false; - config = port->config = xzalloc(sizeof *config); - config->flags |= TNL_F_PMTUD; - config->flags |= TNL_F_HDR_CACHE; + config.flags |= TNL_F_PMTUD; + config.flags |= TNL_F_HDR_CACHE; SHASH_FOR_EACH (node, args) { if (!strcmp(node->name, "remote_ip")) { struct in_addr in_addr; if (lookup_ip(node->data, &in_addr)) { - VLOG_WARN("%s: bad %s 'remote_ip'", name, port->type); + VLOG_WARN("%s: bad %s 'remote_ip'", name, type); } else { - config->daddr = in_addr.s_addr; + config.daddr = in_addr.s_addr; } } else if (!strcmp(node->name, "local_ip")) { struct in_addr in_addr; if (lookup_ip(node->data, &in_addr)) { - VLOG_WARN("%s: bad %s 'local_ip'", name, port->type); + VLOG_WARN("%s: bad %s 'local_ip'", name, type); } else { - config->saddr = in_addr.s_addr; + config.saddr = in_addr.s_addr; } } else if (!strcmp(node->name, "key") && is_gre) { if (!strcmp(node->data, "flow")) { - config->flags |= TNL_F_IN_KEY_MATCH; - config->flags |= TNL_F_OUT_KEY_ACTION; + config.flags |= TNL_F_IN_KEY_MATCH; + config.flags |= TNL_F_OUT_KEY_ACTION; } else { - config->out_key = config->in_key = htonl(atoi(node->data)); + config.out_key = config.in_key = htonl(atoi(node->data)); } } else if (!strcmp(node->name, "in_key") && is_gre) { if (!strcmp(node->data, "flow")) { - config->flags |= TNL_F_IN_KEY_MATCH; + config.flags |= TNL_F_IN_KEY_MATCH; } else { - config->in_key = htonl(atoi(node->data)); + config.in_key = htonl(atoi(node->data)); } } else if (!strcmp(node->name, "out_key") && is_gre) { if (!strcmp(node->data, "flow")) { - config->flags |= TNL_F_OUT_KEY_ACTION; + config.flags |= TNL_F_OUT_KEY_ACTION; } else { - config->out_key = htonl(atoi(node->data)); + config.out_key = htonl(atoi(node->data)); } } else if (!strcmp(node->name, "tos")) { if (!strcmp(node->data, "inherit")) { - config->flags |= TNL_F_TOS_INHERIT; + config.flags |= TNL_F_TOS_INHERIT; } else { - config->tos = atoi(node->data); + config.tos = atoi(node->data); } } else if (!strcmp(node->name, "ttl")) { if (!strcmp(node->data, "inherit")) { - config->flags |= TNL_F_TTL_INHERIT; + config.flags |= TNL_F_TTL_INHERIT; } else { - config->ttl = atoi(node->data); + config.ttl = atoi(node->data); } } else if (!strcmp(node->name, "csum") && is_gre) { if (!strcmp(node->data, "true")) { - config->flags |= TNL_F_CSUM; + config.flags |= TNL_F_CSUM; } } else if (!strcmp(node->name, "pmtud")) { if (!strcmp(node->data, "false")) { - config->flags &= ~TNL_F_PMTUD; + config.flags &= ~TNL_F_PMTUD; } } else if (!strcmp(node->name, "header_cache")) { if (!strcmp(node->data, "false")) { - config->flags &= ~TNL_F_HDR_CACHE; + config.flags &= ~TNL_F_HDR_CACHE; } } else if (!strcmp(node->name, "ipsec_local_ip")) { ipsec_ip_set = true; @@ -536,7 +506,7 @@ parse_tunnel_config(struct vport_info *port, const struct shash *args) ipsec_mech_set = true; } else { VLOG_WARN("%s: unknown %s argument '%s'", - name, port->type, node->name); + name, type, node->name); } } @@ -544,22 +514,25 @@ parse_tunnel_config(struct vport_info *port, const struct shash *args) * IPsec local IP address and authentication mechanism have been defined. */ if (ipsec_ip_set && ipsec_mech_set) { VLOG_INFO("%s: header caching disabled due to use of IPsec", name); - config->flags &= ~TNL_F_HDR_CACHE; + config.flags &= ~TNL_F_HDR_CACHE; } - if (!config->daddr) { + if (!config.daddr) { VLOG_WARN("%s: %s type requires valid 'remote_ip' argument", - name, port->type); + name, type); return EINVAL; } + BUILD_ASSERT(sizeof config <= VPORT_CONFIG_SIZE); + memcpy(configp, &config, sizeof config); return 0; } static int -parse_patch_config(struct vport_info *port, const struct shash *args) +parse_patch_config(const struct netdev_dev *dev, const struct shash *args, + void *configp) { - const char *name = port->devname; + const char *name = netdev_dev_get_name(dev); const char *peer; peer = shash_find_data(args, "peer"); @@ -573,7 +546,7 @@ parse_patch_config(struct vport_info *port, const struct shash *args) return EINVAL; } - if (strlen(peer) >= IFNAMSIZ) { + if (strlen(peer) >= MIN(IFNAMSIZ, VPORT_CONFIG_SIZE)) { VLOG_WARN("%s: patch 'peer' arg too long", name); return EINVAL; } @@ -583,7 +556,7 @@ parse_patch_config(struct vport_info *port, const struct shash *args) return EINVAL; } - port->config = xstrdup(peer); + strncpy(configp, peer, VPORT_CONFIG_SIZE); return 0; } @@ -645,19 +618,18 @@ parse_patch_config(struct vport_info *port, const struct shash *args) netdev_vport_poll_add, \ netdev_vport_poll_remove, -static const struct vport_class vport_gre_class - = { { "gre", VPORT_FUNCTIONS }, parse_tunnel_config }; - -static const struct vport_class vport_capwap_class - = { { "capwap", VPORT_FUNCTIONS }, parse_tunnel_config }; - -static const struct vport_class vport_patch_class - = { { "patch", VPORT_FUNCTIONS }, parse_patch_config }; - void netdev_vport_register(void) { - netdev_register_provider(&vport_gre_class.netdev_class); - netdev_register_provider(&vport_capwap_class.netdev_class); - netdev_register_provider(&vport_patch_class.netdev_class); + static const struct vport_class vport_classes[] = { + { { "gre", VPORT_FUNCTIONS }, parse_tunnel_config }, + { { "capwap", VPORT_FUNCTIONS }, parse_tunnel_config }, + { { "patch", VPORT_FUNCTIONS }, parse_patch_config } + }; + + int i; + + for (i = 0; i < ARRAY_SIZE(vport_classes); i++) { + netdev_register_provider(&vport_classes[i].netdev_class); + } } diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h index 0898ec0b..eb05a097 100644 --- a/lib/netdev-vport.h +++ b/lib/netdev-vport.h @@ -23,6 +23,9 @@ struct odp_port; struct shash; void netdev_vport_register(void); + +void netdev_vport_get_config(const struct netdev *, void *config); + int netdev_vport_get_stats(const struct netdev *, struct netdev_stats *); int netdev_vport_set_stats(struct netdev *, const struct netdev_stats *); diff --git a/lib/netdev.c b/lib/netdev.c index d108e9ca..c7de9064 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -75,6 +75,7 @@ netdev_initialize(void) #ifdef HAVE_NETLINK netdev_register_provider(&netdev_linux_class); + netdev_register_provider(&netdev_internal_class); netdev_register_provider(&netdev_tap_class); netdev_vport_register(); #endif @@ -167,6 +168,13 @@ netdev_unregister_provider(const char *type) return 0; } +const struct netdev_class * +netdev_lookup_provider(const char *type) +{ + netdev_initialize(); + return shash_find_data(&netdev_classes, type && type[0] ? type : "system"); +} + /* Clears 'types' and enumerates the types of all currently registered netdev * providers into it. The caller must first initialize the svec. */ void @@ -252,28 +260,6 @@ update_device_args(struct netdev_dev *dev, const struct shash *args) qsort(dev->args, dev->n_args, sizeof *dev->args, compare_args); } -static int -create_device(struct netdev_options *options, struct netdev_dev **netdev_devp) -{ - struct netdev_class *netdev_class; - int error; - - if (!options->type || strlen(options->type) == 0) { - /* Default to system. */ - options->type = "system"; - } - - netdev_class = shash_find_data(&netdev_classes, options->type); - if (!netdev_class) { - return EAFNOSUPPORT; - } - - error = netdev_class->create(netdev_class, options->name, options->args, - netdev_devp); - assert(error || (*netdev_devp)->netdev_class == netdev_class); - return error; -} - /* Opens the network device named 'name' (e.g. "eth0") and returns zero if * successful, otherwise a positive errno value. On success, sets '*netdevp' * to the new network device, otherwise to null. @@ -302,14 +288,20 @@ netdev_open(struct netdev_options *options, struct netdev **netdevp) netdev_dev = shash_find_data(&netdev_dev_shash, options->name); if (!netdev_dev) { - error = create_device(options, &netdev_dev); + const struct netdev_class *class; + + class = netdev_lookup_provider(options->type); + if (!class) { + VLOG_WARN("could not create netdev %s of unknown type %s", + options->name, options->type); + return EAFNOSUPPORT; + } + error = class->create(class, options->name, options->args, + &netdev_dev); if (error) { - if (error == EAFNOSUPPORT) { - VLOG_WARN("could not create netdev %s of unknown type %s", - options->name, options->type); - } return error; } + assert(netdev_dev->netdev_class == class); update_device_args(netdev_dev, options->args); } else if (!shash_is_empty(options->args) && diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 1c27ef24..7e83b0bf 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1465,6 +1465,7 @@ make_ofport(const struct odp_port *odp_port) memset(&netdev_options, 0, sizeof netdev_options); netdev_options.name = odp_port->devname; + netdev_options.type = odp_port->type; netdev_options.ethertype = NETDEV_ETH_TYPE_NONE; error = netdev_open(&netdev_options, &netdev); diff --git a/utilities/ovs-dpctl.8.in b/utilities/ovs-dpctl.8.in index cb9874d7..958b817e 100644 --- a/utilities/ovs-dpctl.8.in +++ b/utilities/ovs-dpctl.8.in @@ -21,10 +21,8 @@ host's other network devices. To intercept and process traffic on a given network device, use the \fBadd\-if\fR command to explicitly add that network device to the datapath. .PP -Do not use \fBovs\-dpctl\fR commands to modify datapaths if -\fBovs\-vswitchd\fR(8) is in use. Instead, modify the -\fBovs\-vswitchd\fR configuration file and send \fBSIGHUP\fR to the -\fBovs\-vswitchd\fR process. +If \fBovs\-vswitchd\fR(8) is in use, use \fBovs\-vsctl\fR(8) instead +of \fBovs\-dpctl\fR. .PP Most \fBovs\-dpctl\fR commands that work with datapaths take an argument that specifies the name of the datapath, in one of the following @@ -66,10 +64,14 @@ A \fInetdev\fR may be followed by a comma-separated list of options. The following options are currently supported: . .RS -.IP "\fBinternal\fR" -Instead of attaching an existing \fInetdev\fR, creates an internal -port (analogous to the local port) with that name. +.IP "\fBtype=\fItype\fR" +Specifies the type of port to add. The default type is \fBsystem\fR. +.IP "\fIkey\fB=\fIvalue\fR" +Adds an arbitrary key-value option to the port's configuration. .RE +.IP +\fBovs\-vswitchd.conf.db\fR(5) documents the available port types and +options. . .TP \fBdel\-if \fIdp netdev\fR... diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c index 04f6c316..bed50faa 100644 --- a/utilities/ovs-dpctl.c +++ b/utilities/ovs-dpctl.c @@ -37,6 +37,7 @@ #include "dynamic-string.h" #include "netdev.h" #include "odp-util.h" +#include "shash.h" #include "svec.h" #include "timeval.h" #include "util.h" @@ -238,45 +239,55 @@ do_add_if(int argc OVS_UNUSED, char *argv[]) run(parsed_dpif_open(argv[1], false, &dpif), "opening datapath"); for (i = 2; i < argc; i++) { char *save_ptr = NULL; - char *devname, *suboptions; - int flags = 0; + struct netdev_options options; + struct netdev *netdev; + struct shash args; + char *option; int error; - devname = strtok_r(argv[i], ",", &save_ptr); - if (!devname) { + options.name = strtok_r(argv[i], ",", &save_ptr); + options.type = "system"; + options.args = &args; + options.ethertype = NETDEV_ETH_TYPE_NONE; + + if (!options.name) { ovs_error(0, "%s is not a valid network device name", argv[i]); continue; } - suboptions = strtok_r(NULL, "", &save_ptr); - if (suboptions) { - enum { - AP_INTERNAL - }; - static char *options[] = { - "internal" - }; - - while (*suboptions != '\0') { - char *value; - - switch (getsubopt(&suboptions, options, &value)) { - case AP_INTERNAL: - flags |= ODP_PORT_INTERNAL; - break; - - default: - ovs_error(0, "unknown suboption '%s'", value); - break; - } + shash_init(&args); + while ((option = strtok_r(NULL, "", &save_ptr)) != NULL) { + char *save_ptr_2 = NULL; + char *key, *value; + + key = strtok_r(option, "=", &save_ptr_2); + value = strtok_r(NULL, "", &save_ptr_2); + if (!value) { + value = ""; + } + + if (!strcmp(key, "type")) { + options.type = value; + } else if (!shash_add_once(&args, key, value)) { + ovs_error(0, "duplicate \"%s\" option", key); } } - error = dpif_port_add(dpif, devname, flags, NULL); + error = netdev_open(&options, &netdev); + if (error) { + ovs_error(error, "%s: failed to open network device", + options.name); + } else { + error = dpif_port_add(dpif, netdev, NULL); + if (error) { + ovs_error(error, "adding %s to %s failed", + options.name, argv[1]); + } else { + error = if_up(options.name); + } + netdev_close(netdev); + } if (error) { - ovs_error(error, "adding %s to %s failed", devname, argv[1]); - failure = true; - } else if (if_up(devname)) { failure = true; } } @@ -363,9 +374,11 @@ show_dpif(struct dpif *dpif) } query_ports(dpif, &ports, &n_ports); for (i = 0; i < n_ports; i++) { - printf("\tport %u: %s", ports[i].port, ports[i].devname); - if (ports[i].flags & ODP_PORT_INTERNAL) { - printf(" (internal)"); + const struct odp_port *p = &ports[i]; + + printf("\tport %u: %s", p->port, p->devname); + if (strcmp(p->type, "system")) { + printf(" (%s)", p->type); } printf("\n"); } diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c index 153267f3..0cd919a6 100644 --- a/utilities/ovs-openflowd.c +++ b/utilities/ovs-openflowd.c @@ -128,10 +128,19 @@ main(int argc, char *argv[]) size_t i; SVEC_FOR_EACH (i, port, &s.ports) { - error = dpif_port_add(dpif, port, 0, NULL); + struct netdev *netdev; + + error = netdev_open_default(port, &netdev); + if (error) { + ovs_fatal(error, "%s: failed to open network device", port); + } + + error = dpif_port_add(dpif, netdev, NULL); if (error) { ovs_fatal(error, "failed to add %s as a port", port); } + + netdev_close(netdev); } } diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index bf34535a..9916414d 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -376,10 +376,7 @@ bridge_configure_once(const struct ovsrec_open_vswitch *cfg) /* Initializes 'options' and fills it with the options for 'if_cfg'. Merges * keys from "options" and "other_config", preferring "options" keys over - * "other_config" keys. - * - * The value strings in '*options' are taken directly from if_cfg, not copied, - * so the caller should not modify or free them. */ + * "other_config" keys. */ static void iface_get_options(const struct ovsrec_interface *if_cfg, struct shash *options) { @@ -401,68 +398,6 @@ iface_get_options(const struct ovsrec_interface *if_cfg, struct shash *options) } } -/* Returns the type of network device that 'iface' should have. (This is - * ordinarily the same type as the interface, but the network devices for - * "internal" ports have type "system".) */ -static const char * -iface_get_netdev_type(const struct iface *iface) -{ - return !strcmp(iface->type, "internal") ? "system" : iface->type; -} - -/* Attempt to create the network device for 'iface' through the netdev - * library. */ -static int -create_iface_netdev(struct iface *iface) -{ - struct netdev_options netdev_options; - struct shash options; - int error; - - memset(&netdev_options, 0, sizeof netdev_options); - netdev_options.name = iface->cfg->name; - netdev_options.type = iface_get_netdev_type(iface); - netdev_options.args = &options; - netdev_options.ethertype = NETDEV_ETH_TYPE_NONE; - - iface_get_options(iface->cfg, &options); - - error = netdev_open(&netdev_options, &iface->netdev); - - if (iface->netdev) { - iface->enabled = netdev_get_carrier(iface->netdev); - } - - shash_destroy(&options); - - return error; -} - -static int -reconfigure_iface_netdev(struct iface *iface) -{ - const char *netdev_type, *iface_type; - struct shash options; - int error; - - /* Skip reconfiguration if the device has the wrong type. This shouldn't - * happen, but... */ - iface_type = iface_get_netdev_type(iface); - netdev_type = netdev_get_type(iface->netdev); - if (iface_type && strcmp(netdev_type, iface_type)) { - VLOG_WARN("%s: attempting change device type from %s to %s", - iface->cfg->name, netdev_type, iface_type); - return EINVAL; - } - - /* Reconfigure device. */ - iface_get_options(iface->cfg, &options); - error = netdev_reconfigure(iface->netdev, &options); - shash_destroy(&options); - - return error; -} - /* Callback for iterate_and_prune_ifaces(). */ static bool check_iface(struct bridge *br, struct iface *iface, void *aux OVS_UNUSED) @@ -701,18 +636,16 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) SHASH_FOR_EACH (node, &want_ifaces) { const char *if_name = node->name; struct iface *iface = node->data; - bool internal = !iface || !strcmp(iface->type, "internal"); struct odp_port *dpif_port = shash_find_data(&cur_ifaces, if_name); + const char *type = iface ? iface->type : "internal"; int error; /* If we have a port or a netdev already, and it's not the type we * want, then delete the port (if any) and close the netdev (if * any). */ - if (internal - ? dpif_port && !(dpif_port->flags & ODP_PORT_INTERNAL) - : (iface->netdev - && strcmp(iface->type, netdev_get_type(iface->netdev)))) - { + if ((dpif_port && strcmp(dpif_port->type, type)) + || (iface && iface->netdev + && strcmp(type, netdev_get_type(iface->netdev)))) { if (dpif_port) { error = ofproto_port_del(br->ofproto, dpif_port->port); if (error) { @@ -726,48 +659,62 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg) } } - /* If it's not an internal port, open (possibly create) the - * netdev. */ - if (!internal) { - if (!iface->netdev) { - error = create_iface_netdev(iface); - if (error) { - VLOG_WARN("could not create iface %s: %s", iface->name, - strerror(error)); - continue; - } - } else { - reconfigure_iface_netdev(iface); + /* If the port doesn't exist or we don't have the netdev open, + * we need to do more work. */ + if (!dpif_port || (iface && !iface->netdev)) { + struct netdev_options options; + struct netdev *netdev; + struct shash args; + + /* First open the network device. */ + options.name = if_name; + options.type = type; + options.args = &args; + options.ethertype = NETDEV_ETH_TYPE_NONE; + + shash_init(&args); + if (iface) { + iface_get_options(iface->cfg, &args); } - } + error = netdev_open(&options, &netdev); + shash_destroy(&args); - /* If it's not part of the datapath, add it. */ - if (!dpif_port) { - error = dpif_port_add(br->dpif, if_name, - internal ? ODP_PORT_INTERNAL : 0, NULL); - if (error == EFBIG) { - VLOG_ERR("ran out of valid port numbers on %s", - dpif_name(br->dpif)); - break; - } else if (error) { - VLOG_ERR("failed to add %s interface to %s: %s", - if_name, dpif_name(br->dpif), strerror(error)); + if (error) { + VLOG_WARN("could not open network device %s (%s)", + if_name, strerror(error)); continue; } - } - /* If it's an internal port, open the netdev. */ - if (internal) { - if (iface && !iface->netdev) { - error = create_iface_netdev(iface); + /* Then add the port if we haven't already. */ + if (!dpif_port) { + error = dpif_port_add(br->dpif, netdev, NULL); if (error) { - VLOG_WARN("could not create iface %s: %s", iface->name, - strerror(error)); - continue; + netdev_close(netdev); + if (error == EFBIG) { + VLOG_ERR("ran out of valid port numbers on %s", + dpif_name(br->dpif)); + break; + } else { + VLOG_ERR("failed to add %s interface to %s: %s", + if_name, dpif_name(br->dpif), + strerror(error)); + continue; + } } } - } else { - assert(iface->netdev != NULL); + + /* Update 'iface'. */ + if (iface) { + iface->netdev = netdev; + iface->enabled = netdev_get_carrier(iface->netdev); + } + } else if (iface && iface->netdev) { + struct shash args; + + shash_init(&args); + iface_get_options(iface->cfg, &args); + netdev_reconfigure(iface->netdev, &args); + shash_destroy(&args); } } free(dpif_ports);