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

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

[Junfeng Guo]

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

[PATCH 1/2] net/ice: fix variable type in pattern parsing for raw flow

[Junfeng Guo]

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.

Fixes: 25be39cc1760 ("net/ice: enable protocol agnostic flow offloading in FDIR")
Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS")
Cc: stable@dpdk.org

Signed-off-by: Ting Xu <ting.xu@intel.com>
Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
---
 drivers/net/ice/ice_fdir_filter.c | 2 +-
 drivers/net/ice/ice_hash.c        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
index 480b369af8..e8842bc242 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -1876,7 +1876,7 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
 				(uint8_t *)(uintptr_t)raw_mask->pattern;
 			uint8_t *tmp_spec, *tmp_mask;
 			uint16_t tmp_val = 0;
-			uint8_t pkt_len = 0;
+			uint16_t pkt_len = 0;
 			uint8_t tmp = 0;
 			int i, j;
 
diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
index f35727856e..52646e9408 100644
--- a/drivers/net/ice/ice_hash.c
+++ b/drivers/net/ice/ice_hash.c
@@ -653,8 +653,8 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
 	struct ice_parser_profile prof;
 	struct ice_parser_result rslt;
+	uint16_t spec_len, pkt_len;
 	uint8_t *pkt_buf, *msk_buf;
-	uint8_t spec_len, pkt_len;
 	uint8_t tmp_val = 0;
 	uint8_t tmp_c = 0;
 	int i, j;
-- 
2.25.1

[PATCH 2/2] net/iavf: fix variable type in pattern parsing for raw flow

[Junfeng Guo]

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.

Fixes: bc0e85586eaf ("net/iavf: support VF RSS flow rule with raw pattern")
Cc: stable@dpdk.org

Signed-off-by: Ting Xu <ting.xu@intel.com>
Signed-off-by: Junfeng Guo <junfeng.guo@intel.com>
---
 drivers/net/iavf/iavf_hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index ae6fb38594..cf4d677101 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -886,8 +886,8 @@ iavf_hash_parse_raw_pattern(const struct rte_flow_item *item,
 			struct iavf_rss_meta *meta)
 {
 	const struct rte_flow_item_raw *raw_spec, *raw_mask;
+	uint16_t spec_len, pkt_len;
 	uint8_t *pkt_buf, *msk_buf;
-	uint8_t spec_len, pkt_len;
 	uint8_t tmp_val = 0;
 	uint8_t tmp_c = 0;
 	int i, j;
-- 
2.25.1

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

[Xu, Ting]

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

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

[David Marchand]

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
>


-- 
David Marchand

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

[Zhang, Qi Z]

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

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

[Guo, Junfeng]

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

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

[Zhang, Qi Z]

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