[dpdk-dev] [PATCH v10 1/7] ethdev: add set ptype function

Andrew Rybchenko arybchenko at solarflare.com
Thu Oct 10 10:46:41 CEST 2019


On 10/9/19 6:33 PM, pbhagavatula at marvell.com wrote:
> From: Pavan Nikhilesh <pbhagavatula at 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.ananyev at intel.com>
> Signed-off-by: Pavan Nikhilesh <pbhagavatula at marvell.com>
> Reviewed-by: Andrew Rybchenko <arybchenko at 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

> +									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).

> +
> +	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]


More information about the dev mailing list