DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
To: "Naga Harish K, S V" <s.v.naga.harish.k@intel.com>,
	"Jayatheerthan, Jay" <jay.jayatheerthan@intel.com>,
	"Carrillo, Erik G" <erik.g.carrillo@intel.com>,
	"Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>,
	Jerin Jacob <jerinj@marvell.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Van Haaren, Harry" <harry.van.haaren@intel.com>,
	"hofors@lysator.liu.se" <hofors@lysator.liu.se>
Subject: Re: [PATCH 2/4] eventdev: have ethernet Rx adapter appropriately report idle
Date: Thu, 13 Oct 2022 09:53:31 +0000	[thread overview]
Message-ID: <0e613472-bac9-9189-f4a6-ba8bee0a1961@ericsson.com> (raw)
In-Reply-To: <DM6PR11MB3868478E76825E5536D0E02BA1259@DM6PR11MB3868.namprd11.prod.outlook.com>

On 2022-10-13 03:32, Naga Harish K, S V wrote:
> 
> 
>> -----Original Message-----
>> From: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>
>> Sent: Tuesday, October 11, 2022 12:40 PM
>> To: mattias.ronnblom <mattias.ronnblom@ericsson.com>; Carrillo, Erik G
>> <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
>> <abhinandan.gujjar@intel.com>; Jerin Jacob <jerinj@marvell.com>; Naga
>> Harish K, S V <s.v.naga.harish.k@intel.com>
>> Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>;
>> hofors@lysator.liu.se; mattias.ronnblom <mattias.ronnblom@ericsson.com>
>> Subject: RE: [PATCH 2/4] eventdev: have ethernet Rx adapter appropriately
>> report idle
>>
>> @Harish, Could you review the patch ?
>>
>> -Jay
>>
>>> -----Original Message-----
>>> From: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>> Sent: Monday, October 10, 2022 8:24 PM
>>> To: Jayatheerthan, Jay <jay.jayatheerthan@intel.com>; Carrillo, Erik G
>>> <erik.g.carrillo@intel.com>; Gujjar, Abhinandan S
>>> <abhinandan.gujjar@intel.com>; Jerin Jacob <jerinj@marvell.com>
>>> Cc: dev@dpdk.org; Van Haaren, Harry <harry.van.haaren@intel.com>;
>>> hofors@lysator.liu.se; mattias.ronnblom
>>> <mattias.ronnblom@ericsson.com>
>>> Subject: [PATCH 2/4] eventdev: have ethernet Rx adapter appropriately
>>> report idle
>>>
>>> Update the Event Ethernet Rx Adapter's service function to report as
>>> idle (i.e., return -EAGAIN) in case no Ethernet frames were received
>>> from the ethdev and no events were enqueued to the event device.
>>>
>>> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
>>> ---
>>>   lib/eventdev/rte_event_eth_rx_adapter.c | 56
>>> ++++++++++++++++++-------
>>>   1 file changed, 41 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/lib/eventdev/rte_event_eth_rx_adapter.c
>>> b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> index 5c3021a184..cf7bbd4d69 100644
>>> --- a/lib/eventdev/rte_event_eth_rx_adapter.c
>>> +++ b/lib/eventdev/rte_event_eth_rx_adapter.c
>>> @@ -1184,7 +1184,7 @@ rxa_intr_thread(void *arg)
>>>   /* Dequeue <port, q> from interrupt ring and enqueue received
>>>    * mbufs to eventdev
>>>    */
>>> -static inline void
>>> +static inline bool
>>>   rxa_intr_ring_dequeue(struct event_eth_rx_adapter *rx_adapter)  {
>>>   	uint32_t n;
>>> @@ -1194,20 +1194,27 @@ rxa_intr_ring_dequeue(struct
>> event_eth_rx_adapter *rx_adapter)
>>>   	struct rte_event_eth_rx_adapter_stats *stats;
>>>   	rte_spinlock_t *ring_lock;
>>>   	uint8_t max_done = 0;
>>> +	bool work = false;
>>>
>>>   	if (rx_adapter->num_rx_intr == 0)
>>> -		return;
>>> +		return work;
>>>
>>>   	if (rte_ring_count(rx_adapter->intr_ring) == 0
>>>   		&& !rx_adapter->qd_valid)
>>> -		return;
>>> +		return work;
>>>
>>>   	buf = &rx_adapter->event_enqueue_buffer;
>>>   	stats = &rx_adapter->stats;
>>>   	ring_lock = &rx_adapter->intr_ring_lock;
>>>
>>> -	if (buf->count >= BATCH_SIZE)
>>> -		rxa_flush_event_buffer(rx_adapter, buf, stats);
>>> +	if (buf->count >= BATCH_SIZE) {
>>> +		uint16_t n;
>>> +
>>> +		n = rxa_flush_event_buffer(rx_adapter, buf, stats);
>>> +
>>> +		if (likely(n > 0))
>>> +			work = true;
>>> +	}
>>>
>>>   	while (rxa_pkt_buf_available(buf)) {
>>>   		struct eth_device_info *dev_info;
>>> @@ -1289,7 +1296,12 @@ rxa_intr_ring_dequeue(struct
>> event_eth_rx_adapter *rx_adapter)
>>>   	}
>>>
>>>   done:
>>> -	rx_adapter->stats.rx_intr_packets += nb_rx;
>>> +	if (nb_rx > 0) {
> 
> How are the performance numbers before and after this patch?
> Trying to understand the performance impact, as new condition is added to the service function Datapath.
> 
I haven't tested the RX and TX adapters separately, but if you run them 
on the same core, I get the following result:

Without patches, with stats disabled: 16,0 Mpps
Without patches, with stats enabled: 16,1 Mpps
With patches, with stats disabled: 16,1 Mpps
With patches, with stats enabled: 16,2 Mpps

So these patches, with this particular hardware, compiler, and test 
application, adding a tiny bit of additional logic actually make the 
RX+TX adapter perform better. This is contrary to what you might expect, 
and I'm sure YMMV.

Enabling service core statistics (which boils down to a 2x rdtsc and 
some cheap arithmetic in rte_service.c) actually make the RX+TX adapter 
core perform better, both before and after this patchset. Also contrary 
to what you might expect.

The results are consistent across multiple runs.

GCC 11.2.0 and AMD Zen 3 @ 3,7 GHz. Event device is DSW and I/O is the 
ring Ethdev.

>>> +		rx_adapter->stats.rx_intr_packets += nb_rx;
>>> +		work = true;
>>> +	}
>>> +
>>> +	return work;
>>>   }
>>>
>>>   /*
>>> @@ -1305,7 +1317,7 @@ rxa_intr_ring_dequeue(struct
>> event_eth_rx_adapter *rx_adapter)
>>>    * the hypervisor's switching layer where adjustments can be made to deal
>> with
>>>    * it.
>>>    */
>>> -static inline void
>>> +static inline bool
>>>   rxa_poll(struct event_eth_rx_adapter *rx_adapter)  {
>>>   	uint32_t num_queue;
>>> @@ -1314,6 +1326,7 @@ rxa_poll(struct event_eth_rx_adapter
>> *rx_adapter)
>>>   	struct rte_event_eth_rx_adapter_stats *stats = NULL;
>>>   	uint32_t wrr_pos;
>>>   	uint32_t max_nb_rx;
>>> +	bool work = false;
>>>
>>>   	wrr_pos = rx_adapter->wrr_pos;
>>>   	max_nb_rx = rx_adapter->max_nb_rx;
>>> @@ -1329,14 +1342,20 @@ rxa_poll(struct event_eth_rx_adapter
>> *rx_adapter)
>>>   		/* Don't do a batch dequeue from the rx queue if there isn't
>>>   		 * enough space in the enqueue buffer.
>>>   		 */
>>> -		if (buf->count >= BATCH_SIZE)
>>> -			rxa_flush_event_buffer(rx_adapter, buf, stats);
>>> +		if (buf->count >= BATCH_SIZE) {
>>> +			uint16_t n;
>>> +
>>> +			n = rxa_flush_event_buffer(rx_adapter, buf, stats);
>>> +
>>> +			if (likely(n > 0))
>>> +				work = true;
> 
> Same as above
> 
>>> +		}
>>>   		if (!rxa_pkt_buf_available(buf)) {
>>>   			if (rx_adapter->use_queue_event_buf)
>>>   				goto poll_next_entry;
>>>   			else {
>>>   				rx_adapter->wrr_pos = wrr_pos;
>>> -				return;
>>> +				break;
>>>   			}
>>>   		}
>>>
>>> @@ -1352,6 +1371,11 @@ rxa_poll(struct event_eth_rx_adapter
>> *rx_adapter)
>>>   		if (++wrr_pos == rx_adapter->wrr_len)
>>>   			wrr_pos = 0;
>>>   	}
>>> +
>>> +	if (nb_rx > 0)
>>> +		work = true;
>>> +
>>> +	return work;
> 
> Same as above
> 
>>>   }
>>>
>>>   static void
>>> @@ -1384,12 +1408,14 @@ static int
>>>   rxa_service_func(void *args)
>>>   {
>>>   	struct event_eth_rx_adapter *rx_adapter = args;
>>> +	bool intr_work;
>>> +	bool poll_work;
>>>
>>>   	if (rte_spinlock_trylock(&rx_adapter->rx_lock) == 0)
>>> -		return 0;
>>> +		return -EAGAIN;
>>>   	if (!rx_adapter->rxa_started) {
>>>   		rte_spinlock_unlock(&rx_adapter->rx_lock);
>>> -		return 0;
>>> +		return -EAGAIN;
>>>   	}
>>>
>>>   	if (rx_adapter->ena_vector) {
>>> @@ -1410,12 +1436,12 @@ rxa_service_func(void *args)
>>>   		}
>>>   	}
>>>
>>> -	rxa_intr_ring_dequeue(rx_adapter);
>>> -	rxa_poll(rx_adapter);
>>> +	intr_work = rxa_intr_ring_dequeue(rx_adapter);
>>> +	poll_work = rxa_poll(rx_adapter);
>>>
>>>   	rte_spinlock_unlock(&rx_adapter->rx_lock);
>>>
>>> -	return 0;
>>> +	return intr_work || poll_work ? 0 : -EAGAIN;
>>>   }
>>>
>>>   static void *
>>> --
>>> 2.34.1
> 


  reply	other threads:[~2022-10-13  9:53 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-10 14:54 [PATCH 0/4] Have event adapters report idle status Mattias Rönnblom
2022-10-10 14:54 ` [PATCH 1/4] eventdev: have crypto adapter appropriately report idle Mattias Rönnblom
2022-10-17  5:47   ` Gujjar, Abhinandan S
2022-10-10 14:54 ` [PATCH 2/4] eventdev: have ethernet Rx " Mattias Rönnblom
2022-10-11  7:10   ` Jayatheerthan, Jay
2022-10-13  1:32     ` Naga Harish K, S V
2022-10-13  9:53       ` Mattias Rönnblom [this message]
2022-10-14 17:36         ` Jerin Jacob
2022-10-17 12:36           ` Naga Harish K, S V
2022-10-18  9:19             ` Jayatheerthan, Jay
2022-10-10 14:54 ` [PATCH 3/4] eventdev: have ethernet Tx " Mattias Rönnblom
2022-10-11  7:10   ` Jayatheerthan, Jay
2022-10-13  1:30     ` Naga Harish K, S V
2022-10-18  9:19       ` Jayatheerthan, Jay
2022-10-19 11:32         ` Jerin Jacob
2022-10-10 14:54 ` [PATCH 4/4] eventdev: have timer " Mattias Rönnblom
2022-10-17 21:42   ` Carrillo, Erik G

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e613472-bac9-9189-f4a6-ba8bee0a1961@ericsson.com \
    --to=mattias.ronnblom@ericsson.com \
    --cc=abhinandan.gujjar@intel.com \
    --cc=dev@dpdk.org \
    --cc=erik.g.carrillo@intel.com \
    --cc=harry.van.haaren@intel.com \
    --cc=hofors@lysator.liu.se \
    --cc=jay.jayatheerthan@intel.com \
    --cc=jerinj@marvell.com \
    --cc=s.v.naga.harish.k@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).