DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
@ 2018-07-12  2:09 Rosen Xu
  2018-07-12  5:17 ` Ori Kam
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Rosen Xu @ 2018-07-12  2:09 UTC (permalink / raw)
  To: dev; +Cc: rosen.xu, ferruh.yigit, orika, stable

Rte_fdir_conf of rte_eth_conf should be initialized before
port initialization.

Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
Cc: stable@dpdk.org

Signed-off-by: Rosen Xu <rosen.xu@intel.com>
---
 examples/flow_filtering/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
index f595034..aa03e23 100644
--- a/examples/flow_filtering/main.c
+++ b/examples/flow_filtering/main.c
@@ -132,6 +132,12 @@
 				DEV_TX_OFFLOAD_SCTP_CKSUM  |
 				DEV_TX_OFFLOAD_TCP_TSO,
 		},
+		.fdir_conf = {
+			.mode = RTE_FDIR_MODE_PERFECT,
+			.pballoc = RTE_FDIR_PBALLOC_64K,
+			.status = RTE_FDIR_REPORT_STATUS,
+			.drop_queue = 127,
+		},
 	};
 	struct rte_eth_txconf txq_conf;
 	struct rte_eth_rxconf rxq_conf;
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  2:09 [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization Rosen Xu
@ 2018-07-12  5:17 ` Ori Kam
  2018-07-12  5:26   ` Xu, Rosen
  2018-07-21  7:50 ` [dpdk-dev] [PATCH v2] " Rosen Xu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Ori Kam @ 2018-07-12  5:17 UTC (permalink / raw)
  To: Rosen Xu, dev; +Cc: ferruh.yigit, stable, Ori Kam

Hi Rosen,

Why do the fdir_conf must be initialized?

What is the issue you are seeing?

Best,
Ori

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> Sent: Thursday, July 12, 2018 5:10 AM
> To: dev@dpdk.org
> Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> <orika@mellanox.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> Rte_fdir_conf of rte_eth_conf should be initialized before
> port initialization.
> 
> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow
> API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> ---
>  examples/flow_filtering/main.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
> index f595034..aa03e23 100644
> --- a/examples/flow_filtering/main.c
> +++ b/examples/flow_filtering/main.c
> @@ -132,6 +132,12 @@
>  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
>  				DEV_TX_OFFLOAD_TCP_TSO,
>  		},
> +		.fdir_conf = {
> +			.mode = RTE_FDIR_MODE_PERFECT,
> +			.pballoc = RTE_FDIR_PBALLOC_64K,
> +			.status = RTE_FDIR_REPORT_STATUS,
> +			.drop_queue = 127,
> +		},
>  	};
>  	struct rte_eth_txconf txq_conf;
>  	struct rte_eth_rxconf rxq_conf;
> --
> 1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  5:17 ` Ori Kam
@ 2018-07-12  5:26   ` Xu, Rosen
  2018-07-12  5:58     ` Ori Kam
  0 siblings, 1 reply; 18+ messages in thread
From: Xu, Rosen @ 2018-07-12  5:26 UTC (permalink / raw)
  To: Ori Kam, dev; +Cc: Yigit, Ferruh, stable

Hi Ori,

examples/flow_filtering sample app fails on i40e [1] because i40e requires explicit FDIR configuration.

But rte_flow in and hardware independent ways of describing flow-action, it shouldn't require specific config options for specific hardware.

Is there any chance driver select the FDIR config automatically based on rte_flow rule, unless explicitly a FDIR config set by user?

[1]
Flow can't be created 1 message: Check the mode in fdir_conf.
EAL: Error - exiting with code: 1

> -----Original Message-----
> From: Ori Kam [mailto:orika@mellanox.com]
> Sent: Thursday, July 12, 2018 13:17
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
> <orika@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> Hi Rosen,
> 
> Why do the fdir_conf must be initialized?
> 
> What is the issue you are seeing?
> 
> Best,
> Ori
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > Sent: Thursday, July 12, 2018 5:10 AM
> > To: dev@dpdk.org
> > Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> > <orika@mellanox.com>; stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> > initialization
> >
> > Rte_fdir_conf of rte_eth_conf should be initialized before port
> > initialization.
> >
> > Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow
> > API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > ---
> >  examples/flow_filtering/main.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/examples/flow_filtering/main.c
> > b/examples/flow_filtering/main.c index f595034..aa03e23 100644
> > --- a/examples/flow_filtering/main.c
> > +++ b/examples/flow_filtering/main.c
> > @@ -132,6 +132,12 @@
> >  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
> >  				DEV_TX_OFFLOAD_TCP_TSO,
> >  		},
> > +		.fdir_conf = {
> > +			.mode = RTE_FDIR_MODE_PERFECT,
> > +			.pballoc = RTE_FDIR_PBALLOC_64K,
> > +			.status = RTE_FDIR_REPORT_STATUS,
> > +			.drop_queue = 127,
> > +		},
> >  	};
> >  	struct rte_eth_txconf txq_conf;
> >  	struct rte_eth_rxconf rxq_conf;
> > --
> > 1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  5:26   ` Xu, Rosen
@ 2018-07-12  5:58     ` Ori Kam
  2018-07-12  6:22       ` Xu, Rosen
  0 siblings, 1 reply; 18+ messages in thread
From: Ori Kam @ 2018-07-12  5:58 UTC (permalink / raw)
  To: Xu, Rosen, dev; +Cc: Yigit, Ferruh, stable

Hi,

PSB 

> -----Original Message-----
> From: Xu, Rosen [mailto:rosen.xu@intel.com]
> Sent: Thursday, July 12, 2018 8:27 AM
> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> Hi Ori,
> 
> examples/flow_filtering sample app fails on i40e [1] because i40e requires
> explicit FDIR configuration.
> 
> But rte_flow in and hardware independent ways of describing flow-action, it
> shouldn't require specific config options for specific hardware.
> 

I don't understand why using rte flow require the use of fdir.
it doesn't make sense to me, that  new API will need old one.

> Is there any chance driver select the FDIR config automatically based on
> rte_flow rule, unless explicitly a FDIR config set by user?

I don't know how the i40e driver is implemented but I know
that Mellanox convert the other way around, if fdir is given it is 
converted to rte_flow.


> 
> [1]
> Flow can't be created 1 message: Check the mode in fdir_conf.
> EAL: Error - exiting with code: 1
> 
> > -----Original Message-----
> > From: Ori Kam [mailto:orika@mellanox.com]
> > Sent: Thursday, July 12, 2018 13:17
> > To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
> > <orika@mellanox.com>
> > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> rte_fdir_conf
> > initialization
> >
> > Hi Rosen,
> >
> > Why do the fdir_conf must be initialized?
> >
> > What is the issue you are seeing?
> >
> > Best,
> > Ori
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > > Sent: Thursday, July 12, 2018 5:10 AM
> > > To: dev@dpdk.org
> > > Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> > > <orika@mellanox.com>; stable@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> > > initialization
> > >
> > > Rte_fdir_conf of rte_eth_conf should be initialized before port
> > > initialization.
> > >
> > > Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow
> > > API")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > > ---
> > >  examples/flow_filtering/main.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/examples/flow_filtering/main.c
> > > b/examples/flow_filtering/main.c index f595034..aa03e23 100644
> > > --- a/examples/flow_filtering/main.c
> > > +++ b/examples/flow_filtering/main.c
> > > @@ -132,6 +132,12 @@
> > >  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
> > >  				DEV_TX_OFFLOAD_TCP_TSO,
> > >  		},
> > > +		.fdir_conf = {
> > > +			.mode = RTE_FDIR_MODE_PERFECT,
> > > +			.pballoc = RTE_FDIR_PBALLOC_64K,
> > > +			.status = RTE_FDIR_REPORT_STATUS,
> > > +			.drop_queue = 127,
> > > +		},
> > >  	};
> > >  	struct rte_eth_txconf txq_conf;
> > >  	struct rte_eth_rxconf rxq_conf;
> > > --
> > > 1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  5:58     ` Ori Kam
@ 2018-07-12  6:22       ` Xu, Rosen
  2018-07-17  5:15         ` Ori Kam
  0 siblings, 1 reply; 18+ messages in thread
From: Xu, Rosen @ 2018-07-12  6:22 UTC (permalink / raw)
  To: Ori Kam, dev; +Cc: Yigit, Ferruh, stable, Gilmore, Walter E

Hi Ori,

Pls see my reply.

Hi Walter and Ferruh,

I need your voice :)

> -----Original Message-----
> From: Ori Kam [mailto:orika@mellanox.com]
> Sent: Thursday, July 12, 2018 13:58
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> Hi,
> 
> PSB
> 
> > -----Original Message-----
> > From: Xu, Rosen [mailto:rosen.xu@intel.com]
> > Sent: Thursday, July 12, 2018 8:27 AM
> > To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> > rte_fdir_conf initialization
> >
> > Hi Ori,
> >
> > examples/flow_filtering sample app fails on i40e [1] because i40e
> > requires explicit FDIR configuration.
> >
> > But rte_flow in and hardware independent ways of describing
> > flow-action, it shouldn't require specific config options for specific
> hardware.
> >
> 
> I don't understand why using rte flow require the use of fdir.
> it doesn't make sense to me, that  new API will need old one.

It's a good question, I also have this question about Mellanox NIC Driver mlx5_flow.c.
In this file many flow functions call fdir. :)

> > Is there any chance driver select the FDIR config automatically based
> > on rte_flow rule, unless explicitly a FDIR config set by user?
> 
> I don't know how the i40e driver is implemented but I know that Mellanox
> convert the other way around, if fdir is given it is converted to rte_flow.

Firstly, rte_fdir_conf is part of rte_eth_conf definition.
	struct rte_eth_conf {
		......
		struct rte_fdir_conf fdir_conf; /**< FDIR configuration. */
		......
	};
Secondly, default value of rte_eth_conf.fdir_conf.mode is RTE_FDIR_MODE_NONE, which means Disable FDIR support.
Thirdly, flow_filtering should align with test-pmd, in test-pmd all fdir_conf is initialized.
 
> 
> >
> > [1]
> > Flow can't be created 1 message: Check the mode in fdir_conf.
> > EAL: Error - exiting with code: 1
> >
> > > -----Original Message-----
> > > From: Ori Kam [mailto:orika@mellanox.com]
> > > Sent: Thursday, July 12, 2018 13:17
> > > To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> > > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
> > > <orika@mellanox.com>
> > > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> > rte_fdir_conf
> > > initialization
> > >
> > > Hi Rosen,
> > >
> > > Why do the fdir_conf must be initialized?
> > >
> > > What is the issue you are seeing?
> > >
> > > Best,
> > > Ori
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > > > Sent: Thursday, July 12, 2018 5:10 AM
> > > > To: dev@dpdk.org
> > > > Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> > > > <orika@mellanox.com>; stable@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> > > > initialization
> > > >
> > > > Rte_fdir_conf of rte_eth_conf should be initialized before port
> > > > initialization.
> > > >
> > > > Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of
> flow
> > > > API")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > > > ---
> > > >  examples/flow_filtering/main.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/examples/flow_filtering/main.c
> > > > b/examples/flow_filtering/main.c index f595034..aa03e23 100644
> > > > --- a/examples/flow_filtering/main.c
> > > > +++ b/examples/flow_filtering/main.c
> > > > @@ -132,6 +132,12 @@
> > > >  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
> > > >  				DEV_TX_OFFLOAD_TCP_TSO,
> > > >  		},
> > > > +		.fdir_conf = {
> > > > +			.mode = RTE_FDIR_MODE_PERFECT,
> > > > +			.pballoc = RTE_FDIR_PBALLOC_64K,
> > > > +			.status = RTE_FDIR_REPORT_STATUS,
> > > > +			.drop_queue = 127,
> > > > +		},
> > > >  	};
> > > >  	struct rte_eth_txconf txq_conf;
> > > >  	struct rte_eth_rxconf rxq_conf;
> > > > --
> > > > 1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  6:22       ` Xu, Rosen
@ 2018-07-17  5:15         ` Ori Kam
  2018-07-17  9:57           ` Ferruh Yigit
  0 siblings, 1 reply; 18+ messages in thread
From: Ori Kam @ 2018-07-17  5:15 UTC (permalink / raw)
  To: Xu, Rosen, dev; +Cc: Yigit, Ferruh, stable, Gilmore, Walter E

Sorry for the late response,

> -----Original Message-----
> From: Xu, Rosen [mailto:rosen.xu@intel.com]
> Sent: Thursday, July 12, 2018 9:23 AM
> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Gilmore, Walter
> E <walter.e.gilmore@intel.com>
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> Hi Ori,
> 
> Pls see my reply.
> 
> Hi Walter and Ferruh,
> 
> I need your voice :)
> 
> > -----Original Message-----
> > From: Ori Kam [mailto:orika@mellanox.com]
> > Sent: Thursday, July 12, 2018 13:58
> > To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> rte_fdir_conf
> > initialization
> >
> > Hi,
> >
> > PSB
> >
> > > -----Original Message-----
> > > From: Xu, Rosen [mailto:rosen.xu@intel.com]
> > > Sent: Thursday, July 12, 2018 8:27 AM
> > > To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> > > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> > > rte_fdir_conf initialization
> > >
> > > Hi Ori,
> > >
> > > examples/flow_filtering sample app fails on i40e [1] because i40e
> > > requires explicit FDIR configuration.
> > >
> > > But rte_flow in and hardware independent ways of describing
> > > flow-action, it shouldn't require specific config options for specific
> > hardware.
> > >
> >
> > I don't understand why using rte flow require the use of fdir.
> > it doesn't make sense to me, that  new API will need old one.
> 
> It's a good question, I also have this question about Mellanox NIC Driver
> mlx5_flow.c.
> In this file many flow functions call fdir. :)

The only functions that are calling fdir are fdir function,
and you can see that inside of the create function we convert the fdir 
Into rte flow.

> 
> > > Is there any chance driver select the FDIR config automatically based
> > > on rte_flow rule, unless explicitly a FDIR config set by user?
> >
> > I don't know how the i40e driver is implemented but I know that Mellanox
> > convert the other way around, if fdir is given it is converted to rte_flow.
> 
> Firstly, rte_fdir_conf is part of rte_eth_conf definition.
> 	struct rte_eth_conf {
> 		......
> 		struct rte_fdir_conf fdir_conf; /**< FDIR configuration. */
> 		......
> 	};
> Secondly, default value of rte_eth_conf.fdir_conf.mode is
> RTE_FDIR_MODE_NONE, which means Disable FDIR support.
> Thirdly, flow_filtering should align with test-pmd, in test-pmd all fdir_conf is
> initialized.
> 

This sounds to me correct we don't want to enable fdir.
Why should the example app for rte flow use fdir? And align to 
testpmd which support everything in in all modes?


> >
> > >
> > > [1]
> > > Flow can't be created 1 message: Check the mode in fdir_conf.
> > > EAL: Error - exiting with code: 1
> > >
> > > > -----Original Message-----
> > > > From: Ori Kam [mailto:orika@mellanox.com]
> > > > Sent: Thursday, July 12, 2018 13:17
> > > > To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> > > > Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
> > > > <orika@mellanox.com>
> > > > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> > > rte_fdir_conf
> > > > initialization
> > > >
> > > > Hi Rosen,
> > > >
> > > > Why do the fdir_conf must be initialized?
> > > >
> > > > What is the issue you are seeing?
> > > >
> > > > Best,
> > > > Ori
> > > >
> > > > > -----Original Message-----
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> > > > > Sent: Thursday, July 12, 2018 5:10 AM
> > > > > To: dev@dpdk.org
> > > > > Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> > > > > <orika@mellanox.com>; stable@dpdk.org
> > > > > Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add
> rte_fdir_conf
> > > > > initialization
> > > > >
> > > > > Rte_fdir_conf of rte_eth_conf should be initialized before port
> > > > > initialization.
> > > > >
> > > > > Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of
> > flow
> > > > > API")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> > > > > ---
> > > > >  examples/flow_filtering/main.c | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/examples/flow_filtering/main.c
> > > > > b/examples/flow_filtering/main.c index f595034..aa03e23 100644
> > > > > --- a/examples/flow_filtering/main.c
> > > > > +++ b/examples/flow_filtering/main.c
> > > > > @@ -132,6 +132,12 @@
> > > > >  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
> > > > >  				DEV_TX_OFFLOAD_TCP_TSO,
> > > > >  		},
> > > > > +		.fdir_conf = {
> > > > > +			.mode = RTE_FDIR_MODE_PERFECT,
> > > > > +			.pballoc = RTE_FDIR_PBALLOC_64K,
> > > > > +			.status = RTE_FDIR_REPORT_STATUS,
> > > > > +			.drop_queue = 127,
> > > > > +		},
> > > > >  	};
> > > > >  	struct rte_eth_txconf txq_conf;
> > > > >  	struct rte_eth_rxconf rxq_conf;
> > > > > --
> > > > > 1.8.3.1


Best,
Ori

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-17  5:15         ` Ori Kam
@ 2018-07-17  9:57           ` Ferruh Yigit
  2018-07-17 13:04             ` Ori Kam
  0 siblings, 1 reply; 18+ messages in thread
From: Ferruh Yigit @ 2018-07-17  9:57 UTC (permalink / raw)
  To: Ori Kam, Xu, Rosen, dev; +Cc: stable, Gilmore, Walter E, Qi Zhang

On 7/17/2018 6:15 AM, Ori Kam wrote:
> Sorry for the late response,
> 
>> -----Original Message-----
>> From: Xu, Rosen [mailto:rosen.xu@intel.com]
>> Sent: Thursday, July 12, 2018 9:23 AM
>> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Gilmore, Walter
>> E <walter.e.gilmore@intel.com>
>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
>> initialization
>>
>> Hi Ori,
>>
>> Pls see my reply.
>>
>> Hi Walter and Ferruh,
>>
>> I need your voice :)
>>
>>> -----Original Message-----
>>> From: Ori Kam [mailto:orika@mellanox.com]
>>> Sent: Thursday, July 12, 2018 13:58
>>> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>> rte_fdir_conf
>>> initialization
>>>
>>> Hi,
>>>
>>> PSB
>>>
>>>> -----Original Message-----
>>>> From: Xu, Rosen [mailto:rosen.xu@intel.com]
>>>> Sent: Thursday, July 12, 2018 8:27 AM
>>>> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>>>> rte_fdir_conf initialization
>>>>
>>>> Hi Ori,
>>>>
>>>> examples/flow_filtering sample app fails on i40e [1] because i40e
>>>> requires explicit FDIR configuration.
>>>>
>>>> But rte_flow in and hardware independent ways of describing
>>>> flow-action, it shouldn't require specific config options for specific
>>> hardware.
>>>>
>>>
>>> I don't understand why using rte flow require the use of fdir.
>>> it doesn't make sense to me, that  new API will need old one.
>>
>> It's a good question, I also have this question about Mellanox NIC Driver
>> mlx5_flow.c.
>> In this file many flow functions call fdir. :)
> 
> The only functions that are calling fdir are fdir function,
> and you can see that inside of the create function we convert the fdir 
> Into rte flow.
> 
>>
>>>> Is there any chance driver select the FDIR config automatically based
>>>> on rte_flow rule, unless explicitly a FDIR config set by user?
>>>
>>> I don't know how the i40e driver is implemented but I know that Mellanox
>>> convert the other way around, if fdir is given it is converted to rte_flow.
>>
>> Firstly, rte_fdir_conf is part of rte_eth_conf definition.
>> 	struct rte_eth_conf {
>> 		......
>> 		struct rte_fdir_conf fdir_conf; /**< FDIR configuration. */
>> 		......
>> 	};
>> Secondly, default value of rte_eth_conf.fdir_conf.mode is
>> RTE_FDIR_MODE_NONE, which means Disable FDIR support.
>> Thirdly, flow_filtering should align with test-pmd, in test-pmd all fdir_conf is
>> initialized.
>>
> 
> This sounds to me correct we don't want to enable fdir.
> Why should the example app for rte flow use fdir? And align to 
> testpmd which support everything in in all modes?

In i40e fdir is used to implement filters, that is why rte_flow rules
requires/depends some fdir configurations.

In long term I agree it is better if driver doesn't require any fdir
configuration for rte_flow programing, although not sure if this is completely
possible, cc'ed Qi for more comment.

For short term I am for getting this patch so that sample app can run on i40e
too, and fdir configuration shouldn't effect others. Perhaps it can be good to
add a comment to say why that config option is added and it is a temporary
workaround.

> 
> 
>>>
>>>>
>>>> [1]
>>>> Flow can't be created 1 message: Check the mode in fdir_conf.
>>>> EAL: Error - exiting with code: 1
>>>>
>>>>> -----Original Message-----
>>>>> From: Ori Kam [mailto:orika@mellanox.com]
>>>>> Sent: Thursday, July 12, 2018 13:17
>>>>> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
>>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
>>>>> <orika@mellanox.com>
>>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>>>> rte_fdir_conf
>>>>> initialization
>>>>>
>>>>> Hi Rosen,
>>>>>
>>>>> Why do the fdir_conf must be initialized?
>>>>>
>>>>> What is the issue you are seeing?
>>>>>
>>>>> Best,
>>>>> Ori
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
>>>>>> Sent: Thursday, July 12, 2018 5:10 AM
>>>>>> To: dev@dpdk.org
>>>>>> Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
>>>>>> <orika@mellanox.com>; stable@dpdk.org
>>>>>> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add
>> rte_fdir_conf
>>>>>> initialization
>>>>>>
>>>>>> Rte_fdir_conf of rte_eth_conf should be initialized before port
>>>>>> initialization.
>>>>>>
>>>>>> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of
>>> flow
>>>>>> API")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
>>>>>> ---
>>>>>>  examples/flow_filtering/main.c | 6 ++++++
>>>>>>  1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/examples/flow_filtering/main.c
>>>>>> b/examples/flow_filtering/main.c index f595034..aa03e23 100644
>>>>>> --- a/examples/flow_filtering/main.c
>>>>>> +++ b/examples/flow_filtering/main.c
>>>>>> @@ -132,6 +132,12 @@
>>>>>>  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
>>>>>>  				DEV_TX_OFFLOAD_TCP_TSO,
>>>>>>  		},
>>>>>> +		.fdir_conf = {
>>>>>> +			.mode = RTE_FDIR_MODE_PERFECT,
>>>>>> +			.pballoc = RTE_FDIR_PBALLOC_64K,
>>>>>> +			.status = RTE_FDIR_REPORT_STATUS,
>>>>>> +			.drop_queue = 127,
>>>>>> +		},
>>>>>>  	};
>>>>>>  	struct rte_eth_txconf txq_conf;
>>>>>>  	struct rte_eth_rxconf rxq_conf;
>>>>>> --
>>>>>> 1.8.3.1
> 
> 
> Best,
> Ori
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-17  9:57           ` Ferruh Yigit
@ 2018-07-17 13:04             ` Ori Kam
  2018-07-17 15:49               ` Ferruh Yigit
  0 siblings, 1 reply; 18+ messages in thread
From: Ori Kam @ 2018-07-17 13:04 UTC (permalink / raw)
  To: Ferruh Yigit, Xu, Rosen, dev; +Cc: stable, Gilmore, Walter E, Qi Zhang

Hi,

PSB

Thanks,
Ori

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Tuesday, July 17, 2018 12:57 PM
> To: Ori Kam <orika@mellanox.com>; Xu, Rosen <rosen.xu@intel.com>;
> dev@dpdk.org
> Cc: stable@dpdk.org; Gilmore, Walter E <walter.e.gilmore@intel.com>; Qi
> Zhang <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> On 7/17/2018 6:15 AM, Ori Kam wrote:
> > Sorry for the late response,
> >
> >> -----Original Message-----
> >> From: Xu, Rosen [mailto:rosen.xu@intel.com]
> >> Sent: Thursday, July 12, 2018 9:23 AM
> >> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> >> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Gilmore,
> Walter
> >> E <walter.e.gilmore@intel.com>
> >> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> rte_fdir_conf
> >> initialization
> >>
> >> Hi Ori,
> >>
> >> Pls see my reply.
> >>
> >> Hi Walter and Ferruh,
> >>
> >> I need your voice :)
> >>
> >>> -----Original Message-----
> >>> From: Ori Kam [mailto:orika@mellanox.com]
> >>> Sent: Thursday, July 12, 2018 13:58
> >>> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> >>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> >>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> >> rte_fdir_conf
> >>> initialization
> >>>
> >>> Hi,
> >>>
> >>> PSB
> >>>
> >>>> -----Original Message-----
> >>>> From: Xu, Rosen [mailto:rosen.xu@intel.com]
> >>>> Sent: Thursday, July 12, 2018 8:27 AM
> >>>> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
> >>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
> >>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> >>>> rte_fdir_conf initialization
> >>>>
> >>>> Hi Ori,
> >>>>
> >>>> examples/flow_filtering sample app fails on i40e [1] because i40e
> >>>> requires explicit FDIR configuration.
> >>>>
> >>>> But rte_flow in and hardware independent ways of describing
> >>>> flow-action, it shouldn't require specific config options for specific
> >>> hardware.
> >>>>
> >>>
> >>> I don't understand why using rte flow require the use of fdir.
> >>> it doesn't make sense to me, that  new API will need old one.
> >>
> >> It's a good question, I also have this question about Mellanox NIC Driver
> >> mlx5_flow.c.
> >> In this file many flow functions call fdir. :)
> >
> > The only functions that are calling fdir are fdir function,
> > and you can see that inside of the create function we convert the fdir
> > Into rte flow.
> >
> >>
> >>>> Is there any chance driver select the FDIR config automatically based
> >>>> on rte_flow rule, unless explicitly a FDIR config set by user?
> >>>
> >>> I don't know how the i40e driver is implemented but I know that
> Mellanox
> >>> convert the other way around, if fdir is given it is converted to rte_flow.
> >>
> >> Firstly, rte_fdir_conf is part of rte_eth_conf definition.
> >> 	struct rte_eth_conf {
> >> 		......
> >> 		struct rte_fdir_conf fdir_conf; /**< FDIR configuration. */
> >> 		......
> >> 	};
> >> Secondly, default value of rte_eth_conf.fdir_conf.mode is
> >> RTE_FDIR_MODE_NONE, which means Disable FDIR support.
> >> Thirdly, flow_filtering should align with test-pmd, in test-pmd all fdir_conf
> is
> >> initialized.
> >>
> >
> > This sounds to me correct we don't want to enable fdir.
> > Why should the example app for rte flow use fdir? And align to
> > testpmd which support everything in in all modes?
> 
> In i40e fdir is used to implement filters, that is why rte_flow rules
> requires/depends some fdir configurations.
> 
> In long term I agree it is better if driver doesn't require any fdir
> configuration for rte_flow programing, although not sure if this is completely
> possible, cc'ed Qi for more comment.
> 
> For short term I am for getting this patch so that sample app can run on i40e
> too, and fdir configuration shouldn't effect others. Perhaps it can be good to
> add a comment to say why that config option is added and it is a temporary
> workaround.
> 

Assuming that the setting for the fdir are fixed for all possible rte_flow rules
I can agree for this workaround but we must add some comment in the code
and also add this comment in the example documentation.

It will be a problem if other PMD will require different default setting.
In this case we must find a better solution.


> >
> >
> >>>
> >>>>
> >>>> [1]
> >>>> Flow can't be created 1 message: Check the mode in fdir_conf.
> >>>> EAL: Error - exiting with code: 1
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Ori Kam [mailto:orika@mellanox.com]
> >>>>> Sent: Thursday, July 12, 2018 13:17
> >>>>> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> >>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
> >>>>> <orika@mellanox.com>
> >>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
> >>>> rte_fdir_conf
> >>>>> initialization
> >>>>>
> >>>>> Hi Rosen,
> >>>>>
> >>>>> Why do the fdir_conf must be initialized?
> >>>>>
> >>>>> What is the issue you are seeing?
> >>>>>
> >>>>> Best,
> >>>>> Ori
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
> >>>>>> Sent: Thursday, July 12, 2018 5:10 AM
> >>>>>> To: dev@dpdk.org
> >>>>>> Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> >>>>>> <orika@mellanox.com>; stable@dpdk.org
> >>>>>> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add
> >> rte_fdir_conf
> >>>>>> initialization
> >>>>>>
> >>>>>> Rte_fdir_conf of rte_eth_conf should be initialized before port
> >>>>>> initialization.
> >>>>>>
> >>>>>> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of
> >>> flow
> >>>>>> API")
> >>>>>> Cc: stable@dpdk.org
> >>>>>>
> >>>>>> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> >>>>>> ---
> >>>>>>  examples/flow_filtering/main.c | 6 ++++++
> >>>>>>  1 file changed, 6 insertions(+)
> >>>>>>
> >>>>>> diff --git a/examples/flow_filtering/main.c
> >>>>>> b/examples/flow_filtering/main.c index f595034..aa03e23 100644
> >>>>>> --- a/examples/flow_filtering/main.c
> >>>>>> +++ b/examples/flow_filtering/main.c
> >>>>>> @@ -132,6 +132,12 @@
> >>>>>>  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
> >>>>>>  				DEV_TX_OFFLOAD_TCP_TSO,
> >>>>>>  		},
> >>>>>> +		.fdir_conf = {
> >>>>>> +			.mode = RTE_FDIR_MODE_PERFECT,
> >>>>>> +			.pballoc = RTE_FDIR_PBALLOC_64K,
> >>>>>> +			.status = RTE_FDIR_REPORT_STATUS,
> >>>>>> +			.drop_queue = 127,
> >>>>>> +		},
> >>>>>>  	};
> >>>>>>  	struct rte_eth_txconf txq_conf;
> >>>>>>  	struct rte_eth_rxconf rxq_conf;
> >>>>>> --
> >>>>>> 1.8.3.1
> >
> >
> > Best,
> > Ori
> >


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-17 13:04             ` Ori Kam
@ 2018-07-17 15:49               ` Ferruh Yigit
  0 siblings, 0 replies; 18+ messages in thread
From: Ferruh Yigit @ 2018-07-17 15:49 UTC (permalink / raw)
  To: Ori Kam, Xu, Rosen, dev; +Cc: stable, Gilmore, Walter E, Qi Zhang

On 7/17/2018 2:04 PM, Ori Kam wrote:
> Hi,
> 
> PSB
> 
> Thanks,
> Ori
> 
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Tuesday, July 17, 2018 12:57 PM
>> To: Ori Kam <orika@mellanox.com>; Xu, Rosen <rosen.xu@intel.com>;
>> dev@dpdk.org
>> Cc: stable@dpdk.org; Gilmore, Walter E <walter.e.gilmore@intel.com>; Qi
>> Zhang <qi.z.zhang@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf
>> initialization
>>
>> On 7/17/2018 6:15 AM, Ori Kam wrote:
>>> Sorry for the late response,
>>>
>>>> -----Original Message-----
>>>> From: Xu, Rosen [mailto:rosen.xu@intel.com]
>>>> Sent: Thursday, July 12, 2018 9:23 AM
>>>> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Gilmore,
>> Walter
>>>> E <walter.e.gilmore@intel.com>
>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>> rte_fdir_conf
>>>> initialization
>>>>
>>>> Hi Ori,
>>>>
>>>> Pls see my reply.
>>>>
>>>> Hi Walter and Ferruh,
>>>>
>>>> I need your voice :)
>>>>
>>>>> -----Original Message-----
>>>>> From: Ori Kam [mailto:orika@mellanox.com]
>>>>> Sent: Thursday, July 12, 2018 13:58
>>>>> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
>>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
>>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>>>> rte_fdir_conf
>>>>> initialization
>>>>>
>>>>> Hi,
>>>>>
>>>>> PSB
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Xu, Rosen [mailto:rosen.xu@intel.com]
>>>>>> Sent: Thursday, July 12, 2018 8:27 AM
>>>>>> To: Ori Kam <orika@mellanox.com>; dev@dpdk.org
>>>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org
>>>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>>>>>> rte_fdir_conf initialization
>>>>>>
>>>>>> Hi Ori,
>>>>>>
>>>>>> examples/flow_filtering sample app fails on i40e [1] because i40e
>>>>>> requires explicit FDIR configuration.
>>>>>>
>>>>>> But rte_flow in and hardware independent ways of describing
>>>>>> flow-action, it shouldn't require specific config options for specific
>>>>> hardware.
>>>>>>
>>>>>
>>>>> I don't understand why using rte flow require the use of fdir.
>>>>> it doesn't make sense to me, that  new API will need old one.
>>>>
>>>> It's a good question, I also have this question about Mellanox NIC Driver
>>>> mlx5_flow.c.
>>>> In this file many flow functions call fdir. :)
>>>
>>> The only functions that are calling fdir are fdir function,
>>> and you can see that inside of the create function we convert the fdir
>>> Into rte flow.
>>>
>>>>
>>>>>> Is there any chance driver select the FDIR config automatically based
>>>>>> on rte_flow rule, unless explicitly a FDIR config set by user?
>>>>>
>>>>> I don't know how the i40e driver is implemented but I know that
>> Mellanox
>>>>> convert the other way around, if fdir is given it is converted to rte_flow.
>>>>
>>>> Firstly, rte_fdir_conf is part of rte_eth_conf definition.
>>>> 	struct rte_eth_conf {
>>>> 		......
>>>> 		struct rte_fdir_conf fdir_conf; /**< FDIR configuration. */
>>>> 		......
>>>> 	};
>>>> Secondly, default value of rte_eth_conf.fdir_conf.mode is
>>>> RTE_FDIR_MODE_NONE, which means Disable FDIR support.
>>>> Thirdly, flow_filtering should align with test-pmd, in test-pmd all fdir_conf
>> is
>>>> initialized.
>>>>
>>>
>>> This sounds to me correct we don't want to enable fdir.
>>> Why should the example app for rte flow use fdir? And align to
>>> testpmd which support everything in in all modes?
>>
>> In i40e fdir is used to implement filters, that is why rte_flow rules
>> requires/depends some fdir configurations.
>>
>> In long term I agree it is better if driver doesn't require any fdir
>> configuration for rte_flow programing, although not sure if this is completely
>> possible, cc'ed Qi for more comment.
>>
>> For short term I am for getting this patch so that sample app can run on i40e
>> too, and fdir configuration shouldn't effect others. Perhaps it can be good to
>> add a comment to say why that config option is added and it is a temporary
>> workaround.
>>
> 
> Assuming that the setting for the fdir are fixed for all possible rte_flow rules
> I can agree for this workaround but we must add some comment in the code
> and also add this comment in the example documentation.
> 
> It will be a problem if other PMD will require different default setting.
> In this case we must find a better solution.

+1 for commenting code, and as far as I know fdir config only used by Intel PMDs
which we need to confirm all Intel PMDs are OK with change.

> 
> 
>>>
>>>
>>>>>
>>>>>>
>>>>>> [1]
>>>>>> Flow can't be created 1 message: Check the mode in fdir_conf.
>>>>>> EAL: Error - exiting with code: 1
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Ori Kam [mailto:orika@mellanox.com]
>>>>>>> Sent: Thursday, July 12, 2018 13:17
>>>>>>> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
>>>>>>> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Ori Kam
>>>>>>> <orika@mellanox.com>
>>>>>>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add
>>>>>> rte_fdir_conf
>>>>>>> initialization
>>>>>>>
>>>>>>> Hi Rosen,
>>>>>>>
>>>>>>> Why do the fdir_conf must be initialized?
>>>>>>>
>>>>>>> What is the issue you are seeing?
>>>>>>>
>>>>>>> Best,
>>>>>>> Ori
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Rosen Xu
>>>>>>>> Sent: Thursday, July 12, 2018 5:10 AM
>>>>>>>> To: dev@dpdk.org
>>>>>>>> Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
>>>>>>>> <orika@mellanox.com>; stable@dpdk.org
>>>>>>>> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: add
>>>> rte_fdir_conf
>>>>>>>> initialization
>>>>>>>>
>>>>>>>> Rte_fdir_conf of rte_eth_conf should be initialized before port
>>>>>>>> initialization.
>>>>>>>>
>>>>>>>> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of
>>>>> flow
>>>>>>>> API")
>>>>>>>> Cc: stable@dpdk.org
>>>>>>>>
>>>>>>>> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
>>>>>>>> ---
>>>>>>>>  examples/flow_filtering/main.c | 6 ++++++
>>>>>>>>  1 file changed, 6 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/examples/flow_filtering/main.c
>>>>>>>> b/examples/flow_filtering/main.c index f595034..aa03e23 100644
>>>>>>>> --- a/examples/flow_filtering/main.c
>>>>>>>> +++ b/examples/flow_filtering/main.c
>>>>>>>> @@ -132,6 +132,12 @@
>>>>>>>>  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
>>>>>>>>  				DEV_TX_OFFLOAD_TCP_TSO,
>>>>>>>>  		},
>>>>>>>> +		.fdir_conf = {
>>>>>>>> +			.mode = RTE_FDIR_MODE_PERFECT,
>>>>>>>> +			.pballoc = RTE_FDIR_PBALLOC_64K,
>>>>>>>> +			.status = RTE_FDIR_REPORT_STATUS,
>>>>>>>> +			.drop_queue = 127,
>>>>>>>> +		},
>>>>>>>>  	};
>>>>>>>>  	struct rte_eth_txconf txq_conf;
>>>>>>>>  	struct rte_eth_rxconf rxq_conf;
>>>>>>>> --
>>>>>>>> 1.8.3.1
>>>
>>>
>>> Best,
>>> Ori
>>>
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v2] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  2:09 [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization Rosen Xu
  2018-07-12  5:17 ` Ori Kam
@ 2018-07-21  7:50 ` Rosen Xu
  2018-07-22  6:33   ` Ori Kam
  2018-07-22 10:39 ` [dpdk-dev] [PATCH v3] " Rosen Xu
  2018-07-31 12:52 ` [dpdk-dev] [PATCH v4] " Rosen Xu
  3 siblings, 1 reply; 18+ messages in thread
From: Rosen Xu @ 2018-07-21  7:50 UTC (permalink / raw)
  To: dev; +Cc: rosen.xu, ferruh.yigit, orika, walter.e.gilmore, qi.z.zhang, stable

Rte_fdir_conf of rte_eth_conf should be initialized before
port initialization. It is a workaround solution when work
with Intel I40e.

Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
Cc: stable@dpdk.org

Signed-off-by: Rosen Xu <rosen.xu@intel.com>

v2 updates:
===========
 - Take more test on I40e
 - Add comments
---
 examples/flow_filtering/main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
index f595034..73d646c 100644
--- a/examples/flow_filtering/main.c
+++ b/examples/flow_filtering/main.c
@@ -132,6 +132,17 @@
 				DEV_TX_OFFLOAD_SCTP_CKSUM  |
 				DEV_TX_OFFLOAD_TCP_TSO,
 		},
+		/*
+		 * Initialize fdir_conf of ete_eth_conf
+		 * it is a workaround solution when work with Intel I40e
+		 * and it is not the normal way
+		 */
+		.fdir_conf = {
+			.mode = RTE_FDIR_MODE_PERFECT,
+			.pballoc = RTE_FDIR_PBALLOC_64K,
+			.status = RTE_FDIR_REPORT_STATUS,
+			.drop_queue = 127,
+		},
 	};
 	struct rte_eth_txconf txq_conf;
 	struct rte_eth_rxconf rxq_conf;
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-21  7:50 ` [dpdk-dev] [PATCH v2] " Rosen Xu
@ 2018-07-22  6:33   ` Ori Kam
  2018-07-22 10:37     ` Xu, Rosen
  0 siblings, 1 reply; 18+ messages in thread
From: Ori Kam @ 2018-07-22  6:33 UTC (permalink / raw)
  To: Rosen Xu, dev; +Cc: ferruh.yigit, walter.e.gilmore, qi.z.zhang, stable



> -----Original Message-----
> From: Rosen Xu [mailto:rosen.xu@intel.com]
> Sent: Saturday, July 21, 2018 10:50 AM
> To: dev@dpdk.org
> Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> <orika@mellanox.com>; walter.e.gilmore@intel.com; qi.z.zhang@intel.com;
> stable@dpdk.org
> Subject: [PATCH v2] examples/flow_filtering: add rte_fdir_conf initialization
> 
> Rte_fdir_conf of rte_eth_conf should be initialized before
> port initialization. It is a workaround solution when work
> with Intel I40e.
> 
> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow
> API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> 
> v2 updates:
> ===========
>  - Take more test on I40e
>  - Add comments
> ---
>  examples/flow_filtering/main.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
> index f595034..73d646c 100644
> --- a/examples/flow_filtering/main.c
> +++ b/examples/flow_filtering/main.c
> @@ -132,6 +132,17 @@
>  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
>  				DEV_TX_OFFLOAD_TCP_TSO,
>  		},
> +		/*
> +		 * Initialize fdir_conf of ete_eth_conf
> +		 * it is a workaround solution when work with Intel I40e
> +		 * and it is not the normal way
> +		 */
> +		.fdir_conf = {
> +			.mode = RTE_FDIR_MODE_PERFECT,
> +			.pballoc = RTE_FDIR_PBALLOC_64K,
> +			.status = RTE_FDIR_REPORT_STATUS,
> +			.drop_queue = 127,
> +		},
>  	};
>  	struct rte_eth_txconf txq_conf;
>  	struct rte_eth_rxconf rxq_conf;
> --
> 1.8.3.1

Just small comment I think work should be replaced with working.
Both in the commit log and code comment.

Acked-by: Ori Kam <orika@mellanox.com>

Thanks,
Ori

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v2] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-22  6:33   ` Ori Kam
@ 2018-07-22 10:37     ` Xu, Rosen
  0 siblings, 0 replies; 18+ messages in thread
From: Xu, Rosen @ 2018-07-22 10:37 UTC (permalink / raw)
  To: Ori Kam, dev; +Cc: Yigit, Ferruh, Gilmore, Walter E, Zhang, Qi Z, stable

Hi,

I have fixed it, thanks.

> -----Original Message-----
> From: Ori Kam [mailto:orika@mellanox.com]
> Sent: Sunday, July 22, 2018 14:34
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Gilmore, Walter E
> <walter.e.gilmore@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> stable@dpdk.org
> Subject: RE: [PATCH v2] examples/flow_filtering: add rte_fdir_conf
> initialization
> 
> 
> 
> > -----Original Message-----
> > From: Rosen Xu [mailto:rosen.xu@intel.com]
> > Sent: Saturday, July 21, 2018 10:50 AM
> > To: dev@dpdk.org
> > Cc: rosen.xu@intel.com; ferruh.yigit@intel.com; Ori Kam
> > <orika@mellanox.com>; walter.e.gilmore@intel.com;
> > qi.z.zhang@intel.com; stable@dpdk.org
> > Subject: [PATCH v2] examples/flow_filtering: add rte_fdir_conf
> > initialization
> >
> > Rte_fdir_conf of rte_eth_conf should be initialized before port
> > initialization. It is a workaround solution when work with Intel I40e.
> >
> > Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow
> > API")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> >
> > v2 updates:
> > ===========
> >  - Take more test on I40e
> >  - Add comments
> > ---
> >  examples/flow_filtering/main.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/examples/flow_filtering/main.c
> > b/examples/flow_filtering/main.c index f595034..73d646c 100644
> > --- a/examples/flow_filtering/main.c
> > +++ b/examples/flow_filtering/main.c
> > @@ -132,6 +132,17 @@
> >  				DEV_TX_OFFLOAD_SCTP_CKSUM  |
> >  				DEV_TX_OFFLOAD_TCP_TSO,
> >  		},
> > +		/*
> > +		 * Initialize fdir_conf of ete_eth_conf
> > +		 * it is a workaround solution when work with Intel I40e
> > +		 * and it is not the normal way
> > +		 */
> > +		.fdir_conf = {
> > +			.mode = RTE_FDIR_MODE_PERFECT,
> > +			.pballoc = RTE_FDIR_PBALLOC_64K,
> > +			.status = RTE_FDIR_REPORT_STATUS,
> > +			.drop_queue = 127,
> > +		},
> >  	};
> >  	struct rte_eth_txconf txq_conf;
> >  	struct rte_eth_rxconf rxq_conf;
> > --
> > 1.8.3.1
> 
> Just small comment I think work should be replaced with working.
> Both in the commit log and code comment.

It's fixed.

> Acked-by: Ori Kam <orika@mellanox.com>
> 
> Thanks,
> Ori

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v3] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  2:09 [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization Rosen Xu
  2018-07-12  5:17 ` Ori Kam
  2018-07-21  7:50 ` [dpdk-dev] [PATCH v2] " Rosen Xu
@ 2018-07-22 10:39 ` Rosen Xu
  2018-07-23 20:32   ` Ferruh Yigit
  2018-07-26 17:42   ` Thomas Monjalon
  2018-07-31 12:52 ` [dpdk-dev] [PATCH v4] " Rosen Xu
  3 siblings, 2 replies; 18+ messages in thread
From: Rosen Xu @ 2018-07-22 10:39 UTC (permalink / raw)
  To: dev; +Cc: rosen.xu, ferruh.yigit, orika, walter.e.gilmore, qi.z.zhang, stable

Rte_fdir_conf of rte_eth_conf should be initialized before
port initialization. It is a workaround solution when working
with Intel I40e.

Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
Cc: stable@dpdk.org

Signed-off-by: Rosen Xu <rosen.xu@intel.com>
Acked-by: Ori Kam <orika@mellanox.com>

v3 updates:
===========
 - Fix small comment of commit log and code comment

v2 updates:
===========
 - Take more test on I40e
 - Add comments
---
 examples/flow_filtering/main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
index f595034..e3f99f5 100644
--- a/examples/flow_filtering/main.c
+++ b/examples/flow_filtering/main.c
@@ -132,6 +132,17 @@
 				DEV_TX_OFFLOAD_SCTP_CKSUM  |
 				DEV_TX_OFFLOAD_TCP_TSO,
 		},
+		/*
+		 * Initialize fdir_conf of ete_eth_conf
+		 * it is a workaround solution when working with Intel I40e
+		 * and it is not the normal way
+		 */
+		.fdir_conf = {
+			.mode = RTE_FDIR_MODE_PERFECT,
+			.pballoc = RTE_FDIR_PBALLOC_64K,
+			.status = RTE_FDIR_REPORT_STATUS,
+			.drop_queue = 127,
+		},
 	};
 	struct rte_eth_txconf txq_conf;
 	struct rte_eth_rxconf rxq_conf;
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v3] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-22 10:39 ` [dpdk-dev] [PATCH v3] " Rosen Xu
@ 2018-07-23 20:32   ` Ferruh Yigit
  2018-07-26 17:42   ` Thomas Monjalon
  1 sibling, 0 replies; 18+ messages in thread
From: Ferruh Yigit @ 2018-07-23 20:32 UTC (permalink / raw)
  To: Rosen Xu, dev; +Cc: orika, walter.e.gilmore, qi.z.zhang, stable

On 7/22/2018 11:39 AM, Rosen Xu wrote:
> Rte_fdir_conf of rte_eth_conf should be initialized before
> port initialization. It is a workaround solution when working
> with Intel I40e.
> 
> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> Acked-by: Ori Kam <orika@mellanox.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v3] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-22 10:39 ` [dpdk-dev] [PATCH v3] " Rosen Xu
  2018-07-23 20:32   ` Ferruh Yigit
@ 2018-07-26 17:42   ` Thomas Monjalon
  2018-07-31 12:55     ` Xu, Rosen
  1 sibling, 1 reply; 18+ messages in thread
From: Thomas Monjalon @ 2018-07-26 17:42 UTC (permalink / raw)
  To: Rosen Xu; +Cc: dev, ferruh.yigit, orika, walter.e.gilmore, qi.z.zhang, stable

22/07/2018 12:39, Rosen Xu:
> Rte_fdir_conf of rte_eth_conf should be initialized before
> port initialization. It is a workaround solution when working
> with Intel I40e.
[...]
> +		/*
> +		 * Initialize fdir_conf of ete_eth_conf

Typo and lack of punctuation.

> +		 * it is a workaround solution when working with Intel I40e
> +		 * and it is not the normal way

It is not said why it is needed,
and what are we waiting to remove the workaround.

> +		 */
> +		.fdir_conf = {
> +			.mode = RTE_FDIR_MODE_PERFECT,
> +			.pballoc = RTE_FDIR_PBALLOC_64K,
> +			.status = RTE_FDIR_REPORT_STATUS,
> +			.drop_queue = 127,
> +		},

Please work on a v4 with better explanations.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH v4] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-12  2:09 [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization Rosen Xu
                   ` (2 preceding siblings ...)
  2018-07-22 10:39 ` [dpdk-dev] [PATCH v3] " Rosen Xu
@ 2018-07-31 12:52 ` Rosen Xu
  2018-08-05 20:13   ` Thomas Monjalon
  3 siblings, 1 reply; 18+ messages in thread
From: Rosen Xu @ 2018-07-31 12:52 UTC (permalink / raw)
  To: dev
  Cc: thomas, rosen.xu, ferruh.yigit, orika, walter.e.gilmore,
	qi.z.zhang, stable

Rte_fdir_conf of rte_eth_conf should be initialized before
port initialization. It is a workaround solution when working
with Intel I40e.

Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
Cc: stable@dpdk.org

Signed-off-by: Rosen Xu <rosen.xu@intel.com>
Acked-by: Ori Kam <orika@mellanox.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

v4 updates:
===========
 - Fix typo and lack of punctuation.
 - Add why it is needed, and what are we waiting to remove the workaround.

v3 updates:
===========
 - Fix small comment of commit log and code comment

v2 updates:
===========
 - Take more test on I40e
 - Add comments
---
 examples/flow_filtering/main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/examples/flow_filtering/main.c b/examples/flow_filtering/main.c
index f595034..ce91e8a 100644
--- a/examples/flow_filtering/main.c
+++ b/examples/flow_filtering/main.c
@@ -132,6 +132,22 @@
 				DEV_TX_OFFLOAD_SCTP_CKSUM  |
 				DEV_TX_OFFLOAD_TCP_TSO,
 		},
+		/*
+		 * Initialize fdir_conf of rte_eth_conf.
+		 * Fdir is used in flow filtering for I40e,
+		 * so rte_flow rules involve some fdir
+		 * configurations. In long term it's better
+		 * that drivers don't require any fdir
+		 * configuration for rte_flow, but we need to
+		 * get this workaround so that sample app can
+		 * run on I40e.
+		 */
+		.fdir_conf = {
+			.mode = RTE_FDIR_MODE_PERFECT,
+			.pballoc = RTE_FDIR_PBALLOC_64K,
+			.status = RTE_FDIR_REPORT_STATUS,
+			.drop_queue = 127,
+		},
 	};
 	struct rte_eth_txconf txq_conf;
 	struct rte_eth_rxconf rxq_conf;
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v3] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-26 17:42   ` Thomas Monjalon
@ 2018-07-31 12:55     ` Xu, Rosen
  0 siblings, 0 replies; 18+ messages in thread
From: Xu, Rosen @ 2018-07-31 12:55 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, Yigit, Ferruh, orika, Gilmore, Walter E, Zhang, Qi Z, stable

Hi Thomas,

This is my reply.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, July 27, 2018 1:42
> To: Xu, Rosen <rosen.xu@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>;
> orika@mellanox.com; Gilmore, Walter E <walter.e.gilmore@intel.com>;
> Zhang, Qi Z <qi.z.zhang@intel.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] examples/flow_filtering: add
> rte_fdir_conf initialization
> 
> 22/07/2018 12:39, Rosen Xu:
> > Rte_fdir_conf of rte_eth_conf should be initialized before port
> > initialization. It is a workaround solution when working with Intel
> > I40e.
> [...]
> > +		/*
> > +		 * Initialize fdir_conf of ete_eth_conf
> 
> Typo and lack of punctuation.

Fixed.

> > +		 * it is a workaround solution when working with Intel I40e
> > +		 * and it is not the normal way
> 
> It is not said why it is needed,
> and what are we waiting to remove the workaround.

Added.

> > +		 */
> > +		.fdir_conf = {
> > +			.mode = RTE_FDIR_MODE_PERFECT,
> > +			.pballoc = RTE_FDIR_PBALLOC_64K,
> > +			.status = RTE_FDIR_REPORT_STATUS,
> > +			.drop_queue = 127,
> > +		},
> 
> Please work on a v4 with better explanations.
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH v4] examples/flow_filtering: add rte_fdir_conf initialization
  2018-07-31 12:52 ` [dpdk-dev] [PATCH v4] " Rosen Xu
@ 2018-08-05 20:13   ` Thomas Monjalon
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Monjalon @ 2018-08-05 20:13 UTC (permalink / raw)
  To: Rosen Xu; +Cc: dev, ferruh.yigit, orika, walter.e.gilmore, qi.z.zhang, stable

31/07/2018 14:52, Rosen Xu:
> Rte_fdir_conf of rte_eth_conf should be initialized before
> port initialization. It is a workaround solution when working
> with Intel I40e.
> 
> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> Acked-by: Ori Kam <orika@mellanox.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> v4 updates:
> ===========
>  - Fix typo and lack of punctuation.
>  - Add why it is needed, and what are we waiting to remove the workaround.

Applied, thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-08-05 20:14 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-12  2:09 [dpdk-dev] [PATCH] examples/flow_filtering: add rte_fdir_conf initialization Rosen Xu
2018-07-12  5:17 ` Ori Kam
2018-07-12  5:26   ` Xu, Rosen
2018-07-12  5:58     ` Ori Kam
2018-07-12  6:22       ` Xu, Rosen
2018-07-17  5:15         ` Ori Kam
2018-07-17  9:57           ` Ferruh Yigit
2018-07-17 13:04             ` Ori Kam
2018-07-17 15:49               ` Ferruh Yigit
2018-07-21  7:50 ` [dpdk-dev] [PATCH v2] " Rosen Xu
2018-07-22  6:33   ` Ori Kam
2018-07-22 10:37     ` Xu, Rosen
2018-07-22 10:39 ` [dpdk-dev] [PATCH v3] " Rosen Xu
2018-07-23 20:32   ` Ferruh Yigit
2018-07-26 17:42   ` Thomas Monjalon
2018-07-31 12:55     ` Xu, Rosen
2018-07-31 12:52 ` [dpdk-dev] [PATCH v4] " Rosen Xu
2018-08-05 20:13   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).