From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <rasland@mellanox.com>
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 <dev@dpdk.org>; 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 <rasland@mellanox.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
CC: Thomas Monjalon <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>,
Shahaf Shuler <shahafs@mellanox.com>, Ori Katz <orik@mellanox.com>
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: <DB5PR05MB125494F33F6A865B56C42E80C2E80@DB5PR05MB1254.eurprd05.prod.outlook.com>
References: <20180720105742.12669-1-thomas@monjalon.net>
<1538047196-13789-1-git-send-email-rasland@mellanox.com>
<DC7AFAB1-E08E-4DC4-9901-3A67629C162D@intel.com>
In-Reply-To: <DC7AFAB1-E08E-4DC4-9901-3A67629C162D@intel.com>
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: <DB5PR05MB19090D19883FCEC84CA7D7E9C2E80@DB5PR05MB1909.eurprd05.prod.outlook.com>
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 <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: 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 <keith.wiles@intel.com>=20
Sent: Thursday, September 27, 2018 4:18 PM
To: Raslan Darawsheh <rasland@mellanox.com>
Cc: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Shahaf Shuler <sha=
hafs@mellanox.com>; Ori Katz <orik@mellanox.com>
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 <rasland@mellanox.com> 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 <rasland@mellanox.com>
> ---
> 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