DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
@ 2019-08-08 12:28 Nilanjan Sarkar
  2019-10-18 16:24 ` Yigit, Ferruh
  0 siblings, 1 reply; 8+ messages in thread
From: Nilanjan Sarkar @ 2019-08-08 12:28 UTC (permalink / raw)
  To: dev; +Cc: Nilanjan Sarkar

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.

Signed-off-by: Nilanjan Sarkar <nsarkar@sandvine.com>
---
 lib/librte_ethdev/rte_ethdev.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index dc6596bc9..bd8b8fee4 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4569,6 +4569,35 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * Buffer a single packet for future transmission on Tx buffer. This buffer
+ * can be sent to a port and queue of a NIC using rte_eth_tx_buffer_flush ()
+ * call.
+ *
+ * This function enqueues a packet to Tx buffer. In case there is no space
+ * in Tx buffer, this function fails.
+ * Tx buffer will be flushed using rte_eth_tx_buffer_flush () call. It is
+ * application's responsibility to flush the Tx buffer in regular interval.
+ *
+ * @param buffer
+ *  Buffer used to collect packets to be sent.
+ * @param tx_pkt
+ *  Pointer to the packet mbuf to be sent.
+ * @return
+ *  0 = packet has been buffered for later transmission
+ *  -1 = Packet can not be buffered since it reached limit
+ */
+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;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.22.0


Disclaimer:
This communication (including any attachments) is intended for the use of the intended recipient(s) only and may contain information that is considered confidential, proprietary, sensitive and/or otherwise legally protected. Any unauthorized use or dissemination of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender by return e-mail message and delete all copies of the original communication. Thank you for your cooperation.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
  2019-08-08 12:28 [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer Nilanjan Sarkar
@ 2019-10-18 16:24 ` Yigit, Ferruh
  2019-11-11 16:56   ` Ferruh Yigit
  0 siblings, 1 reply; 8+ messages in thread
From: Yigit, Ferruh @ 2019-10-18 16:24 UTC (permalink / raw)
  To: Nilanjan Sarkar, dev
  Cc: Thomas Monjalon, Andrew Rybchenko, Konstantin Ananyev,
	Bruce Richardson, Kinsella, Ray, Olivier MATZ, Jerin Jacob

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?

Thanks,
ferruh

> 
> Signed-off-by: Nilanjan Sarkar <nsarkar@sandvine.com>
> ---
>  lib/librte_ethdev/rte_ethdev.h | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index dc6596bc9..bd8b8fee4 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -4569,6 +4569,35 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
>  	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>  }
>  
> +/**
> + * Buffer a single packet for future transmission on Tx buffer. This buffer
> + * can be sent to a port and queue of a NIC using rte_eth_tx_buffer_flush ()
> + * call.
> + *
> + * This function enqueues a packet to Tx buffer. In case there is no space
> + * in Tx buffer, this function fails.
> + * Tx buffer will be flushed using rte_eth_tx_buffer_flush () call. It is
> + * application's responsibility to flush the Tx buffer in regular interval.
> + *
> + * @param buffer
> + *  Buffer used to collect packets to be sent.
> + * @param tx_pkt
> + *  Pointer to the packet mbuf to be sent.
> + * @return
> + *  0 = packet has been buffered for later transmission
> + *  -1 = Packet can not be buffered since it reached limit
> + */
> +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;
> +}
> +
>  #ifdef __cplusplus
>  }
>  #endif
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
  2019-10-18 16:24 ` Yigit, Ferruh
@ 2019-11-11 16:56   ` Ferruh Yigit
  2019-11-11 17:30     ` Thomas Monjalon
  0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2019-11-11 16:56 UTC (permalink / raw)
  To: Yigit, Ferruh, Nilanjan Sarkar, dev
  Cc: Thomas Monjalon, Andrew Rybchenko, Konstantin Ananyev,
	Bruce Richardson, Kinsella, Ray, Olivier MATZ, Jerin Jacob

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?

> 
>>
>> Signed-off-by: Nilanjan Sarkar <nsarkar@sandvine.com>
>> ---
>>  lib/librte_ethdev/rte_ethdev.h | 29 +++++++++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index dc6596bc9..bd8b8fee4 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -4569,6 +4569,35 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
>>  	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
>>  }
>>  
>> +/**
>> + * Buffer a single packet for future transmission on Tx buffer. This buffer
>> + * can be sent to a port and queue of a NIC using rte_eth_tx_buffer_flush ()
>> + * call.
>> + *
>> + * This function enqueues a packet to Tx buffer. In case there is no space
>> + * in Tx buffer, this function fails.
>> + * Tx buffer will be flushed using rte_eth_tx_buffer_flush () call. It is
>> + * application's responsibility to flush the Tx buffer in regular interval.
>> + *
>> + * @param buffer
>> + *  Buffer used to collect packets to be sent.
>> + * @param tx_pkt
>> + *  Pointer to the packet mbuf to be sent.
>> + * @return
>> + *  0 = packet has been buffered for later transmission
>> + *  -1 = Packet can not be buffered since it reached limit
>> + */
>> +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;
>> +}
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
  2019-11-11 16:56   ` Ferruh Yigit
@ 2019-11-11 17:30     ` Thomas Monjalon
  2019-11-12  7:17       ` Andrew Rybchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2019-11-11 17:30 UTC (permalink / raw)
  To: dev
  Cc: Ferruh Yigit, Yigit, Ferruh, Nilanjan Sarkar, Andrew Rybchenko,
	Konstantin Ananyev, Bruce Richardson, Kinsella, Ray,
	Olivier MATZ, Jerin Jacob

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?



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
  2019-11-11 17:30     ` Thomas Monjalon
@ 2019-11-12  7:17       ` Andrew Rybchenko
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Rybchenko @ 2019-11-12  7:17 UTC (permalink / raw)
  To: Thomas Monjalon, dev
  Cc: Ferruh Yigit, Yigit, Ferruh, Nilanjan Sarkar, Konstantin Ananyev,
	Bruce Richardson, Kinsella, Ray, Olivier MATZ, Jerin Jacob

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
@ 2019-08-21  5:59 Nilanjan Sarkar
  0 siblings, 0 replies; 8+ messages in thread
From: Nilanjan Sarkar @ 2019-08-21  5:59 UTC (permalink / raw)
  To: dev

Hello Thomas M,

Can you please review this path "http://patches.dpdk.org/patch/57593/"? I see this has been assigned to you. Kindly provide your feedback.

Regards,
Nilanjan

Disclaimer:
This communication (including any attachments) is intended for the use of the intended recipient(s) only and may contain information that is considered confidential, proprietary, sensitive and/or otherwise legally protected. Any unauthorized use or dissemination of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender by return e-mail message and delete all copies of the original communication. Thank you for your cooperation.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
@ 2019-08-08 11:53 Nilanjan Sarkar
  0 siblings, 0 replies; 8+ messages in thread
From: Nilanjan Sarkar @ 2019-08-08 11:53 UTC (permalink / raw)
  To: dev; +Cc: Nilanjan Sarkar

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.

Signed-off-by: Nilanjan Sarkar <nsarkar@sandvine.com>
---
 lib/librte_ethdev/rte_ethdev.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index dc6596bc9..4f72af5ea 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4569,6 +4569,35 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
 	return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
 }
 
+/**
+ * Buffer a single packet for future transmission on Tx buffer. This buffer 
+ * can be sent to a port and queue of a NIC using rte_eth_tx_buffer_flush ()
+ * call. 
+ *
+ * This function enqueues a packet to Tx buffer. In case there is no space
+ * in Tx buffer, this function fails. 
+ * Tx buffer will be flushed using rte_eth_tx_buffer_flush () call. It is 
+ * application's responsibility to flush the Tx buffer in regular interval.
+ *
+ * @param buffer
+ *  Buffer used to collect packets to be sent.
+ * @param tx_pkt
+ *  Pointer to the packet mbuf to be sent.
+ * @return
+ *  0 = packet has been buffered for later transmission
+ *  -1 = Packet can not be buffered since it reached limit
+ */
+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;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.22.0


Disclaimer:
This communication (including any attachments) is intended for the use of the intended recipient(s) only and may contain information that is considered confidential, proprietary, sensitive and/or otherwise legally protected. Any unauthorized use or dissemination of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender by return e-mail message and delete all copies of the original communication. Thank you for your cooperation.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer
@ 2019-08-08 11:17 Nilanjan Sarkar
  0 siblings, 0 replies; 8+ messages in thread
From: Nilanjan Sarkar @ 2019-08-08 11:17 UTC (permalink / raw)
  To: dev

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.

Signed-off-by: Nilanjan Sarkar <nsarkar@sandvine.com>
---
lib/librte_ethdev/rte_ethdev.h | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index dc6596b..8055928 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -4569,6 +4569,37 @@ rte_eth_tx_buffer(uint16_t port_id, uint16_t queue_id,
        return rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
}

+/**
+ * Buffer a single packet for future transmission on Tx buffer. This buffer
+ * can be sent to a port and queue of a NIC using rte_eth_tx_buffer_flush ()
+ * call.
+ *
+ * This function enqueues a packet to Tx buffer. In case there is no space
+ * in Tx buffer, this function fails.
+ * Tx buffer will be flushed using rte_eth_tx_buffer_flush () call. It is
+ * application's responsibility to flush the Tx buffer in regular interval.
+ *
+ * @param buffer
+ *  Buffer used to collect packets to be sent.
+ * @param tx_pkt
+ *  Pointer to the packet mbuf to be sent.
+ * @return
+ *  0 = packet has been buffered for later transmission
+ *  -1 = Packet can not be buffered since it reached limit
+ */
+
+static __rte_always_inline int8_t
+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;
+}
+
#ifdef __cplusplus
}
#endif

--
2.7.4

Disclaimer:
This communication (including any attachments) is intended for the use of the intended recipient(s) only and may contain information that is considered confidential, proprietary, sensitive and/or otherwise legally protected. Any unauthorized use or dissemination of this communication is strictly prohibited. If you have received this communication in error, please immediately notify the sender by return e-mail message and delete all copies of the original communication. Thank you for your cooperation.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-11-12  7:18 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 12:28 [dpdk-dev] [PATCH] eal: added new api to only enqueue a packet in tx buffer Nilanjan Sarkar
2019-10-18 16:24 ` Yigit, Ferruh
2019-11-11 16:56   ` Ferruh Yigit
2019-11-11 17:30     ` Thomas Monjalon
2019-11-12  7:17       ` Andrew Rybchenko
  -- strict thread matches above, loose matches on Subject: below --
2019-08-21  5:59 Nilanjan Sarkar
2019-08-08 11:53 Nilanjan Sarkar
2019-08-08 11:17 Nilanjan Sarkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).