Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
Hi, Xiaolong
> -Original Message-
> From: Ye, Xiaolong
> Sent: Tuesday, April 14, 2020 3:54 PM
> To: Wang, ShougangX
> Cc: dev@dpdk.org; Lu, Wenzhuo ; Yang, Qiming
> ; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: fix link status after port reset
>
> Hi, Shougang
>
> On 04/13, Shougang Wang wrote:
> >It's a normal behavior to change the link status to up after resetting
> >the port. So it is unnecessary to set link down before starting port,
> >and changing the link state(link up/down) frequently will cause link
> >speed unstable.
> >
> >Fixes: c3f2fbff78cf ("net/ixgbe: fix link status")
> >Cc: sta...@dpdk.org
> >
> >Signed-off-by: Shougang Wang
> >---
> > drivers/net/ixgbe/ixgbe_ethdev.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> >diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >b/drivers/net/ixgbe/ixgbe_ethdev.c
> >index 23b3f5b0c..206358b85 100644
> >--- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >@@ -1196,7 +1196,6 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> void *init_params __rte_unused)
> > diag = ixgbe_bypass_init_hw(hw);
> > #else
> > diag = ixgbe_init_hw(hw);
> >-hw->mac.autotry_restart = false;
>
> Why do we need this change? Seems it is not mentioned in the commit log.
In c3f2fbff78cf, port was set to down by following 2 steps.
1. Setting autotry_restart flag to false to prevent NIC from linking up by
itself.
2. Calling ixgbe_dev_set_link_down().
Force to set link status to down is unnecessary operation before starting port,
so I revert these 2 changes in this patch.
Thanks
Shougang
Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS flow
Hi, Beilei
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, May 8, 2020 9:04 AM
> To: Wang, ShougangX ; dev@dpdk.org
> Subject: RE: [PATCH] net/i40e: fix hash enable issue in RSS flow
>
>
>
> > -Original Message-
> > From: Wang, ShougangX
> > Sent: Thursday, May 7, 2020 5:44 PM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Wang, ShougangX
> >
> > Subject: [PATCH] net/i40e: fix hash enable issue in RSS flow
> >
> > This patch fixes the issue that failed to create an RSS rule with type
> > L2- payload.
> >
> > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> > flow")
> >
> > Signed-off-by: Shougang Wang
> > ---
> > drivers/net/i40e/i40e_flow.c | 14 ++
> > 1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c index 7e64ae53a..f5f2f0d5d 100644L2-
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -4511,6 +4511,7 @@ i40e_flow_parse_rss_pattern(__rte_unused
> struct
> > rte_eth_dev *dev,
>
> <...>
>
> > @@ -4544,8 +4545,7 @@ i40e_flow_parse_rss_pattern(__rte_unused
> struct
> > rte_eth_dev *dev, if
> > (i40e_match_pattern(i40e_rss_pctype_patterns[i].item_array,
> > items)) {
> > p_info->types = i40e_rss_pctype_patterns[i].type; -rte_free(items);
> > -return 0;
> > +break;
> > }
> > }
> >
> > @@ -4580,11 +4580,9 @@ i40e_flow_parse_rss_pattern(__rte_unused
> > struct rte_eth_dev *dev,
> > }
> > break;
> > default:
> > -rte_flow_error_set(error, EINVAL,
> > -RTE_FLOW_ERROR_TYPE_ITEM,
> > -item,
> > -"Not support range");
> > -return -rte_errno;
> > +p_info->action_flag = 0;
> > +memset(info, 0, sizeof(struct i40e_queue_regions)); return 0;
> > }
> > }
> >
> > @@ -4640,7 +4638,7 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> > *dev, return -rte_errno; }
> >
> > -if (p_info.action_flag) {
> > +if (p_info.action_flag && rss->queue_num) {
> > for (n = 0; n < 64; n++) {
> > if (rss->types & (hf_bit << n)) {
> > conf_info->region[0].hw_flowtype[0] = n;
> > --
> > 2.17.1
>
> Are the above changes relating to L2-payload?
>
Yes, in order to resolve the conflict between hash enable and queue region
which caused by ether pattern, there are also a little bit changes for queue
region.
Thanks.
Shougang
Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS flow
Hi, Jeff
Thanks for your review.
This patch can be treated as a work around, the codes about p_info->action_flag
will be reconstructed in the future.
Thanks.
Shougang
> -Original Message-
> From: Guo, Jia
> Sent: Wednesday, May 13, 2020 4:41 PM
> To: Xu, HailinX ; Wang, ShougangX
> ; Xing, Beilei ;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS flow
>
> hi, hailin
>
> Seems that this patch is fixing a issue about l2-playload but it is related
> with
> the priors wrong usage of the pattern.
>
> The rework need further to do and sincerely it it not very easy to review
> it. But if consider it for the work around solution, commend as below.
>
>
> On 5/8/2020 11:32 AM, Xu, HailinX wrote:
> > Tested-by: Xu, Hailin
> >
> > Regards,
> > Xu, Hailin
> >
> > -----Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wang, ShougangX
> > Sent: Friday, May 8, 2020 9:52 AM
> > To: Xing, Beilei ; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix hash enable issue in RSS
> > flow
> >
> > Hi, Beilei
> >
> >> -Original Message-
> >> From: Xing, Beilei
> >> Sent: Friday, May 8, 2020 9:04 AM
> >> To: Wang, ShougangX ; dev@dpdk.org
> >> Subject: RE: [PATCH] net/i40e: fix hash enable issue in RSS flow
> >>
> >>
> >>
> >>> -Original Message-
> >>> From: Wang, ShougangX
> >>> Sent: Thursday, May 7, 2020 5:44 PM
> >>> To: dev@dpdk.org
> >>> Cc: Xing, Beilei ; Wang, ShougangX
> >>>
> >>> Subject: [PATCH] net/i40e: fix hash enable issue in RSS flow
> >>>
> >>> This patch fixes the issue that failed to create an RSS rule with
> >>> type
> >>> L2- payload.
> >>>
> >>> Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> >>> flow")
> >>>
> >>> Signed-off-by: Shougang Wang
> >>> ---
> >>> drivers/net/i40e/i40e_flow.c | 14 ++
> >>> 1 file changed, 6 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/drivers/net/i40e/i40e_flow.c
> >>> b/drivers/net/i40e/i40e_flow.c index 7e64ae53a..f5f2f0d5d 100644L2-
> >>> --- a/drivers/net/i40e/i40e_flow.c
> >>> +++ b/drivers/net/i40e/i40e_flow.c
> >>> @@ -4511,6 +4511,7 @@ i40e_flow_parse_rss_pattern(__rte_unused
> >> struct
> >>> rte_eth_dev *dev,
> >> <...>
> >>
> >>> @@ -4544,8 +4545,7 @@ i40e_flow_parse_rss_pattern(__rte_unused
> >> struct
> >>> rte_eth_dev *dev, if
> >>> (i40e_match_pattern(i40e_rss_pctype_patterns[i].item_array,
> >>> items)) {
> >>> p_info->types = i40e_rss_pctype_patterns[i].type;
> >>> -rte_free(items); -return 0;
> >>> +break;
> >>> }
> >>> }
> >>>
> >>> @@ -4580,11 +4580,9 @@ i40e_flow_parse_rss_pattern(__rte_unused
> >>> struct rte_eth_dev *dev,
> >>> }
> >>> break;
> >>> default:
> >>> -rte_flow_error_set(error, EINVAL,
> >>> -RTE_FLOW_ERROR_TYPE_ITEM,
> >>> -item,
> >>> -"Not support range");
> >>> -return -rte_errno;
> >>> +p_info->action_flag = 0;
> >>> +memset(info, 0, sizeof(struct i40e_queue_regions)); return 0;
>
>
> Could you check if all case have set
>
> p_info->action_flag such as RTE_FLOW_ITEM_TYPE_VLAN case.
>
>
> >>> }
> >>> }
> >>>
> >>> @@ -4640,7 +4638,7 @@ i40e_flow_parse_rss_action(struct
> rte_eth_dev
> >>> *dev, return -rte_errno; }
> >>>
> >>> -if (p_info.action_flag) {
> >>> +if (p_info.action_flag && rss->queue_num) {
> >>> for (n = 0; n < 64; n++) {
> >>> if (rss->types & (hf_bit << n)) {
> >>> conf_info->region[0].hw_flowtype[0] = n;
> >>> --
> >>> 2.17.1
> >> Are the above changes relating to L2-payload?
> >>
> > Yes, in order to resolve the conflict between hash enable and queue region
> which caused by ether pattern, there are also a little bit changes for queue
> region.
> >
> > Thanks.
> > Shougang
Re: [dpdk-dev] [PATCH v3] net/i40e: fix queue region issue in RSS flow
Hi, Bernad
> -Original Message-
> From: Iremonger, Bernard
> Sent: Wednesday, May 13, 2020 6:12 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ; Wang,
> ShougangX ; sta...@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] net/i40e: fix queue region issue in RSS
> flow
>
> Hi Shougang,
>
> > -Original Message-
> > From: dev On Behalf Of Shougang Wang
> > Sent: Wednesday, May 13, 2020 4:33 AM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Guo, Jia
> > ; Wang, ShougangX ;
> > sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v3] net/i40e: fix queue region issue in RSS
> > flow
> >
> > This patch fixes the issue that the queue region does not take effect
> > due to incorrectly setting the flow type.
> >
> > Fixes: ecad87d22383 ("net/i40e: move RSS to flow API")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Shougang Wang
> > Reviewed-by: Jeff Guo
> > ---
> > drivers/net/i40e/i40e_flow.c | 35
> -
> > --
> > 1 file changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c index 7e64ae53a..2f937567b 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -4625,6 +4625,34 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> > *dev, uint32_t index = 0; uint64_t hf_bit = 1;
> >
> > +static const struct {
> > +uint64_t rss_type;
> > +enum i40e_filter_pctype pctype;
> > +} pctype_match_table[] = {
> > +{ETH_RSS_FRAG_IPV4,
> > +I40E_FILTER_PCTYPE_FRAG_IPV4},
> > +{ETH_RSS_NONFRAG_IPV4_TCP,
> > +I40E_FILTER_PCTYPE_NONF_IPV4_TCP},
> > +{ETH_RSS_NONFRAG_IPV4_UDP,
> > +I40E_FILTER_PCTYPE_NONF_IPV4_UDP},
> > +{ETH_RSS_NONFRAG_IPV4_SCTP,
> > +I40E_FILTER_PCTYPE_NONF_IPV4_SCTP},
> > +{ETH_RSS_NONFRAG_IPV4_OTHER,
> > +I40E_FILTER_PCTYPE_NONF_IPV4_OTHER},
> > +{ETH_RSS_FRAG_IPV6,
> > +I40E_FILTER_PCTYPE_FRAG_IPV6},
> > +{ETH_RSS_NONFRAG_IPV6_TCP,
> > +I40E_FILTER_PCTYPE_NONF_IPV6_TCP},
> > +{ETH_RSS_NONFRAG_IPV6_UDP,
> > +I40E_FILTER_PCTYPE_NONF_IPV6_UDP},
> > +{ETH_RSS_NONFRAG_IPV6_SCTP,
> > +I40E_FILTER_PCTYPE_NONF_IPV6_SCTP},
> > +{ETH_RSS_NONFRAG_IPV6_OTHER,
> > +I40E_FILTER_PCTYPE_NONF_IPV6_OTHER},
> > +{ETH_RSS_L2_PAYLOAD,
> > +I40E_FILTER_PCTYPE_L2_PAYLOAD},
> > +};
>
> I don't think this is a complete list of RSS offload types.
> See file librte_ethdev/rte_ethdev,h lines 496 to 523.
> See also app/test-pmd/config.c lines 77 to 121.
>
Thanks for your review.
We can not contain all the RSS offload types. We need i40e_filter_pctype to
configure the queue region. It only defines some basic pctypes and there is no
regular between i40e_filter_pctype and RSS offload types. So for "flow create",
it can only configure queue region for these basic pctypes. What we can do is
associate them with RSS offload types.
Thanks.
Shougang
Re: [dpdk-dev] [PATCH v3] net/i40e: fix queue region issue in RSS flow
Hi, Wei
> -Original Message-
> From: Zhao1, Wei
> Sent: Sunday, June 28, 2020 2:02 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ; Wang,
> ShougangX ; sta...@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] net/i40e: fix queue region issue in RSS
> flow
>
> Hi, shougang
>
[snip]
> > diff --git a/drivers/net/i40e/i40e_flow.c
> > b/drivers/net/i40e/i40e_flow.c index 7e64ae53a..2f937567b 100644
> > --- a/drivers/net/i40e/i40e_flow.c
> > +++ b/drivers/net/i40e/i40e_flow.c
> > @@ -4625,6 +4625,34 @@ i40e_flow_parse_rss_action(struct rte_eth_dev
> > *dev,
> > uint32_t index = 0;
> > uint64_t hf_bit = 1;
> >
> > + static const struct {
> > + uint64_t rss_type;
> > + enum i40e_filter_pctype pctype;
> > + } pctype_match_table[] = {
> > + {ETH_RSS_FRAG_IPV4,
> > + I40E_FILTER_PCTYPE_FRAG_IPV4},
> > + {ETH_RSS_NONFRAG_IPV4_TCP,
> > + I40E_FILTER_PCTYPE_NONF_IPV4_TCP},
> > + {ETH_RSS_NONFRAG_IPV4_UDP,
> > + I40E_FILTER_PCTYPE_NONF_IPV4_UDP},
> > + {ETH_RSS_NONFRAG_IPV4_SCTP,
> > + I40E_FILTER_PCTYPE_NONF_IPV4_SCTP},
> > + {ETH_RSS_NONFRAG_IPV4_OTHER,
> > + I40E_FILTER_PCTYPE_NONF_IPV4_OTHER},
> > + {ETH_RSS_FRAG_IPV6,
> > + I40E_FILTER_PCTYPE_FRAG_IPV6},
> > + {ETH_RSS_NONFRAG_IPV6_TCP,
> > + I40E_FILTER_PCTYPE_NONF_IPV6_TCP},
> > + {ETH_RSS_NONFRAG_IPV6_UDP,
> > + I40E_FILTER_PCTYPE_NONF_IPV6_UDP},
> > + {ETH_RSS_NONFRAG_IPV6_SCTP,
> > + I40E_FILTER_PCTYPE_NONF_IPV6_SCTP},
> > + {ETH_RSS_NONFRAG_IPV6_OTHER,
> > + I40E_FILTER_PCTYPE_NONF_IPV6_OTHER},
> > + {ETH_RSS_L2_PAYLOAD,
> > + I40E_FILTER_PCTYPE_L2_PAYLOAD},
> > + };
> > +
>
> For x772, the pctype for UDP is different, this table should be different
> also.
>
> if (hw->mac.type == I40E_MAC_X722)
> ..
Thanks for your review, I will perfect this table.
Thanks.
Shougang
Re: [dpdk-dev] [PATCH] net/ice: fix wild pointer
> -Original Message-
> From: Ye, Xiaolong
> Sent: Thursday, November 7, 2019 11:30 AM
> To: Wang, ShougangX
> Cc: dev@dpdk.org; Yang, Qiming ; Xing, Beilei
> ; Cao, Yahui ; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] net/ice: fix wild pointer
>
> On 11/07, Ye Xiaolong wrote:
> >On 11/05, Wang ShougangX wrote:
> >>To avoid wild pointer, pointers should be set to NULL after free them.
> >>
> >>Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
> >>Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
> >>Cc: sta...@dpdk.org
> >>
> >>Signed-off-by: Wang ShougangX
> >>---
> >> drivers/net/ice/ice_fdir_filter.c | 8 +++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/net/ice/ice_fdir_filter.c
> >>b/drivers/net/ice/ice_fdir_filter.c
> >>index 736ccd54e..d2c754f07 100644
> >>--- a/drivers/net/ice/ice_fdir_filter.c
> >>+++ b/drivers/net/ice/ice_fdir_filter.c
> >>@@ -403,6 +403,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
> >>rte_free(fdir_info->hash_map);
> >>if (fdir_info->hash_table)
> >>rte_hash_free(fdir_info->hash_table);
> >>+
> >>+ fdir_info->hash_map = NULL;
> >>+ fdir_info->hash_table = NULL;
> >> }
> >>
> >> /*
> >>@@ -525,10 +528,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
> >>
> >>for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
> >> ptype < ICE_FLTR_PTYPE_MAX;
> >>-ptype++)
> >>+ptype++) {
> >>rte_free(hw->fdir_prof[ptype]);
> >>+ hw->fdir_prof[ptype] = NULL;
> >>+ }
> >>
> >>rte_free(hw->fdir_prof);
> >>+ hw->fdir_prof = NULL;
> >> }
> >>
> >> /* Remove a profile for some filter type */
> >>--
> >>2.17.1
> >>
> >
> >Reviewed-by: Xiaolong Ye
> >
> >Applied to dpdk-next-net-intel. Thanks.
>
> Please ignore this mail, I'm still waiting for your new patchset.
>
OK, I will make a patchset.
> Thanks,
> Xiaolong
Thanks.
Shougang
[dpdk-dev] [PATCH v2 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 46 +++ 2 files changed, 42 insertions(+), 5 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v2 2/4] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from ICE_FLTR_PTYPE_NONF_IPV4_UDP.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 31705c164..87634e4de 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -583,7 +583,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
[dpdk-dev] [PATCH v2 1/4] net/ice: fix memzone reserve and release in FDIR
To avoid memzone reserve failure and memory leak, following
resources management should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index de67e5934..0a39ca6ff 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -325,6 +325,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 736ccd54e..31705c164 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -140,6 +140,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -493,19 +499,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -619,6 +629,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH v2 3/4] net/ice: fix FDIR counter resource release
All the counter resources should be cleaned up when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 87634e4de..94a6e96df 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -265,6 +265,9 @@ ice_fdir_counter_release(struct ice_pf *pf)
for (i = 0; i < container->index_free; i++)
rte_free(container->pools[i]);
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
+
return 0;
}
--
2.17.1
[dpdk-dev] [PATCH v2 4/4] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 94a6e96df..e6e925b4e 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -165,6 +165,10 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
+ /* To avoid wild pointer, unused field pointer should be NULL */
+ hw->fdir_prof[ICE_FLTR_PTYPE_NONF_NONE] = NULL;
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -180,9 +184,13 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fail_mem:
for (fltr_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
fltr_ptype < ptype;
-fltr_ptype++)
+fltr_ptype++) {
rte_free(hw->fdir_prof[fltr_ptype]);
+ hw->fdir_prof[fltr_ptype] = NULL;
+ }
+
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -262,8 +270,10 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
TAILQ_INIT(&container->pool_list);
container->index_free = 0;
@@ -412,6 +422,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -538,10 +551,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
Re: [dpdk-dev] [PATCH v2 1/4] net/ice: fix memzone reserve and release in FDIR
Hi, Xiaolong
> -Original Message-
> From: Ye, Xiaolong
> Sent: Monday, November 11, 2019 4:10 PM
> To: Wang, ShougangX
> Cc: dev@dpdk.org; Yang, Qiming ; Xing, Beilei
> ; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] net/ice: fix memzone reserve and
> release in FDIR
>
> On 11/07, Wang ShougangX wrote:
> >To avoid memzone reserve failure and memory leak, following resources
> >management should be added.
> >- Check if the FDIR Memzone already exists before reserving.
>
> In what scenario it will reserve FDIR memzone twice?
Currently, there is no scenario.
It is a failsafe method, just like i40e_memzone_reserve().
>
> >- Free FDIR memzone when teardown and other failure scenarios.
> >
> >Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
> >Cc: sta...@dpdk.org
>
> No need to cc stable since commit 84dc7a95a2d3 is in this release.
Got it.
>
> >
> >Signed-off-by: Wang ShougangX
> >---
> > drivers/net/ice/ice_ethdev.h | 1 +
> > drivers/net/ice/ice_fdir_filter.c | 19 ++-
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/ice/ice_ethdev.h
> >b/drivers/net/ice/ice_ethdev.h index de67e5934..0a39ca6ff 100644
> >--- a/drivers/net/ice/ice_ethdev.h
> >+++ b/drivers/net/ice/ice_ethdev.h
> >@@ -325,6 +325,7 @@ struct ice_fdir_info {
> > struct ice_rx_queue *rxq;
> > void *prg_pkt; /* memory for fdir program packet */
> > uint64_t dma_addr; /* physic address of packet memory*/
> >+const struct rte_memzone *mz;
> > struct ice_fdir_filter_conf conf;
> >
> > struct ice_fdir_filter_conf **hash_map; diff --git
> >a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
> >index 736ccd54e..31705c164 100644
> >--- a/drivers/net/ice/ice_fdir_filter.c
> >+++ b/drivers/net/ice/ice_fdir_filter.c
> >@@ -140,6 +140,12 @@ static struct ice_flow_parser
> >ice_fdir_parser_comms; static const struct rte_memzone *
> >ice_memzone_reserve(const char *name, uint32_t len, int socket_id) {
> >+const struct rte_memzone *mz;
> >+
> >+mz = rte_memzone_lookup(name);
> >+if (mz)
> >+return mz;
> >+
> > return rte_memzone_reserve_aligned(name, len, socket_id,
> >RTE_MEMZONE_IOVA_CONTIG,
> >ICE_RING_BASE_ALIGN);
> >@@ -493,19 +499,23 @@ ice_fdir_setup(struct ice_pf *pf)
> > }
> > pf->fdir.prg_pkt = mz->addr;
> > pf->fdir.dma_addr = mz->iova;
> >+pf->fdir.mz = mz;
> >
> > err = ice_fdir_prof_alloc(hw);
> > if (err) {
> > PMD_DRV_LOG(ERR, "Cannot allocate memory for "
> > "flow director profile.");
> > err = -ENOMEM;
> >-goto fail_mem;
> >+goto fail_prof;
> > }
> >
> > PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming
> queue %u.",
> > vsi->base_queue);
> > return ICE_SUCCESS;
> >
> >+fail_prof:
> >+rte_memzone_free(pf->fdir.mz);
> >+pf->fdir.mz = NULL;
> > fail_mem:
> > ice_rx_queue_release(pf->fdir.rxq);
> > pf->fdir.rxq = NULL;
> >@@ -619,6 +629,13 @@ ice_fdir_teardown(struct ice_pf *pf)
> > ice_fdir_prof_free(hw);
> > ice_release_vsi(vsi);
> > pf->fdir.fdir_vsi = NULL;
> >+
> >+if (pf->fdir.mz) {
> >+err = rte_memzone_free(pf->fdir.mz);
> >+pf->fdir.mz = NULL;
> >+if (err)
> >+PMD_DRV_LOG(ERR, "Failed to free memezone.");
>
> Be more specific about the error, like "Failed to free memzone for flow
> director."
Got it.
>
>
>
> Thanks,
> Xiaolong
>
> >+}
> > }
> >
> > static int
> >--
> >2.17.1
> >
Thanks.
Shougang
[dpdk-dev] [PATCH v3 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v3 changes: Changed error log. v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 46 +++ 2 files changed, 42 insertions(+), 5 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v3 1/4] net/ice: fix memzone reserve and release in FDIR
To avoid memzone reserve failure and memory leak, following
resources management should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index c55713cc1..d4ab18436 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -334,6 +334,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index afab5af7f..a89c506c0 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -127,6 +127,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -481,19 +487,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -607,6 +617,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free FDIR memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH v3 4/4] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 361969f71..78754f365 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -152,6 +152,10 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
+ /* To avoid wild pointer, unused field pointer should be NULL */
+ hw->fdir_prof[ICE_FLTR_PTYPE_NONF_NONE] = NULL;
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -167,9 +171,13 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fail_mem:
for (fltr_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
fltr_ptype < ptype;
-fltr_ptype++)
+fltr_ptype++) {
rte_free(hw->fdir_prof[fltr_ptype]);
+ hw->fdir_prof[fltr_ptype] = NULL;
+ }
+
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -249,8 +257,10 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
TAILQ_INIT(&container->pool_list);
container->index_free = 0;
@@ -400,6 +410,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -526,10 +539,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
[dpdk-dev] [PATCH v3 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v3 changes: Changed error log. v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 46 +++ 2 files changed, 42 insertions(+), 5 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v3 3/4] net/ice: fix FDIR counter resource release
All the counter resources should be cleaned up when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index f728b9062..361969f71 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -252,6 +252,9 @@ ice_fdir_counter_release(struct ice_pf *pf)
for (i = 0; i < container->index_free; i++)
rte_free(container->pools[i]);
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
+
return 0;
}
--
2.17.1
[dpdk-dev] [PATCH v3 2/4] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from ICE_FLTR_PTYPE_NONF_IPV4_UDP.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index a89c506c0..f728b9062 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
[dpdk-dev] [PATCH v3 1/4] net/ice: fix memzone reserve and release in FDIR
To avoid memzone reserve failure and memory leak, following
resources management should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index c55713cc1..d4ab18436 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -334,6 +334,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index afab5af7f..a89c506c0 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -127,6 +127,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -481,19 +487,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -607,6 +617,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free FDIR memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH v3 3/4] net/ice: fix FDIR counter resource release
All the counter resources should be cleaned up when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index f728b9062..361969f71 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -252,6 +252,9 @@ ice_fdir_counter_release(struct ice_pf *pf)
for (i = 0; i < container->index_free; i++)
rte_free(container->pools[i]);
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
+
return 0;
}
--
2.17.1
[dpdk-dev] [PATCH v3 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v3 changes: Changed error log. v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 46 +++ 2 files changed, 42 insertions(+), 5 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v3 4/4] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 361969f71..78754f365 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -152,6 +152,10 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
+ /* To avoid wild pointer, unused field pointer should be NULL */
+ hw->fdir_prof[ICE_FLTR_PTYPE_NONF_NONE] = NULL;
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -167,9 +171,13 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fail_mem:
for (fltr_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
fltr_ptype < ptype;
-fltr_ptype++)
+fltr_ptype++) {
rte_free(hw->fdir_prof[fltr_ptype]);
+ hw->fdir_prof[fltr_ptype] = NULL;
+ }
+
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -249,8 +257,10 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
TAILQ_INIT(&container->pool_list);
container->index_free = 0;
@@ -400,6 +410,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -526,10 +539,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
[dpdk-dev] [PATCH v3 2/4] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from ICE_FLTR_PTYPE_NONF_IPV4_UDP.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index a89c506c0..f728b9062 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
[dpdk-dev] [PATCH v3 2/4] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from ICE_FLTR_PTYPE_NONF_IPV4_UDP.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index a89c506c0..f728b9062 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
[dpdk-dev] [PATCH v3 1/4] net/ice: fix memzone reserve and release in FDIR
To avoid memzone reserve failure and memory leak, following
resources management should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index c55713cc1..d4ab18436 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -334,6 +334,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index afab5af7f..a89c506c0 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -127,6 +127,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -481,19 +487,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -607,6 +617,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free FDIR memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH v3 3/4] net/ice: fix FDIR counter resource release
All the counter resources should be cleaned up when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index f728b9062..361969f71 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -252,6 +252,9 @@ ice_fdir_counter_release(struct ice_pf *pf)
for (i = 0; i < container->index_free; i++)
rte_free(container->pools[i]);
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
+
return 0;
}
--
2.17.1
[dpdk-dev] [PATCH v3 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v3 changes: Changed error log. v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 46 +++ 2 files changed, 42 insertions(+), 5 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v3 4/4] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 361969f71..78754f365 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -152,6 +152,10 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
+ /* To avoid wild pointer, unused field pointer should be NULL */
+ hw->fdir_prof[ICE_FLTR_PTYPE_NONF_NONE] = NULL;
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -167,9 +171,13 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fail_mem:
for (fltr_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
fltr_ptype < ptype;
-fltr_ptype++)
+fltr_ptype++) {
rte_free(hw->fdir_prof[fltr_ptype]);
+ hw->fdir_prof[fltr_ptype] = NULL;
+ }
+
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -249,8 +257,10 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
TAILQ_INIT(&container->pool_list);
container->index_free = 0;
@@ -400,6 +410,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -526,10 +539,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
Re: [dpdk-dev] [PATCH v3 2/4] net/ice: fix removal of FDIR profile
Hi, Qi
> -Original Message-
> From: Zhang, Qi Z
> Sent: Tuesday, November 12, 2019 3:51 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Yang, Qiming ; Xing, Beilei
> ; Wang, ShougangX
> Subject: RE: [dpdk-dev] [PATCH v3 2/4] net/ice: fix removal of FDIR profile
>
>
>
> > -Original Message-----
> > From: dev On Behalf Of Wang ShougangX
> > Sent: Tuesday, November 12, 2019 8:51 AM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming ; Xing, Beilei
> > ; Wang, ShougangX
> > Subject: [dpdk-dev] [PATCH v3 2/4] net/ice: fix removal of FDIR
> > profile
> >
> > The removal of FDIR profile should start from
> > ICE_FLTR_PTYPE_NONF_IPV4_UDP.
> >
> > Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
> >
> > Signed-off-by: Wang ShougangX
> > ---
> > drivers/net/ice/ice_fdir_filter.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ice/ice_fdir_filter.c
> > b/drivers/net/ice/ice_fdir_filter.c
> > index a89c506c0..f728b9062 100644
> > --- a/drivers/net/ice/ice_fdir_filter.c
> > +++ b/drivers/net/ice/ice_fdir_filter.c
> > @@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf) {
> > enum ice_fltr_ptype ptype;
> >
> > - for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
> > + for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
>
> Is "ptype = ICE_FLTR_PTYPE_NONF_NONE + 1" better?, so we don't need to
> worry profile be re-ordered or new one be inserted in future.
yes, it is better. I will change it in next version. Thank you very much.
>
>
> > ptype < ICE_FLTR_PTYPE_MAX;
> > ptype++) {
> > ice_fdir_prof_rm(pf, ptype, false);
> > --
> > 2.17.1
Thanks.
Shougang
[dpdk-dev] [PATCH v4 3/4] net/ice: fix FDIR counter resource release
All the counter resources should be cleaned up when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
Acked-by: Qi Zhang
---
drivers/net/ice/ice_fdir_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 039e00a28..82dd283f7 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -252,6 +252,9 @@ ice_fdir_counter_release(struct ice_pf *pf)
for (i = 0; i < container->index_free; i++)
rte_free(container->pools[i]);
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
+
return 0;
}
--
2.17.1
[dpdk-dev] [PATCH v4 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v4 changes: Removed unnecessary pointer initialization operations. Changed for loop start index in ice_fdir_prof_alloc(). v3 changes: Changed error log. v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 43 +++ 2 files changed, 39 insertions(+), 5 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v4 4/4] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 82dd283f7..ac03819ed 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -152,6 +152,7 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -167,9 +168,13 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fail_mem:
for (fltr_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
fltr_ptype < ptype;
-fltr_ptype++)
+fltr_ptype++) {
rte_free(hw->fdir_prof[fltr_ptype]);
+ hw->fdir_prof[fltr_ptype] = NULL;
+ }
+
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -249,8 +254,10 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
TAILQ_INIT(&container->pool_list);
container->index_free = 0;
@@ -400,6 +407,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -526,10 +536,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
[dpdk-dev] [PATCH v4 1/4] net/ice: fix memzone reserve and release in FDIR
To avoid memzone reserve failure and memory leak, following
resources management should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Signed-off-by: Wang ShougangX
Acked-by: Qi Zhang
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index c55713cc1..d4ab18436 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -334,6 +334,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index afab5af7f..a89c506c0 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -127,6 +127,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -481,19 +487,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -607,6 +617,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free FDIR memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH v4 2/4] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from the next
of ICE_FLTR_PTYPE_NONF_NONE.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index a89c506c0..039e00a28 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
Re: [dpdk-dev] [PATCH v4 2/4] net/ice: fix removal of FDIR profile
Hi, Qiming
> -Original Message-
> From: Yang, Qiming
> Sent: Tuesday, November 12, 2019 6:37 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Zhang, Qi Z
> Subject: RE: [PATCH v4 2/4] net/ice: fix removal of FDIR profile
>
> Hi,
>
> > -Original Message-
> > From: Wang, ShougangX
> > Sent: Tuesday, November 12, 2019 11:50 AM
> > To: dev@dpdk.org
> > Cc: Yang, Qiming ; Zhang, Qi Z
> > ; Wang, ShougangX
> > Subject: [PATCH v4 2/4] net/ice: fix removal of FDIR profile
> >
> > The removal of FDIR profile should start from the next of
> > ICE_FLTR_PTYPE_NONF_NONE.
> >
> > Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
> >
> > Signed-off-by: Wang ShougangX
> > ---
> > drivers/net/ice/ice_fdir_filter.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ice/ice_fdir_filter.c
> > b/drivers/net/ice/ice_fdir_filter.c
> > index a89c506c0..039e00a28 100644
> > --- a/drivers/net/ice/ice_fdir_filter.c
> > +++ b/drivers/net/ice/ice_fdir_filter.c
> > @@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf) {
> > enum ice_fltr_ptype ptype;
> >
> > - for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
> > + for (ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
>
> I think we should also use ICE_FLTR_PTYPE_NONF_NONE + 1 instead of
> ICE_FLTR_PTYPE_TCP_IPV4 in prof_add, To make sure it is symmetric. And it
> should be contained in this patch.
OK, I will replace them.
>
> > ptype < ICE_FLTR_PTYPE_MAX;
> > ptype++) {
> > ice_fdir_prof_rm(pf, ptype, false);
> > --
> > 2.17.1
[dpdk-dev] [PATCH v5 1/4] net/ice: fix memzone reserve and release in FDIR
To avoid memzone reserve failure and memory leak, following
resources management should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Signed-off-by: Wang ShougangX
Acked-by: Qi Zhang
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index c55713cc1..d4ab18436 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -334,6 +334,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index afab5af7f..a89c506c0 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -127,6 +127,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -481,19 +487,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -607,6 +617,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free FDIR memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH v5 2/4] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from the next
of ICE_FLTR_PTYPE_NONF_NONE.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index a89c506c0..39c0efbdc 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -152,7 +152,7 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
- for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
+ for (ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
if (!hw->fdir_prof[ptype]) {
@@ -165,7 +165,7 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
return 0;
fail_mem:
- for (fltr_ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
+ for (fltr_ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
fltr_ptype < ptype;
fltr_ptype++)
rte_free(hw->fdir_prof[fltr_ptype]);
@@ -521,7 +521,7 @@ ice_fdir_prof_free(struct ice_hw *hw)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
+ for (ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++)
rte_free(hw->fdir_prof[ptype]);
@@ -571,7 +571,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
[dpdk-dev] [PATCH v5 0/4] net/ice: fix memory release in FDIR
These patches include FDIR memory resource fixes related to ICE driver. Patch 1: fix memzone reserve and release in FDIR Patch 2: fix removal of FDIR profile Patch 3: fix FDIR counter resource release Patch 4: fix wild pointer --- v5 changes: Changed loop start index in ice_fdir_prof_alloc(). Changed loop start index in ice_fdir_prof_free(). v4 changes: Removed unnecessary pointer initialization operations. Changed loop start index in ice_fdir_prof_rm_all(). v3 changes: Changed error log. v2 changes: Merged patches related to CVL memory resources management into patchset. Wang ShougangX (4): net/ice: fix memzone reserve and release in FDIR net/ice: fix removal of FDIR profile net/ice: fix FDIR counter resource release net/ice: fix wild pointer drivers/net/ice/ice_ethdev.h | 1 + drivers/net/ice/ice_fdir_filter.c | 49 ++- 2 files changed, 42 insertions(+), 8 deletions(-) -- 2.17.1
[dpdk-dev] [PATCH v5 4/4] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 19 ---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index e3b633e23..dd0e1b0c9 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -167,9 +167,14 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fail_mem:
for (fltr_ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
fltr_ptype < ptype;
-fltr_ptype++)
+fltr_ptype++) {
rte_free(hw->fdir_prof[fltr_ptype]);
+ hw->fdir_prof[fltr_ptype] = NULL;
+ }
+
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
+
return -ENOMEM;
}
@@ -249,8 +254,10 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
TAILQ_INIT(&container->pool_list);
container->index_free = 0;
@@ -400,6 +407,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -526,10 +536,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_NONE + 1;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
[dpdk-dev] [PATCH v5 3/4] net/ice: fix FDIR counter resource release
All the counter resources should be cleaned up when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Signed-off-by: Wang ShougangX
Acked-by: Qi Zhang
---
drivers/net/ice/ice_fdir_filter.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 39c0efbdc..e3b633e23 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -252,6 +252,9 @@ ice_fdir_counter_release(struct ice_pf *pf)
for (i = 0; i < container->index_free; i++)
rte_free(container->pools[i]);
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
+
return 0;
}
--
2.17.1
Re: [dpdk-dev] [PATCH] net/ixgbe: fix qos sched sample app performance drop
Hi, Xiaolong
> -Original Message-
[snip]
> >+static void ixgbe_dev_macsec_init(struct rte_eth_dev *dev);
> >+
> > /*
> > * Define VF Stats MACRO for Non "cleared on read" register
> > */
> >@@ -1095,6 +1097,8 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev,
> >void *init_params __rte_unused)
> >
> > PMD_INIT_FUNC_TRACE();
> >
> >+ixgbe_dev_macsec_init(eth_dev);
>
> This is not needed as dev_private is allocated by rte_zmalloc_socket.
>
OK, I will remove it.
> >+
> > eth_dev->dev_ops = &ixgbe_eth_dev_ops;
> > eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
> > eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts; @@ -2545,7 +2549,7 @@
> >ixgbe_dev_start(struct rte_eth_dev *dev)
> > uint32_t *link_speeds;
> > struct ixgbe_tm_conf *tm_conf =
> > IXGBE_DEV_PRIVATE_TO_TM_CONF(dev->data->dev_private);
> >-struct ixgbe_macsec_setting *macsec_ctrl =
> >+struct ixgbe_macsec_setting *macsec_setting =
> > IXGBE_DEV_PRIVATE_TO_MACSEC_SETTING(dev->data-
> >dev_private);
> >
> > PMD_INIT_FUNC_TRACE();
> >@@ -2799,9 +2803,11 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
> > */
> > ixgbe_dev_link_update(dev, 0);
> >
> >-/* setup the macsec ctrl register */
> >-ixgbe_dev_macsec_register_enable(dev, macsec_ctrl);
> >-
> >+/* setup the macsec setting register */
> >+if (macsec_setting->encrypt_en != 0 ||
> >+macsec_setting->replayprotect_en != 0) {
> >+ixgbe_dev_macsec_register_enable(dev, macsec_setting);
> >+}
>
> Can we safely assume that if encrypt_en and replayprotect_en equals zero,
> then we don't need to call ixgbe_dev_macsec_register_enable at all? Since this
> enable routine is more about just encrypt_en/replayprotect_en, is that any
> usercase when user set macsec offload with both encrypt_en and
> replayprotect_en are 0?
>
As you said, this is a problem. I will fix it. Thanks a lot.
Thanks.
Shougang
[dpdk-dev] [PATCH] net/ice: fix memory release in FDIR
To avoid wild pointer and memory leak, following resources management
should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
- Set pointers to NULL after free them.
- Release all counter resources when teardown.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 41 ---
2 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index de67e5934..0a39ca6ff 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -325,6 +325,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 736ccd54e..059a579dc 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -140,6 +140,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -159,6 +165,10 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
+ /* To avoid wild pointer, unused field pointer should be NULL */
+ hw->fdir_prof[ICE_FLTR_PTYPE_NONF_NONE] = NULL;
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -177,6 +187,7 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fltr_ptype++)
rte_free(hw->fdir_prof[fltr_ptype]);
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -256,8 +267,13 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
+
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
return 0;
}
@@ -403,6 +419,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -493,19 +512,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -525,10 +548,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
@@ -573,7 +599,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
@@ -619,6 +645,13 @@ ice_fdir_teardown(struct ice_pf *pf)
i
Re: [dpdk-dev] [PATCH] net/ice: fix memory release in FDIR
Hi, Beilei > -Original Message- > From: Xing, Beilei > Sent: Tuesday, November 5, 2019 1:09 PM > To: Wang, ShougangX ; dev@dpdk.org > Cc: Yang, Qiming ; Cao, Yahui > ; sta...@dpdk.org > Subject: RE: [PATCH] net/ice: fix memory release in FDIR > > > > > -Original Message- > > From: Wang, ShougangX > > Sent: Tuesday, November 5, 2019 4:44 AM > > To: dev@dpdk.org > > Cc: Yang, Qiming ; Xing, Beilei > > ; Cao, Yahui ; Wang, > > ShougangX ; sta...@dpdk.org > > Subject: [PATCH] net/ice: fix memory release in FDIR > > > > To avoid wild pointer and memory leak, following resources management > > should be added. > > - Check if the FDIR Memzone already exists before reserving. > > - Free FDIR memzone when teardown and other failure scenarios. > > - Set pointers to NULL after free them. > > - Release all counter resources when teardown. > > Thanks for the patch, but could you split the patch according to different > issues? > > Beilei OK, I will split them in next version. Thanks. Shougang
[dpdk-dev] [PATCH] net/ice: fix memzone reserve and release in FDIR
To avoid wild pointer and memory leak, following resources management
should be added.
- Check if the FDIR Memzone already exists before reserving.
- Free FDIR memzone when teardown and other failure scenarios.
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_ethdev.h | 1 +
drivers/net/ice/ice_fdir_filter.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index de67e5934..0a39ca6ff 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -325,6 +325,7 @@ struct ice_fdir_info {
struct ice_rx_queue *rxq;
void *prg_pkt; /* memory for fdir program packet */
uint64_t dma_addr; /* physic address of packet memory*/
+ const struct rte_memzone *mz;
struct ice_fdir_filter_conf conf;
struct ice_fdir_filter_conf **hash_map;
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 736ccd54e..31705c164 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -140,6 +140,12 @@ static struct ice_flow_parser ice_fdir_parser_comms;
static const struct rte_memzone *
ice_memzone_reserve(const char *name, uint32_t len, int socket_id)
{
+ const struct rte_memzone *mz;
+
+ mz = rte_memzone_lookup(name);
+ if (mz)
+ return mz;
+
return rte_memzone_reserve_aligned(name, len, socket_id,
RTE_MEMZONE_IOVA_CONTIG,
ICE_RING_BASE_ALIGN);
@@ -493,19 +499,23 @@ ice_fdir_setup(struct ice_pf *pf)
}
pf->fdir.prg_pkt = mz->addr;
pf->fdir.dma_addr = mz->iova;
+ pf->fdir.mz = mz;
err = ice_fdir_prof_alloc(hw);
if (err) {
PMD_DRV_LOG(ERR, "Cannot allocate memory for "
"flow director profile.");
err = -ENOMEM;
- goto fail_mem;
+ goto fail_prof;
}
PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.",
vsi->base_queue);
return ICE_SUCCESS;
+fail_prof:
+ rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
fail_mem:
ice_rx_queue_release(pf->fdir.rxq);
pf->fdir.rxq = NULL;
@@ -619,6 +629,13 @@ ice_fdir_teardown(struct ice_pf *pf)
ice_fdir_prof_free(hw);
ice_release_vsi(vsi);
pf->fdir.fdir_vsi = NULL;
+
+ if (pf->fdir.mz) {
+ err = rte_memzone_free(pf->fdir.mz);
+ pf->fdir.mz = NULL;
+ if (err)
+ PMD_DRV_LOG(ERR, "Failed to free memezone.");
+ }
}
static int
--
2.17.1
[dpdk-dev] [PATCH] net/ice: fix FDIR counter resource release
To avoid memory leak, all the counter resources should be
released when teardown.
Fixes: 0f880c3df192 ("net/ice: add flow director counter resource init/release")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 736ccd54e..4a4349824 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -256,8 +256,13 @@ ice_fdir_counter_release(struct ice_pf *pf)
&fdir_info->counter;
uint8_t i;
- for (i = 0; i < container->index_free; i++)
+ for (i = 0; i < container->index_free; i++) {
rte_free(container->pools[i]);
+ container->pools[i] = NULL;
+ }
+
+ TAILQ_INIT(&container->pool_list);
+ container->index_free = 0;
return 0;
}
--
2.17.1
[dpdk-dev] [PATCH] net/ice: fix removal of FDIR profile
The removal of FDIR profile should start from ICE_FLTR_PTYPE_NONF_IPV4_UDP.
Fixes: 109e8e06249e ("net/ice: configure HW flow director rule")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 736ccd54e..e426c1431 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -159,6 +159,10 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
if (!hw->fdir_prof)
return -ENOMEM;
}
+
+ /* To avoid wild pointer, unused field pointer should be NULL */
+ hw->fdir_prof[ICE_FLTR_PTYPE_NONF_NONE] = NULL;
+
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
@@ -177,6 +181,7 @@ ice_fdir_prof_alloc(struct ice_hw *hw)
fltr_ptype++)
rte_free(hw->fdir_prof[fltr_ptype]);
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
return -ENOMEM;
}
@@ -573,7 +578,7 @@ ice_fdir_prof_rm_all(struct ice_pf *pf)
{
enum ice_fltr_ptype ptype;
- for (ptype = ICE_FLTR_PTYPE_NONF_NONE;
+ for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
ptype++) {
ice_fdir_prof_rm(pf, ptype, false);
--
2.17.1
[dpdk-dev] [PATCH] net/ice: fix wild pointer
To avoid wild pointer, pointers should be set to NULL after free them.
Fixes: 1a2fc1799f09 ("net/ice: reject duplicated flow for flow director")
Fixes: 84dc7a95a2d3 ("net/ice: enable flow director engine")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
drivers/net/ice/ice_fdir_filter.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c
b/drivers/net/ice/ice_fdir_filter.c
index 736ccd54e..d2c754f07 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -403,6 +403,9 @@ ice_fdir_release_filter_list(struct ice_pf *pf)
rte_free(fdir_info->hash_map);
if (fdir_info->hash_table)
rte_hash_free(fdir_info->hash_table);
+
+ fdir_info->hash_map = NULL;
+ fdir_info->hash_table = NULL;
}
/*
@@ -525,10 +528,13 @@ ice_fdir_prof_free(struct ice_hw *hw)
for (ptype = ICE_FLTR_PTYPE_NONF_IPV4_UDP;
ptype < ICE_FLTR_PTYPE_MAX;
-ptype++)
+ptype++) {
rte_free(hw->fdir_prof[ptype]);
+ hw->fdir_prof[ptype] = NULL;
+ }
rte_free(hw->fdir_prof);
+ hw->fdir_prof = NULL;
}
/* Remove a profile for some filter type */
--
2.17.1
[dpdk-dev] [PATCH] app/testpmd: fix Segment fault when start fwd
This patch fixed the reset function to avoid crash when user don't
call port reset , port stop and port start functions as sequence.
Fixes: 97f1e19679 ("app/testpmd: add port reset command")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
app/test-pmd/testpmd.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e8e2a39b6..273a7aa02 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2344,6 +2344,9 @@ reset_port(portid_t pid)
if (port_id_is_invalid(pid, ENABLED_WARN))
return;
+ printf("Stopping ports...\n");
+ stop_port(pid);
+
printf("Resetting ports...\n");
RTE_ETH_FOREACH_DEV(pi) {
@@ -2372,6 +2375,9 @@ reset_port(portid_t pid)
}
}
+ printf("Starting ports...\n");
+ start_port(pid);
+
printf("Done\n");
}
--
2.17.1
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix Segment fault when start fwd
Hi Ferruh
Thanks for your reply.
> Not sure if 'reset' command should do more than it says, if there is a
> requirement that port should be stopped, why not add this condition with an
> error message so that user can stop the port in advance if she wants.
Firstly, port must be stopped before reset. Usually, port is stopped by
rte_eth_dev_reset(),
so testpmd does not prompt user to stop port. Although it can stop port, but
testpmd does not
change its own port status flag to "RTE_PORT_STOPPED" and it will cause "port
start" to fail.
So I add this patch to stop port as same as running "port stop" command.
> - 'reset_port()' function has a loop inside, 'RTE_ETH_FOREACH_DEV()' but it
> works on single port, the loop looked unnecessary to me, can you please check
> and remove the loop if required?
"port reset" supports reset all ports (testpmd > port reset all), so this loop
is necessary.
> - I am not able to see 'reset' has been documented in
> 'doc/guides/testpmd_app_ug/testpmd_funcs.rst', it may be missing, can you
> please check and add it if required?
OK. I will add it in the next patch.
Thanks.
Shougang
> -Original Message-
> From: Yigit, Ferruh
> Sent: Saturday, September 14, 2019 1:35 AM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Yang, Qiming
> ; sta...@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix Segment fault when start
> fwd
>
> On 9/6/2019 2:28 AM, Wang ShougangX wrote:
> > This patch fixed the reset function to avoid crash when user don't
> > call port reset , port stop and port start functions as sequence.
> >
> > Fixes: 97f1e19679 ("app/testpmd: add port reset command")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Wang ShougangX
> > ---
> > app/test-pmd/testpmd.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > e8e2a39b6..273a7aa02 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2344,6 +2344,9 @@ reset_port(portid_t pid)
> > if (port_id_is_invalid(pid, ENABLED_WARN))
> > return;
> >
> > + printf("Stopping ports...\n");
> > + stop_port(pid);
> > +
> > printf("Resetting ports...\n");
> >
> > RTE_ETH_FOREACH_DEV(pi) {
> > @@ -2372,6 +2375,9 @@ reset_port(portid_t pid)
> > }
> > }
> >
> > + printf("Starting ports...\n");
> > + start_port(pid);
> > +
> > printf("Done\n");
> > }
>
> Hi Shougang,
>
> Not sure if 'reset' command should do more than it says, if there is a
> requirement that port should be stopped, why not add this condition with an
> error message so that user can stop the port in advance if she wants.
>
>
> Btw, a few things related,
> - 'reset_port()' function has a loop inside, 'RTE_ETH_FOREACH_DEV()' but it
> works on single port, the loop looked unnecessary to me, can you please check
> and remove the loop if required?
>
> - I am not able to see 'reset' has been documented in
> 'doc/guides/testpmd_app_ug/testpmd_funcs.rst', it may be missing, can you
> please check and add it if required?
>
> Thanks,
> ferruh
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix Segment fault when start fwd
> Let's assume port already stopped before calling the reset, reset will cause
> port
> to be started.
Indeed, automatically start looks strange in this case. User explicitly do the
"port start" command should be better.
> I am for user explicitly do the stop, reset and start commands, instead of
> reset
> automatically stop and start later.
There are two reasons that it is necessary to stop automatically in "port
reset" command:
1) Even without this patch, user does not need to manually execute "port stop"
command
before "port reset" too, because port will be stopped in rte_eth_dev_reset()
function.
But this function does not update the port status flag of testpmd. It makes the
port state
recorded by testpmd inconsistent with the actual port state. So I add stop
processing in
reset_port() function to stop port as same as running "port stop" command.
2) If let user to stop port manually when he wants to reset port, it is not
easy to use than before.
Thanks.
Shougang
> -Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, September 16, 2019 11:28 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Yang, Qiming
> ; sta...@dpdk.org
> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix Segment fault when start
> fwd
>
> On 9/16/2019 7:37 AM, Wang, ShougangX wrote:
> > Hi Ferruh
> >
> > Thanks for your reply.
> >
> >> Not sure if 'reset' command should do more than it says, if there is
> >> a requirement that port should be stopped, why not add this condition
> >> with an error message so that user can stop the port in advance if she
> >> wants.
> >
> > Firstly, port must be stopped before reset. Usually, port is stopped
> > by rte_eth_dev_reset(), so testpmd does not prompt user to stop port.
> > Although it can stop port, but testpmd does not change its own port status
> flag to "RTE_PORT_STOPPED" and it will cause "port start" to fail.
> > So I add this patch to stop port as same as running "port stop" command.
>
> Let's assume port already stopped before calling the reset, reset will cause
> port
> to be started.
> I am for user explicitly do the stop, reset and start commands, instead of
> reset
> automatically stop and start later.
>
> >
> >> - 'reset_port()' function has a loop inside, 'RTE_ETH_FOREACH_DEV()'
> >> but it works on single port, the loop looked unnecessary to me, can
> >> you please check and remove the loop if required?
> >
> > "port reset" supports reset all ports (testpmd > port reset all), so this
> > loop is
> necessary.
>
> Got it.
>
> >
> >> - I am not able to see 'reset' has been documented in
> >> 'doc/guides/testpmd_app_ug/testpmd_funcs.rst', it may be missing, can
> >> you please check and add it if required?
> >
> > OK. I will add it in the next patch.
> >
> > Thanks.
> > Shougang
> >
> >
> >> -Original Message-
> >> From: Yigit, Ferruh
> >> Sent: Saturday, September 14, 2019 1:35 AM
> >> To: Wang, ShougangX ; dev@dpdk.org
> >> Cc: Lu, Wenzhuo ; Yang, Qiming
> >> ; sta...@dpdk.org
> >> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix Segment fault
> >> when start fwd
> >>
> >> On 9/6/2019 2:28 AM, Wang ShougangX wrote:
> >>> This patch fixed the reset function to avoid crash when user don't
> >>> call port reset , port stop and port start functions as sequence.
> >>>
> >>> Fixes: 97f1e19679 ("app/testpmd: add port reset command")
> >>> Cc: sta...@dpdk.org
> >>>
> >>> Signed-off-by: Wang ShougangX
> >>> ---
> >>> app/test-pmd/testpmd.c | 6 ++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>> e8e2a39b6..273a7aa02 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -2344,6 +2344,9 @@ reset_port(portid_t pid)
> >>> if (port_id_is_invalid(pid, ENABLED_WARN))
> >>> return;
> >>>
> >>> + printf("Stopping ports...\n");
> >>> + stop_port(pid);
> >>> +
> >>> printf("Resetting ports...\n");
> >>>
> >>> RTE_ETH_FOREACH_DEV(pi) {
> >>> @@ -2372,6 +2375,9 @@ reset_port(portid_t pid)
> >>> }
> >>> }
> >>>
> >>> + printf("Starting ports...\n");
> >>> + start_port(pid);
> >>> +
> >>> printf("Done\n");
> >>> }
> >>
> >> Hi Shougang,
> >>
> >> Not sure if 'reset' command should do more than it says, if there is
> >> a requirement that port should be stopped, why not add this condition
> >> with an error message so that user can stop the port in advance if she
> >> wants.
> >>
> >>
> >> Btw, a few things related,
> >> - 'reset_port()' function has a loop inside, 'RTE_ETH_FOREACH_DEV()'
> >> but it works on single port, the loop looked unnecessary to me, can
> >> you please check and remove the loop if required?
> >>
> >> - I am not able to see 'reset' has been documented in
> >> 'doc/guides/testpmd_app_ug/testpmd_funcs.rst', it may be missing, can
> >> you please check and add it if required?
> >>
> >> Thanks,
> >> ferruh
[dpdk-dev] [PATCH v2] app/testpmd: fix Segment fault when start fwd
This patch fixed the reset function to avoid crash when user don't
call port stop, port reset and port start functions as sequence.
Fixes: 97f1e19679 ("app/testpmd: add port reset command")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
app/test-pmd/testpmd.c | 2 ++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 +
2 files changed, 11 insertions(+)
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e8e2a39b6..9224aa1f7 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2344,6 +2344,8 @@ reset_port(portid_t pid)
if (port_id_is_invalid(pid, ENABLED_WARN))
return;
+ stop_port(pid);
+
printf("Resetting ports...\n");
RTE_ETH_FOREACH_DEV(pi) {
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 313e0707e..2c459810c 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2041,6 +2041,15 @@ Close all ports or a specific port::
testpmd> port close (port_id|all)
+port reset
+~~
+
+Reset all ports or a specific port::
+
+ testpmd> port reset (port_id|all)
+
+User should (re-)start the port after reset.
+
port config - queue ring size
~
--
2.17.1
Re: [dpdk-dev] [PATCH v2] app/testpmd: fix Segment fault when start fwd
Hi Bernard
> The app/test-pmd/cmdline.c file should be updated at line 760 to add help
> text
> for the "port reset (port_id|all)\n" command.
Thank you very much, I will add in next patch.
Thanks.
Shougang
> -Original Message-
> From: Iremonger, Bernard
> Sent: Monday, September 23, 2019 6:20 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Lu, Wenzhuo ; Yang, Qiming
> ; Yigit, Ferruh ; Wang,
> ShougangX ; sta...@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] app/testpmd: fix Segment fault when start
> fwd
>
> Hi Shougangx
>
> > -Original Message-----
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wang ShougangX
> > Sent: Friday, September 20, 2019 4:14 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo ; Yang, Qiming
> > ; Yigit, Ferruh ; Wang,
> > ShougangX ; sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix Segment fault when
> > start fwd
> >
> > This patch fixed the reset function to avoid crash when user don't
> > call port stop, port reset and port start functions as sequence.
> >
> > Fixes: 97f1e19679 ("app/testpmd: add port reset command")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Wang ShougangX
> > ---
> > app/test-pmd/testpmd.c | 2 ++
> > doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 +
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > e8e2a39b6..9224aa1f7 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -2344,6 +2344,8 @@ reset_port(portid_t pid)
> > if (port_id_is_invalid(pid, ENABLED_WARN))
> > return;
> >
> > + stop_port(pid);
> > +
> > printf("Resetting ports...\n");
> >
> > RTE_ETH_FOREACH_DEV(pi) {
>
> The app/test-pmd/cmdline.c file should be updated at line 760 to add help
> text
> for the "port reset (port_id|all)\n" command.
>
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index 313e0707e..2c459810c 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -2041,6 +2041,15 @@ Close all ports or a specific port::
> >
> > testpmd> port close (port_id|all)
> >
> > +port reset
> > +~~
> > +
> > +Reset all ports or a specific port::
> > +
> > + testpmd> port reset (port_id|all)
> > +
> > +User should (re-)start the port after reset.
> > +
> > port config - queue ring size
> > ~
> >
> > --
> > 2.17.1
>
> Regards,
>
> Bernard.
Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix Segment fault when start fwd
Hi Regards, Ferruh > -Original Message- > From: Iremonger, Bernard > Sent: Monday, September 23, 2019 10:18 PM > To: Yigit, Ferruh ; Wang, ShougangX > ; dev@dpdk.org > Cc: Lu, Wenzhuo ; Yang, Qiming > ; sta...@dpdk.org > Subject: RE: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix Segment fault > when start fwd > > Hi ShougangX, Ferruh, > > > -Original Message- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit > > Sent: Monday, September 23, 2019 12:04 PM > > To: Wang, ShougangX ; dev@dpdk.org > > Cc: Lu, Wenzhuo ; Yang, Qiming > > ; sta...@dpdk.org > > Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix Segment > > fault when start fwd > > > > On 9/17/2019 10:22 AM, Wang, ShougangX wrote: > > >> Let's assume port already stopped before calling the reset, reset > > >> will cause port to be started. > > > > > > Indeed, automatically start looks strange in this case. User > > > explicitly do the "port start" command should be better. > > > > > >> I am for user explicitly do the stop, reset and start commands, > > >> instead of reset automatically stop and start later. > > > > > > There are two reasons that it is necessary to stop automatically in > > > "port > > reset" command: > > > 1) Even without this patch, user does not need to manually execute > > > "port stop" command before "port reset" too, because port will be > > stopped in rte_eth_dev_reset() function. > > > But this function does not update the port status flag of testpmd. > > > It makes the port state recorded by testpmd inconsistent with the > > > actual port state. So I add stop processing in > > > reset_port() function to stop port as same as running "port stop" > > command. > > > > I see 'rte_eth_dev_reset()' API calls the 'rte_eth_dev_stop()' but it > > isn't enough as you said and a testpmd stop is required, instead of > > doing in the function it is easy to detect port is not stopped and > > return an error in the 'reset', it is up to user to stop and do the reset. > > > > > > > > 2) If let user to stop port manually when he wants to reset port, it > > > is not > > easy to use than before. > > > > I still prefer 'Explicit is better than implicit.' [1], yes it is two > > commands now to reset the port if it is already started but I don't see > > this is a > problem. > > > > Anyway this is not a big deal, if testpmd maintainers are OK for it we > > can continue with this... > > I am inclined to agree with Ferruh, that it would be better to return an > error if > the port is not stopped when "port reset" is called rather that silently > stopping > the port. > I will adopt your suggestion. > > > [1] see: The Zen of Python :) > > > > > > > > Thanks. > > > Shougang > > > > > > > > >> -Original Message- > > >> From: Yigit, Ferruh > > >> Sent: Monday, September 16, 2019 11:28 PM > > >> To: Wang, ShougangX ; dev@dpdk.org > > >> Cc: Lu, Wenzhuo ; Yang, Qiming > > >> ; sta...@dpdk.org > > >> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix Segment fault > > >> when start fwd > > >> > > >> On 9/16/2019 7:37 AM, Wang, ShougangX wrote: > > >>> Hi Ferruh > > >>> > > >>> Thanks for your reply. > > >>> > > >>>> Not sure if 'reset' command should do more than it says, if there > > >>>> is a requirement that port should be stopped, why not add this > > >>>> condition with an error message so that user can stop the port in > > advance if she wants. > > >>> > > >>> Firstly, port must be stopped before reset. Usually, port is > > >>> stopped by rte_eth_dev_reset(), so testpmd does not prompt user to stop > port. > > >>> Although it can stop port, but testpmd does not change its own > > >>> port status > > >> flag to "RTE_PORT_STOPPED" and it will cause "port start" to fail. > > >>> So I add this patch to stop port as same as running "port stop" command. > > >> > > >> Let's assume port already stopped before calling the reset, reset > > >> will cause port to be started. > > >> I am
[dpdk-dev] [PATCH v3] app/testpmd: fix Segment fault when start fwd
This patch fixed the reset function to avoid crash when user don't
call port stop, port reset functions as sequence.
Fixes: 97f1e19679 ("app/testpmd: add port reset command")
Cc: sta...@dpdk.org
Signed-off-by: Wang ShougangX
---
app/test-pmd/cmdline.c | 3 +++
app/test-pmd/testpmd.c | 6 ++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 +
3 files changed, 18 insertions(+)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 56783aa13..9d0c3db5e 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -758,6 +758,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"port close (port_id|all)\n"
"Close all ports or port_id.\n\n"
+ "port reset (port_id|all)\n"
+ "Reset all ports or port_id.\n\n"
+
"port attach (ident)\n"
"Attach physical or virtual dev by pci address or
virtual device name\n\n"
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e8e2a39b6..9b7a1521a 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2344,6 +2344,12 @@ reset_port(portid_t pid)
if (port_id_is_invalid(pid, ENABLED_WARN))
return;
+ if ((pid == (portid_t)RTE_PORT_ALL && !all_ports_stopped()) ||
+ (pid != (portid_t)RTE_PORT_ALL && !port_is_stopped(pid))) {
+ printf("Can not reset port(s), please stop port(s) first.\n");
+ return;
+ }
+
printf("Resetting ports...\n");
RTE_ETH_FOREACH_DEV(pi) {
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 313e0707e..b1a14cc4e 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2041,6 +2041,15 @@ Close all ports or a specific port::
testpmd> port close (port_id|all)
+port reset
+~~
+
+Reset all ports or a specific port::
+
+ testpmd> port reset (port_id|all)
+
+User should stop port(s) before resetting and (re-)start after reset.
+
port config - queue ring size
~
--
2.17.1
Re: [dpdk-dev] [RFC] net/i40e: refactor of hash flow
Hi, Alvin > -Original Message- > From: dev On Behalf Of Zhang,Alvin > Sent: Friday, October 23, 2020 2:56 PM > To: dev@dpdk.org > Cc: Zhang, AlvinX > Subject: [dpdk-dev] [RFC] net/i40e: refactor of hash flow > > From: Alvin Zhang > > 1. Delete original code. > 2. Add 2 tables(pattern RSS type matched PCTYPE, RSS type to input set). > 3. Parse RSS pattern and RSS type to get PCTYPE. > 4. Parse RSS action to get queues, RSS function and hash field. > 5. Create and destroy RSS filters. > 6. Create new files for hash flows. > 7. Update doc. > > Signed-off-by: Alvin Zhang > --- > doc/guides/nics/i40e.rst |4 +- > drivers/net/i40e/i40e_ethdev.c | 840 ++--- > drivers/net/i40e/i40e_ethdev.h | 43 +- > drivers/net/i40e/i40e_flow.c | 617 +-- > drivers/net/i40e/i40e_hash.c | 1315 > > drivers/net/i40e/i40e_hash.h | 34 ++ > drivers/net/i40e/meson.build |1 + > 7 files changed, 1587 insertions(+), 1267 deletions(-) > create mode 100644 drivers/net/i40e/i40e_hash.c > create mode 100644 drivers/net/i40e/i40e_hash.h > > diff --git a/drivers/net/i40e/i40e_hash.c b/drivers/net/i40e/i40e_hash.c > +#define I40E_HASH_VLAN_RSS_MASK (ETH_RSS_S_VLAN | > ETH_RSS_C_VLAN) > +#define I40E_HASH_L2_RSS_MASK(ETH_RSS_ETH | > ETH_RSS_L2_SRC_ONLY | \ > + ETH_RSS_L2_SRC_ONLY) This should be ETH_RSS_L2_DST_ONLY, right? > + > +#define I40E_HASH_L23_RSS_MASK (I40E_HASH_L2_RSS_MASK | > \ > + I40E_HASH_VLAN_RSS_MASK | \ > + ETH_RSS_L3_SRC_ONLY | \ > + ETH_RSS_L3_SRC_ONLY) ETH_RSS_L3_DST_ONLY ? > + > +#define I40E_HASH_L234_RSS_MASK (I40E_HASH_L23_RSS_MASK > | \ > + ETH_RSS_PORT | > ETH_RSS_L3_SRC_ONLY | \ > + ETH_RSS_L3_SRC_ONLY) ETH_RSS_L3_DST_ONLY ? Thanks Shougang
Re: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table
Hi, Qi > -Original Message- > From: Zhang, Qi Z > Sent: Wednesday, July 22, 2020 1:31 PM > To: Wang, ShougangX ; dev@dpdk.org > Cc: Xing, Beilei ; Guo, Jia ; Wang, > ShougangX ; sta...@dpdk.org > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table > > > > > -Original Message- > > From: dev On Behalf Of Shougang Wang > > Sent: Tuesday, July 21, 2020 1:49 PM > > To: dev@dpdk.org > > Cc: Xing, Beilei ; Guo, Jia > > ; Wang, ShougangX ; > > sta...@dpdk.org > > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up > > table > > > > The hash look up table(LUT) will not be initializing when starting > > testpmd with --disable-rss. So that some invalid queue indexes may > > still in the LUT. When enable RSS by creating RSS rule, some packets will > > not > be into the valid queues. > > This patch fixes this issue by initializing the LUT when creating an RSS > > rule. > > Could you explain why you only initialize the LUT when creating an RSS rule > but not at dev_init or dev_start? It is good to initialize the LUT at dev_start, I will fix it. > What if user configure LUT table before create a RSS rule? Does that mean > the LUT table will be flushed? Yes. Thanks. Shougang
Re: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table
Hi, Jeff
> -Original Message-
> From: Guo, Jia
> Sent: Wednesday, July 22, 2020 1:51 PM
> To: Xie, WeiX ; Wang, ShougangX
> ; dev@dpdk.org
> Cc: Xing, Beilei ; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up table
>
> hi, shougang
>
> On 7/21/2020 2:41 PM, Xie, WeiX wrote:
> > Tested-by: Zhang, XiX
> >
> > Regards,
> > Xie Wei
> >
> >
> > -Original Message-
> > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Shougang Wang
> > Sent: Tuesday, July 21, 2020 1:49 PM
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Guo, Jia
> > ; Wang, ShougangX ;
> > sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix incorrect hash look up
> > table
> >
> > The hash look up table(LUT) will not be initializing when starting testpmd
> with --disable-rss. So that some invalid queue indexes may still in the LUT.
> When enable RSS by creating RSS rule, some packets will not be into the valid
> queues.
> > This patch fixes this issue by initializing the LUT when creating an RSS
> > rule.
> >
> > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> > flow")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Shougang Wang
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 134 -
> > 1 file changed, 63 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 393b5320f..e56543393 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -13070,6 +13070,55 @@ i40e_rss_conf_init(struct
> i40e_rte_flow_rss_conf *out,
> > return 0;
> > }
> >
> > +/* If conf is NULL, function will init hash LUT with default
> > +configration*/
>
>
> Please fix the checkpatch issue here.
>
>
> > static int i40e_rss_set_lut(struct i40e_pf *pf,
>
>
> In order to eliminate any confuse with current i40e_set_rss_lut, please
> change the name, such as "i40e_rss_lut_init" or other better naming.
>
Qi also gave me some comments. I will define i40e_rss_lut_init() to initialize
the LUT.
>
> > +struct i40e_rte_flow_rss_conf *conf) {
> > + struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> > + uint32_t lut = 0;
> > + uint16_t j, num;
> > + uint32_t i;
> > +
> > + /* If both VMDQ and RSS enabled, not all of PF queues are
> configured.
> > +* It's necessary to calculate the actual PF queues that are configured.
> > +*/
> > + if (pf->dev_data->dev_conf.rxmode.mq_mode &
> ETH_MQ_RX_VMDQ_FLAG)
> > + num = i40e_pf_calc_configured_queues_num(pf);
> > + else
> > + num = pf->dev_data->nb_rx_queues;
> > +
> > + if (conf == NULL)
> > + num = RTE_MIN(num, I40E_MAX_Q_PER_TC);
> > + else
> > + num = RTE_MIN(num, conf->conf.queue_num);
> > + PMD_DRV_LOG(INFO, "Max of contiguous %u PF queues are
> configured",
> > + num);
>
>
> Alignment should match open parenthesis.
>
>
> > +
> > + if (num == 0) {
> > + PMD_DRV_LOG(ERR,
> > + "No PF queues are configured to enable RSS for
> port %u",
> > + pf->dev_data->port_id);
> > + return -ENOTSUP;
> > + }
> > +
> > + /* Fill in redirection table */
> > + for (i = 0, j = 0; i < hw->func_caps.rss_table_size; i++, j++) {
> > + if (j == num)
> > + j = 0;
> > + if (conf == NULL)
> > + lut = (lut << 8) | (j & ((0x1 <<
> > + hw->func_caps.rss_table_entry_width) - 1));
> > + else
> > + lut = (lut << 8) | (conf->conf.queue[j] & ((0x1 <<
> > + hw->func_caps.rss_table_entry_width) - 1));
> > + if ((i & 3) == 3)
> > + I40E_WRITE_REG(hw, I40E_PFQF_HLUT(i >> 2), lut);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > /* Write HENA register to enable hash */ static int
> i40e_rss_hash_set(struct i40e_pf *pf, struct i40e_rte_flow_rss_conf
> *rss_conf) @@ -13318,12 +13367,24 @@ static int
> i40e_rss_enable_hash(struct i40e_pf *pf,
> > struct i40e_rte_flow_rss_conf *conf)
> > {
> > + enum rte_eth_rx_mq_mode mq_mode
Re: [dpdk-dev] [PATCH v3] net/i40e: fix incorrect hash look up table
> -Original Message-
> From: Zhang, Qi Z
> Sent: Thursday, July 23, 2020 8:53 PM
> To: Yang, Qiming ; Wang, ShougangX
> ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ; Wang,
> ShougangX ; sta...@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3] net/i40e: fix incorrect hash look up table
>
>
>
> > -Original Message-
> > From: dev On Behalf Of Yang, Qiming
> > Sent: Thursday, July 23, 2020 9:57 AM
> > To: Wang, ShougangX ; dev@dpdk.org
> > Cc: Xing, Beilei ; Guo, Jia
> > ; Wang, ShougangX ;
> > sta...@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3] net/i40e: fix incorrect hash look
> > up table
> >
> > I don't understand why you add new function and new function mostly do
> > the same thing.
> > Why don't add fix in original code.
> >
> > > static int
> > > i40e_pf_config_mq_rx(struct i40e_pf *pf) {
> > > - int ret = 0;
> > > + int ret;
> > > enum rte_eth_rx_mq_mode mq_mode = pf->dev_data-
> > > >dev_conf.rxmode.mq_mode;
> > >
> > > + /* Initialize hash look up table */
> > > + ret = i40e_pf_init_rss_lut(pf);
> > > + if (ret)
> > > + return ret;
> > > +
> > > /* RSS setup */
> > > if (mq_mode & ETH_MQ_RX_RSS_FLAG)
>
> I agree with Qiming, if we move this check into i40e_pf_config_rss, dose that
> looks we don't need to create a new function?
Indeed, I will do like this.
>
> > > ret = i40e_pf_config_rss(pf);
> > > --
> > > 2.17.1
>
Re: [dpdk-dev] [PATCH v4] net/i40e: fix incorrect hash look up table
Hi, Jeff
> -Original Message-
> From: Guo, Jia
> Sent: Friday, July 24, 2020 11:58 AM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Xing, Beilei ; sta...@dpdk.org
> Subject: Re: [PATCH v4] net/i40e: fix incorrect hash look up table
>
> hi, shougang
>
> On 7/24/2020 10:47 AM, Shougang Wang wrote:
> > The hash look up table (LUT) is managed by global register but it is
> > not initialized when RSS is disabled. Once user wants to enable RSS
> > during runtime, the LUT will not be initialized.
> > This patch fixes the issue by initializing the LUT whether RSS enabled
> > or not.
>
>
> "whatever" but not "whether"?
"whatever", I will fix it.
>
>
> >
> > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> > flow")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Shougang Wang
> > ---
> > v4:
> > -Updated code.
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 15 ---
> > 1 file changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 05d5f2861..0a3f5e3c1 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -8985,6 +8985,7 @@ static int
> > i40e_pf_config_rss(struct i40e_pf *pf)
> > {
> > struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> > + enum rte_eth_rx_mq_mode mq_mode =
> > +pf->dev_data->dev_conf.rxmode.mq_mode;
> > struct rte_eth_rss_conf rss_conf;
> > uint32_t i, lut = 0;
> > uint16_t j, num;
> > @@ -9022,7 +9023,8 @@ i40e_pf_config_rss(struct i40e_pf *pf)
> > }
> >
> > rss_conf = pf->dev_data->dev_conf.rx_adv_conf.rss_conf;
> > - if ((rss_conf.rss_hf & pf->adapter->flow_types_mask) == 0) {
> > + if ((rss_conf.rss_hf & pf->adapter->flow_types_mask) == 0 ||
> > + !(mq_mode & ETH_MQ_RX_RSS_FLAG)) {
> > i40e_pf_disable_rss(pf);
> > return 0;
> > }
> > @@ -9198,16 +9200,7 @@ i40e_tunnel_filter_handle(struct rte_eth_dev
> *dev,
> > static int
> > i40e_pf_config_mq_rx(struct i40e_pf *pf)
>
>
> This function is still need or could it be replace by i40e_pf_config_rss?
It can be replaced by i40e_pf_config_rss, thanks for your review.
Re: [dpdk-dev] [PATCH v4] net/i40e: fix incorrect hash look up table
Hi, Qiming
> -Original Message-
> From: Yang, Qiming
> Sent: Friday, July 24, 2020 1:08 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ; Wang,
> ShougangX ; sta...@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v4] net/i40e: fix incorrect hash look up table
>
> Hi, Shougang
> This version looks better, only two small questions.
> Once somebody gave comments to your patch, you should reply the
> comments and CC when you send patch next version.
> You don't include me in this version, don't forget this next time.
Got it, I will keep in mind.
> Qiming
> > -Original Message-
> > From: dev On Behalf Of Shougang Wang
> > Sent: Friday, July 24, 2020 10:47
> > To: dev@dpdk.org
> > Cc: Xing, Beilei ; Guo, Jia
> > ; Wang, ShougangX ;
> > sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v4] net/i40e: fix incorrect hash look up
> > table
> >
> > The hash look up table (LUT) is managed by global register but it is
> > not initialized when RSS is disabled. Once user wants to enable RSS
> > during runtime, the LUT will not be initialized.
> > This patch fixes the issue by initializing the LUT whether RSS enabled or
> > not.
> >
> > Fixes: feaae285b342 ("net/i40e: support hash configuration in RSS
> > flow")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Shougang Wang
> > ---
> > v4:
> > -Updated code.
> > ---
> > drivers/net/i40e/i40e_ethdev.c | 15 ---
> > 1 file changed, 4 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index 05d5f2861..0a3f5e3c1 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -8985,6 +8985,7 @@ static int
> > i40e_pf_config_rss(struct i40e_pf *pf) {
> > struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> > + enum rte_eth_rx_mq_mode mq_mode =
> > +pf->dev_data->dev_conf.rxmode.mq_mode;
>
> No tab?
Actually, this line is on the same line as the code above, but it looks like
two lines here.
I will adjust the position of this definition to follow the "Christmas tree" in
next version.
>
> > struct rte_eth_rss_conf rss_conf;
> > uint32_t i, lut = 0;
> > uint16_t j, num;
> > @@ -9022,7 +9023,8 @@ i40e_pf_config_rss(struct i40e_pf *pf)
> > }
> >
> > rss_conf = pf->dev_data->dev_conf.rx_adv_conf.rss_conf;
> > - if ((rss_conf.rss_hf & pf->adapter->flow_types_mask) == 0) {
> > + if ((rss_conf.rss_hf & pf->adapter->flow_types_mask) == 0 ||
> > + !(mq_mode & ETH_MQ_RX_RSS_FLAG)) {
> > i40e_pf_disable_rss(pf);
> > return 0;
> > }
> > @@ -9198,16 +9200,7 @@ i40e_tunnel_filter_handle(struct rte_eth_dev
> > *dev, static int i40e_pf_config_mq_rx(struct i40e_pf *pf) {
> > - int ret = 0;
> > - enum rte_eth_rx_mq_mode mq_mode = pf->dev_data-
> > >dev_conf.rxmode.mq_mode;
> > -
> > - /* RSS setup */
> > - if (mq_mode & ETH_MQ_RX_RSS_FLAG)
> > - ret = i40e_pf_config_rss(pf);
> > - else
> > - i40e_pf_disable_rss(pf);
> > -
> > - return ret;
> > + return i40e_pf_config_rss(pf);
>
> If only have one function call in this function, we can delete it, and just
> use
> i40e_pf_config_rss(pf) instead of i40e_pf_config_mq_rx.
Got it. Thanks for your review.
Thanks.
Shougang
Re: [dpdk-dev] [PATCH v2] net/i40e: fix link status
Tested-by: Shougang Wang
> -Original Message-
> From: dev On Behalf Of Guinan Sun
> Sent: Thursday, July 30, 2020 6:25 PM
> To: Xing, Beilei ; Guo, Jia ;
> dev@dpdk.org
> Cc: Sun, GuinanX ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] net/i40e: fix link status
>
> If the PF driver supports the new speed reporting capabilities then use
> link_event_adv instead of link_event to get the speed.
>
> Fixes: 2a73125b7041 ("i40evf: fix link info update")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Guinan Sun
> ---
> v2:
> * Modify commit log.
> * Add code comments.
> * Delete useless codes.
> ---
> drivers/net/i40e/base/virtchnl.h | 16 +++-
> drivers/net/i40e/i40e_ethdev_vf.c | 42
> +--
> 2 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/i40e/base/virtchnl.h
> b/drivers/net/i40e/base/virtchnl.h
> index 4f498ca45..9c64fd469 100644
> --- a/drivers/net/i40e/base/virtchnl.h
> +++ b/drivers/net/i40e/base/virtchnl.h
> @@ -240,7 +240,8 @@ VIRTCHNL_CHECK_STRUCT_LEN(16,
> virtchnl_vsi_resource);
> #define VIRTCHNL_VF_OFFLOAD_ENCAP0X0010
> #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM 0X0020
> #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM0X0040
> -
> +/* Define below the capability flags that are not offloads */
> +#define VIRTCHNL_VF_CAP_ADV_LINK_SPEED 0x0080
> #define VF_BASE_MODE_OFFLOADS (VIRTCHNL_VF_OFFLOAD_L2 | \
> VIRTCHNL_VF_OFFLOAD_VLAN | \
> VIRTCHNL_VF_OFFLOAD_RSS_PF)
> @@ -536,10 +537,23 @@ enum virtchnl_event_codes { struct
> virtchnl_pf_event {
> enum virtchnl_event_codes event;
> union {
> + /* If the PF driver does not support the new speed reporting
> + * capabilities then use link_event else use link_event_adv to
> + * get the speed and link information. The ability to
> understand
> + * new speeds is indicated by setting the capability flag
> + * VIRTCHNL_VF_CAP_ADV_LINK_SPEED in vf_cap_flags
> parameter
> + * in virtchnl_vf_resource struct and can be used to
> determine
> + * which link event struct to use below.
> + */
> struct {
> enum virtchnl_link_speed link_speed;
> bool link_status;
> } link_event;
> + struct {
> + /* link_speed provided in Mbps */
> + u32 link_speed;
> + u8 link_status;
> + } link_event_adv;
> } event_data;
>
> int severity;
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 69cab8e73..f8cf45fbe 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -1386,8 +1386,46 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev,
> uint8_t *msg,
> break;
> case VIRTCHNL_EVENT_LINK_CHANGE:
> PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event");
> - vf->link_up = pf_msg->event_data.link_event.link_status;
> - vf->link_speed = pf_msg-
> >event_data.link_event.link_speed;
> +
> + if (vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> + vf->link_up =
> + pf_msg-
> >event_data.link_event_adv.link_status;
> +
> + switch (pf_msg-
> >event_data.link_event_adv.link_speed) {
> + case ETH_SPEED_NUM_100M:
> + vf->link_speed = I40E_LINK_SPEED_100MB;
> + break;
> + case ETH_SPEED_NUM_1G:
> + vf->link_speed = I40E_LINK_SPEED_1GB;
> + break;
> + case ETH_SPEED_NUM_2_5G:
> + vf->link_speed = I40E_LINK_SPEED_2_5GB;
> + break;
> + case ETH_SPEED_NUM_5G:
> + vf->link_speed = I40E_LINK_SPEED_5GB;
> + break;
> + case ETH_SPEED_NUM_10G:
> + vf->link_speed = I40E_LINK_SPEED_10GB;
> + break;
> + case ETH_SPEED_NUM_20G:
> + vf->link_speed = I40E_LINK_SPEED_20GB;
> + break;
> + case ETH_SPEED_NUM_25G:
> + vf->link_speed = I40E_LINK_SPEED_25GB;
> + break;
> + case ETH_SPEED_NUM_40G:
> + vf->link_speed = I40E_LINK_SPEED_40GB;
> + break;
> + default:
> + vf->link_speed =
> I40E_LINK_SPEED_UNKNOWN;
> + break;
> + }
> +
Re: [dpdk-dev] [PATCH v2] net/i40e: fix link status
Hi Beilei
> -Original Message-
> From: dev On Behalf Of Xing, Beilei
> Sent: Friday, July 31, 2020 10:30 AM
> To: Sun, GuinanX ; Guo, Jia ;
> dev@dpdk.org
> Cc: Sun, GuinanX ; sta...@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix link status
>
>
>
> > -Original Message-
> > From: dev On Behalf Of Guinan Sun
> > Sent: Thursday, July 30, 2020 6:25 PM
> > To: Xing, Beilei ; Guo, Jia
> > ; dev@dpdk.org
> > Cc: Sun, GuinanX ; sta...@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] net/i40e: fix link status
> >
> > If the PF driver supports the new speed reporting capabilities then
> > use link_event_adv instead of link_event to get the speed.
> >
> > Fixes: 2a73125b7041 ("i40evf: fix link info update")
> > Cc: sta...@dpdk.org
> >
> > Signed-off-by: Guinan Sun
> > ---
> > v2:
> > * Modify commit log.
> > * Add code comments.
> > * Delete useless codes.
> > ---
> > drivers/net/i40e/base/virtchnl.h | 16 +++-
> > drivers/net/i40e/i40e_ethdev_vf.c | 42
> +--
> > 2 files changed, 55 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/base/virtchnl.h
> > b/drivers/net/i40e/base/virtchnl.h
> > index 4f498ca45..9c64fd469 100644
> > --- a/drivers/net/i40e/base/virtchnl.h
> > +++ b/drivers/net/i40e/base/virtchnl.h
> > @@ -240,7 +240,8 @@ VIRTCHNL_CHECK_STRUCT_LEN(16,
> > virtchnl_vsi_resource);
> > #define VIRTCHNL_VF_OFFLOAD_ENCAP 0X0010
> > #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM 0X0020
> > #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM 0X0040
> > -
> > +/* Define below the capability flags that are not offloads */
> > +#define VIRTCHNL_VF_CAP_ADV_LINK_SPEED 0x0080
>
> You defined the new capability VIRTCHNL_VF_CAP_ADV_LINK_SPEED in the
> patch, but didn't request the capability for i40evf?
This capability is given by i40e kernel pf if VIRTCHNL_VF_CAP_ADV_LINK_SPEED is
defined. So vf doesn't need to request it.
Thanks.
Shougang
Re: [dpdk-dev] [PATCH v3] net/i40e: fix link status
Tested-by: Shougang Wang
Thanks.
Shougang
> -Original Message-
> From: dev On Behalf Of Guinan Sun
> Sent: Thursday, August 6, 2020 4:17 PM
> To: dev@dpdk.org
> Cc: Xing, Beilei ; Guo, Jia ; Sun,
> GuinanX ; sta...@dpdk.org
> Subject: [dpdk-dev] [PATCH v3] net/i40e: fix link status
>
> If the PF driver supports the new speed reporting capabilities then use
> link_event_adv instead of link_event to get the speed.
>
> Fixes: 2a73125b7041 ("i40evf: fix link info update")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Guinan Sun
> ---
> v3:
> * request the capability for i40evf
> v2:
> * modify commit log
> * add code comments
> * delete useless code
> ---
> drivers/net/i40e/base/virtchnl.h | 16 ++-
> drivers/net/i40e/i40e_ethdev_vf.c | 45
> ---
> 2 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/i40e/base/virtchnl.h
> b/drivers/net/i40e/base/virtchnl.h
> index 4f498ca45..9c64fd469 100644
> --- a/drivers/net/i40e/base/virtchnl.h
> +++ b/drivers/net/i40e/base/virtchnl.h
> @@ -240,7 +240,8 @@ VIRTCHNL_CHECK_STRUCT_LEN(16,
> virtchnl_vsi_resource);
> #define VIRTCHNL_VF_OFFLOAD_ENCAP0X0010
> #define VIRTCHNL_VF_OFFLOAD_ENCAP_CSUM 0X0020
> #define VIRTCHNL_VF_OFFLOAD_RX_ENCAP_CSUM0X0040
> -
> +/* Define below the capability flags that are not offloads */
> +#define VIRTCHNL_VF_CAP_ADV_LINK_SPEED 0x0080
> #define VF_BASE_MODE_OFFLOADS (VIRTCHNL_VF_OFFLOAD_L2 | \
> VIRTCHNL_VF_OFFLOAD_VLAN | \
> VIRTCHNL_VF_OFFLOAD_RSS_PF)
> @@ -536,10 +537,23 @@ enum virtchnl_event_codes { struct
> virtchnl_pf_event {
> enum virtchnl_event_codes event;
> union {
> + /* If the PF driver does not support the new speed reporting
> + * capabilities then use link_event else use link_event_adv to
> + * get the speed and link information. The ability to
> understand
> + * new speeds is indicated by setting the capability flag
> + * VIRTCHNL_VF_CAP_ADV_LINK_SPEED in vf_cap_flags
> parameter
> + * in virtchnl_vf_resource struct and can be used to
> determine
> + * which link event struct to use below.
> + */
> struct {
> enum virtchnl_link_speed link_speed;
> bool link_status;
> } link_event;
> + struct {
> + /* link_speed provided in Mbps */
> + u32 link_speed;
> + u8 link_status;
> + } link_event_adv;
> } event_data;
>
> int severity;
> diff --git a/drivers/net/i40e/i40e_ethdev_vf.c
> b/drivers/net/i40e/i40e_ethdev_vf.c
> index 69cab8e73..ccf5d8c57 100644
> --- a/drivers/net/i40e/i40e_ethdev_vf.c
> +++ b/drivers/net/i40e/i40e_ethdev_vf.c
> @@ -469,7 +469,8 @@ i40evf_get_vf_resource(struct rte_eth_dev *dev)
> VIRTCHNL_VF_OFFLOAD_RSS_AQ |
> VIRTCHNL_VF_OFFLOAD_RSS_REG |
> VIRTCHNL_VF_OFFLOAD_VLAN |
> -VIRTCHNL_VF_OFFLOAD_RX_POLLING;
> +VIRTCHNL_VF_OFFLOAD_RX_POLLING |
> +VIRTCHNL_VF_CAP_ADV_LINK_SPEED;
> args.in_args = (uint8_t *)∩︀
> args.in_args_size = sizeof(caps);
> } else {
> @@ -1386,8 +1387,46 @@ i40evf_handle_pf_event(struct rte_eth_dev *dev,
> uint8_t *msg,
> break;
> case VIRTCHNL_EVENT_LINK_CHANGE:
> PMD_DRV_LOG(DEBUG, "VIRTCHNL_EVENT_LINK_CHANGE
> event");
> - vf->link_up = pf_msg->event_data.link_event.link_status;
> - vf->link_speed = pf_msg-
> >event_data.link_event.link_speed;
> +
> + if (vf->vf_res->vf_cap_flags &
> VIRTCHNL_VF_CAP_ADV_LINK_SPEED) {
> + vf->link_up =
> + pf_msg-
> >event_data.link_event_adv.link_status;
> +
> + switch (pf_msg-
> >event_data.link_event_adv.link_speed) {
> + case ETH_SPEED_NUM_100M:
> + vf->link_speed = I40E_LINK_SPEED_100MB;
> + break;
> + case ETH_SPEED_NUM_1G:
> + vf->link_speed = I40E_LINK_SPEED_1GB;
> + break;
> + case ETH_SPEED_NUM_2_5G:
> + vf->link_speed = I40E_LINK_SPEED_2_5GB;
> + break;
> + case ETH_SPEED_NUM_5G:
> + vf->link_speed = I40E_LINK_SPEED_5GB;
> + break;
> + case ETH_SPEED_NUM_10G:
> + vf->link_speed = I40E_LINK_SPEED_10GB;
> + break;
> + case ETH_SPEED_NUM_20G:
> + vf->link_speed = I4
Re: [dpdk-dev] [PATCH] net/ice: fix incorrect firmware version
> -Original Message-
> From: Yang, Qiming
> Sent: Thursday, August 6, 2020 5:16 PM
> To: Wang, ShougangX ; dev@dpdk.org
> Cc: Zhang, Qi Z ; sta...@dpdk.org
> Subject: RE: [PATCH] net/ice: fix incorrect firmware version
>
>
>
> > -Original Message-
> > From: Wang, ShougangX
> > Sent: Thursday, August 6, 2020 16:25
> > To: dev@dpdk.org
> > Cc: Yang, Qiming ; Zhang, Qi Z
> > ; Wang, ShougangX ;
> > sta...@dpdk.org
> > Subject: [PATCH] net/ice: fix incorrect firmware version
> >
> > Kernel driver shows firmware version as hex but ice PMD shows as decimal.
> > This patch fixes the issue to make consistent with kernel driver.
> >
> > Fixes: ac882a0eda69 ("net/ice/base: store NVM version in extracted
> > format")
>
> Fix line not correct, should be the code you fixed
OK, I will correct it.
Thanks
Shougang
Re: [dpdk-dev] 19.11.4 patches review and test
Hi, Luca and Bo
> > - The MD5 is not same between kernel ethtool and dpdk ethtool when
> > testing userspace_ethtool/retrieve_eeprom. It has been fixed on
> > 20.08.patch link: http://patches.dpdk.org/patch/72354/
>
> Shougang and Qi, this patch applies when backported but build fails, as some
> internal structures are different. Do you consider this issue a blocking one
> for
> the release and want to do the work to backport the fixes to 19.11?
>
This patch can't be applied because the solution depends on updating ice shared
code.
I'm not sure the workload and impact of updating shared code. It is a small bug
and it is
not a DPDK main feature. So I think it is not a block issue for release and I
don't suggest
to fix it in 19.11. How do you think of it?
> 01186263c2c7 ("net/ice: fix EEPROM data")
>
> ../drivers/net/ice/ice_ethdev.c: In function ‘ice_get_eeprom_length’:
> ../drivers/net/ice/ice_ethdev.c:3979:16: error: ‘struct ice_nvm_info’
> has no member named ‘flash_size’
> return hw->nvm.flash_size;
Thanks.
Shougang
> -Original Message-
> From: Luca Boccassi
> Sent: Wednesday, August 26, 2020 5:50 PM
> To: Wang, ShougangX ; Burakov, Anatoly
> ; Zhang, Qi Z
> Cc: dev@dpdk.org; sta...@dpdk.org; Chen, BoX C
> Subject: Re: [dpdk-dev] 19.11.4 patches review and test
>
> On Wed, 2020-08-26 at 02:30 +, Chen, BoX C wrote:
> > Hi Luca,
> > Update LTS 19.11.4 test result for Intel part. No new issue is found except
> known issues.
>
> Thank you!
>
> > * Intel(R) Testing
> >
> > # Basic Intel(R) NIC testing
> > * PF(i40e):Passed
> > - Exception message when starting testpmd for testing external
> > memory. It has been fixed on 20.05.patch link:
> > http://patches.dpdk.org/patch/66041/
>
> Anatoly, is it safe to backport this patch to 19.11? It applies and builds
> cleanly
> when cherry-picked:
>
> d1c7c0cdf7ba ("vfio: map contiguous areas in one go")
>
> > * PF(ixgbe):Passed
> > * PF(ice):Passed
> > - The MD5 is not same between kernel ethtool and dpdk ethtool when
> > testing userspace_ethtool/retrieve_eeprom. It has been fixed on
> > 20.08.patch link: http://patches.dpdk.org/patch/72354/
>
> Shougang and Qi, this patch applies when backported but build fails, as some
> internal structures are different. Do you consider this issue a blocking one
> for
> the release and want to do the work to backport the fixes to 19.11?
>
> 01186263c2c7 ("net/ice: fix EEPROM data")
>
> ../drivers/net/ice/ice_ethdev.c: In function ‘ice_get_eeprom_length’:
> ../drivers/net/ice/ice_ethdev.c:3979:16: error: ‘struct ice_nvm_info’
> has no member named ‘flash_size’
> return hw->nvm.flash_size;
>
> > - With latest ice driver and firmware package exception found when port
> reset vf of testpmd. It is also found on latest dpdk version, dev is
> debugging.
> > * VF(i40e):Passed
> > * VF(ixgbe):Passed
> > * VF(ice):Passed
> > * Build or compile: Passed
> > * Intel NIC single core/NIC performance: Passed
> >
> > #Basic cryptodev and virtio testing
> > * vhost/virtio basic loopback, PVP and performance test: Passed.
> > - udp-fragmentation-offload can't be setup on vm Ubuntu1910.it is
> kernel issue, tracked on: https://bugzilla.kernel.org/show_bug.cgi?id=207075
> > - l3fwd-power can wake up lcore but can not sleep again. It is also
> > found
> on 20.08, dev is debugging.
> > * cryptodev Function: Passed.
> > - fips_cryptodev test failed for TDES. It is also found on 20.08,
> > dev is debugging. https://bugs.dpdk.org/show_bug.cgi?id=512
> > * cryptodev Performance: Passed.
> > - known unstable issue of test case 1c1t 3CPM. not effect LTS release.
> >
> > Thanks.
> > Regards,
> > Chen Bo
> >
> > > -Original Message-
> > > From: dev On Behalf Of Luca Boccassi
> > > Sent: August 19, 2020 2:12
> > > To: sta...@dpdk.org
> > > Cc: dev@dpdk.org; Abhishek Marathe
> ;
> > > Akhil Goyal ; Ali Alnubani
> > > ; Walker, Benjamin
> > > ; David Christensen
> > > ; Hemant Agrawal
> ;
> > > Stokes, Ian ; Jerin Jacob
> > > ; Mcnamara, John ;
> > > Ju-Hyoung Lee ; Kevin Traynor
> > > ; Pei Zhang ; Yu, PingX
> > > ; Xu, Qian Q ; Raslan
> > > Darawsheh ; Thomas Monjalon
> > > ; Peng, Yuan ; Chen,
> > > Zhaoyan
> > > Subject: [dpdk-dev] 19.11.4 patches review and test
> > >
> > > Hi all,
> > >
> > > Here is a list of patc
