-----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 filling info

Hi Konstantin,

On 2/25/2016 11:46 PM, Ananyev, Konstantin wrote:

quoted

Hi Jainfeng,

quoted

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 <redacted>
---
  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_ethdev.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 rte_eth_dev_info *dev_info)
  	dev_info->driver_name = 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 = &rte_eth_devices[port_id];
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_ptype_info_get, -ENOTSUP);
+	all_ptypes = (*dev->dev_ops->dev_ptype_info_get)(dev);
+
+	if (!all_ptypes)
+		return 0;
+
+	for (i = 0, ret = 0; all_ptypes[i] != RTE_PTYPE_UNKNOWN; ++i)
+		if (all_ptypes[i] & ptype_mask)
+			ret++;
+	if (ret == 0)
+		return 0;
+
+	*p_ptypes = (uint32_t *)malloc(sizeof(uint32_t) * ret);
+	if (*p_ptypes == NULL)
+		return -ENOMEM;

I thought we all agreed to follow snprintf()-like logic and avoid any memory allocations inside that function.
So why malloc() again?
Konstantin

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.

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/free a space for that date:
on a stack, inside a data segment (global variable), on heap (malloc),
at hugepages via rte_malloc, somewhere else.  

BTW, if you had concerns about that approach, why didn't you provide any arguments
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. 
Do you have any good reason for that?

Konstantin