From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id CC55F1B465 for ; Thu, 27 Sep 2018 15:18:45 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Sep 2018 06:18:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,310,1534834800"; d="scan'208";a="77814407" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga006.jf.intel.com with ESMTP; 27 Sep 2018 06:17:41 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 27 Sep 2018 06:17:41 -0700 Received: from fmsmsx117.amr.corp.intel.com ([169.254.3.34]) by fmsmsx156.amr.corp.intel.com ([169.254.13.194]) with mapi id 14.03.0319.002; Thu, 27 Sep 2018 06:17:41 -0700 From: "Wiles, Keith" To: Raslan Darawsheh CC: "thomas@monjalon.net" , "dev@dpdk.org" , "shahafs@mellanox.com" , "orik@mellanox.com" Thread-Topic: [PATCH v3 1/2] net/tap: change queue fd to be pointers to process private Thread-Index: AQHUVlQZuIITwBEvUUKU905O3AO3NKUEkaCA Date: Thu, 27 Sep 2018 13:17:41 +0000 Message-ID: References: <20180720105742.12669-1-thomas@monjalon.net> <1538047196-13789-1-git-send-email-rasland@mellanox.com> In-Reply-To: <1538047196-13789-1-git-send-email-rasland@mellanox.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.81.84] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 1/2] net/tap: change queue fd to be pointers to process private X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Sep 2018 13:18:46 -0000 > On Sep 27, 2018, at 6:19 AM, Raslan Darawsheh wrot= e: >=20 > change the fds for the queues to be pointers and add new process private > structure and make the queue fds point to it. >=20 > Signed-off-by: Raslan Darawsheh > --- > 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(-) >=20 > 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 @@ >=20 > static struct rte_vdev_driver pmd_tap_drv; > static struct rte_vdev_driver pmd_tun_drv; > +static struct pmd_process_private *process_private; Maybe I do not see some minor point for making fd a pointer to fd when we c= ould have an array of process_private[RTE_PMD_TAP_MAX_QUEUES] instead of a = pointer type here. Then we do not need to allocate the memory each PMD and = they would still have a private copy. Remove the array of rx/tx fds in the = structure. This way it appears we can remove the code below that is making = fd a pointer to fd. It just seems overly complex to me at the cost of a few= more bytes of memory. This would remove int fd; from the structure and add a pointer to the pid_p= rocess_private instead, which is private by default. Did I miss some detail here that makes my comment wrong? >=20 > static const char *valid_arguments[] =3D { > ETH_TAP_IFACE_ARG, > @@ -331,7 +332,7 @@ pmd_rx_burst(void *queue, struct rte_mbuf **bufs, uin= t16_t nb_pkts) > uint16_t data_off =3D rte_pktmbuf_headroom(mbuf); > int len; >=20 > - len =3D readv(rxq->fd, *rxq->iovecs, > + len =3D 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_mb= ufs, > tap_tx_l4_cksum(l4_cksum, l4_phdr_cksum, l4_raw_cksum); >=20 > /* copy the tx frame data */ > - n =3D writev(txq->fd, iovecs, j); > + n =3D writev(*txq->fd, iovecs, j); > if (n <=3D 0) > break; > (*num_packets)++; > @@ -976,13 +977,13 @@ tap_dev_close(struct rte_eth_dev *dev) > tap_flow_implicit_flush(internals, NULL); >=20 > for (i =3D 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) { > - if (internals->rxq[i].fd !=3D -1) { > - close(internals->rxq[i].fd); > - internals->rxq[i].fd =3D -1; > + if (*internals->rxq[i].fd !=3D -1) { > + close(*internals->rxq[i].fd); > + *internals->rxq[i].fd =3D -1; > } > - if (internals->txq[i].fd !=3D -1) { > - close(internals->txq[i].fd); > - internals->txq[i].fd =3D -1; > + if (*internals->txq[i].fd !=3D -1) { > + close(*internals->txq[i].fd); > + *internals->txq[i].fd =3D -1; > } > } >=20 > @@ -1007,9 +1008,9 @@ tap_rx_queue_release(void *queue) > { > struct rx_queue *rxq =3D queue; >=20 > - if (rxq && (rxq->fd > 0)) { > - close(rxq->fd); > - rxq->fd =3D -1; > + if (rxq && rxq->fd && (*rxq->fd > 0)) { > + close(*rxq->fd); > + *rxq->fd =3D -1; > rte_pktmbuf_free(rxq->pool); > rte_free(rxq->iovecs); > rxq->pool =3D NULL; > @@ -1022,9 +1023,9 @@ tap_tx_queue_release(void *queue) > { > struct tx_queue *txq =3D queue; >=20 > - if (txq && (txq->fd > 0)) { > - close(txq->fd); > - txq->fd =3D -1; > + if (txq && txq->fd && (*txq->fd > 0)) { > + close(*txq->fd); > + *txq->fd =3D -1; > } > } >=20 > @@ -1214,13 +1215,13 @@ tap_setup_queue(struct rte_eth_dev *dev, > struct rte_gso_ctx *gso_ctx; >=20 > if (is_rx) { > - fd =3D &rx->fd; > - other_fd =3D &tx->fd; > + fd =3D rx->fd; > + other_fd =3D tx->fd; > dir =3D "rx"; > gso_ctx =3D NULL; > } else { > - fd =3D &tx->fd; > - other_fd =3D &rx->fd; > + fd =3D tx->fd; > + other_fd =3D rx->fd; > dir =3D "tx"; > gso_ctx =3D &tx->gso_ctx; > } > @@ -1331,7 +1332,7 @@ tap_rx_queue_setup(struct rte_eth_dev *dev, > } >=20 > 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); >=20 > return 0; >=20 > @@ -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"); >=20 > return 0; > @@ -1633,6 +1634,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, c= har *tap_name, > goto error_exit_nodev; > } >=20 > + process_private =3D (struct pmd_process_private *) > + rte_zmalloc_socket(tap_name, sizeof(struct pmd_process_private), > + RTE_CACHE_LINE_SIZE, dev->device->numa_node); > + > pmd =3D dev->data->dev_private; > pmd->dev =3D dev; > snprintf(pmd->name, sizeof(pmd->name), "%s", tap_name); > @@ -1669,8 +1674,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, c= har *tap_name, > /* Presetup the fds to -1 as being not valid */ > pmd->ka_fd =3D -1; > for (i =3D 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) { > - pmd->rxq[i].fd =3D -1; > - pmd->txq[i].fd =3D -1; > + process_private->rxq_fds[i] =3D -1; > + process_private->txq_fds[i] =3D -1; > + pmd->rxq[i].fd =3D &process_private->rxq_fds[i]; > + pmd->txq[i].fd =3D &process_private->txq_fds[i]; > } >=20 > if (pmd->type =3D=3D 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 =3D 0; i < RTE_PMD_TAP_MAX_QUEUES; i++) { > - if (internals->rxq[i].fd !=3D -1) { > - close(internals->rxq[i].fd); > - internals->rxq[i].fd =3D -1; > + if (*internals->rxq[i].fd !=3D -1) { > + close(*internals->rxq[i].fd); > + *internals->rxq[i].fd =3D -1; > } > - if (internals->txq[i].fd !=3D -1) { > - close(internals->txq[i].fd); > - internals->txq[i].fd =3D -1; > + if (*internals->txq[i].fd !=3D -1) { > + close(*internals->txq[i].fd); > + *internals->txq[i].fd =3D -1; > } > } >=20 > diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.= h > index 44e2773..4146f5c 100644 > --- a/drivers/net/tap/rte_eth_tap.h > +++ b/drivers/net/tap/rte_eth_tap.h > @@ -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 { > }; >=20 > 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 */ > }; >=20 > +struct pmd_process_private { > + int rxq_fds[RTE_PMD_TAP_MAX_QUEUES]; > + int txq_fds[RTE_PMD_TAP_MAX_QUEUES]; > +}; > + > /* tap_intr.c */ >=20 > int tap_rx_intr_vec_set(struct rte_eth_dev *dev, int set); > diff --git a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c > index fc59018..a4b212d 100644 > --- a/drivers/net/tap/tap_intr.c > +++ b/drivers/net/tap/tap_intr.c > @@ -71,7 +71,7 @@ tap_rx_intr_vec_install(struct rte_eth_dev *dev) > struct rx_queue *rxq =3D pmd->dev->data->rx_queues[i]; >=20 > /* Skip queues that cannot request interrupts. */ > - if (!rxq || rxq->fd <=3D 0) { > + if (!rxq || !rxq->fd || *rxq->fd <=3D 0) { > /* Use invalid intr_vec[] index to disable entry. */ > intr_handle->intr_vec[i] =3D > 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] =3D RTE_INTR_VEC_RXTX_OFFSET + count; > - intr_handle->efds[count] =3D rxq->fd; > + intr_handle->efds[count] =3D *rxq->fd; > count++; > } > if (!count) > --=20 > 2.7.4 >=20 Regards, Keith