From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id F3A102BC9 for ; Thu, 25 Feb 2016 18:16:56 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP; 25 Feb 2016 09:16:40 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,498,1449561600"; d="scan'208";a="923866398" Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99]) by fmsmga002.fm.intel.com with ESMTP; 25 Feb 2016 09:16:38 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.237]) by IRSMSX107.ger.corp.intel.com ([163.33.3.99]) with mapi id 14.03.0248.002; Thu, 25 Feb 2016 17:16:37 +0000 From: "Ananyev, Konstantin" To: "Tan, Jianfeng" , "dev@dpdk.org" Thread-Topic: [PATCH v3 01/12] ethdev: add API to query packet type filling info Thread-Index: AQHRb9x63YVJTZ3yWkKEQ5Gzpzo6uZ882isQgAAbaICAAARWUA== Date: Thu, 25 Feb 2016 17:16:37 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725836B0B5DE@irsmsx105.ger.corp.intel.com> References: <1451544799-70776-1-git-send-email-jianfeng.tan@intel.com> <1456386842-112571-1-git-send-email-jianfeng.tan@intel.com> <1456386842-112571-2-git-send-email-jianfeng.tan@intel.com> <2601191342CEEE43887BDE71AB97725836B0B4F8@irsmsx105.ger.corp.intel.com> <56CF2D81.2090904@intel.com> In-Reply-To: <56CF2D81.2090904@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjZkNTMwY2YtYzQxMC00MDI4LWJmYTctNjg3MzAzYjEyMDVhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IjYwaDh6cEtSUHNpWEpCaDZZV3Y0YUVcL2VlcEN4V1RXMGl5N2h5aWdvbnJVPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 01/12] ethdev: add API to query packet type filling info X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Feb 2016 17:16:57 -0000 > -----Original Message----- > From: Tan, Jianfeng > Sent: Thursday, February 25, 2016 4:36 PM > To: Ananyev, Konstantin; dev@dpdk.org > Cc: Zhang, Helin; nelio.laranjeiro@6wind.com; adrien.mazarguil@6wind.com;= rahul.lakkireddy@chelsio.com > Subject: Re: [PATCH v3 01/12] ethdev: add API to query packet type fillin= g info >=20 > Hi Konstantin, >=20 > On 2/25/2016 11:46 PM, Ananyev, Konstantin wrote: > > Hi Jainfeng, > > > >> Add a new API rte_eth_dev_get_ptype_info to query whether/what packet > >> type can be filled by given pmd rx burst function. > >> > >> Signed-off-by: Jianfeng Tan > >> --- > >> lib/librte_ether/rte_ethdev.c | 32 ++++++++++++++++++++++++++++++++ > >> lib/librte_ether/rte_ethdev.h | 23 +++++++++++++++++++++++ > >> 2 files changed, 55 insertions(+) > >> > >> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethd= ev.c > >> index 1257965..b52555b 100644 > >> --- a/lib/librte_ether/rte_ethdev.c > >> +++ b/lib/librte_ether/rte_ethdev.c > >> @@ -1576,6 +1576,38 @@ rte_eth_dev_info_get(uint8_t port_id, struct rt= e_eth_dev_info *dev_info) > >> dev_info->driver_name =3D dev->data->drv_name; > >> } > >> > >> +int > >> +rte_eth_dev_get_ptype_info(uint8_t port_id, uint32_t ptype_mask, > >> + uint32_t **p_ptypes) > >> +{ > >> + int i, j, ret; > >> + struct rte_eth_dev *dev; > >> + const uint32_t *all_ptypes; > >> + > >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > >> + dev =3D &rte_eth_devices[port_id]; > >> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_ptype_info_get, -ENOTSUP)= ; > >> + all_ptypes =3D (*dev->dev_ops->dev_ptype_info_get)(dev); > >> + > >> + if (!all_ptypes) > >> + return 0; > >> + > >> + for (i =3D 0, ret =3D 0; all_ptypes[i] !=3D RTE_PTYPE_UNKNOWN; ++i) > >> + if (all_ptypes[i] & ptype_mask) > >> + ret++; > >> + if (ret =3D=3D 0) > >> + return 0; > >> + > >> + *p_ptypes =3D (uint32_t *)malloc(sizeof(uint32_t) * ret); > >> + if (*p_ptypes =3D=3D NULL) > >> + return -ENOMEM; > > > > I thought we all agreed to follow snprintf()-like logic and avoid any m= emory allocations inside that function. > > So why malloc() again? > > Konstantin > > >=20 > Sorry for the setback. I still have concerns that snprintf()-like needs > to be called twice by an application to get the ptype info. So I write > this implementation for you to comment. >=20 > So what's the reason why we should avoid memory allocations inside that > function? By the same reason none of other rte_ethdev get_info() style functions (rte_eth_dev_info_get, rte_eth_rx_queue_info_get, rte_eth_xstats_get, rte_eth_dev_rss_reta_query, etc) allocate space themselves. It is a good practice to let user to decide himself how/where to allocate/f= ree a space for that date: on a stack, inside a data segment (global variable), on heap (malloc), at hugepages via rte_malloc, somewhere else. =20 BTW, if you had concerns about that approach, why didn't you provide any ar= guments when it was discussed/agreed? Instead you came up a month later with same old approach that voids my and = other reviewers comments and even v2 of your own patch.=20 Do you have any good reason for that? Konstantin