[dpdk-dev] [PATCH 35/38] net/dpaa: add support for packet type parsing

[Shreyansh Jain]

On Wednesday 28 June 2017 09:20 PM, Ferruh Yigit wrote:
> On 6/16/2017 6:41 AM, Shreyansh Jain wrote:
>> Signed-off-by: Hemant Agrawal <hemant.agrawal at nxp.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
> 
> <...>
> 
>> +static const uint32_t *
>> +dpaa_supported_ptypes_get(struct rte_eth_dev *dev)
>> +{
>> +	static const uint32_t ptypes[] = {
>> +		/*todo -= add more types */
>> +		RTE_PTYPE_L2_ETHER,
>> +		RTE_PTYPE_L3_IPV4,
>> +		RTE_PTYPE_L3_IPV4_EXT,
>> +		RTE_PTYPE_L3_IPV6,
>> +		RTE_PTYPE_L3_IPV6_EXT,
>> +		RTE_PTYPE_L4_TCP,
>> +		RTE_PTYPE_L4_UDP,
>> +		RTE_PTYPE_L4_SCTP
>> +	};
>> +
>> +	PMD_INIT_FUNC_TRACE();
>> +
>> +	if (dev->rx_pkt_burst == dpaa_eth_queue_rx)
> 
> Isn't this only rx function exists? Is this check required?

Yes, for now we only have a single function. But, just like other driver, we can add more in near future based on some variation of RX.
In fact, this is more to be in sync with how other drivers implement this function (albeit, we only have a single Rx variant).

> 
>> +		return ptypes;
>> +	return NULL;
>> +}
>>  
>>  static int dpaa_eth_dev_start(struct rte_eth_dev *dev)
>>  {
>> @@ -159,6 +180,10 @@ static void dpaa_eth_dev_info(struct rte_eth_dev *dev,
>>  	dev_info->max_vfs = 0;
>>  	dev_info->max_vmdq_pools = ETH_16_POOLS;
>>  	dev_info->flow_type_rss_offloads = DPAA_RSS_OFFLOAD_ALL;
>> +	dev_info->rx_offload_capa =
>> +		(DEV_RX_OFFLOAD_IPV4_CKSUM |
>> +		DEV_RX_OFFLOAD_UDP_CKSUM  |
>> +		DEV_RX_OFFLOAD_TCP_CKSUM);
> 
> I guess this patch also enable L3/L4 Rx checksum offload, can you please
> update commit log.

Ok. I will do that

> 
> And should ol_flags set with one of the PKT_RX_IP_CKSUM_BAD,
> PKT_RX_IP_CKSUM_GOOD, PKT_RX_IP_CKSUM_NONE? Also with L4 versions of these?

Yes. I will fix that.

> 
> <...>
> 
>> +
>> +	m->tx_offload = annot->parse.ip_off[0];
>> +	m->tx_offload |= (annot->parse.l4_off - annot->parse.ip_off[0])
>> +					<< DPAA_PKT_L3_LEN_SHIFT;
> 
> This is a received mbuf right? Is it required to set tx_offload flag?
> 
>> +
>> +	/* Set the hash values */
>> +	m->hash.rss = (uint32_t)(rte_be_to_cpu_64(annot->hash));> +	m->ol_flags = PKT_RX_RSS_HASH;
>> +
>> +	/* Check if Vlan is present */
>> +	if (prs & DPAA_PARSE_VLAN_MASK)
>> +		m->ol_flags |= PKT_RX_VLAN_PKT;
> 
> I guess PKT_RX_VLAN_STRIPPED is the preferred flag now.
> 
> <...>
> 

I will re-check the above (and fix).