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