[v4,1/2] net/tap: change queue fd to be pointers to process private
Checks
Commit Message
change the fds for the queues to be pointers and add new process private
structure and make the queue fds point to it.
Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
---
drivers/net/tap/rte_eth_tap.c | 63 ++++++++++++++++++++++++-------------------
drivers/net/tap/rte_eth_tap.h | 9 +++++--
drivers/net/tap/tap_intr.c | 4 +--
3 files changed, 44 insertions(+), 32 deletions(-)
Comments
On 10/2/2018 11:34 AM, Raslan Darawsheh wrote:
> change the fds for the queues to be pointers and add new process private
> structure and make the queue fds point to it.
>
> Signed-off-by: Raslan Darawsheh <rasland@mellanox.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 63 ++++++++++++++++++++++++-------------------
> drivers/net/tap/rte_eth_tap.h | 9 +++++--
> drivers/net/tap/tap_intr.c | 4 +--
> 3 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index ad5ae98..8cc4552 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -64,6 +64,7 @@
>
> static struct rte_vdev_driver pmd_tap_drv;
> static struct rte_vdev_driver pmd_tun_drv;
> +static struct pmd_process_private *process_private;
This is the handlers for eth_dev queues, this should be _per eth_dev_, you can't
have a single global variable for this, we will see the problems below.
<...>
> @@ -1633,6 +1634,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> goto error_exit_nodev;
> }
>
> + process_private = (struct pmd_process_private *)
> + rte_zmalloc_socket(tap_name, sizeof(struct pmd_process_private),
> + RTE_CACHE_LINE_SIZE, dev->device->numa_node);
For each tap device, you overwrite the "process_private",
- So it has the address of last created tap device, we will come back this one
- and problem is how to free this back if an eth_dev removed? We lost references
except last one.
> +
> pmd = dev->data->dev_private;
> pmd->dev = dev;
> snprintf(pmd->name, sizeof(pmd->name), "%s", tap_name);
> @@ -1669,8 +1674,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
> /* Presetup the fds to -1 as being not valid */
> pmd->ka_fd = -1;
> for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
> - pmd->rxq[i].fd = -1;
> - pmd->txq[i].fd = -1;
> + process_private->rxq_fds[i] = -1;
> + process_private->txq_fds[i] = -1;
> + pmd->rxq[i].fd = &process_private->rxq_fds[i];
> + pmd->txq[i].fd = &process_private->txq_fds[i];
for eth_dev, dev->data->dev_private->rxq[i].fd points to a memory address,
and remember "dev->data->dev_private" is shared between primary and secondary,
when secondary updates its rxq[i].fd to point its memory block, won't it corrupt
the primary???
I think redirection is not helping here, I am not sure to the this within the
shared memory, you need a not shared area.
@@ -64,6 +64,7 @@
static struct rte_vdev_driver pmd_tap_drv;
static struct rte_vdev_driver pmd_tun_drv;
+static struct pmd_process_private *process_private;
static const char *valid_arguments[] = {
ETH_TAP_IFACE_ARG,
@@ -331,7 +332,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts)
uint16_t data_off = rte_pktmbuf_headroom(mbuf);
int len;
- len = readv(rxq->fd, *rxq->iovecs,
+ len = readv(*rxq->fd, *rxq->iovecs,
1 +
(rxq->rxmode->offloads & DEV_RX_OFFLOAD_SCATTER ?
rxq->nb_rx_desc : 1));
@@ -595,7 +596,7 @@ tap_write_mbufs(struct tx_queue *txq, uint16_t num_mbufs,
tap_tx_l4_cksum(l4_cksum, l4_phdr_cksum, l4_raw_cksum);
/* copy the tx frame data */
- n = writev(txq->fd, iovecs, j);
+ n = writev(*txq->fd, iovecs, j);
if (n <= 0)
break;
(*num_packets)++;
@@ -976,13 +977,13 @@ tap_dev_close(struct rte_eth_dev *dev)
tap_flow_implicit_flush(internals, NULL);
for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
- if (internals->rxq[i].fd != -1) {
- close(internals->rxq[i].fd);
- internals->rxq[i].fd = -1;
+ if (*internals->rxq[i].fd != -1) {
+ close(*internals->rxq[i].fd);
+ *internals->rxq[i].fd = -1;
}
- if (internals->txq[i].fd != -1) {
- close(internals->txq[i].fd);
- internals->txq[i].fd = -1;
+ if (*internals->txq[i].fd != -1) {
+ close(*internals->txq[i].fd);
+ *internals->txq[i].fd = -1;
}
}
@@ -1007,9 +1008,9 @@ tap_rx_queue_release(void *queue)
{
struct rx_queue *rxq = queue;
- if (rxq && (rxq->fd > 0)) {
- close(rxq->fd);
- rxq->fd = -1;
+ if (rxq && rxq->fd && (*rxq->fd > 0)) {
+ close(*rxq->fd);
+ *rxq->fd = -1;
rte_pktmbuf_free(rxq->pool);
rte_free(rxq->iovecs);
rxq->pool = NULL;
@@ -1022,9 +1023,9 @@ tap_tx_queue_release(void *queue)
{
struct tx_queue *txq = queue;
- if (txq && (txq->fd > 0)) {
- close(txq->fd);
- txq->fd = -1;
+ if (txq && txq->fd && (*txq->fd > 0)) {
+ close(*txq->fd);
+ *txq->fd = -1;
}
}
@@ -1214,13 +1215,13 @@ tap_setup_queue(struct rte_eth_dev *dev,
struct rte_gso_ctx *gso_ctx;
if (is_rx) {
- fd = &rx->fd;
- other_fd = &tx->fd;
+ fd = rx->fd;
+ other_fd = tx->fd;
dir = "rx";
gso_ctx = NULL;
} else {
- fd = &tx->fd;
- other_fd = &rx->fd;
+ fd = tx->fd;
+ other_fd = rx->fd;
dir = "tx";
gso_ctx = &tx->gso_ctx;
}
@@ -1331,7 +1332,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev,
}
TAP_LOG(DEBUG, " RX TUNTAP device name %s, qid %d on fd %d",
- internals->name, rx_queue_id, internals->rxq[rx_queue_id].fd);
+ internals->name, rx_queue_id, *internals->rxq[rx_queue_id].fd);
return 0;
@@ -1371,7 +1372,7 @@ tap_tx_queue_setup(struct rte_eth_dev *dev,
return -1;
TAP_LOG(DEBUG,
" TX TUNTAP device name %s, qid %d on fd %d csum %s",
- internals->name, tx_queue_id, internals->txq[tx_queue_id].fd,
+ internals->name, tx_queue_id, *internals->txq[tx_queue_id].fd,
txq->csum ? "on" : "off");
return 0;
@@ -1633,6 +1634,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
goto error_exit_nodev;
}
+ process_private = (struct pmd_process_private *)
+ rte_zmalloc_socket(tap_name, sizeof(struct pmd_process_private),
+ RTE_CACHE_LINE_SIZE, dev->device->numa_node);
+
pmd = dev->data->dev_private;
pmd->dev = dev;
snprintf(pmd->name, sizeof(pmd->name), "%s", tap_name);
@@ -1669,8 +1674,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_name,
/* Presetup the fds to -1 as being not valid */
pmd->ka_fd = -1;
for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
- pmd->rxq[i].fd = -1;
- pmd->txq[i].fd = -1;
+ process_private->rxq_fds[i] = -1;
+ process_private->txq_fds[i] = -1;
+ pmd->rxq[i].fd = &process_private->rxq_fds[i];
+ pmd->txq[i].fd = &process_private->txq_fds[i];
}
if (pmd->type == ETH_TUNTAP_TYPE_TAP) {
@@ -2089,13 +2096,13 @@ rte_pmd_tap_remove(struct rte_vdev_device *dev)
tap_nl_final(internals->nlsk_fd);
}
for (i = 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) {
- if (internals->rxq[i].fd != -1) {
- close(internals->rxq[i].fd);
- internals->rxq[i].fd = -1;
+ if (*internals->rxq[i].fd != -1) {
+ close(*internals->rxq[i].fd);
+ *internals->rxq[i].fd = -1;
}
- if (internals->txq[i].fd != -1) {
- close(internals->txq[i].fd);
- internals->txq[i].fd = -1;
+ if (*internals->txq[i].fd != -1) {
+ close(*internals->txq[i].fd);
+ *internals->txq[i].fd = -1;
}
}
@@ -46,7 +46,7 @@ struct rx_queue {
struct rte_mempool *mp; /* Mempool for RX packets */
uint32_t trigger_seen; /* Last seen Rx trigger value */
uint16_t in_port; /* Port ID */
- int fd;
+ int *fd;
struct pkt_stats stats; /* Stats for this RX queue */
uint16_t nb_rx_desc; /* max number of mbufs available */
struct rte_eth_rxmode *rxmode; /* RX features */
@@ -56,7 +56,7 @@ struct rx_queue {
};
struct tx_queue {
- int fd;
+ int *fd;
int type; /* Type field - TUN|TAP */
uint16_t *mtu; /* Pointer to MTU from dev_data */
uint16_t csum:1; /* Enable checksum offloading */
@@ -92,6 +92,11 @@ struct pmd_internals {
int ka_fd; /* keep-alive file descriptor */
};
+struct pmd_process_private {
+ int rxq_fds[RTE_PMD_TAP_MAX_QUEUES];
+ int txq_fds[RTE_PMD_TAP_MAX_QUEUES];
+};
+
/* tap_intr.c */
int tap_rx_intr_vec_set(struct rte_eth_dev *dev, int set);
@@ -71,7 +71,7 @@ tap_rx_intr_vec_install(struct rte_eth_dev *dev)
struct rx_queue *rxq = pmd->dev->data->rx_queues[i];
/* Skip queues that cannot request interrupts. */
- if (!rxq || rxq->fd <= 0) {
+ if (!rxq || !rxq->fd || *rxq->fd <= 0) {
/* Use invalid intr_vec[] index to disable entry. */
intr_handle->intr_vec[i] =
RTE_INTR_VEC_RXTX_OFFSET +
@@ -79,7 +79,7 @@ tap_rx_intr_vec_install(struct rte_eth_dev *dev)
continue;
}
intr_handle->intr_vec[i] = RTE_INTR_VEC_RXTX_OFFSET + count;
- intr_handle->efds[count] = rxq->fd;
+ intr_handle->efds[count] = *rxq->fd;
count++;
}
if (!count)