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

  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

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

> -----Original Message-----
> From: Guo, Junfeng <junfeng.guo@intel.com>
> Sent: Thursday, June 15, 2023 1:17 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Xu, Ting <ting.xu@intel.com>; Guo,
> Junfeng <junfeng.guo@intel.com>
> Subject: [PATCH 0/2] fix variable type in pattern parsing for raw flow
> 
> 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
> 
>  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

Acked-by: Ting Xu <ting.xu@intel.com>
  

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?


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

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

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

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

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Applied to dpdk-next-net-intel.

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