[dpdk-dev] [PATCH v2 4/5] net/tap: create netdevice during probing

Pascal Mazon pascal.mazon at 6wind.com
Tue Apr 18 10:17:56 CEST 2017


This has three main benefits:
 - tun_alloc is now generic again for any queue,
 - mtu no longer needs to be handled in tap_setup_queue(),
 - an actual netdevice is created as soon as the device is probed.

On top of it, code in eth_dev_tap_create() has been reworked to have a
more logical behavior; initialization can now fail if a remote is
requested but cannot be set up.

Fixes: 2bc06869cd94 ("net/tap: add remote netdevice traffic capture")

Signed-off-by: Pascal Mazon <pascal.mazon at 6wind.com>
---
 drivers/net/tap/rte_eth_tap.c | 217 ++++++++++++++++++++++--------------------
 1 file changed, 115 insertions(+), 102 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index 403aca9f7307..24ad9ead3b4c 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -113,10 +113,6 @@ enum ioctl_mode {
 	REMOTE_ONLY,
 };
 
-static int
-tap_ioctl(struct pmd_internals *pmd, unsigned long request,
-	  struct ifreq *ifr, int set, enum ioctl_mode mode);
-
 static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
 
 /* Tun/Tap allocation routine
@@ -125,7 +121,7 @@ static int tap_intr_handle_set(struct rte_eth_dev *dev, int set);
  * supplied name.
  */
 static int
-tun_alloc(struct pmd_internals *pmd, uint16_t qid)
+tun_alloc(struct pmd_internals *pmd)
 {
 	struct ifreq ifr;
 #ifdef IFF_MULTI_QUEUE
@@ -224,75 +220,6 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 			strerror(errno));
 	}
 
-	if (qid == 0) {
-		struct ifreq ifr;
-
-		/*
-		 * pmd->eth_addr contains the desired MAC, either from remote
-		 * or from a random assignment. Sync it with the tap netdevice.
-		 */
-		ifr.ifr_hwaddr.sa_family = AF_LOCAL;
-		rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr,
-			   ETHER_ADDR_LEN);
-		if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
-			goto error;
-
-		pmd->if_index = if_nametoindex(pmd->name);
-		if (!pmd->if_index) {
-			RTE_LOG(ERR, PMD,
-				"Could not find ifindex for %s: rte_flow won't be usable.\n",
-				pmd->name);
-			return fd;
-		}
-		if (!pmd->flower_support)
-			return fd;
-		if (qdisc_create_multiq(pmd->nlsk_fd, pmd->if_index) < 0) {
-			RTE_LOG(ERR, PMD,
-				"Could not create multiq qdisc for %s: rte_flow won't be usable.\n",
-				pmd->name);
-			return fd;
-		}
-		if (qdisc_create_ingress(pmd->nlsk_fd, pmd->if_index) < 0) {
-			RTE_LOG(ERR, PMD,
-				"Could not create multiq qdisc for %s: rte_flow won't be usable.\n",
-				pmd->name);
-			return fd;
-		}
-		if (pmd->remote_if_index) {
-			/*
-			 * Flush usually returns negative value because it tries
-			 * to delete every QDISC (and on a running device, one
-			 * QDISC at least is needed). Ignore negative return
-			 * value.
-			 */
-			qdisc_flush(pmd->nlsk_fd, pmd->remote_if_index);
-			if (qdisc_create_ingress(pmd->nlsk_fd,
-						 pmd->remote_if_index) < 0)
-				goto remote_fail;
-			LIST_INIT(&pmd->implicit_flows);
-			if (tap_flow_implicit_create(
-				    pmd, TAP_REMOTE_LOCAL_MAC) < 0)
-				goto remote_fail;
-			if (tap_flow_implicit_create(
-				    pmd, TAP_REMOTE_BROADCAST) < 0)
-				goto remote_fail;
-			if (tap_flow_implicit_create(
-				    pmd, TAP_REMOTE_BROADCASTV6) < 0)
-				goto remote_fail;
-			if (tap_flow_implicit_create(
-				    pmd, TAP_REMOTE_TX) < 0)
-				goto remote_fail;
-		}
-	}
-
-	return fd;
-
-remote_fail:
-	RTE_LOG(ERR, PMD,
-		"Could not set up remote flow rules for %s: remote disabled.\n",
-		pmd->name);
-	pmd->remote_if_index = 0;
-	tap_flow_implicit_flush(pmd, NULL);
 	return fd;
 
 error:
@@ -829,22 +756,12 @@ tap_setup_queue(struct rte_eth_dev *dev,
 	if (fd == -1) {
 		RTE_LOG(INFO, PMD, "Add queue to TAP %s for qid %d\n",
 			pmd->name, qid);
-		fd = tun_alloc(pmd, qid);
+		fd = tun_alloc(pmd);
 		if (fd < 0) {
-			RTE_LOG(ERR, PMD, "tun_alloc(%s, %d) failed\n",
-				pmd->name, qid);
+			RTE_LOG(ERR, PMD, "%s: tun_alloc() failed.\n",
+				pmd->name);
 			return -1;
 		}
-		if (qid == 0) {
-			struct ifreq ifr;
-
-			ifr.ifr_mtu = dev->data->mtu;
-			if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1,
-				      LOCAL_AND_REMOTE) < 0) {
-				close(fd);
-				return -1;
-			}
-		}
 	}
 
 	rx->fd = fd;
@@ -1132,6 +1049,7 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface,
 	struct rte_eth_dev *dev = NULL;
 	struct pmd_internals *pmd = NULL;
 	struct rte_eth_dev_data *data = NULL;
+	struct ifreq ifr;
 	int i;
 
 	RTE_LOG(DEBUG, PMD, "  TAP device on numa %u\n", rte_socket_id());
@@ -1208,37 +1126,132 @@ eth_dev_tap_create(const char *name, char *tap_name, char *remote_iface,
 		eth_random_addr((uint8_t *)&pmd->eth_addr);
 	}
 
+	/* Immediately create the netdevice (this will create the 1st queue). */
+	if (tap_setup_queue(dev, pmd, 0) == -1)
+		goto error_exit;
+
+	ifr.ifr_mtu = dev->data->mtu;
+	if (tap_ioctl(pmd, SIOCSIFMTU, &ifr, 1, LOCAL_AND_REMOTE) < 0)
+		goto error_exit;
+
+	memset(&ifr, 0, sizeof(struct ifreq));
+	ifr.ifr_hwaddr.sa_family = AF_LOCAL;
+	rte_memcpy(ifr.ifr_hwaddr.sa_data, &pmd->eth_addr, ETHER_ADDR_LEN);
+	if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0)
+		goto error_exit;
+
 	tap_kernel_support(pmd);
-	if (!pmd->flower_support)
-		return 0;
-	LIST_INIT(&pmd->flows);
+	if (!pmd->flower_support) {
+		if (remote_iface[0]) {
+			RTE_LOG(ERR, PMD,
+				"%s: kernel does not support TC rules, required for remote feature.",
+				pmd->name);
+			goto error_exit;
+		} else {
+			RTE_LOG(INFO, PMD,
+				"%s: kernel too old for Flow API support.\n",
+				pmd->name);
+			return 0;
+		}
+	}
+
 	/*
-	 * If no netlink socket can be created, then it will fail when
-	 * creating/destroying flow rules.
+	 * Set up everything related to rte_flow:
+	 * - netlink socket
+	 * - tap / remote if_index
+	 * - mandatory QDISCs
+	 * - rte_flow actual/implicit lists
+	 * - implicit rules
 	 */
 	pmd->nlsk_fd = nl_init(0);
-	if (strlen(remote_iface)) {
-		struct ifreq ifr;
+	if (pmd->nlsk_fd == -1) {
+		RTE_LOG(WARNING, PMD, "%s: failed to create netlink socket.",
+			pmd->name);
+		goto disable_rte_flow;
+	}
+	pmd->if_index = if_nametoindex(pmd->name);
+	if (!pmd->if_index) {
+		RTE_LOG(ERR, PMD, "%s: failed to get if_index.", pmd->name);
+		goto disable_rte_flow;
+	}
+	if (qdisc_create_multiq(pmd->nlsk_fd, pmd->if_index) < 0) {
+		RTE_LOG(ERR, PMD, "%s: failed to create multiq qdisc.",
+			pmd->name);
+		goto disable_rte_flow;
+	}
+	if (qdisc_create_ingress(pmd->nlsk_fd, pmd->if_index) < 0) {
+		RTE_LOG(ERR, PMD, "%s: failed to create ingress qdisc.",
+			pmd->name);
+		goto disable_rte_flow;
+	}
+	LIST_INIT(&pmd->flows);
 
+	if (strlen(remote_iface)) {
 		pmd->remote_if_index = if_nametoindex(remote_iface);
+		if (!pmd->remote_if_index) {
+			RTE_LOG(ERR, PMD, "%s: failed to get %s if_index.",
+				pmd->name, remote_iface);
+			goto error_remote;
+		}
 		snprintf(pmd->remote_iface, RTE_ETH_NAME_MAX_LEN,
 			 "%s", remote_iface);
-		if (!pmd->remote_if_index) {
-			RTE_LOG(ERR, PMD, "Could not find %s ifindex: "
-				"remote interface will remain unconfigured\n",
-				remote_iface);
-			return 0;
+		if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0) {
+			RTE_LOG(ERR, PMD, "%s: failed to get %s MAC address.",
+				pmd->name, pmd->remote_iface);
+			goto error_remote;
 		}
-		if (tap_ioctl(pmd, SIOCGIFHWADDR, &ifr, 0, REMOTE_ONLY) < 0)
-			goto error_exit;
 		rte_memcpy(&pmd->eth_addr, ifr.ifr_hwaddr.sa_data,
 			   ETHER_ADDR_LEN);
+		/* The desired MAC is already in ifreq after SIOCGIFHWADDR. */
+		if (tap_ioctl(pmd, SIOCSIFHWADDR, &ifr, 0, LOCAL_ONLY) < 0) {
+			RTE_LOG(ERR, PMD, "%s: failed to get %s MAC address.",
+				pmd->name, remote_iface);
+			goto error_remote;
+		}
+
+		/*
+		 * Flush usually returns negative value because it tries to
+		 * delete every QDISC (and on a running device, one QDISC at
+		 * least is needed). Ignore negative return value.
+		 */
+		qdisc_flush(pmd->nlsk_fd, pmd->remote_if_index);
+		if (qdisc_create_ingress(pmd->nlsk_fd,
+					 pmd->remote_if_index) < 0) {
+			RTE_LOG(ERR, PMD, "%s: failed to create ingress qdisc.",
+				pmd->remote_iface);
+			goto error_remote;
+		}
+		LIST_INIT(&pmd->implicit_flows);
+		if (tap_flow_implicit_create(pmd, TAP_REMOTE_TX) < 0 ||
+		    tap_flow_implicit_create(pmd, TAP_REMOTE_LOCAL_MAC) < 0 ||
+		    tap_flow_implicit_create(pmd, TAP_REMOTE_BROADCAST) < 0 ||
+		    tap_flow_implicit_create(pmd, TAP_REMOTE_BROADCASTV6) < 0) {
+			RTE_LOG(ERR, PMD,
+				"%s: failed to create implicit rules.",
+				pmd->name);
+			goto error_remote;
+		}
 	}
 
 	return 0;
 
+disable_rte_flow:
+	RTE_LOG(ERR, PMD, " Disabling rte flow support: %s(%d)\n",
+		strerror(errno), errno);
+	if (strlen(remote_iface)) {
+		RTE_LOG(ERR, PMD, "Remote feature requires flow support.\n");
+		goto error_exit;
+	}
+	pmd->flower_support = 0;
+	return 0;
+
+error_remote:
+	RTE_LOG(ERR, PMD, " Can't set up remote feature: %s(%d)\n",
+		strerror(errno), errno);
+	tap_flow_implicit_flush(pmd, NULL);
+
 error_exit:
-	RTE_LOG(DEBUG, PMD, "TAP Unable to initialize %s\n", name);
+	RTE_LOG(ERR, PMD, "TAP Unable to initialize %s\n", name);
 
 	rte_free(data);
 	rte_free(pmd);
-- 
2.12.0.306.g4a9b9b3



More information about the dev mailing list