Hi Ilya, This patch does not apply to head of master, currently "c899576 build-windows: cccl fail compilation on Wimplicit-function-declaration".
I'll don't have any comments on the code right now but if you can tell me the commit it's based on I'll check it out. Thanks, Billy > -----Original Message----- > From: [email protected] [mailto:ovs-dev- > [email protected]] On Behalf Of Ilya Maximets > Sent: Friday, May 19, 2017 2:38 PM > To: [email protected]; Daniele Di Proietto <[email protected]>; > Darrell Ball <[email protected]> > Cc: Ilya Maximets <[email protected]>; Heetae Ahn > <[email protected]> > Subject: [ovs-dev] [PATCH v4 2/3] netdev-dpdk: Fix device leak on port > deletion. > > Currently, once created device in dpdk will exist forever even after del-port > operation untill we manually call 'ovs-appctl netdev-dpdk/detach <name>', > where <name> is not the port's name but the name of dpdk eth device or pci > address. > > Few issues with current implementation: > > 1. Different API for usual (system) and DPDK devices. > (We have to call 'ovs-appctl netdev-dpdk/detach' each > time after 'del-port' to actually free the device) > This is a big issue mostly for virtual DPDK devices. > > 2. Follows from 1: > For DPDK devices 'del-port' leads just to > 'rte_eth_dev_stop' and subsequent 'add-port' will > just start the already existing device. Such behaviour > will not reset the device to initial state as it could > be expected. For example: virtual pcap pmd will continue > reading input file instead of reading it from the beginning. > > 3. Follows from 2: > After execution of the following commands 'port1' will be > configured with the 'old-options' while 'ovs-vsctl show' > will show us 'new-options' in dpdk-devargs field: > > ovs-vsctl add-port port1 -- set interface port1 type=dpdk \ > options:dpdk-devargs=<eth_pmd_name1>,<old-options> > ovs-vsctl del-port port1 > ovs-vsctl add-port port1 -- set interface port1 type=dpdk \ > options:dpdk-devargs=<eth_pmd_name1>,<new-options> > > 4. Follows from 1: > Not detached device consumes 'port_id'. Since we have very > limited number of 'port_id's (32 in common case) this may > lead to quick exhausting of id pool and inability to add any > other port. > > To avoid above issues we need to detach all the attached devices on port > destruction. > appctl 'netdev-dpdk/detach' removed because not needed anymore. > > We need to use internal 'attached' variable to track ports on which > rte_eth_dev_attach() was called and returned successfully to avoid closing > and detaching devices that do not support hotplug or by any other reason > attached using the 'dpdk-extra' cmdline options. > > CC: Ciara Loftus <[email protected]> > Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming") > Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs > (vdevs)") > Signed-off-by: Ilya Maximets <[email protected]> > --- > Documentation/howto/dpdk.rst | 5 ++- > lib/netdev-dpdk.c | 72 > ++++++++++++-------------------------------- > 2 files changed, 22 insertions(+), 55 deletions(-) > > diff --git a/Documentation/howto/dpdk.rst > b/Documentation/howto/dpdk.rst index 3bd9e07..7c06239 100644 > --- a/Documentation/howto/dpdk.rst > +++ b/Documentation/howto/dpdk.rst > @@ -342,10 +342,9 @@ Then it can be attached to OVS:: > $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx type=dpdk \ > options:dpdk-devargs=0000:01:00.0 > > -It is also possible to detach a port from ovs, the user has to remove the > -port > using the del-port command, then it can be detached using:: > +Detaching will be performed while processing del-port command:: > > - $ ovs-appctl netdev-dpdk/detach 0000:01:00.0 > + $ ovs-vsctl del-port dpdkx > > This feature is not supported with VFIO and does not work with some NICs. > For more information please refer to the `DPDK Port Hotplug Framework diff > --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 1586e41..a41679f 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -360,6 +360,9 @@ struct netdev_dpdk { > /* Device arguments for dpdk ports */ > char *devargs; > > + /* If true, device was attached by rte_eth_dev_attach(). */ > + bool attached; > + > /* In dpdk_list. */ > struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); > > @@ -853,6 +856,7 @@ common_construct(struct netdev *netdev, unsigned > int port_no, > dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu); > ovsrcu_index_init(&dev->vid, -1); > dev->vhost_reconfigured = false; > + dev->attached = false; > > ovsrcu_init(&dev->qos_conf, NULL); > > @@ -998,10 +1002,21 @@ static void > netdev_dpdk_destruct(struct netdev *netdev) { > struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); > + char devname[RTE_ETH_NAME_MAX_LEN]; > > ovs_mutex_lock(&dpdk_mutex); > > rte_eth_dev_stop(dev->port_id); > + > + if (dev->attached) { > + rte_eth_dev_close(dev->port_id); > + if (rte_eth_dev_detach(dev->port_id, devname) < 0) { > + VLOG_ERR("Device '%s' can not be detached", dev->devargs); > + } else { > + VLOG_INFO("Device '%s' detached", devname); > + } > + } > + > free(dev->devargs); > common_destruct(dev); > > @@ -1113,7 +1128,8 @@ netdev_dpdk_lookup_by_port_id(int port_id) } > > static int > -netdev_dpdk_process_devargs(const char *devargs, char **errp) > +netdev_dpdk_process_devargs(struct netdev_dpdk *dev, > + const char *devargs, char **errp) > { > /* Get the name up to the first comma. */ > char *name = xmemdup0(devargs, strcspn(devargs, ",")); @@ -1125,6 > +1141,7 @@ netdev_dpdk_process_devargs(const char *devargs, char > **errp) > /* Device not found in DPDK, attempt to attach it */ > if (!rte_eth_dev_attach(devargs, &new_port_id)) { > /* Attach successful */ > + dev->attached = true; > VLOG_INFO("Device '%s' attached to DPDK", devargs); > } else { > /* Attach unsuccessful */ > @@ -1211,7 +1228,8 @@ netdev_dpdk_set_config(struct netdev *netdev, > const struct smap *args, > * is valid */ > if (!(dev->devargs && !strcmp(dev->devargs, new_devargs) > && rte_eth_dev_is_valid_port(dev->port_id))) { > - int new_port_id = netdev_dpdk_process_devargs(new_devargs, > errp); > + int new_port_id = netdev_dpdk_process_devargs(dev, > new_devargs, > + errp); > if (!rte_eth_dev_is_valid_port(new_port_id)) { > err = EINVAL; > } else if (new_port_id == dev->port_id) { @@ -2438,53 +2456,6 @@ > netdev_dpdk_set_admin_state(struct unixctl_conn *conn, int argc, > unixctl_command_reply(conn, "OK"); > } > > -static void > -netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED, > - const char *argv[], void *aux OVS_UNUSED) > -{ > - int ret; > - char *response; > - uint8_t port_id; > - char devname[RTE_ETH_NAME_MAX_LEN]; > - struct netdev_dpdk *dev; > - > - ovs_mutex_lock(&dpdk_mutex); > - > - if (!rte_eth_dev_count() || rte_eth_dev_get_port_by_name(argv[1], > - &port_id)) { > - response = xasprintf("Device '%s' not found in DPDK", argv[1]); > - goto error; > - } > - > - dev = netdev_dpdk_lookup_by_port_id(port_id); > - if (dev) { > - response = xasprintf("Device '%s' is being used by interface '%s'. " > - "Remove it before detaching", > - argv[1], netdev_get_name(&dev->up)); > - goto error; > - } > - > - rte_eth_dev_close(port_id); > - > - ret = rte_eth_dev_detach(port_id, devname); > - if (ret < 0) { > - response = xasprintf("Device '%s' can not be detached", argv[1]); > - goto error; > - } > - > - response = xasprintf("Device '%s' has been detached", argv[1]); > - > - ovs_mutex_unlock(&dpdk_mutex); > - unixctl_command_reply(conn, response); > - free(response); > - return; > - > -error: > - ovs_mutex_unlock(&dpdk_mutex); > - unixctl_command_reply_error(conn, response); > - free(response); > -} > - > /* > * Set virtqueue flags so that we do not receive interrupts. > */ > @@ -2760,9 +2731,6 @@ netdev_dpdk_class_init(void) > unixctl_command_register("netdev-dpdk/set-admin-state", > "[netdev] up|down", 1, 2, > netdev_dpdk_set_admin_state, NULL); > - unixctl_command_register("netdev-dpdk/detach", > - "pci address of device", 1, 1, > - netdev_dpdk_detach, NULL); > > ovsthread_once_done(&once); > } > -- > 2.7.4 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
