On 11/2/19 1:25 AM, Thomas Monjalon wrote: > 01/11/2019 11:55, Andrew Rybchenko: >> On 10/31/19 7:38 PM, Pavan Nikhilesh Bhagavatula wrote: >>>> 29/10/2019 16:37, pbhagavat...@marvell.com: >>>>> From: Pavan Nikhilesh <pbhagavat...@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." >>> @arybche...@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? > "Reduce range of packet types to handle."
OK >>>>> + * 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()? > Maybe the word "allowed" would better fit? allowed could be treated as everything else is disallowed. So, I'm not sure.
