>On 10/9/19 6:33 PM, pbhagavat...@marvell.com wrote:
>> From: Pavan Nikhilesh <pbhagavat...@marvell.com>
>>
>> Add `rte_eth_dev_set_supported_ptypes` function that will allow the
>> application to inform the PMD the packet types it is interested in.
>> Based on the ptypes set PMDs can optimize their Rx path.
>>
>> -If application doesn’t want any ptype information it can call
>> `rte_eth_dev_set_supported_ptypes(ethdev_id,
>RTE_PTYPE_UNKNOWN, NULL, 0)`
>> and PMD may skip packet type processing and set
>rte_mbuf::packet_type to
>> RTE_PTYPE_UNKNOWN.
>>
>> -If application doesn’t call `rte_eth_dev_set_supported_ptypes` PMD
>can
>> return `rte_mbuf::packet_type` with
>`rte_eth_dev_get_supported_ptypes`.
>>
>> -If application is interested only in L2/L3 layer, it can inform the PMD
>> to update `rte_mbuf::packet_type` with L2/L3 ptype by calling
>> `rte_eth_dev_set_supported_ptypes(ethdev_id,
>> RTE_PTYPE_L2_MASK | RTE_PTYPE_L3_MASK, NULL,
>0)`.
>>
>> Suggested-by: Konstantin Ananyev <konstantin.anan...@intel.com>
>> Signed-off-by: Pavan Nikhilesh <pbhagavat...@marvell.com>
>> Reviewed-by: Andrew Rybchenko <arybche...@solarflare.com>
>
>Sorry, but few nits below.
>
>[snip]
>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c
>b/lib/librte_ethdev/rte_ethdev.c
>> index 17d183e1f..349483707 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -2602,6 +2602,90 @@
>rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t
>ptype_mask,
>> return j;
>> }
>>
>> +int
>> +rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
>ptype_mask,
>> + uint32_t *set_ptypes, unsigned int
>num)
>> +{
>> + const uint32_t valid_ptype_masks[] = {
>> + RTE_PTYPE_L2_MASK,
>> + RTE_PTYPE_L3_MASK,
>> + RTE_PTYPE_L4_MASK,
>> + RTE_PTYPE_TUNNEL_MASK,
>> + RTE_PTYPE_INNER_L2_MASK,
>> + RTE_PTYPE_INNER_L3_MASK,
>> + RTE_PTYPE_INNER_L4_MASK,
>> + };
>> + const uint32_t *all_ptypes;
>> + struct rte_eth_dev *dev;
>> + unsigned int i, j;
>> + int ret;
>> +
>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> + dev = &rte_eth_devices[port_id];
>> +
>> + if (num > 0 && set_ptypes == NULL)
>> + return -EINVAL;
>> +
>> + if (*dev->dev_ops->dev_supported_ptypes_get == NULL ||
>> + *dev->dev_ops->dev_supported_ptypes_set
>== NULL) {
>> + ret = 0;
>> + goto ptype_unknown;
>> + }
>> +
>> + if (ptype_mask == 0) {
>> + ret = (*dev->dev_ops-
>>dev_supported_ptypes_set)(dev,
>> + ptype_mask);
>> + goto ptype_unknown;
>> + }
>> +
>> + for (i = 0; i <
>sizeof(valid_ptype_masks)/sizeof(valid_ptype_masks[0]);
>
>RTE_DIM should be used instead of sizeof/sizeof
Ah! yes checkpatch was preempting me to use ARRAY_SIZE I forgot RTE_DIM exists.
>
>> +
> i++) {
>> + uint32_t mask = ptype_mask & valid_ptype_masks[i];
>> + if (mask && mask != valid_ptype_masks[i]) {
>> + ret = -EINVAL;
>> + goto ptype_unknown;
>> + }
>> + }
>> +
>> + if (ptype_mask & ~RTE_PTYPE_ALL_MASK) {
>> + ret = -EINVAL;
>> + goto ptype_unknown;
>> + }
>
>It does not protect against more masks added in the future and lost
>in your list above but still used in request.
>It is better just add extra variable and remove bits
>from there while masks are checked in the loop. Then it should be
>checked here that the mask is 0 (no bits are left and everything is
>checked).
>
Makes sense, as it would probably break if future updates to ptype_masks
are not added to the above list.
I will roll a v11.
Thanks,
Pavan.
>> +
>> + all_ptypes = (*dev->dev_ops-
>>dev_supported_ptypes_get)(dev);
>> + if (all_ptypes == NULL) {
>> + ret = 0;
>> + goto ptype_unknown;
>> + }
>> +
>> + /*
>> + * Accodommodate as many set_ptypes as possible. If the
>supplied
>> + * set_ptypes array is insufficient fill it partially.
>> + */
>> + for (i = 0, j = 0; set_ptypes != NULL &&
>> + (all_ptypes[i] !=
>RTE_PTYPE_UNKNOWN); ++i) {
>> + if (ptype_mask & all_ptypes[i]) {
>> + if (j < num - 1) {
>> + set_ptypes[j] = all_ptypes[i];
>> + j++;
>> + continue;
>> + }
>> + break;
>> + }
>> + }
>> +
>> + if (set_ptypes != NULL && j < num)
>> + set_ptypes[j] = RTE_PTYPE_UNKNOWN;
>> +
>> + return (*dev->dev_ops->dev_supported_ptypes_set)(dev,
>ptype_mask);
>> +
>> +ptype_unknown:
>> + if (num > 0)
>> + set_ptypes[0] = RTE_PTYPE_UNKNOWN;
>> +
>> + return ret;
>> +}
>> +
>> void
>> rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr
>*mac_addr)
>> {
>
>[snip]
