[dpdk-dev] [PATCH 08/12] pmd/mlx4: add dev_ptype_info_get implementation

[Tan, Jianfeng]

On 1/4/2016 7:11 PM, Adrien Mazarguil wrote:
> Hi Jianfeng,
>
> I'm only commenting the mlx4/mlx5 bits in this message, see below.
>
> On Thu, Dec 31, 2015 at 02:53:15PM +0800, Jianfeng Tan wrote:
>> Signed-off-by: Jianfeng Tan <jianfeng.tan at intel.com>
>> ---
>>   drivers/net/mlx4/mlx4.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
>> index 207bfe2..85afa32 100644
>> --- a/drivers/net/mlx4/mlx4.c
>> +++ b/drivers/net/mlx4/mlx4.c
>> @@ -2836,6 +2836,8 @@ rxq_cleanup(struct rxq *rxq)
>>    * @param flags
>>    *   RX completion flags returned by poll_length_flags().
>>    *
>> + * @note: fix mlx4_dev_ptype_info_get() if any change here.
>> + *
>>    * @return
>>    *   Packet type for struct rte_mbuf.
>>    */
>> @@ -4268,6 +4270,30 @@ mlx4_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *info)
>>   	priv_unlock(priv);
>>   }
>>   
>> +static int
>> +mlx4_dev_ptype_info_get(struct rte_eth_dev *dev, uint32_t ptype_mask,
>> +		uint32_t ptypes[])
>> +{
>> +	int num = 0;
>> +
>> +	if ((dev->rx_pkt_burst == mlx4_rx_burst)
>> +			|| (dev->rx_pkt_burst == mlx4_rx_burst_sp)) {
>> +		/* refers to rxq_cq_to_pkt_type() */
>> +		if ((ptype_mask & RTE_PTYPE_L3_MASK) == RTE_PTYPE_L3_MASK) {
>> +			ptypes[num++] = RTE_PTYPE_L3_IPV4;
>> +			ptypes[num++] = RTE_PTYPE_L3_IPV6;
>> +		}
>> +
>> +		if ((ptype_mask & RTE_PTYPE_INNER_L3_MASK) == RTE_PTYPE_INNER_L3_MASK) {
>> +			ptypes[num++] = RTE_PTYPE_INNER_L3_IPV4;
>> +			ptypes[num++] = RTE_PTYPE_INNER_L3_IPV6;
>> +		}
>> +	} else
>> +		num = -ENOTSUP;
>> +
>> +	return num;
>> +}
> I think checking for mlx4_rx_burst and mlx4_rx_burst_sp is unnecessary at
> the moment, all RX burst functions do update the packet_type field, no need
> for extra complexity.
>
> Same comment for mlx5.

Hi Mazarguil,

My original thought is that rx_pkt_burst could be also set as 
removed_rx_burst, which does not make sense indeed
because it's only possible when the device is closed.

Another consideration is to keep same style with other devices. Each 
kind of device could have several rx burst functions.
So current implementation can keep extensibility to add new rx burst 
functions. How do you think of it?

Thanks,
Jianfeng

>
>> +
>>   /**
>>    * DPDK callback to get device statistics.
>>    *
>> @@ -4989,6 +5015,7 @@ static const struct eth_dev_ops mlx4_dev_ops = {
>>   	.stats_reset = mlx4_stats_reset,
>>   	.queue_stats_mapping_set = NULL,
>>   	.dev_infos_get = mlx4_dev_infos_get,
>> +	.dev_ptypes_info_get = mlx4_dev_ptype_info_get,
>>   	.vlan_filter_set = mlx4_vlan_filter_set,
>>   	.vlan_tpid_set = NULL,
>>   	.vlan_strip_queue_set = NULL,
>> -- 
>> 2.1.4
>>