mir/ovs
2
0
Fork 0
mirror of https://github.com/openvswitch/ovs synced 2025-08-31 14:25:26 +00:00
Code Issues Releases Wiki Activity

netdev-dpdk: Fix device leak on port deletion.

Browse Source 
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 <ciara.loftus@intel.com>
Fixes: 55e075e65e ("netdev-dpdk: Arbitrary 'dpdk' port naming")
Fixes: 69876ed786 ("netdev-dpdk: Add support for virtual DPDK PMDs (vdevs)")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Billy O'Mahony <billy.o.mahony@intel.com>
This commit is contained in:
Ilya Maximets
2017-05-19 16:37:31 +03:00
committed by Ben Pfaff
parent 03d6399e61
commit 5dcde09c80
2 changed files with 22 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
Documentation/howto/dpdk.rst
View File

@@ -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

72
lib/netdev-dpdk.c
View File

@@ -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);
}