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

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Sep 22, 2021 at 09:51:42AM +0800, fengchengwen wrote:
> On 2021/9/22 2:11, Jerin Jacob wrote:
> > 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.
> 
> +1 for move to fastpath.
> 

Will move in next revision.