Hi, PSB
Thanks, Ori > -----Original Message----- > From: Ferruh Yigit [mailto:ferruh.yi...@intel.com] > Sent: Tuesday, July 17, 2018 12:57 PM > To: Ori Kam <or...@mellanox.com>; Xu, Rosen <rosen...@intel.com>; > dev@dpdk.org > Cc: sta...@dpdk.org; Gilmore, Walter E <walter.e.gilm...@intel.com>; Qi > Zhang <qi.z.zh...@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...@intel.com] > >> Sent: Thursday, July 12, 2018 9:23 AM > >> To: Ori Kam <or...@mellanox.com>; dev@dpdk.org > >> Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; sta...@dpdk.org; Gilmore, > Walter > >> E <walter.e.gilm...@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:or...@mellanox.com] > >>> Sent: Thursday, July 12, 2018 13:58 > >>> To: Xu, Rosen <rosen...@intel.com>; dev@dpdk.org > >>> Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; sta...@dpdk.org > >>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: add > >> rte_fdir_conf > >>> initialization > >>> > >>> Hi, > >>> > >>> PSB > >>> > >>>> -----Original Message----- > >>>> From: Xu, Rosen [mailto:rosen...@intel.com] > >>>> Sent: Thursday, July 12, 2018 8:27 AM > >>>> To: Ori Kam <or...@mellanox.com>; dev@dpdk.org > >>>> Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; sta...@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:or...@mellanox.com] > >>>>> Sent: Thursday, July 12, 2018 13:17 > >>>>> To: Xu, Rosen <rosen...@intel.com>; dev@dpdk.org > >>>>> Cc: Yigit, Ferruh <ferruh.yi...@intel.com>; sta...@dpdk.org; Ori Kam > >>>>> <or...@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-boun...@dpdk.org] On Behalf Of Rosen Xu > >>>>>> Sent: Thursday, July 12, 2018 5:10 AM > >>>>>> To: dev@dpdk.org > >>>>>> Cc: rosen...@intel.com; ferruh.yi...@intel.com; Ori Kam > >>>>>> <or...@mellanox.com>; sta...@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: sta...@dpdk.org > >>>>>> > >>>>>> Signed-off-by: Rosen Xu <rosen...@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 > >
