Re: [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 11/11/19 8:30 PM, Thomas Monjalon wrote:
> 11/11/2019 17:56, Ferruh Yigit:
>> On 10/18/2019 5:24 PM, Yigit, Ferruh wrote:
>>> On 8/8/2019 1:28 PM, Nilanjan Sarkar wrote:
>>>> This api is similar like api `rte_eth_tx_buffer` except it
>>>> does not attempt to flush the buffer in case buffer is full.
>>>> The advantage is that, this api does not need port id and
>>>> queue id. In case port id and queue id are shared within threads
>>>> then application can not buffer a packet until it gets access
>>>> to port and queue. So this function segregate buffering
>>>> job from flushing job and thus removes dependency on port and queue.
>>>
>>> Hi Nilanjan,
>>>
>>> Sorry, the patch seems missed because of the misleading module info in the patch
>>> title, this is not an 'eal' patch but a 'ethdev' patch ...
>>>
>>> Related to the API, it looks like target is to reduce the critical section which
>>> looks reasonable to me.
>>>
>>> A concern is related to the making this function inline, we are discussing
>>> moving existing inline functions to regular functions, this may have performance
>>> affect but if the drop is acceptable what about making this an ethdev API?
>>>
>>
>> There was no response on making the new proposed API a proper function.
>>
>> @Thomas, @Andrew, et al,
>>
>> What do you think about a new static inline ethdev API?
>>
>>>> +static __rte_always_inline int
>>>> +rte_eth_tx_enqueue(struct rte_eth_dev_tx_buffer *buffer, struct rte_mbuf *tx_pkt)
>>>> +{
>>>> +	if (buffer->length < buffer->size) {
>>>> +		buffer->pkts[buffer->length++] = tx_pkt;
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	return -1;
>>>> +}
> 
> It looks reasonnable.
> But the function name should include _buffer_
> What about rte_eth_tx_buffer_enqueue?

+1