From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <beilei.xing@intel.com>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id A60781B19B
 for <dev@dpdk.org>; Fri, 29 Sep 2017 09:31:26 +0200 (CEST)
Received: from fmsmga004.fm.intel.com ([10.253.24.48])
 by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 29 Sep 2017 00:31:25 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.42,451,1500966000"; d="scan'208";a="317592233"
Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206])
 by fmsmga004.fm.intel.com with ESMTP; 29 Sep 2017 00:31:25 -0700
Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by
 FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS)
 id 14.3.319.2; Fri, 29 Sep 2017 00:31:22 -0700
Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by
 FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS)
 id 14.3.319.2; Fri, 29 Sep 2017 00:31:21 -0700
Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.159]) by
 SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002;
 Fri, 29 Sep 2017 15:30:37 +0800
From: "Xing, Beilei" <beilei.xing@intel.com>
To: "Rybalchenko, Kirill" <kirill.rybalchenko@intel.com>, "dev@dpdk.org"
 <dev@dpdk.org>
CC: "Chilikin, Andrey" <andrey.chilikin@intel.com>, "Wu, Jingjing"
 <jingjing.wu@intel.com>
Thread-Topic: [PATCH v6 1/2] net/i40e: get information about protocols
 defined in ddp profile
Thread-Index: AQHTOF5hdF4XFDHA8kqSxjvnbe1nwaLLcv8w
Date: Fri, 29 Sep 2017 07:30:36 +0000
Message-ID: <94479800C636CB44BD422CB454846E0132038CC5@SHSMSX101.ccr.corp.intel.com>
References: <1505843612-40976-1-git-send-email-kirill.rybalchenko@intel.com>
 <1506605599-31693-1-git-send-email-kirill.rybalchenko@intel.com>
 <1506605599-31693-2-git-send-email-kirill.rybalchenko@intel.com>
In-Reply-To: <1506605599-31693-2-git-send-email-kirill.rybalchenko@intel.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-originating-ip: [10.239.127.40]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v6 1/2] net/i40e: get information about
 protocols defined in ddp profile
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 29 Sep 2017 07:31:27 -0000

Seems you didn't address my questions except the memory access error in you=
r last version.

> -----Original Message-----
> From: Rybalchenko, Kirill
> Sent: Thursday, September 28, 2017 9:33 PM
> To: dev@dpdk.org
> Cc: Rybalchenko, Kirill <kirill.rybalchenko@intel.com>; Chilikin, Andrey
> <andrey.chilikin@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH v6 1/2] net/i40e: get information about protocols defined=
 in
> ddp profile
>=20
> This patch adds new package info types to get list of protocols, pctypes =
and
> ptypes defined in a profile
>=20
> ---
> v3
> info_size parameter always represents size of the info buffer in bytes
>=20
> v6
> fix bug with wrong usage of info_size parameter
>=20
> Signed-off-by: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
> ---
>  drivers/net/i40e/rte_pmd_i40e.c | 174
> ++++++++++++++++++++++++++++++++++++++++
>  drivers/net/i40e/rte_pmd_i40e.h |  25 ++++++
>  2 files changed, 199 insertions(+)
>=20
> diff --git a/drivers/net/i40e/rte_pmd_i40e.c
> b/drivers/net/i40e/rte_pmd_i40e.c index c08e07a..f57e59b 100644
> --- a/drivers/net/i40e/rte_pmd_i40e.c
> +++ b/drivers/net/i40e/rte_pmd_i40e.c
> @@ -1706,6 +1706,26 @@ rte_pmd_i40e_process_ddp_package(uint8_t
> port, uint8_t *buff,
>  	return status;
>  }
>=20
> +/* Get number of tvl records in the section */ static unsigned int
> +i40e_get_tlv_section_size(struct i40e_profile_section_header *sec) {
> +	unsigned int i, nb_rec, nb_tlv =3D 0;
> +	struct i40e_profile_tlv_section_record *tlv;
> +
> +	if (!sec)
> +		return nb_tlv;
> +
> +	/* get number of records in the section */
> +	nb_rec =3D sec->section.size / sizeof(struct
> i40e_profile_tlv_section_record);
> +	for (i =3D 0; i < nb_rec; ) {
> +		tlv =3D (struct i40e_profile_tlv_section_record *)&sec[1 + i];
> +		i +=3D tlv->len;
> +		nb_tlv++;
> +	}
> +	return nb_tlv;
> +}
> +
>  int rte_pmd_i40e_get_ddp_info(uint8_t *pkg_buff, uint32_t pkg_size,
>  	uint8_t *info_buff, uint32_t info_size,
>  	enum rte_pmd_i40e_package_info type)
> @@ -1860,6 +1880,160 @@ int rte_pmd_i40e_get_ddp_info(uint8_t
> *pkg_buff, uint32_t pkg_size,
>  		return I40E_SUCCESS;
>  	}
>=20
> +	/* get number of protocols */
> +	if (type =3D=3D RTE_PMD_I40E_PKG_INFO_PROTOCOL_NUM) {
> +		struct i40e_profile_section_header *proto;
> +
> +		if (info_size < sizeof(uint32_t)) {
> +			PMD_DRV_LOG(ERR, "Invalid information buffer
> size");
> +			return -EINVAL;
> +		}
> +		proto =3D i40e_find_section_in_profile(SECTION_TYPE_PROTO,
> +						     (struct
> i40e_profile_segment *)
> +						     i40e_seg_hdr);
> +		*(uint32_t *)info_buff =3D i40e_get_tlv_section_size(proto);
> +		return I40E_SUCCESS;
> +	}
> +
> +	/* get list of protocols */
> +	if (type =3D=3D RTE_PMD_I40E_PKG_INFO_PROTOCOL_LIST) {
> +		uint32_t i, j, nb_rec, nb_rec_buf;
> +		struct rte_pmd_i40e_proto_info *pinfo;
> +		struct i40e_profile_section_header *proto;
> +		struct i40e_profile_tlv_section_record *tlv;
> +
> +		proto =3D i40e_find_section_in_profile(SECTION_TYPE_PROTO,
> +						     (struct
> i40e_profile_segment *)
> +						     i40e_seg_hdr);
> +		nb_rec =3D i40e_get_tlv_section_size(proto);

If number of records is different with number of TLV, I think it's better t=
o change nb_rec with nb_tlv, a little confused here.

> +		nb_rec_buf =3D info_size / sizeof(struct
> rte_pmd_i40e_proto_info);

How about changing nb_rec_buf with nb_proto_info?

> +		if (nb_rec_buf < nb_rec) {
> +			PMD_DRV_LOG(ERR, "Invalid information buffer
> size");
> +			return -EINVAL;
> +		}
> +		pinfo =3D (struct rte_pmd_i40e_proto_info *)info_buff;
> +		for (i =3D 0; i < nb_rec_buf; i++) {
> +			pinfo[i].proto_id =3D RTE_PMD_I40E_PROTO_UNUSED;
> +			memset(pinfo[i].name, 0,
> RTE_PMD_I40E_DDP_NAME_SIZE);
> +		}
> +		if (nb_rec =3D=3D 0)
> +			return I40E_SUCCESS;


Why not check nb_rec just when getting the value? =20

<...>

Same comments for RTE_PMD_I40E_PKG_INFO_PCTYPE_LIST and RTE_PMD_I40E_PKG_IN=
FO_PTYPE_LIST.