Oops the patch series is probably the reason.
> -----Original Message----- > From: Ilya Maximets [mailto:[email protected]] > Sent: Friday, May 26, 2017 2:51 PM > To: O Mahony, Billy <[email protected]>; [email protected]; > Daniele Di Proietto <[email protected]>; Darrell Ball > <[email protected]> > Cc: Heetae Ahn <[email protected]> > Subject: Re: [ovs-dev] [PATCH v4 2/3] netdev-dpdk: Fix device leak on port > deletion. > > On 26.05.2017 16:38, O Mahony, Billy wrote: > > Hi Ilya, > > > > This patch does not apply to head of master, currently "c899576 build- > windows: cccl fail compilation on Wimplicit-function-declaration". > > Hmm. I'm able to apply it using 'git am'. Have you applied first patch of this > series? > > > 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. > > Originally, these patches are made on top of > 126fb3e8abc2 ("ovn-ctl: Start ovn-northd even if ovsdb-servers are not > running") but they are still applicable on top of current master. > > Bets regards, Ilya Maximets. > > > > > 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
