[dpdk-stable] patch 'net/tap: let kernel choose tun device name' has been queued to LTS release 18.11.1
Stephen Hemminger
stephen at networkplumber.org
Wed Feb 6 18:45:39 CET 2019
On Fri, 1 Feb 2019 18:28:52 +0800
<hfli at netitest.com> wrote:
> Hi Guys,
>
>
>
> We have gotten a weird problem after we upgrade DPDK version from 18.05.1 to
> 18.11, we need call a expect script in C code by system() which execute some
> commands on our network devices.
>
>
>
> But it will be crashed after call function rte_eal_memory_init(), I have
> enclosed the expect script and crash output in attachment. Can anyone dig
> into this?
>
>
>
> spawn telnet 192.168.16.230
>
> *** buffer overflow detected ***: /usr/bin/expect terminated
>
> ======= Backtrace: =========
>
> /lib64/libc.so.6(__fortify_fail+0x37)[0x7f033f6759e7]
>
> /lib64/libc.so.6(+0x115b62)[0x7f033f673b62]
>
> /lib64/libc.so.6(+0x117947)[0x7f033f675947]
>
> /lib64/libtcl8.5.so(+0xe37d8)[0x7f033ff147d8]
>
> /lib64/libpthread.so.0(+0x7dd5)[0x7f033f146dd5]
>
> /lib64/libc.so.6(clone+0x6d)[0x7f033f65bead]
>
>
>
>
>
>
>
>
>
> Thanks and Regards,
>
> Haifeng
>
>
>
> -----邮件原件-----
>
> 发件人: Kevin Traynor <ktraynor at redhat.com>
>
> 发送时间: 2019年1月31日 23:49
>
> 收件人: Stephen Hemminger <stephen at networkplumber.org>
>
> 抄送: Haifeng Li <hfli at netitest.com>; dpdk stable <stable at dpdk.org>
>
> 主题: patch 'net/tap: let kernel choose tun device name' has been queued to
> LTS release 18.11.1
>
>
>
> Hi,
>
>
>
> FYI, your patch has been queued to LTS release 18.11.1
>
>
>
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
>
> It will be pushed if I get no objections before 02/07/19. So please shout if
> anyone has objections.
>
>
>
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for
> rebasing
>
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
>
>
>
> Thanks.
>
>
>
> Kevin Traynor
>
>
>
> ---
>
> From d3034e04e58ca7bed78182fa9a1439afe4a73d4b Mon Sep 17 00:00:00 2001
>
> From: Stephen Hemminger <stephen at networkplumber.org>
>
> Date: Fri, 11 Jan 2019 12:35:18 -0800
>
> Subject: [PATCH] net/tap: let kernel choose tun device name
>
>
>
> [ upstream commit b5235d61f3b7d2b1c2b9ee2601f3aca2b151b508 ]
>
>
>
> Assigning tun and tap index in DPDK tap device driver is racy and fails if
> used with primary/secondary. Instead use the kernel feature of device
> wildcarding where if a name with %d is used the kernel will fill in the next
> available device.
>
>
>
> Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
>
>
>
> Reported-by: Haifeng Li <hfli at netitest.com>
>
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org> Acked-by Keith
> Wiles <keith.wiles at intel.com>
>
> ---
>
> drivers/net/tap/rte_eth_tap.c | 33 +++++++++++++++++----------------
>
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
>
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index aef8b0b9f..a93429973 100644
>
> --- a/drivers/net/tap/rte_eth_tap.c
>
> +++ b/drivers/net/tap/rte_eth_tap.c
>
> @@ -79,7 +79,4 @@ static const char *valid_arguments[] = { };
>
> -static unsigned int tap_unit;
>
> -static unsigned int tun_unit;
>
> -
>
> static char tuntap_name[8];
>
> @@ -151,6 +148,4 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
>
> snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
>
> - TAP_LOG(DEBUG, "ifr_name '%s'", ifr.ifr_name);
>
> -
>
> fd = open(TUN_TAP_DEV_PATH, O_RDWR);
>
> if (fd < 0) {
>
> @@ -186,4 +181,11 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
>
> }
>
> + /*
>
> + * Name passed to kernel might be wildcard like dtun%d
>
> + * and need to find the resulting device.
>
> + */
>
> + TAP_LOG(DEBUG, "Device name is '%s'", ifr.ifr_name);
>
> + strlcpy(pmd->name, ifr.ifr_name, RTE_ETH_NAME_MAX_LEN);
>
> +
>
> if (is_keepalive) {
>
> /*
>
> @@ -1756,4 +1758,5 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char
> *tap_name,
>
> goto error_exit;
>
> }
>
> + TAP_LOG(DEBUG, "allocated %s", pmd->name);
>
> ifr.ifr_mtu = dev->data->mtu;
>
> @@ -1895,6 +1898,6 @@ set_interface_name(const char *key __rte_unused,
>
> strlcpy(name, value, RTE_ETH_NAME_MAX_LEN);
>
> else
>
> - snprintf(name, RTE_ETH_NAME_MAX_LEN, "%s%d",
>
> - DEFAULT_TAP_NAME, tap_unit - 1);
>
> + /* use tap%d which causes kernel to choose next available
> */
>
> + strlcpy(name, DEFAULT_TAP_NAME "%d", RTE_ETH_NAME_MAX_LEN);
>
> return 0;
>
> @@ -2003,6 +2006,6 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>
> }
>
> - snprintf(tun_name, sizeof(tun_name), "%s%u",
>
> - DEFAULT_TUN_NAME, tun_unit++);
>
> + /* use tun%d which causes kernel to choose next available */
>
> + strlcpy(tun_name, DEFAULT_TUN_NAME "%d", RTE_ETH_NAME_MAX_LEN);
>
> if (params && (params[0] != '\0')) {
>
> @@ -2024,9 +2027,8 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>
> pmd_link.link_speed = ETH_SPEED_NUM_10G;
>
> - TAP_LOG(NOTICE, "Initializing pmd_tun for %s as %s",
>
> - name, tun_name);
>
> + TAP_LOG(NOTICE, "Initializing pmd_tun for %s", name);
>
> ret = eth_dev_tap_create(dev, tun_name, remote_iface, 0,
>
> - ETH_TUNTAP_TYPE_TUN);
>
> + ETH_TUNTAP_TYPE_TUN);
>
> leave:
>
> @@ -2034,5 +2036,4 @@ leave:
>
> TAP_LOG(ERR, "Failed to create pmd for %s as %s",
>
> name, tun_name);
>
> - tun_unit--; /* Restore the unit number */
>
> }
>
> rte_kvargs_free(kvlist);
>
> @@ -2190,6 +2191,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>
> speed = ETH_SPEED_NUM_10G;
>
> - snprintf(tap_name, sizeof(tap_name), "%s%u",
>
> - DEFAULT_TAP_NAME, tap_unit++);
>
> +
>
> + /* use tap%d which causes kernel to choose next available */
>
> + strlcpy(tap_name, DEFAULT_TAP_NAME "%d", RTE_ETH_NAME_MAX_LEN);
>
> memset(remote_iface, 0, RTE_ETH_NAME_MAX_LEN);
>
> @@ -2255,5 +2257,4 @@ leave:
>
> tap_devices_count--;
>
> }
>
> - tap_unit--; /* Restore the unit number */
>
> }
>
> rte_kvargs_free(kvlist);
>
> --
>
> 2.19.0
>
>
>
> ---
>
> Diff of the applied patch vs upstream commit (please double-check if
> non-empty:
>
> ---
>
> --- - 2019-01-31 15:44:06.948575581 +0000
>
> +++ 0052-net-tap-let-kernel-choose-tun-device-name.patch 2019-01-31
> 15:44:05.000000000 +0000
>
> @@ -1,15 +1,16 @@
>
> -From b5235d61f3b7d2b1c2b9ee2601f3aca2b151b508 Mon Sep 17 00:00:00 2001
>
> +From d3034e04e58ca7bed78182fa9a1439afe4a73d4b Mon Sep 17 00:00:00 2001
>
> From: Stephen Hemminger <stephen at networkplumber.org>
>
> Date: Fri, 11 Jan 2019 12:35:18 -0800
>
> Subject: [PATCH] net/tap: let kernel choose tun device name
>
> +[ upstream commit b5235d61f3b7d2b1c2b9ee2601f3aca2b151b508 ]
>
> +
>
> Assigning tun and tap index in DPDK tap device driver is racy and fails if
> used with primary/secondary. Instead use the kernel feature of device
> wildcarding where if a name with %d is used the kernel will fill in the
> next available device.
>
> Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD")
>
> -Cc: stable at dpdk.org
>
> Reported-by: Haifeng Li <hfli at netitest.com>
>
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org> @@ -19,10 +20,
> 10 @@
>
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> -index d699b4fb0..ed8483cb9 100644
>
> +index aef8b0b9f..a93429973 100644
>
> --- a/drivers/net/tap/rte_eth_tap.c
>
> +++ b/drivers/net/tap/rte_eth_tap.c
>
> -@@ -80,7 +80,4 @@ static const char *valid_arguments[] = {
>
> +@@ -79,7 +79,4 @@ static const char *valid_arguments[] = {
>
> };
>
>
>
> -static unsigned int tap_unit;
>
> @@ -30,14 +31,14 @@
>
> -
>
> static char tuntap_name[8];
>
>
>
> -@@ -152,6 +149,4 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
>
> - strlcpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
>
> +@@ -151,6 +148,4 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
>
> + snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
>
>
>
> - TAP_LOG(DEBUG, "ifr_name '%s'", ifr.ifr_name);
>
> -
>
> fd = open(TUN_TAP_DEV_PATH, O_RDWR);
>
> if (fd < 0) {
>
> -@@ -187,4 +182,11 @@ tun_alloc(struct pmd_internals *pmd, int is_keepalive)
>
> +@@ -186,4 +181,11 @@ tun_alloc(struct pmd_internals *pmd, int
>
> +is_keepalive)
>
> }
>
>
>
> + /*
>
> @@ -49,22 +50,22 @@
>
> +
>
> if (is_keepalive) {
>
> /*
>
> -@@ -1757,4 +1759,5 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char
> *tap_name,
>
> +@@ -1756,4 +1758,5 @@ eth_dev_tap_create(struct rte_vdev_device *vdev,
>
> +char *tap_name,
>
> goto error_exit;
>
> }
>
> + TAP_LOG(DEBUG, "allocated %s", pmd->name);
>
>
>
> ifr.ifr_mtu = dev->data->mtu;
>
> -@@ -1919,6 +1922,6 @@ set_interface_name(const char *key __rte_unused,
>
> +@@ -1895,6 +1898,6 @@ set_interface_name(const char *key __rte_unused,
>
> strlcpy(name, value, RTE_ETH_NAME_MAX_LEN);
>
> - } else {
>
> + else
>
> - snprintf(name, RTE_ETH_NAME_MAX_LEN, "%s%d",
>
> - DEFAULT_TAP_NAME, tap_unit - 1);
>
> + /* use tap%d which causes kernel to choose next available */
>
> + strlcpy(name, DEFAULT_TAP_NAME "%d", RTE_ETH_NAME_MAX_LEN);
>
> - }
>
> +
>
> return 0;
>
> -@@ -2033,6 +2036,6 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>
> +@@ -2003,6 +2006,6 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>
> }
>
>
>
> - snprintf(tun_name, sizeof(tun_name), "%s%u",
>
> @@ -73,25 +74,25 @@
>
> + strlcpy(tun_name, DEFAULT_TUN_NAME "%d", RTE_ETH_NAME_MAX_LEN);
>
>
>
> if (params && (params[0] != '\0')) {
>
> -@@ -2054,9 +2057,8 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>
> +@@ -2024,9 +2027,8 @@ rte_pmd_tun_probe(struct rte_vdev_device *dev)
>
> pmd_link.link_speed = ETH_SPEED_NUM_10G;
>
>
>
> -- TAP_LOG(DEBUG, "Initializing pmd_tun for %s as %s",
>
> +- TAP_LOG(NOTICE, "Initializing pmd_tun for %s as %s",
>
> - name, tun_name);
>
> -+ TAP_LOG(DEBUG, "Initializing pmd_tun for %s", name);
>
> ++ TAP_LOG(NOTICE, "Initializing pmd_tun for %s", name);
>
>
>
> ret = eth_dev_tap_create(dev, tun_name, remote_iface, 0,
>
> - ETH_TUNTAP_TYPE_TUN);
>
> + ETH_TUNTAP_TYPE_TUN);
>
>
>
> leave:
>
> -@@ -2064,5 +2066,4 @@ leave:
>
> +@@ -2034,5 +2036,4 @@ leave:
>
> TAP_LOG(ERR, "Failed to create pmd for %s as %s",
>
> name, tun_name);
>
> - tun_unit--; /* Restore the unit number */
>
> }
>
> rte_kvargs_free(kvlist);
>
> -@@ -2220,6 +2221,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>
> +@@ -2190,6 +2191,7 @@ rte_pmd_tap_probe(struct rte_vdev_device *dev)
>
>
>
> speed = ETH_SPEED_NUM_10G;
>
> - snprintf(tap_name, sizeof(tap_name), "%s%u",
>
> @@ -101,7 +102,7 @@
>
> + strlcpy(tap_name, DEFAULT_TAP_NAME "%d", RTE_ETH_NAME_MAX_LEN);
>
> memset(remote_iface, 0, RTE_ETH_NAME_MAX_LEN);
>
>
>
> -@@ -2284,5 +2286,4 @@ leave:
>
> +@@ -2255,5 +2257,4 @@ leave:
>
> tap_devices_count--;
>
> }
>
> - tap_unit--; /* Restore the unit number */
>
I wonder if the real problem here is the number of open file descriptors that
get leftover from the new EAL memory changes.
More information about the stable
mailing list