diff mbox series

[1/5] net/mlx5: fix protocol size for raw encap judgement

Message ID 493405ea9f0f86ef8e0cfa08cab0d5ae764ae8be.1605493464.git.jackmin@nvidia.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series fix protocol size calculation |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Xiaoyu Min Nov. 16, 2020, 7:55 a.m. UTC 
  From: Xiaoyu Min <jackmin@nvidia.com>

The rte_flow_item_eth and rte_flow_item_vlan items are refined.
The structs do not exactly represent the packet bits captured on the
wire anymore.
Should use real header instead of the whole struct.

Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.

Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")

Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c | 12 ++++++------
 drivers/net/mlx5/mlx5_flow.h |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

Comments

Matan Azrad Nov. 17, 2020, 1:25 p.m. UTC | #1 
From: Xiaoyu Min
> The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> The structs do not exactly represent the packet bits captured on the wire
> anymore.
> Should use real header instead of the whole struct.
> 
> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> 
> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> items")
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
Thomas Monjalon Nov. 22, 2020, 2:21 p.m. UTC | #2 
17/11/2020 14:25, Matan Azrad:
> From: Xiaoyu Min
> > The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> > The structs do not exactly represent the packet bits captured on the wire
> > anymore.
> > Should use real header instead of the whole struct.
> > 
> > Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> > 
> > Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> > items")
> > 
> > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>

It has been partially fixed later (and already merged) by Slava:
http://git.dpdk.org/dpdk/commit/?id=f9210259cac
Thomas Monjalon Nov. 22, 2020, 3:32 p.m. UTC | #3 
16/11/2020 08:55, Xiaoyu Min:
> From: Xiaoyu Min <jackmin@nvidia.com>
> 
> The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> The structs do not exactly represent the packet bits captured on the
> wire anymore.
> Should use real header instead of the whole struct.
> 
> Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> 
> Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN items")
> 
> Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c | 12 ++++++------
>  drivers/net/mlx5/mlx5_flow.h |  4 ++--
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 324349ed19..34b7a2f137 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -3655,8 +3655,8 @@ flow_check_hairpin_split(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
>  			raw_encap = actions->conf;
>  			if (raw_encap->size >
> -			    (sizeof(struct rte_flow_item_eth) +
> -			     sizeof(struct rte_flow_item_ipv4)))
> +			    (sizeof(struct rte_ether_hdr) +
> +			     sizeof(struct rte_ipv4_hdr)))
>  				split++;
>  			action_n++;
>  			break;
> @@ -4092,8 +4092,8 @@ flow_hairpin_split(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
>  			raw_encap = actions->conf;
>  			if (raw_encap->size >
> -			    (sizeof(struct rte_flow_item_eth) +
> -			     sizeof(struct rte_flow_item_ipv4))) {
> +			    (sizeof(struct rte_ether_hdr) +
> +			     sizeof(struct rte_ipv4_hdr))) {
>  				memcpy(actions_tx, actions,
>  				       sizeof(struct rte_flow_action));
>  				actions_tx++;
> @@ -4107,8 +4107,8 @@ flow_hairpin_split(struct rte_eth_dev *dev,
>  		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
>  			raw_decap = actions->conf;
>  			if (raw_decap->size <
> -			    (sizeof(struct rte_flow_item_eth) +
> -			     sizeof(struct rte_flow_item_ipv4))) {
> +			    (sizeof(struct rte_ether_hdr) +
> +			     sizeof(struct rte_ipv4_hdr))) {
>  				memcpy(actions_tx, actions,
>  				       sizeof(struct rte_flow_action));
>  				actions_tx++;
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 5fac8672fc..3b28c48a1d 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -345,8 +345,8 @@ enum mlx5_feature_name {
>  #define MLX5_GENEVE_OPT_LEN_0 14
>  #define MLX5_GENEVE_OPT_LEN_1 63
>  
> -#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_flow_item_eth) + \
> -					  sizeof(struct rte_flow_item_ipv4))
> +#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_ether_hdr) + \
> +					  sizeof(struct rte_ipv4_hdr))

This constant should be used above in hairpin functions.
Will change while merging.
Matan Azrad Nov. 22, 2020, 4:04 p.m. UTC | #4 
From: Thomas Monjalon
 
> 16/11/2020 08:55, Xiaoyu Min:
> > From: Xiaoyu Min <jackmin@nvidia.com>
> >
> > The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> > The structs do not exactly represent the packet bits captured on the
> > wire anymore.
> > Should use real header instead of the whole struct.
> >
> > Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> >
> > Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> > items")
> >
> > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow.c | 12 ++++++------
> > drivers/net/mlx5/mlx5_flow.h |  4 ++--
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > b/drivers/net/mlx5/mlx5_flow.c index 324349ed19..34b7a2f137 100644
> > --- a/drivers/net/mlx5/mlx5_flow.c
> > +++ b/drivers/net/mlx5/mlx5_flow.c
> > @@ -3655,8 +3655,8 @@ flow_check_hairpin_split(struct rte_eth_dev *dev,
> >               case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> >                       raw_encap = actions->conf;
> >                       if (raw_encap->size >
> > -                         (sizeof(struct rte_flow_item_eth) +
> > -                          sizeof(struct rte_flow_item_ipv4)))
> > +                         (sizeof(struct rte_ether_hdr) +
> > +                          sizeof(struct rte_ipv4_hdr)))
> >                               split++;
> >                       action_n++;
> >                       break;
> > @@ -4092,8 +4092,8 @@ flow_hairpin_split(struct rte_eth_dev *dev,
> >               case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> >                       raw_encap = actions->conf;
> >                       if (raw_encap->size >
> > -                         (sizeof(struct rte_flow_item_eth) +
> > -                          sizeof(struct rte_flow_item_ipv4))) {
> > +                         (sizeof(struct rte_ether_hdr) +
> > +                          sizeof(struct rte_ipv4_hdr))) {
> >                               memcpy(actions_tx, actions,
> >                                      sizeof(struct rte_flow_action));
> >                               actions_tx++; @@ -4107,8 +4107,8 @@
> > flow_hairpin_split(struct rte_eth_dev *dev,
> >               case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
> >                       raw_decap = actions->conf;
> >                       if (raw_decap->size <
> > -                         (sizeof(struct rte_flow_item_eth) +
> > -                          sizeof(struct rte_flow_item_ipv4))) {
> > +                         (sizeof(struct rte_ether_hdr) +
> > +                          sizeof(struct rte_ipv4_hdr))) {
> >                               memcpy(actions_tx, actions,
> >                                      sizeof(struct rte_flow_action));
> >                               actions_tx++; diff --git
> > a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> > 5fac8672fc..3b28c48a1d 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -345,8 +345,8 @@ enum mlx5_feature_name {  #define
> > MLX5_GENEVE_OPT_LEN_0 14  #define MLX5_GENEVE_OPT_LEN_1 63
> >
> > -#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct
> rte_flow_item_eth) + \
> > -                                       sizeof(struct rte_flow_item_ipv4))
> > +#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct
> rte_ether_hdr) + \
> > +                                       sizeof(struct rte_ipv4_hdr))
> 
> This constant should be used above in hairpin functions.
> Will change while merging.

+1
Ori Kam Nov. 23, 2020, 7:54 a.m. UTC | #5 
> -----Original Message-----
> From: Matan Azrad <matan@nvidia.com>
> Sent: Sunday, November 22, 2020 6:05 PM
> Subject: RE: [dpdk-dev] [PATCH 1/5] net/mlx5: fix protocol size for raw encap
> judgement
> 
> 
> 
> From: Thomas Monjalon
> 
> > 16/11/2020 08:55, Xiaoyu Min:
> > > From: Xiaoyu Min <jackmin@nvidia.com>
> > >
> > > The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> > > The structs do not exactly represent the packet bits captured on the
> > > wire anymore.
> > > Should use real header instead of the whole struct.
> > >
> > > Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> > >
> > > Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> > > items")
> > >
> > > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> > > ---
> > >  drivers/net/mlx5/mlx5_flow.c | 12 ++++++------
> > > drivers/net/mlx5/mlx5_flow.h |  4 ++--
> > >  2 files changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > > b/drivers/net/mlx5/mlx5_flow.c index 324349ed19..34b7a2f137 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > @@ -3655,8 +3655,8 @@ flow_check_hairpin_split(struct rte_eth_dev
> *dev,
> > >               case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> > >                       raw_encap = actions->conf;
> > >                       if (raw_encap->size >
> > > -                         (sizeof(struct rte_flow_item_eth) +
> > > -                          sizeof(struct rte_flow_item_ipv4)))
> > > +                         (sizeof(struct rte_ether_hdr) +
> > > +                          sizeof(struct rte_ipv4_hdr)))
> > >                               split++;
> > >                       action_n++;
> > >                       break;
> > > @@ -4092,8 +4092,8 @@ flow_hairpin_split(struct rte_eth_dev *dev,
> > >               case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> > >                       raw_encap = actions->conf;
> > >                       if (raw_encap->size >
> > > -                         (sizeof(struct rte_flow_item_eth) +
> > > -                          sizeof(struct rte_flow_item_ipv4))) {
> > > +                         (sizeof(struct rte_ether_hdr) +
> > > +                          sizeof(struct rte_ipv4_hdr))) {
> > >                               memcpy(actions_tx, actions,
> > >                                      sizeof(struct rte_flow_action));
> > >                               actions_tx++; @@ -4107,8 +4107,8 @@
> > > flow_hairpin_split(struct rte_eth_dev *dev,
> > >               case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
> > >                       raw_decap = actions->conf;
> > >                       if (raw_decap->size <
> > > -                         (sizeof(struct rte_flow_item_eth) +
> > > -                          sizeof(struct rte_flow_item_ipv4))) {
> > > +                         (sizeof(struct rte_ether_hdr) +
> > > +                          sizeof(struct rte_ipv4_hdr))) {
> > >                               memcpy(actions_tx, actions,
> > >                                      sizeof(struct rte_flow_action));
> > >                               actions_tx++; diff --git
> > > a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> > > 5fac8672fc..3b28c48a1d 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.h
> > > +++ b/drivers/net/mlx5/mlx5_flow.h
> > > @@ -345,8 +345,8 @@ enum mlx5_feature_name {  #define
> > > MLX5_GENEVE_OPT_LEN_0 14  #define MLX5_GENEVE_OPT_LEN_1 63
> > >
> > > -#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct
> > rte_flow_item_eth) + \
> > > -                                       sizeof(struct rte_flow_item_ipv4))
> > > +#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct
> > rte_ether_hdr) + \
> > > +                                       sizeof(struct rte_ipv4_hdr))
> >
> > This constant should be used above in hairpin functions.
> > Will change while merging.
> 
> +1
> 

Why above the hairpin functions? This is correct for all encap decisions.

Best,
Ori
Thomas Monjalon Nov. 23, 2020, 8:12 a.m. UTC | #6 
23/11/2020 08:54, Ori Kam:
> From: Matan Azrad <matan@nvidia.com>
> > From: Thomas Monjalon
> > > 16/11/2020 08:55, Xiaoyu Min:
> > > > From: Xiaoyu Min <jackmin@nvidia.com>
> > > >
> > > > The rte_flow_item_eth and rte_flow_item_vlan items are refined.
> > > > The structs do not exactly represent the packet bits captured on the
> > > > wire anymore.
> > > > Should use real header instead of the whole struct.
> > > >
> > > > Replace the rte_flow_item_* with the existing corresponding rte_*_hdr.
> > > >
> > > > Fixes: 09315fc83861 ("ethdev: add VLAN attributes to ethernet and VLAN
> > > > items")
> > > >
> > > > Signed-off-by: Xiaoyu Min <jackmin@nvidia.com>
> > > > ---
> > > >  drivers/net/mlx5/mlx5_flow.c | 12 ++++++------
> > > > drivers/net/mlx5/mlx5_flow.h |  4 ++--
> > > >  2 files changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > > > b/drivers/net/mlx5/mlx5_flow.c index 324349ed19..34b7a2f137 100644
> > > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > > @@ -3655,8 +3655,8 @@ flow_check_hairpin_split(struct rte_eth_dev
> > *dev,
> > > >               case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> > > >                       raw_encap = actions->conf;
> > > >                       if (raw_encap->size >
> > > > -                         (sizeof(struct rte_flow_item_eth) +
> > > > -                          sizeof(struct rte_flow_item_ipv4)))
> > > > +                         (sizeof(struct rte_ether_hdr) +
> > > > +                          sizeof(struct rte_ipv4_hdr)))
> > > >                               split++;
> > > >                       action_n++;
> > > >                       break;
> > > > @@ -4092,8 +4092,8 @@ flow_hairpin_split(struct rte_eth_dev *dev,
> > > >               case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> > > >                       raw_encap = actions->conf;
> > > >                       if (raw_encap->size >
> > > > -                         (sizeof(struct rte_flow_item_eth) +
> > > > -                          sizeof(struct rte_flow_item_ipv4))) {
> > > > +                         (sizeof(struct rte_ether_hdr) +
> > > > +                          sizeof(struct rte_ipv4_hdr))) {
> > > >                               memcpy(actions_tx, actions,
> > > >                                      sizeof(struct rte_flow_action));
> > > >                               actions_tx++; @@ -4107,8 +4107,8 @@
> > > > flow_hairpin_split(struct rte_eth_dev *dev,
> > > >               case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
> > > >                       raw_decap = actions->conf;
> > > >                       if (raw_decap->size <
> > > > -                         (sizeof(struct rte_flow_item_eth) +
> > > > -                          sizeof(struct rte_flow_item_ipv4))) {
> > > > +                         (sizeof(struct rte_ether_hdr) +
> > > > +                          sizeof(struct rte_ipv4_hdr))) {
> > > >                               memcpy(actions_tx, actions,
> > > >                                      sizeof(struct rte_flow_action));
> > > >                               actions_tx++; diff --git
> > > > a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index
> > > > 5fac8672fc..3b28c48a1d 100644
> > > > --- a/drivers/net/mlx5/mlx5_flow.h
> > > > +++ b/drivers/net/mlx5/mlx5_flow.h
> > > > @@ -345,8 +345,8 @@ enum mlx5_feature_name {  #define
> > > > MLX5_GENEVE_OPT_LEN_0 14  #define MLX5_GENEVE_OPT_LEN_1 63
> > > >
> > > > -#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct
> > > rte_flow_item_eth) + \
> > > > -                                       sizeof(struct rte_flow_item_ipv4))
> > > > +#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct
> > > rte_ether_hdr) + \
> > > > +                                       sizeof(struct rte_ipv4_hdr))
> > >
> > > This constant should be used above in hairpin functions.
> > > Will change while merging.
> > 
> > +1
> > 
> 
> Why above the hairpin functions? This is correct for all encap decisions.

Other functions were already using this macro.
Only hairpin functions were not using it.
So I changed on apply.
diff mbox series

Patch

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 324349ed19..34b7a2f137 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3655,8 +3655,8 @@  flow_check_hairpin_split(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			raw_encap = actions->conf;
 			if (raw_encap->size >
-			    (sizeof(struct rte_flow_item_eth) +
-			     sizeof(struct rte_flow_item_ipv4)))
+			    (sizeof(struct rte_ether_hdr) +
+			     sizeof(struct rte_ipv4_hdr)))
 				split++;
 			action_n++;
 			break;
@@ -4092,8 +4092,8 @@  flow_hairpin_split(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			raw_encap = actions->conf;
 			if (raw_encap->size >
-			    (sizeof(struct rte_flow_item_eth) +
-			     sizeof(struct rte_flow_item_ipv4))) {
+			    (sizeof(struct rte_ether_hdr) +
+			     sizeof(struct rte_ipv4_hdr))) {
 				memcpy(actions_tx, actions,
 				       sizeof(struct rte_flow_action));
 				actions_tx++;
@@ -4107,8 +4107,8 @@  flow_hairpin_split(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_RAW_DECAP:
 			raw_decap = actions->conf;
 			if (raw_decap->size <
-			    (sizeof(struct rte_flow_item_eth) +
-			     sizeof(struct rte_flow_item_ipv4))) {
+			    (sizeof(struct rte_ether_hdr) +
+			     sizeof(struct rte_ipv4_hdr))) {
 				memcpy(actions_tx, actions,
 				       sizeof(struct rte_flow_action));
 				actions_tx++;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 5fac8672fc..3b28c48a1d 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -345,8 +345,8 @@  enum mlx5_feature_name {
 #define MLX5_GENEVE_OPT_LEN_0 14
 #define MLX5_GENEVE_OPT_LEN_1 63
 
-#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_flow_item_eth) + \
-					  sizeof(struct rte_flow_item_ipv4))
+#define MLX5_ENCAPSULATION_DECISION_SIZE (sizeof(struct rte_ether_hdr) + \
+					  sizeof(struct rte_ipv4_hdr))
 
 /* IPv4 fragment_offset field contains relevant data in bits 2 to 15. */
 #define MLX5_IPV4_FRAG_OFFSET_MASK \