DPDK patches and discussions
 help / color / mirror / Atom feed
From: Heng Wang <heng.wang@ericsson.com>
To: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>,
	"Jerin Jacob Kollanukkaran" <jerinj@marvell.com>
Cc: "timothy.mcdaniel@intel.com" <timothy.mcdaniel@intel.com>,
	"Hemant Agrawal" <hemant.agrawal@nxp.com>,
	"Harry van Haaren" <harry.van.haaren@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Svante Järvstråt" <svante.jarvstrat@ericsson.com>,
	"Stefan Sundkvist" <stefan.sundkvist@ericsson.com>,
	"Peter Nilsson" <peter.nilsson@ericsson.com>,
	"Maria Lingemark" <maria.lingemark@ericsson.com>
Subject: RE: Event device early back-pressure indication
Date: Thu, 13 Apr 2023 12:55:29 +0000	[thread overview]
Message-ID: <DB9PR07MB887260FEFC5DA9024F176D249C989@DB9PR07MB8872.eurprd07.prod.outlook.com> (raw)
In-Reply-To: <ac881317-3648-01ec-8ad0-e849cf03aa4d@ericsson.com>

Hi,
  This interaction with eventdev introduces some overhead. Isn't it easier to just create an API to query the available credit for a certain event port?

Regards,
Heng

-----Original Message-----
From: Mattias Rönnblom <mattias.ronnblom@ericsson.com> 
Sent: Thursday, April 13, 2023 8:54 AM
To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Cc: timothy.mcdaniel@intel.com; Hemant Agrawal <hemant.agrawal@nxp.com>; Harry van Haaren <harry.van.haaren@intel.com>; dev@dpdk.org; Svante Järvstråt <svante.jarvstrat@ericsson.com>; Heng Wang <heng.wang@ericsson.com>; Stefan Sundkvist <stefan.sundkvist@ericsson.com>; Peter Nilsson <peter.nilsson@ericsson.com>; Maria Lingemark <maria.lingemark@ericsson.com>
Subject: Event device early back-pressure indication

Hi.

Consider this situation:

An application EAL thread receives an eventdev event (or some other stimuli), which in turn triggers some action. This action results in a number of new events being prepared, and a number of associated state changes in the application.

On attempting to enqueue the newly created batch of RTE_EVENT_OP_NEW events, it turns out the system is very busy, and the event device back pressures (i.e., returns a short count in rte_event_enqueue_new_burst()).

The application may now be a in tough spot, in case:

A) The processing was expensive and/or difficult to reverse (e.g., destructive changes were made to a packet).
B) The application does not have the option to discard the events (and any related mbufs).

In this situation, it would be very beneficial to the application if the event device give could give some assurance that a future enqueue operation will succeed (in its entirety).

 From what I understand from today's Eventdev API, there are no good options. You *may* be able to do some heuristics based on a event device-specific xstat (to infer the event device load), but that is not even close to "good". You may also try some application-level buffering, but that assumes that the packets/state changes are going to be identical, if they are to be sent at a later time. It would drive complexity in the app.

One seemingly clean way to solve this issue is to allow pre-allocation of RTE_NEW_OP_NEW credits. The eventdev API doesn't talk about credits, but at least in the event device implementations I've come across use some kind of credit system internally.

uint16_t
rte_event_alloc_new_credits(uint8_t dev_id, uint8_t port_id, uint16_t count);

In addition to this function, the application would also need some way to indicate, at the point of enqueue, that the credits have already been allocated.

I don't see any need for pre-allocating credits for non-RTE_OP_NEW events. (Some event devices don't even use credits to track such
events.) Back pressure on RTE_OP_FORWARD usually spells disaster, in one form of the other.

You could use a bit in the rte_event struct for the purpose of signaling if its credit is pre-allocated. That would allow this change to happen, without any changes to the enqueue function prototypes.

However, this would require the event device to scan the event array.

I'm not sure I think there is a use case for mixing pre-allocated and non-pre-allocated events in the same burst.

If this burst-level separation is good enough, one could either change the existing rte_enqueue_new_burst() or add a new one. Something like:

uint16_t
rte_enqueue_new_burst(uint8_t dev_id, uint8_t port_id,
                       const struct rte_event ev[],
                       uint16_t nb_events, uint32_t flags);

#define RTE_EVENT_FLAG_PRE_CREDITS_ALLOCATED (UINT32_C(1) << 0)

A related shortcoming of the current eventdev API is that the new_event_threshold is tied to a port, which is impractical for applications which require different threshold for different kinds of events enqueued on the same port. One can use different ports, but that approach does not scale, since there may be significant memory and/or event device hardware resources tied to ports, and thus you cannot allow for a combinatorial explosion of ports.

This issue could be solve by allowing the application to specify the new_event_threshold, either per burst, or per event.

Per event doesn't make a lot of sense in practice, I think, since mixing events with different back pressure points will create head-of-line blocking. An early low-threshold event may prevent higher-indexed high threshold event in the same enqueue burst from being enqueued. This is the same reason it usually doesn't make sense to mix RTE_OP_NEW and RTE_OP_FORWARD events in the same burst.

Although the new_event_threshold seems completely orthogonal to the port to me, it could still serve as the default.

In case you find this a useful feature, it could be added to the credit allocation function.

uint16_t
rte_event_alloc_new_credits(uint8_t dev_id, uint8_t port_id, uint32_t new_event_threshold, uint16_t count);

If that is the only change, the user is required to pre-allocated credits to use a flexible new_event_threshold.

It seems to me that that might be something you can live with. Or, you add new enqueue_new_burst() variant where a new_event_threshold parameter is added.

It may also be useful to have a way to return credits, in case not all allocated was actually needed.

void
rte_event_return_new_credits(...);

Thoughts?

Best regards,
	Mattias

  reply	other threads:[~2023-04-17  8:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-13  6:54 Mattias Rönnblom
2023-04-13 12:55 ` Heng Wang [this message]
2023-04-17 12:52 ` Jerin Jacob
2023-04-17 15:36   ` Mattias Rönnblom
2023-04-19 11:06     ` Jerin Jacob
2023-04-27  9:15       ` Mattias Rönnblom

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=DB9PR07MB887260FEFC5DA9024F176D249C989@DB9PR07MB8872.eurprd07.prod.outlook.com \
    --to=heng.wang@ericsson.com \
    --cc=dev@dpdk.org \
    --cc=harry.van.haaren@intel.com \
    --cc=hemant.agrawal@nxp.com \
    --cc=jerinj@marvell.com \
    --cc=maria.lingemark@ericsson.com \
    --cc=mattias.ronnblom@ericsson.com \
    --cc=peter.nilsson@ericsson.com \
    --cc=stefan.sundkvist@ericsson.com \
    --cc=svante.jarvstrat@ericsson.com \
    --cc=timothy.mcdaniel@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).