Re: [RFC PATCH] ethdev: advertise flow restore in mbuf

[David Marchand <david.marchand@redhat.com>]

On Thu, Jun 1, 2023 at 11:31 AM Ori Kam <orika@nvidia.com> wrote:
>
>
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Thursday, June 1, 2023 11:48 AM
> >
> > On Wed, May 24, 2023 at 8:44 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > > On Wed, May 24, 2023 at 6:00 PM Ori Kam <orika@nvidia.com> wrote:
> > > > > As reported by Ilya [1], unconditionally calling
> > > > > rte_flow_get_restore_info() impacts an application performance for
> > drivers
> > > > > that do not provide this ops.
> > > > > It could also impact processing of packets that require no call to
> > > > > rte_flow_get_restore_info() at all.
> > > > >
> > > > > Advertise in mbuf (via a dynamic flag) whether the driver has more
> > > > > metadata to provide via rte_flow_get_restore_info().
> > > > > The application can then call it only when required.
> > > > >
> > > > > Link: http://inbox.dpdk.org/dev/5248c2ca-f2a6-3fb0-38b8-
> > > > > 7f659bfa40de@ovn.org/
> > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > > ---
> > > > > Note: I did not test this RFC patch yet but I hope we can resume and
> > > > > maybe conclude on the discussion for the tunnel offloading API.
> > > > >
> > > >
> > > > I think your approach has a good base but what happens if
> > > > it is not relevant for all flows? In this case your solution will not work.
> > >
> > > Sorry, I am not following.
> > > Could you develop?
> >
> > I still don't get your comment, could you give an example/usecase
> > where this approach can't work?
> > Thanks.
> >
> I'm think of a use case that some flows have the restore info, while
> other don't for example, we get arp packets or some packets that
> are not tunneled, and we also get tunneled packets.
>
> Or for example PMD supports this flag but the application didn't offload such a rule yet.

Again, maybe I missed something, but my proposal is for a *per packet*
report from the driver.
I am not for a global driver capability, if this is what you have in mind.


>
> In those cases application will be slow even if he didn't offload the rules,
> I assume we can say that if application wants to use this he should know
> that other packets will have some performance issues.
>
> From my point of view if application requested the tunnel offload it should
> always check this function.

With a per packet flag, the application only calls restore_info when
such tunnel offload rules have been requested, and only for packets
that require it.


-- 
David Marchand