From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4FBE0A0C41; Sun, 19 Sep 2021 20:49:21 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C9EB14014E; Sun, 19 Sep 2021 20:49:20 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com [67.231.148.174]) by mails.dpdk.org (Postfix) with ESMTP id F25294014D for ; Sun, 19 Sep 2021 20:49:18 +0200 (CEST) Received: from pps.filterd (m0045849.ppops.net [127.0.0.1]) by mx0a-0016f401.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 18JIf2Hm012168; Sun, 19 Sep 2021 11:49:15 -0700 Received: from nam10-dm6-obe.outbound.protection.outlook.com (mail-dm6nam10lp2108.outbound.protection.outlook.com [104.47.58.108]) by mx0a-0016f401.pphosted.com with ESMTP id 3b5x9e9asc-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sun, 19 Sep 2021 11:49:14 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TtUm7TOP+KOUv2fB94/+7xNboyJhpt8S3sugkCeoDnriMYO6FdkluYVwTXGOkKA686aSQDcrAleDnxRr3xGxtpGEaBx0JfhnpZmhI5mV0+ZI1NWwWgojdsHu+41/8MFC5lFuWhUCqDywJ1IykF2M4n6DaEePLrD4B9naGlVQa8oJL2alvaARzmdHkJM6vWhek5maBwR9KhSKymNhv4MrdkOzfc1j1ZzHiXxA9qOfmsFtBykd1sbGOVlCocIc7ZeGwSgci6mp0u/NMnzTIdfrR3ic3ji4cgPfEuP+LgAsAd6Jse/tVDNlRMoQ4xbxxdXJzl8bUaw9FgRZyeWZdz7rIw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=0neqCtYzy7urOJ5hxYZkm7plzb1B57ZCmmBpHvoNoiM=; b=N2gnmHGqeOaQTy/juPHLG11CVJc0x+do0pCwsakYqVcwOs0cN7ShAkY8sX8gtVJ5HIbzJpL29q94/9K9KMMr5P5I4c6YkOWekrwpkBU7jc8btIqLOWGZzfrItBy3ozKZjBZwCzdPGmS5eb0xUXV8meKhd0UJelEkYdnQnpMmDZZ0/iIO3Qkp7k+w+F5sqNGKbMnzXEyMliVHhDgH/HmWtiwsBIP2JDf54QxbWKTTz4SCi1OEuGKXKlCziFyhe2ggskUjssRLc4jZl3QqC1sWx4Otsf+YilQ2C3o0tfS2Cs/MzMXQ6rf8MmUffMDBcv1m+GUIPehE0LnTvp/DWPRWDQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=0neqCtYzy7urOJ5hxYZkm7plzb1B57ZCmmBpHvoNoiM=; b=r0W9upDfZx4dtNujtK8C7G1urkTAHJDopDoCQkC4H9mJbblTQ/r5kA8HQAcPgLMV8cCKfmfVmnKB2O8FnULJgqAoPLwiDfdU9jAJlC0Z+uxfUJyCgbNodn3m7Ri548tQJV1Sdcp2FwIugFXpUE2rOjqdcCi5S6ZTKeXQpueMJAE= Received: from CO6PR18MB4484.namprd18.prod.outlook.com (2603:10b6:5:359::9) by CO6PR18MB4420.namprd18.prod.outlook.com (2603:10b6:303:139::6) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4523.14; Sun, 19 Sep 2021 18:49:13 +0000 Received: from CO6PR18MB4484.namprd18.prod.outlook.com ([fe80::f068:d9c5:99fe:31cf]) by CO6PR18MB4484.namprd18.prod.outlook.com ([fe80::f068:d9c5:99fe:31cf%9]) with mapi id 15.20.4523.018; Sun, 19 Sep 2021 18:49:13 +0000 From: Akhil Goyal To: "Gujjar, Abhinandan S" , Anoob Joseph , Shijith Thotton CC: Jerin Jacob Kollanukkaran , Pavan Nikhilesh Bhagavatula , Ray Kinsella , Ankur Dwivedi , "dev@dpdk.org" , "thomas@monjalon.net" , "nipun.gupta@nxp.com" , Hemant Agrawal Thread-Topic: [PATCH v3] eventdev: update crypto adapter metadata structures Thread-Index: AQHXnj3Sp0R21bZzb0OpVxTST4ioRquYT0kAgAAc44CAAHOCAIAA7gOAgAADKYCACTblAIAAAqoAgAh9hICAAELK4A== Date: Sun, 19 Sep 2021 18:49:12 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 7e431403-6f73-49af-4b0c-08d97b9e2c75 x-ms-traffictypediagnostic: CO6PR18MB4420: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 78ixjg0cC557NsJeOwlSrNBh6wSMZB5w5Met3+c6xwaiaP7mEmAB8cN3Z9ar7X2E4M01mKbKRP2jPg5XFtLsWhehwN1IjANfvzjHGeIe2+J8QAPwcRKqh+45BXs8vsj+pLsz0jWb/vG74He3BHHTa68jB3PvxUTEesaMbYoFJ2z34/nack3jhGu/OsWnxdIMIl2UyqlxEDb2IyQ9nlTARKvwEmwNKCUDnhqkIKTqYC1iu0BWKjrGN7VV/f7WzQooBAoHBZsp254nvK/0JO44fGx/F6Q9wmHZzNU5c6Qr5OvM0tvCkwKxFh/np6LQFLFo0ZjsvINAsvaDZtskJw8MTvdolCJIQVCyuKVGPwIanSX+QMOKE7Bfo7Bq86+nwFGHngeTYxXWQxI3Gs+UxxNTMCMsWGv2gPEVPXQ0v3gA6VgEeOKJ8qXypEwk6ovpsusOIgs2HIwu0zSNoPS3M+VMjlN5m9Ojp5IYEQ1jpMoj/u+WDv6quMFZCdCN5cUDALTTCTFyjUl+t5D2CRYc7ls7tlg+YzfcVVtPZTGzo5V5O3pW/lTJSo4TVZQJ1XR6IcX8s9rFnbImDUtm1MQOyL+nr3IRB6KoLZ53dlU+t+UsHVqty72ViHPizAufjh2tIhM0j1YafTMq7o1q+mMQVHwdPnbZ3NxYwoW6M3DySM1A6MAyBE9K0Er2e8AaXT7WD0dOwt9PSVhh1O0koMEjqq1M1uC1Q5NwqMlf87+W0J0tbtCnTuTF1o4gfz37TxVjWJqVXWYStGVzP+bIXrMX2JhepWV5y+uQ/RylNN4P9AU2tu8= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CO6PR18MB4484.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(26005)(966005)(122000001)(55016002)(186003)(2906002)(110136005)(38100700002)(316002)(54906003)(4326008)(15650500001)(83380400001)(6506007)(5660300002)(8936002)(64756008)(9686003)(508600001)(33656002)(6636002)(55236004)(76116006)(52536014)(7696005)(66556008)(66476007)(66946007)(66446008)(38070700005)(86362001)(71200400001)(8676002); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?X7iUzDyjxMok05BhxLKxVYY2I/1I8qHS2IMFUjITZxq5wBjqpzIJ09RukX7v?= =?us-ascii?Q?WZmEK1Mr7j2/48MjG8UKuHK+Uhl61RkwcLSXuZROa8hcpKXER14LXcuykhTu?= =?us-ascii?Q?1N9GfTXN0WRRya+yxev6P4sQ5decCoDpN7oGS34P38LBal8qRfO3aNI192/j?= =?us-ascii?Q?SHu8ZFPA8YscO6qXARVFmSWrdsokKp3d26DmAWtWALLaNKxTmh3t9nWmeDOd?= =?us-ascii?Q?svd1n5+4SNMix4HuM4rTs21JKz6IIqSc+Vvfr2ZMDsYTeVMgE4C/Cc/ALOl1?= =?us-ascii?Q?rTBHWevzg/ISS91p52IiZ33Ko1QJeIHTSGzxvsYtKPdgzGDoeLOJQAdroWju?= =?us-ascii?Q?Fe33iz4gLeoA3/VyIqnj1P4gR6ZNqdJMZonus8eYPL7UIdneCiYc73/2eEZ8?= =?us-ascii?Q?eFhWxTGUEWYCtLQFkpEqtukal40hHLC/F9T4nM+h2GyPJ3e/bqP9EwlotZJl?= =?us-ascii?Q?hThEdLAwEnDCLM6SZaFeqF2PlKkeXAjYjp4s/Kn/V/eZG/rG3UE0nM5Wp2zS?= =?us-ascii?Q?YpRfT6SL2LxrPDw5VERpfGGN9Kw0Yd86PDaHE2HIb/W8rPti5d2RYPgiBluh?= =?us-ascii?Q?araFSfLsy1fkVHNzVVBEZFnCxuziFhvIpvkjlXqdHY+mkduRqowYwZ0t/o2I?= =?us-ascii?Q?JA7OpGTKUcrPkKsHCLum4UQpUpRGuvBnWG33vrgD4dLzEyJHaDL4xAStYg5U?= =?us-ascii?Q?rmPPFuunPB0DINNYGxiGU7iJk+wQW7PfXqzK0dxAFcZ0X33HVdxFF+y3vddn?= =?us-ascii?Q?1Numg3K/JLbUA7py0bgYbRq2nAhGgiqGSNphiZOj2YqojpZkJXCqgVr0U/gM?= =?us-ascii?Q?5CfNVLZ3dvb1U+CDvQ6IIkP9v+yhQMUuXUaXLyCoAAIu8lUyRlAAKSHOsigZ?= =?us-ascii?Q?YlhQ7gsxReb8R0LMEr/65WOkrxf9a/XxqIhvnW39Z61328RfZRIMsAG7ioF1?= =?us-ascii?Q?edp97HiVFii7hJtXR6zDMMSTfhOJ+6rqMwbv1/Q03kKe2bo/jF+Mz2VCjLcA?= =?us-ascii?Q?26piTKSMctd+1vDjCwow8uVjQ+sRudO69egLj97kkMd6Ad1mZqWBxq1nsoPg?= =?us-ascii?Q?6RBoFhWT1BSF7nupKAjwkbYdKEXv6oNicqbj2aaWidGmcJmerhE+ZZ3Da7/y?= =?us-ascii?Q?Y17hMzx7GueLY+dvrdZ6BZgIIEA6s7UlkcS6EXOfsUp4NYT29lfY8RXtH+p5?= =?us-ascii?Q?VjRWtTbZKf9UwfgH+wQQUreM24kte6ZSgXMHhbZF9WOi5Dk9ReKebU5VT092?= =?us-ascii?Q?mKBD6ZdQ3duA4EbrF1WbJ42G5UMg4nur2Al2uHCZhukeb3wstQs5jM6kITRo?= =?us-ascii?Q?dZ8UbsBeubyiFh+yczxgiO/G?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: marvell.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO6PR18MB4484.namprd18.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7e431403-6f73-49af-4b0c-08d97b9e2c75 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Sep 2021 18:49:12.7313 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: yQ4MD+2ZzgDHHHDowJWSYApjVVvjckPJ1qqWC57h/Stl4xJjWYq4j9VKdhFgdC/ay0NCvg0/hB6xJ/ZXw3z9nA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR18MB4420 X-Proofpoint-ORIG-GUID: Wcw414iwyED1FPfLzeAz2u9_e_FtX1VQ X-Proofpoint-GUID: Wcw414iwyED1FPfLzeAz2u9_e_FtX1VQ X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.391,FMLib:17.0.607.475 definitions=2021-09-19_06,2021-09-17_02,2020-04-07_01 Subject: Re: [dpdk-dev] [PATCH v3] eventdev: update crypto adapter metadata structures X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Abhinandan, > > > >> >> > > > >> >> >> In crypto adapter metadata, reserved bytes in request info > > > >> >> >> structure is a space holder for response info. It enforces a= n > > > >> >> >> order of operation if the structures are updated using memcp= y > > > >> >> >> to avoid overwriting response info. It is logical to move th= e > > > >> >> >> reserved space out of request info. It also solves the > > > >> >> >> ordering issue > > > >> mentioned before. > > > >> >> >I would like to understand what kind of ordering issue you hav= e > > > >> >> >faced with the current approach. Could you please give an > > > >> >> >example/sequence > > > >> >> and explain? > > > >> >> > > > > >> >> > > > >> >> I have seen this issue with crypto adapter autotest (#n215). > > > >> >> > > > >> >> Example: > > > >> >> rte_memcpy(&m_data.response_info, &response_info, > > > >> >> sizeof(response_info)); rte_memcpy(&m_data.request_info, > > > >> >> &request_info, sizeof(request_info)); > > > >> >> > > > >> >> Here response info is getting overwritten by request info. > > > >> >> Above lines can reordered to fix the issue, but can be ignored > > > >> >> with this > > > >> patch. > > > >> >There is a reason for designing the metadata in this way. > > > >> >Right now, sizeof (union rte_event_crypto_metadata) is 16 bytes. > > > >> >So, the session based case needs just 16 bytes to store the data. > > > >> >Whereas, for sessionless case each crypto_ops requires another 16 > > > bytes. > > > >> > > > > >> >By changing the struct in the following way you are doubling the > > > >> >memory requirement. > > > >> >With the changes, for sessionless case, each crypto op requires 3= 2 > > > >> >bytes of space instead of 16 bytes and the mempool will be bigger= . > > > >> >This will have the perf impact too! > > > >> > > > > >> >You can just copy the individual members(cdev_id & queue_pair_id) > > > >> >after the response_info. > > > >> >OR You have a better way? > > > >> > > > > >> > > > >> I missed the second word of event structure. It adds an extra 8 > > > >> bytes, which is not required. > > > >I guess you meant not adding, it is overlapping with request > information. > > > >> Let me know, what you think of below change to metadata structure. > > > >> > > > >> struct rte_event_crypto_metadata { > > > >> uint64_t response_info; // 8 bytes > > > >With this, it lags clarity saying it is an event information. > > > >> struct rte_event_crypto_request request_info; // 8 bytes }; > > > >> > > > >> Total structure size is 16 bytes. > > > >> > > > >> Response info can be set like below in test app: > > > >> m_data.response_info =3D response_info.event; > > > >> > > > >> The main aim of this patch is to decouple response info from reque= st > > info. > > > >The decoupling was required as it was doing memcpy. > > > >If you copy the individual members of request info(after response), > > > >you don't require it. > > > > > > With this change, the application will be free of such constraints. > > > > [Anoob] Why don't we make it like, > > > > struct rte_event_crypto_metadata { > > uint64_t ev_w0_template; > > /**< Template of the first word of ``struct rte_event`` > > (rte_event.event) to be set in the events generated by crypto adapter i= n > > both RTE_EVENT_CRYPTO_ADAPTER_OP_NEW and > > * RTE_EVENT_CRYPTO_ADAPTER_OP_FORWARD modes. > > */ > > struct rte_event_crypto_request request_info; }; > > > > This way, the usage would become more obvious and we will not have > > additional space reserved as well. >=20 > Thanks for the suggestion. At this point, it is like an application witho= ut > understanding the data structure/ internals > has used memcpy and we are trying to fix it by changing the actual data > structure instead of fixing the application! > With this patch, the other applications(outside of dpdk) which are using = the > data structures in a right are forced to change! > I don't think, this is the right way to handle this. I think, we should b= e > updating the test case and documentation for this rather > than changing the data structure. I expect the next version of this patc= h > should have those changes. Thanks! The point here is about making the specification better and clearer to the = user. If the structure is not clear and is error prone if somebody does not follo= w Proper documentation, we can modify it to reduce possible issues. This is a cleanup which was added in deprecation notice as well in the last= release. This is a ABI break release and we should do this cleanup if it is legit. Applications outside DPDK are notified as per the deprecation notice in the= last release. We have followed standard procedure to modify this structure, hence, no need to worry about that. We have provided, 2-3 suggestions to clean this up. Please suggest which one is best for Intel use case. Having first element as reserved in the structure doesn't look quite obviou= s. This was also highlighted in the original patch, but was not taken up serio= usly as we were not supporting it at that point. See this http://patches.dpdk.org/project/dpdk/patch/1524573807-168522-2-git-send-ema= il-abhinandan.gujjar@intel.com/ -Akhil Please note: Avoid top posting for comments and remove unnecessary lines while commentin= g back.