[dpdk-dev] [PATCH v3 2/4] regex/mlx5: add data path scattered mbuf process

[Suanming Mou]

Hi Slava,

> -----Original Message-----
> From: Slava Ovsiienko <viacheslavo at nvidia.com>
> Sent: Tuesday, March 30, 2021 4:05 PM
> To: Suanming Mou <suanmingm at nvidia.com>; Ori Kam <orika at nvidia.com>
> Cc: dev at dpdk.org; Matan Azrad <matan at nvidia.com>; Raslan Darawsheh
> <rasland at nvidia.com>
> Subject: RE: [PATCH v3 2/4] regex/mlx5: add data path scattered mbuf process
> 
> > -----Original Message-----
> > From: Suanming Mou <suanmingm at nvidia.com>
> > Sent: Tuesday, March 30, 2021 4:39
> > To: Ori Kam <orika at nvidia.com>
> > Cc: dev at dpdk.org; Slava Ovsiienko <viacheslavo at nvidia.com>; Matan
> > Azrad <matan at nvidia.com>; Raslan Darawsheh <rasland at nvidia.com>
> > Subject: [PATCH v3 2/4] regex/mlx5: add data path scattered mbuf
> > process
> >
> Nice feature, but I would fix the typos and reword a bit:
> 
> > UMR WQE can convert multiple mkey's memory sapce to contiguous space.
> Typo: "sapce?"
> 
> And rather not "convert mkey" but "present data buffers scattered within
> multiple mbufs with single indirect mkey".
> 
> 
> > Take advantage of the UMR WQE, scattered mbuf in one operation can be
> > converted to an indirect mkey. The RegEx which only accepts one mkey
> > can now process the whole scattered mbuf.
> I would add "in one operation."
> 
> >
> > The maximum scattered mbuf can be supported in one UMR WQE is now
> > defined as 64. Multiple operations scattered mbufs can be add to one
> > UMR
> Typos: "THE multiple", "added"
> 
> I would reword - "The mbufs from multiple operations can be combined into one
> UMR. Also, I would add few words what UMR is.
> 
> > WQE if there is enough space in the KLM array, since the operations
> > can address their own mbuf's content by the mkey's address and length.
> > However, one operation's scattered mbuf's can't be placed in two
> > different UMR WQE's KLM array, if the UMR WQE's KLM does not has
> > enough free space for one operation, a new UMR WQE will be required.
> I would say "the extra UMR WQE will be engaged"
> 
> >
> > In case the UMR WQE's indirect mkey will be over wrapped by the SQ's
> > WQE move, the meky's index used by the UMR WQE should be the index of
> > last
> typo: "meky"
> 
> > the RegEX WQE in the operations. As one operation consumes one WQE
> > set, build the RegEx WQE by reverse helps address the mkey more efficiently.
> typo: TO address
> 
> With best regards,
> Slava
> 

Thanks very much for helping with the log improvement, I will wait for one or two days to see if there are other new comments and update it with the new version.

BR,
Suanming