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

["Guo, Junfeng" <junfeng.guo@intel.com>]

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Friday, June 16, 2023 13:49
> To: David Marchand <david.marchand@redhat.com>; Guo, Junfeng
> <junfeng.guo@intel.com>
> Cc: 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
> 
> 
> 
> > -----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.

Sure, that sounds good to me!
Please go ahead with the suggested title.

Thanks David for the comment!
Yes, it's more reasonable to describe patch with more specific and valid words.

> 
> >
> >
> > >
> > >  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