Hi Beilei

-----Original Message-----
From: Xing, Beilei
Sent: Thursday, June 1, 2017 3:21 AM
To: Chilikin, Andrey <redacted>; dev@dpdk.org
Cc: Wu, Jingjing <redacted>
Subject: RE: [PATCH] net/i40e: get information about ddp profile

Hi Andrey,

quoted

-----Original Message-----
From: Chilikin, Andrey
Sent: Friday, May 26, 2017 8:42 PM
To: dev@dpdk.org
Cc: Xing, Beilei <redacted>; Wu, Jingjing
[off-list ref]; Chilikin, Andrey [off-list ref]
Subject: [PATCH] net/i40e: get information about ddp profile

This patch adds ability to request information about dynamic device
personalization profile

Signed-off-by: Andrey Chilikin <redacted>
---
 drivers/net/i40e/rte_pmd_i40e.c |  163
++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/rte_pmd_i40e.h |   45 +++++++++++
 2 files changed, 204 insertions(+), 4 deletions(-)

diff --git a/drivers/net/i40e/rte_pmd_i40e.c

b/drivers/net/i40e/rte_pmd_i40e.c index f7ce62b..72e1564 100644

--- a/drivers/net/i40e/rte_pmd_i40e.c
+++ b/drivers/net/i40e/rte_pmd_i40e.c

@@ -1468,7 +1468,7 @@ int rte_pmd_i40e_set_vf_vlan_filter(uint8_t

port, uint16_t vlan_id,
 	return ret;
 }

-#define I40E_PROFILE_INFO_SIZE 48
+#define I40E_PROFILE_INFO_SIZE sizeof(struct
+rte_pmd_i40e_profile_info)
 #define I40E_MAX_PROFILE_NUM 16

 static void

@@ -1520,9 +1520,6 @@ int rte_pmd_i40e_set_vf_vlan_filter(uint8_t

port, uint16_t vlan_id,
 	return status;
 }

-#define I40E_PROFILE_INFO_SIZE 48
-#define I40E_MAX_PROFILE_NUM 16
-
 /* Check if the profile info exists */  static int
i40e_check_profile_info(uint8_t port, uint8_t *profile_info_sec) @@ -
1682,6 +1679,164 @@ int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port,
uint16_t vlan_id,
 	return status;
 }

+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) {
+	uint32_t ret_size;
+	struct i40e_package_header *pkg_hdr;
+	struct i40e_generic_seg_header *i40e_seg_hdr;
+	struct i40e_generic_seg_header *note_seg_hdr;
+	struct i40e_generic_seg_header *metadata_seg_hdr;
+
+	if (!info_buff) {
+		PMD_DRV_LOG(ERR, "Output info buff is invalid.");
+		return -EINVAL;
+	}
+
+	if (!pkg_buff || pkg_size < (sizeof(struct i40e_package_header) +
+		sizeof(struct i40e_metadata_segment) +
+		sizeof(uint32_t) * 2)) {
+		PMD_DRV_LOG(ERR, "Package buff is invalid.");
+		return -EINVAL;
+	}
+
+	pkg_hdr = (struct i40e_package_header *)pkg_buff;
+	if (pkg_hdr->segment_count < 2) {
+		PMD_DRV_LOG(ERR, "Segment_count should be 2 at least.");
+		return -EINVAL;
+	}
+
+	/* Find metadata segment */
+	metadata_seg_hdr =
i40e_find_segment_in_package(SEGMENT_TYPE_METADATA,
+		pkg_hdr);
+
+	/* Find global notes segment */
+	note_seg_hdr =
i40e_find_segment_in_package(SEGMENT_TYPE_NOTES,
+		pkg_hdr);
+
+	/* Find i40e profile segment */
+	i40e_seg_hdr =
i40e_find_segment_in_package(SEGMENT_TYPE_I40E,
+pkg_hdr);
+
+	/* get global header info */
+	if (type == RTE_PMD_I40E_PKG_INFO_GLOBAL_HEADER) {
+		struct rte_pmd_i40e_profile_info *info =
+			(struct rte_pmd_i40e_profile_info *)info_buff;
+
+		if (info_size < sizeof(struct rte_pmd_i40e_profile_info)) {
+			PMD_DRV_LOG(ERR, "Output info buff size is
invalid.");
+			return -EINVAL;
+		}
+
+		if (!metadata_seg_hdr) {
+			PMD_DRV_LOG(ERR, "Failed to find metadata
segment header");
+			return -EINVAL;
+		}
+
+		memset(info, 0, sizeof(struct rte_pmd_i40e_profile_info));
+		info->owner = RTE_PMD_I40E_DDP_OWNER_UNKNOWN;
+		info->track_id =
+			((struct i40e_metadata_segment
*)metadata_seg_hdr)->track_id;
+
+		memcpy(info->name,
+			((struct i40e_metadata_segment
*)metadata_seg_hdr)->name,
+			I40E_DDP_NAME_SIZE);
+		memcpy(&info->version,
+			&((struct i40e_metadata_segment
*)metadata_seg_hdr)->version,
+			sizeof(struct i40e_ddp_version));
+		return I40E_SUCCESS;
+	}
+
+	/* get global note size */
+	if (type == RTE_PMD_I40E_PKG_INFO_GLOBAL_NOTES_SIZE) {
+		if (info_size < sizeof(uint32_t)) {
+			PMD_DRV_LOG(ERR, "Invalid information buffer
size");
+			return -EINVAL;
+		}
+		if (note_seg_hdr == NULL)
+			ret_size = 0;
+		else
+			ret_size = note_seg_hdr->size;
+		*(uint32_t *)info_buff = ret_size;
+		return I40E_SUCCESS;
+	}
+
+	/* get global note */
+	if (type == RTE_PMD_I40E_PKG_INFO_GLOBAL_NOTES) {
+		if (note_seg_hdr == NULL)
+			return -ENOTSUP;
+		if (info_size < note_seg_hdr->size) {
+			PMD_DRV_LOG(ERR, "Information buffer size is too
small");
+			return -EINVAL;
+		}
+		memcpy(info_buff, &note_seg_hdr[1], note_seg_hdr->size);
+		return I40E_SUCCESS;
+	}
+
+	/* get i40e segment header info */
+	if (type == RTE_PMD_I40E_PKG_INFO_HEADER) {
+		struct rte_pmd_i40e_profile_info *info =
+			(struct rte_pmd_i40e_profile_info *)info_buff;
+
+		if (info_size < sizeof(struct rte_pmd_i40e_profile_info)) {
+			PMD_DRV_LOG(ERR, "Output info buff size is
invalid.");
+			return -EINVAL;
+		}
+
+		if (!metadata_seg_hdr) {
+			PMD_DRV_LOG(ERR, "Failed to find metadata
segment header");
+			return -EINVAL;
+		}
+
+		if (!i40e_seg_hdr) {
+			PMD_DRV_LOG(ERR, "Failed to find i40e segment
header");
+			return -EINVAL;
+		}
+
+		memset(info, 0, sizeof(struct rte_pmd_i40e_profile_info));
+		info->owner = RTE_PMD_I40E_DDP_OWNER_UNKNOWN;
+		info->track_id =
+			((struct i40e_metadata_segment
*)metadata_seg_hdr)->track_id;
+
+		memcpy(info->name,
+			((struct i40e_profile_segment *)i40e_seg_hdr)-

quoted

name,

+			I40E_DDP_NAME_SIZE);
+		memcpy(&info->version,
+			&((struct i40e_profile_segment *)i40e_seg_hdr)-

quoted

version,

+			sizeof(struct i40e_ddp_version));
+		return I40E_SUCCESS;
+	}
+
+	/* get number of devices */
+	if (type == RTE_PMD_I40E_PKG_INFO_DEVID_NUM) {
+		if (info_size < sizeof(uint32_t)) {
+			PMD_DRV_LOG(ERR, "Invalid information buffer
size");
+			return -EINVAL;
+		}
+		*(uint32_t *)info_buff =
+			((struct i40e_profile_segment *)i40e_seg_hdr)-

quoted

device_table_count;

+		return I40E_SUCCESS;
+	}
+
+	/* get list of devices */
+	if (type == RTE_PMD_I40E_PKG_INFO_DEVID_LIST) {
+		uint32_t dev_num;
+		dev_num =
+			((struct i40e_profile_segment *)i40e_seg_hdr)-

quoted

device_table_count;

+		if (info_size < sizeof(struct
rte_pmd_i40e_ddp_device_id)*dev_num) {
+			PMD_DRV_LOG(ERR, "Invalid information buffer
size");
+			return -EINVAL;
+		}
+		memcpy(info_buff,
+			((struct i40e_profile_segment *)i40e_seg_hdr)-

quoted

device_table,

+			sizeof(struct
rte_pmd_i40e_ddp_device_id)*dev_num);
+		return I40E_SUCCESS;
+	}
+
+	PMD_DRV_LOG(ERR, "Info type %u is invalid.", type);
+	return -EINVAL;
+}

How about using switch case to replace above if statements?

Not sure if switch case is better here as there is no post-switch processing,
each  if statement has its own return and no breaks are needed.

quoted

+
 int
 rte_pmd_i40e_get_ddp_list(uint8_t port, uint8_t *buff, uint32_t size)
{ diff --git a/drivers/net/i40e/rte_pmd_i40e.h
b/drivers/net/i40e/rte_pmd_i40e.h index 1efb2c4..0f376e2 100644

--- a/drivers/net/i40e/rte_pmd_i40e.h
+++ b/drivers/net/i40e/rte_pmd_i40e.h

@@ -74,6 +74,21 @@ enum rte_pmd_i40e_package_op {
 	RTE_PMD_I40E_PKG_OP_MAX = 32
 };

+/**
+* Types of package information.
+*/
+enum rte_pmd_i40e_package_info {
+	RTE_PMD_I40E_PKG_INFO_UNDEFINED = 0,
+	RTE_PMD_I40E_PKG_INFO_GLOBAL_HEADER,
+	RTE_PMD_I40E_PKG_INFO_GLOBAL_NOTES_SIZE,
+	RTE_PMD_I40E_PKG_INFO_GLOBAL_NOTES,
+	RTE_PMD_I40E_PKG_INFO_GLOBAL_MAX = 1024,
+	RTE_PMD_I40E_PKG_INFO_HEADER,
+	RTE_PMD_I40E_PKG_INFO_DEVID_NUM,
+	RTE_PMD_I40E_PKG_INFO_DEVID_LIST,
+	RTE_PMD_I40E_PKG_INFO_MAX = 0xFFFFFFFF };
+
 #define RTE_PMD_I40E_DDP_NAME_SIZE 32

 /**

@@ -88,6 +103,14 @@ struct rte_pmd_i40e_ddp_version {  };

 /**
+* Device ID for dynamic device personalization.
+*/
+struct rte_pmd_i40e_ddp_device_id {
+	uint32_t vendor_dev_id;
+	uint32_t sub_vendor_dev_id;
+};
+
+/**
  * Profile information in profile info list.
  */
 struct rte_pmd_i40e_profile_info {

@@ -98,6 +121,8 @@ struct rte_pmd_i40e_profile_info {
 	uint8_t name[RTE_PMD_I40E_DDP_NAME_SIZE];  };

+#define RTE_PMD_I40E_DDP_OWNER_UNKNOWN 0xFF
+
 /**
  * Profile information list returned from HW.
  */

@@ -499,6 +524,26 @@ int rte_pmd_i40e_process_ddp_package(uint8_t

port, uint8_t *buff,
 				     enum rte_pmd_i40e_package_op op);

 /**
+* rte_pmd_i40e_get_ddp_info - Get profile's info
+* @param pkg
+*    buffer of package.
+* @param pkg_size
+*    package buffer size
+* @param info
+*    buffer for response
+* @param size
+*    response buffer size
+* @param type
+*    type of information requested
+* @return
+*   - (0) if successful.
+*   - (-EINVAL) if bad parameter.
+*/
+int rte_pmd_i40e_get_ddp_info(uint8_t *pkg, uint32_t pkg_size,
+				     uint8_t *info, uint32_t size,
+				     enum rte_pmd_i40e_package_info type);
+
+/**
  * rte_pmd_i40e_get_ddp_list - Get loaded profile list
  * @param port
  *    port id
--
1.7.0.7

Will you add testpmd CLI to support getting DDP info?
Besides, API info in rte_pmd_i40e_version.map should be updated.

Will add to the second revision of the patch.

/Andrey