Re: [dpdk-dev] [PATCH v3 2/8] dmadev: add burst capacity API

[Jerin Jacob <jerinjacobk@gmail.com>]

On Tue, Sep 21, 2021 at 10:42 PM Pai G, Sunil <sunil.pai.g@intel.com> wrote:
>
> Hi Jerin,
>
> <snipped>
>
> > > > To understand it better, Could you share more details on feedback
> > > > mechanism on your application enqueue
> > > >
> > > > app_enqueue_v1(.., nb_seg)
> > > > {
> > > >              /* Not enough space, Let application handle it by
> > > > dropping or resubmitting */
> > > >              if (rte_dmadev_burst_capacity() < nb_seg)
> > > >                         return -ENOSPC;
> > > >
> > > >             do rte_dma_op() in loop without checking error;
> > > >             return 0; // Success
> > > > }
> > > >
> > > > vs
> > > > app_enqueue_v2(.., nb_seg)
> > > > {
> > > >            int rc;
> > > >
> > > >             rc |= rte_dma_op() in loop without checking error;
> > > >             return rc; // return the actual status to application
> > > > if Not enough space, Let application handle it by dropping or
> > > > resubmitting */ }
> > > >
> > > > Is app_enqueue_v1() and app_enqueue_v2() logically the same from
> > > > application PoV. Right?
> > > >
> > > > If not, could you explain, the version you are planning to do for
> > > > app_enqueue()
> > >
> > > The exact version can be found here :
> > >
> > http://patchwork.ozlabs.org/project/openvswitch/patch/20210907120021.4
> > > 093604-2-sunil.pai.g@intel.com/ Unfortunately, both versions are not
> > > same in our case because of the SW fallback we have for ring full scenario's.
> > > For a packet with 8 nb_segs, if the ring has only space for 4 , we
> > > would avoid this packet with app_enqueue_v1 while going ahead with an
> > enqueue with app_enqueue_v2, resulting in a mix of DMA and CPU copies
> > for a packet which we would want to avoid.
> >
> > Thanks for RFC link. Usage is clear now, Since you are checking the space in
> > the completion handler, I am not sure, what is hard to get the remaining
> > space, Will following logic work to find the empty space. If not, please discuss
> > the issue with this approach. I am trying to avoid yet another fastpath API
> > and complication in driver as there is element checking space in the submit
> > queue and completion queue at least in our hardware.
> >
> >      max_count = nb_desc; (power of 2)
> >      mask = max_count - 1;
> >
> >      for (i = 0; I < n; i++) {
> >           submit_idx = rte_dma_copy();
> >      }
> >      rc = rte_dma_completed(…, &completed_idx..);
> >      space_in_queue =  mask - ((submit_idx – completed_idx) & mask);
> >
>
> Unfortunately, space left in the device (as calculated by the app) still can mean there is no space in the device :|
> i.e its pmd dependent.

I did not pay much attention to Jiayu's reply as I did not know what is DSA.
Now I searched the DSA in ml, I could see the PMD patches.

If it is PMD limitation then I am fine with the proposed API.

@Richardson, Bruce @Laatz, Kevin  @feng Since it is used fastpath, Can
we move to fastpath handler and
remove additional check in fastpath from common code like other APIs.

>
> As Jiayu mentioned before:
> > The fact is that it's very hard for apps to calculate the available space of a DMA ring.
> > For DSA, the available space is decided by three factors: the number of available slots
> > in SW ring, the max batching size of a batch descriptor, and if there are available batch
> > descriptors. The first one is configured by SW, and apps can calculate it. But the second
> > depends on DSA HW, and the third one is hided in DSA driver which is not visible to apps.
> > Considering the complexity of different DMA HW, I think the best way is to hide all details
> > inside DMA dev and provide this check capacity API for apps.
>
> <snipped>
>
> Thanks and regards,
> Sunil
>