From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-HE1-obe.outbound.protection.outlook.com (mail-eopbgr70052.outbound.protection.outlook.com [40.107.7.52]) by dpdk.org (Postfix) with ESMTP id 424222BE3 for ; Tue, 2 Oct 2018 12:30:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=wNVoZTCRXNZEUu18EEqDDNvKboDQA5q3yZP5a/l8Xgo=; b=o37sR7advB/4v5LaKDZn3ehSxMSfetSuomcrUk4bjV5ZqEwieZvyGUtfx9DbjT5PXku2lfHzAS+zhpIAuvN2gpuVXJETTLd6P53eUlqRYw6Z2L/SBXm7rPhQ+PyfrdiSBAItXvr+gSKLqKz04/8cN6HRfwqFoGez9m3VHhm/mek= Received: from DB5PR05MB1254.eurprd05.prod.outlook.com (10.162.157.140) by DB5PR05MB1909.eurprd05.prod.outlook.com (10.166.173.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1185.24; Tue, 2 Oct 2018 10:30:15 +0000 Received: from DB5PR05MB1254.eurprd05.prod.outlook.com ([fe80::3516:b329:5c35:51a5]) by DB5PR05MB1254.eurprd05.prod.outlook.com ([fe80::3516:b329:5c35:51a5%2]) with mapi id 15.20.1185.024; Tue, 2 Oct 2018 10:30:15 +0000 From: Raslan Darawsheh To: "Wiles, Keith" CC: Thomas Monjalon , "dev@dpdk.org" , Shahaf Shuler , Ori Katz Thread-Topic: [PATCH v3 1/2] net/tap: change queue fd to be pointers to process private Thread-Index: AQHUVlQOqMpnxKr0IE63lsM58aCEwaUEHEeAgAeqUrA= Date: Tue, 2 Oct 2018 10:30:15 +0000 Message-ID: References: <20180720105742.12669-1-thomas@monjalon.net> <1538047196-13789-1-git-send-email-rasland@mellanox.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=rasland@mellanox.com; x-originating-ip: [82.213.2.186] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB5PR05MB1909; 6:xtx62VZqRfa4f0tylNrpPglGo5vSlPB/KbnlSYRNO1NWIUNom7yel+hO4crOX+ixkwjkun7fqWARZDo1/bagTr+ZNKsUFXr1dXMGX+d/znz/e62hkyc6MLdCiIxVEP9FLMCvUc4jFxsCzffyN1N2lGGgTPmDXfX4mCBCu/OsZu4MBDZ7jREur1EauG/75ZLNVKGbs/pbI5YJiQmk/+kVkDcOB5GU+NO6cnk+Wp9S9y1beCDbS4FSv9hv+gzru8vL6M7931fGmegh7bzGhCT8KE3TlcRKEI826l5Y0t9tWnC3bXO+INAIk4Xv99s2X17VYxQ/MjF/6Xm6P8GnDuJBJm+N2NrS9mCQ9s9n47A8o3e3+9bhh4Y4cMjjW6yuo4RoGNWnm2it4DNHUs1h85WrxcnEHGbZ8FaLqN2JwXB5WY7/Vae/wUIPzsWt6dFVBhESiVh/wtpAvs+WMHA/n5O71A==; 5:0TJRsTfgDgzeRmC6FTej413oaIxZk88PsiIm1CcqMJ8C4/W+D7NH7le1+Dvt3bchzi2lY/fcp8/BUshkZMvJpd6Qpm8ZL8oZKB/5tyD17RE7Cfdatw2YTGH314pemzWCIPvnem5azpRAZhwNT+LLe0G75+WEoaCmblQDhC8dFAc=; 7:1FAIUX1O79EfMCqgsSQzaTQHloYpzA/8TnDJPIy9ah5bhPRo4BzUFIXAIRkitDxCFNYq6uwXEQ67BAODfsenBkfgBjuCsitdRiGMghUb57BHQCHskSGka2yE12WFJy8kwTST6Acf1t5ulTzfbWApRTItG5BKCDc3y1wF/uGGJS7Il6ItKoIRFKn8eohyt6TjP+eM6IGeOwM+iSGP4Klmj830iJ1KKTxU/cqse4XFfR+MoeDSPsHht50Fn7W+ntUP x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 0a2cb31c-d1d0-46f1-6269-08d628520af7 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534165)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:DB5PR05MB1909; x-ms-traffictypediagnostic: DB5PR05MB1909: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(228905959029699); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231355)(944501410)(52105095)(93006095)(93001095)(3002001)(10201501046)(6055026)(149066)(150057)(6041310)(20161123558120)(20161123562045)(20161123564045)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051); SRVR:DB5PR05MB1909; BCL:0; PCL:0; RULEID:; SRVR:DB5PR05MB1909; x-forefront-prvs: 0813C68E65 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(136003)(396003)(39860400002)(346002)(376002)(13464003)(199004)(189003)(53546011)(6506007)(6436002)(486006)(6116002)(105586002)(3846002)(66066001)(5250100002)(33656002)(86362001)(68736007)(446003)(99286004)(76176011)(9686003)(476003)(5660300001)(6246003)(11346002)(478600001)(229853002)(54906003)(81166006)(8936002)(2900100001)(186003)(97736004)(74316002)(71190400001)(7736002)(4326008)(71200400001)(305945005)(55016002)(102836004)(26005)(14454004)(8676002)(81156014)(7696005)(107886003)(106356001)(6916009)(14444005)(25786009)(316002)(256004)(53936002)(2906002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB5PR05MB1909; H:DB5PR05MB1254.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: b8iL6CmbaIk9U/1XETpLAuM2pxAfCufXcmkobheaKbOg3yzzyHEHwMOH6R8qshzmE2NGIVKLyM9DpE6lq9oK8tKrJHo0T08XSfa/ECkMRyZcB84i2i3nCuuje5yP+FQzfd/iBcMZ1RExbze3k99heZAljdrJqV4loT1FF/+a4LxY/RKICRNvOmbJK3+fO/3Oot5AOlKAXNGVeHW13Q4HqNdJZN+ucjO9N+Dk47JrGg4XILD1frx39cBY9U+YGwBe6YR+TEc3EK73xjfCsGLhpj4YCJoPU2HYDGpm2DoJ6nCE5aq0uW6WAbI1qeqlVaVvYSu0O1NMlNpY27JxXdFq7S+rEVSsamJ/8AU9YILxpUg= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 0a2cb31c-d1d0-46f1-6269-08d628520af7 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Oct 2018 10:30:15.4666 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB5PR05MB1909 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: Tue, 02 Oct 2018 10:30:18 -0000 Hi, what I'm really doing is simply do some private array for all the fd's that= each process will allocate it separately which will allow that each proces= s will be=20 able to access the fd's for the queues in order not to overwrite the shared= ones and it's working for me this way. Now coming to your comment I'm not sure I fully understand it but, what you= are suggesting is to create an array which will be accessed by the process= _id to store these fd's in it. As far as I know we don't have something as process_id in dpdk we only have= the system process id which is not relevant for the array of fd's. Please correct me if I'm wrong,=20 I think this way we'll be limiting the number of secondary processes to num= ber of queues by tap. Meanwhile, in my solution we don't have such limitation. Kindest regards, Raslan Darawsheh -----Original Message----- From: Wiles, Keith =20 Sent: Thursday, September 27, 2018 4:18 PM To: Raslan Darawsheh Cc: Thomas Monjalon ; dev@dpdk.org; Shahaf Shuler ; Ori Katz Subject: Re: [PATCH v3 1/2] net/tap: change queue fd to be pointers to proc= ess private > 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=20 > private structure and make the queue fds point to it. >=20 > Signed-off-by: Raslan Darawsheh > --- > drivers/net/tap/rte_eth_tap.c | 63=20 > ++++++++++++++++++++++++------------------- > 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=20 > 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=20 > 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=20 > +1674,10 @@ eth_dev_tap_create(struct rte_vdev_device *vdev, char *tap_na= me, > /* 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 @@=20 > 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=20 > 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 @@=20 > 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=20 > a/drivers/net/tap/tap_intr.c b/drivers/net/tap/tap_intr.c index=20 > 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) > -- > 2.7.4 >=20 Regards, Keith