[dpdk-dev] [PATCH v15 1/7] ethdev: add set ptype function
Andrew Rybchenko
arybchenko at solarflare.com
Fri Nov 1 11:55:31 CET 2019
On 10/31/19 7:38 PM, Pavan Nikhilesh Bhagavatula wrote:
>> 29/10/2019 16:37, pbhagavatula at marvell.com:
>>> From: Pavan Nikhilesh <pbhagavatula at marvell.com>
>>> Removed Items
>>> -------------
>>> --- a/lib/librte_ethdev/rte_ethdev.h
>>> +++ b/lib/librte_ethdev/rte_ethdev.h
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Inform Ethernet device of the packet types classification the
>> recipient is
>>> + * interested in.
>> This is a bit convoluted.
>> What about this?
>> "Optimize driver handling of packet types by reducing its range."
> @arybchenko at solarflare.com Thoughts?
Optimize is a possible side effect of the operation, but there is
no any promise that something will be optimized.
I thought that current description explains what happens.
Below statements try to explain why it may be useful.
Any other options?
>>> + *
>>> + * Application can use this function to set only specific ptypes that it's
>>> + * interested. This information can be used by the PMD to optimize
>> Rx path.
>>> + *
>>> + * The function accepts an array `set_ptypes` allocated by the caller
>> to
>>> + * store the packet types set by the driver, the last element of the
>> array
>>> + * is set to RTE_PTYPE_UNKNOWN. The size of the `set_ptype` array
>> should be
>>> + * `rte_eth_dev_get_supported_ptypes() + 1` else it might only be
>> filled
>>> + * partially.
>>> + *
>>> + * @param port_id
>>> + * The port identifier of the Ethernet device.
>>> + * @param ptype_mask
>>> + * The ptype family that application is interested in should be
>> bitwise OR of
>>> + * RTE_PTYPE_*_MASK or 0.
>>> + * @param set_ptypes
>>> + * An array pointer to store set packet types, allocated by caller. The
>>> + * function marks the end of array with RTE_PTYPE_UNKNOWN.
>>> + * @param num
>>> + * Size of the array pointed by param ptypes.
>>> + * Should be rte_eth_dev_get_supported_ptypes() + 1 to
>> accommodate the
>>> + * set ptypes.
>>> + * @return
>>> + * - (0) if Success.
>>> + * - (-ENODEV) if *port_id* invalid.
>>> + * - (-EINVAL) if *ptype_mask* is invalid (or) set_ptypes is NULL and
>>> + * num > 0.
>>> + */
>> John, please you check the English wording?
>>
>>> +__rte_experimental
>>> +int rte_eth_dev_set_supported_ptypes(uint16_t port_id, uint32_t
>> ptype_mask,
>>> + uint32_t *set_ptypes, unsigned int
>> num);
>>
>> I don't like the name of the function because they are
>> not "supported" packet types but "requested".
>> What about replacing "set_supported" with "set_allowed", or
>> "white_list"?
> "white_list" seems ok but hope it doesn't call for blacklisting API.
"white_list" suggests that it is guaranteed that nothing else will
be reported. At least for me. However, I agree that set_supported
is not nice and I accepted it just to keep API naming symmetric.
May be it is really misleading here. May be just: rte_eth_dev_set_ptypes()?
More information about the dev
mailing list