rte_event_eth_tx_adapter_enqueue() short enqueue

rte_event_eth_tx_adapter_enqueue() short enqueue

[Mattias Rönnblom]

Hi.

Consider the following situation:

An application does

rte_event_eth_tx_adapter_enqueue()

and due to back-pressure or some other reason not all events/packets 
could be enqueued, and a count lower than the nb_events input parameter 
is returned.

The API says that "/../ the remaining events at the end of ev[] are not 
consumed and the caller has to take care of them /../".

May an event device rearrange the ev array so that any enqueue failures 
are put last in the ev array?

In other words: does the "at the end of ev[]" mean "at the end of ev[] 
as the call has completed", or is the event array supposed to be 
untouched, and thus the same events are at the end both before and after 
the call.

The ev array pointer is not const, so from that perspective it may be 
modified.

This situation may occur for example the bonding driver is used under 
the hood. The bonding driver does this kind of rearrangements on the 
ethdev level.

Thanks,
	Mattias

Re: rte_event_eth_tx_adapter_enqueue() short enqueue

[Bruce Richardson]

On Wed, Nov 27, 2024 at 11:03:31AM +0100, Mattias Rönnblom wrote:
> Hi.
> 
> Consider the following situation:
> 
> An application does
> 
> rte_event_eth_tx_adapter_enqueue()
> 
> and due to back-pressure or some other reason not all events/packets could
> be enqueued, and a count lower than the nb_events input parameter is
> returned.
> 
> The API says that "/../ the remaining events at the end of ev[] are not
> consumed and the caller has to take care of them /../".
> 
> May an event device rearrange the ev array so that any enqueue failures are
> put last in the ev array?
> 
> In other words: does the "at the end of ev[]" mean "at the end of ev[] as
> the call has completed", or is the event array supposed to be untouched, and
> thus the same events are at the end both before and after the call.
> 
> The ev array pointer is not const, so from that perspective it may be
> modified.
> 
> This situation may occur for example the bonding driver is used under the
> hood. The bonding driver does this kind of rearrangements on the ethdev
> level.
> 

Interesting question. I tend to think that we should not proclude this
reordering, as it should allow e.g  an eventdev which is short on space to
selectively enqueue only the high priority events.

Only caveat is that if we do allow the reordering we clarify the
documentation to explicitly state that it is allowed.

/Bruce

Re: rte_event_eth_tx_adapter_enqueue() short enqueue

[Mattias Rönnblom]

On 2024-11-27 11:38, Bruce Richardson wrote:
> On Wed, Nov 27, 2024 at 11:03:31AM +0100, Mattias Rönnblom wrote:
>> Hi.
>>
>> Consider the following situation:
>>
>> An application does
>>
>> rte_event_eth_tx_adapter_enqueue()
>>
>> and due to back-pressure or some other reason not all events/packets could
>> be enqueued, and a count lower than the nb_events input parameter is
>> returned.
>>
>> The API says that "/../ the remaining events at the end of ev[] are not
>> consumed and the caller has to take care of them /../".
>>
>> May an event device rearrange the ev array so that any enqueue failures are
>> put last in the ev array?
>>
>> In other words: does the "at the end of ev[]" mean "at the end of ev[] as
>> the call has completed", or is the event array supposed to be untouched, and
>> thus the same events are at the end both before and after the call.
>>
>> The ev array pointer is not const, so from that perspective it may be
>> modified.
>>
>> This situation may occur for example the bonding driver is used under the
>> hood. The bonding driver does this kind of rearrangements on the ethdev
>> level.
>>
> 
> Interesting question. I tend to think that we should not proclude this
> reordering, as it should allow e.g  an eventdev which is short on space to
> selectively enqueue only the high priority events.
> 

Allowing reordering may be a little surprising to the user. At least it 
would be for me.

Other eventdev APIs enqueue do not allow this kind of reordering (with 
const-marked arrays).

That said, I lean toward agreeing with you, since it will solve the 
ethdev tx_burst() mapping issue mentioned.

> Only caveat is that if we do allow the reordering we clarify the
> documentation to explicitly state that it is allowed.
> 
> /Bruce

Re: rte_event_eth_tx_adapter_enqueue() short enqueue

[Bruce Richardson]

On Wed, Nov 27, 2024 at 11:53:50AM +0100, Mattias Rönnblom wrote:
> On 2024-11-27 11:38, Bruce Richardson wrote:
> > On Wed, Nov 27, 2024 at 11:03:31AM +0100, Mattias Rönnblom wrote:
> > > Hi.
> > > 
> > > Consider the following situation:
> > > 
> > > An application does
> > > 
> > > rte_event_eth_tx_adapter_enqueue()
> > > 
> > > and due to back-pressure or some other reason not all events/packets could
> > > be enqueued, and a count lower than the nb_events input parameter is
> > > returned.
> > > 
> > > The API says that "/../ the remaining events at the end of ev[] are not
> > > consumed and the caller has to take care of them /../".
> > > 
> > > May an event device rearrange the ev array so that any enqueue failures are
> > > put last in the ev array?
> > > 
> > > In other words: does the "at the end of ev[]" mean "at the end of ev[] as
> > > the call has completed", or is the event array supposed to be untouched, and
> > > thus the same events are at the end both before and after the call.
> > > 
> > > The ev array pointer is not const, so from that perspective it may be
> > > modified.
> > > 
> > > This situation may occur for example the bonding driver is used under the
> > > hood. The bonding driver does this kind of rearrangements on the ethdev
> > > level.
> > > 
> > 
> > Interesting question. I tend to think that we should not proclude this
> > reordering, as it should allow e.g  an eventdev which is short on space to
> > selectively enqueue only the high priority events.
> > 
> 
> Allowing reordering may be a little surprising to the user. At least it
> would be for me.
> 
> Other eventdev APIs enqueue do not allow this kind of reordering (with
> const-marked arrays).
> 

That is a good point. I forgot that the events are directly passed to the
enqueue functions rather than being passed as pointers, which could then be
reordered without modifying the underlying events.

> That said, I lean toward agreeing with you, since it will solve the ethdev
> tx_burst() mapping issue mentioned.
> 

If enabling this solves a real problem, then let's allow it, despite the
inconsistency in the APIs. Again, though, we need to to call this out in
the docs very prominently to avoid surprises.

Alternatively, do we want to add a separate API that explicitly allows
reordering, and update the existing API to have a const value parameter?
For drivers that don't implement the reordering they can just not provide
the reordering function and the non-reorder version can be used
transparently instead.

/Bruce