RE: [PATCH 0/2] fix variable type in pattern parsing for raw flow

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, June 15, 2023 3:28 PM
> To: Guo, Junfeng <junfeng.guo@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; dev@dpdk.org; stable@dpdk.org; Xu, Ting
> <ting.xu@intel.com>
> Subject: Re: [PATCH 0/2] fix variable type in pattern parsing for raw flow
> 
> On Thu, Jun 15, 2023 at 7:17 AM Junfeng Guo <junfeng.guo@intel.com>
> wrote:
> >
> > In current pattern parsing function for protocol agnostic flow
> > offloading (raw flow), some of the variables of packet length are
> > defined as uint8_t, which are too small for some large-size packets,
> > such as srv6 (Segment Routing over IPv6 dataplane) type. Change the
> > type to uint16_t.
> >
> > For example, the length of below srv6 paket is 268 B, larger than the
> > max of uint8_t type (i.e., 256).
> > "mac()/ipv6(nextheader=43)/ipv6srh(headerextlength=4,nextheader=41)\
> >                 /ipv6(dst=2001:2:0:0:0:0:0:2)"
> >
> > Junfeng Guo (2):
> >   net/ice: fix variable type in pattern parsing for raw flow
> >   net/iavf: fix variable type in pattern parsing for raw flow
> 
> In the commit title, it is better to describe a functional impact rather than
> repeat the implementation of a fix.
> 
> This makes it easier for people looking for a fix for their specific issue they
> are investigating.
> And, on the contrary, it also makes it easier when looking for a regression on
> a specific feature.
> 
> Here, "fix variable type" gives no clue that it is linked to packet length or the
> protocol agnostic/raw pattern offloading feature.
> 
> So, I don't understand this part of the code, but I think a better title would
> be:
> net/ice: fix protocol agnostic offloading with big packets
> 
> Does it sound ok to you?

+1
Junfeng,  could you comment, if no concern, I will merge patch with the suggested title.

> 
> 
> >
> >  drivers/net/iavf/iavf_hash.c      | 2 +-
> >  drivers/net/ice/ice_fdir_filter.c | 2 +-
> >  drivers/net/ice/ice_hash.c        | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > --
> > 2.25.1
> >
> 
> 
> --
> David Marchand