From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <keith.wiles@intel.com>
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
by dpdk.org (Postfix) with ESMTP id CC55F1B465
for <dev@dpdk.org>; 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" <keith.wiles@intel.com>
To: Raslan Darawsheh <rasland@mellanox.com>
CC: "thomas@monjalon.net" <thomas@monjalon.net>, "dev@dpdk.org"
<dev@dpdk.org>, "shahafs@mellanox.com" <shahafs@mellanox.com>,
"orik@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: <DC7AFAB1-E08E-4DC4-9901-3A67629C162D@intel.com>
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: <C15377980AB0F64C906306D1F0A16730@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 27 Sep 2018 13:18:46 -0000
> On Sep 27, 2018, at 6:19 AM, Raslan Darawsheh <rasland@mellanox.com> 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 <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(-)
>=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