* [dpdk-dev] [PATCH v2] net/ice: remove unnecessary variable
@ 2020-03-03 4:18 Qi Zhang
2020-03-03 8:27 ` Ye Xiaolong
0 siblings, 1 reply; 3+ messages in thread
From: Qi Zhang @ 2020-03-03 4:18 UTC (permalink / raw)
To: xiaolong.ye, yahui.cao, simei.su, wei.zhao; +Cc: dev, Qi Zhang, stable
Remove unnecessary variable "meta" in ice_flow_create and
ice_flow_validate, it should be defined when really be needed:
its ice_parse_engine_create and ice_parse_engine_validate.
In a validate opertion, a meta is not necessary be created during
parser, so NULL will be parsed to low level engine and all engine's
parse_pattern_action need to be modified properly.
The the patch also fix a potentional memory leak in
ice_parse_engine_validate, since meta may not be freed.
Fixes: 4e27d3ed02bd ("net/ice: fix flow API framework")
Cc: stable@dpdk.org
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
v2:
- remove meta free to low level engine
- allow parse meta = NULL to parse_pattern_action.
drivers/net/ice/ice_fdir_filter.c | 3 ++-
drivers/net/ice/ice_generic_flow.c | 24 ++++++++++--------------
drivers/net/ice/ice_hash.c | 13 +++++++------
drivers/net/ice/ice_switch_filter.c | 14 +++++++-------
4 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
index 5a791610f..6342b560c 100644
--- a/drivers/net/ice/ice_fdir_filter.c
+++ b/drivers/net/ice/ice_fdir_filter.c
@@ -1966,7 +1966,8 @@ ice_fdir_parse(struct ice_adapter *ad,
if (ret)
goto error;
- *meta = filter;
+ if (meta)
+ *meta = filter;
error:
rte_free(item);
return ret;
diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
index 38ac799d8..6bcf28de8 100644
--- a/drivers/net/ice/ice_generic_flow.c
+++ b/drivers/net/ice/ice_generic_flow.c
@@ -1375,7 +1375,6 @@ typedef struct ice_flow_engine * (*parse_engine_t)(struct ice_adapter *ad,
struct ice_parser_list *parser_list,
const struct rte_flow_item pattern[],
const struct rte_flow_action actions[],
- void **meta,
struct rte_flow_error *error);
void
@@ -1713,11 +1712,11 @@ ice_parse_engine_create(struct ice_adapter *ad,
struct ice_parser_list *parser_list,
const struct rte_flow_item pattern[],
const struct rte_flow_action actions[],
- void **meta,
struct rte_flow_error *error)
{
struct ice_flow_engine *engine = NULL;
struct ice_flow_parser_node *parser_node;
+ void *meta = NULL;
void *temp;
TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
@@ -1726,7 +1725,7 @@ ice_parse_engine_create(struct ice_adapter *ad,
if (parser_node->parser->parse_pattern_action(ad,
parser_node->parser->array,
parser_node->parser->array_len,
- pattern, actions, meta, error) < 0)
+ pattern, actions, &meta, error) < 0)
continue;
engine = parser_node->parser->engine;
@@ -1737,7 +1736,7 @@ ice_parse_engine_create(struct ice_adapter *ad,
continue;
}
- ret = engine->create(ad, flow, *meta, error);
+ ret = engine->create(ad, flow, meta, error);
if (ret == 0)
return engine;
else if (ret == -EEXIST)
@@ -1752,7 +1751,6 @@ ice_parse_engine_validate(struct ice_adapter *ad,
struct ice_parser_list *parser_list,
const struct rte_flow_item pattern[],
const struct rte_flow_action actions[],
- void **meta,
struct rte_flow_error *error)
{
struct ice_flow_engine *engine = NULL;
@@ -1763,12 +1761,13 @@ ice_parse_engine_validate(struct ice_adapter *ad,
if (parser_node->parser->parse_pattern_action(ad,
parser_node->parser->array,
parser_node->parser->array_len,
- pattern, actions, meta, error) < 0)
+ pattern, actions, NULL, error) < 0)
continue;
engine = parser_node->parser->engine;
break;
}
+
return engine;
}
@@ -1779,7 +1778,6 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
const struct rte_flow_item pattern[],
const struct rte_flow_action actions[],
struct ice_flow_engine **engine,
- void **meta,
parse_engine_t ice_parse_engine,
struct rte_flow_error *error)
{
@@ -1814,7 +1812,7 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
return ret;
*engine = ice_parse_engine(ad, flow, &pf->rss_parser_list,
- pattern, actions, meta, error);
+ pattern, actions, error);
if (*engine != NULL)
return 0;
@@ -1822,11 +1820,11 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR_ONLY:
case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR:
*engine = ice_parse_engine(ad, flow, &pf->dist_parser_list,
- pattern, actions, meta, error);
+ pattern, actions, error);
break;
case ICE_FLOW_CLASSIFY_STAGE_PERMISSION:
*engine = ice_parse_engine(ad, flow, &pf->perm_parser_list,
- pattern, actions, meta, error);
+ pattern, actions, error);
break;
default:
return -EINVAL;
@@ -1845,11 +1843,10 @@ ice_flow_validate(struct rte_eth_dev *dev,
const struct rte_flow_action actions[],
struct rte_flow_error *error)
{
- void *meta;
struct ice_flow_engine *engine;
return ice_flow_process_filter(dev, NULL, attr, pattern, actions,
- &engine, &meta, ice_parse_engine_validate, error);
+ &engine, ice_parse_engine_validate, error);
}
static struct rte_flow *
@@ -1863,7 +1860,6 @@ ice_flow_create(struct rte_eth_dev *dev,
struct rte_flow *flow = NULL;
int ret;
struct ice_flow_engine *engine = NULL;
- void *meta;
flow = rte_zmalloc("ice_flow", sizeof(struct rte_flow), 0);
if (!flow) {
@@ -1874,7 +1870,7 @@ ice_flow_create(struct rte_eth_dev *dev,
}
ret = ice_flow_process_filter(dev, flow, attr, pattern, actions,
- &engine, &meta, ice_parse_engine_create, error);
+ &engine, ice_parse_engine_create, error);
if (ret < 0)
goto free_flow;
flow->engine = engine;
diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
index d891538bd..b37da9920 100644
--- a/drivers/net/ice/ice_hash.c
+++ b/drivers/net/ice/ice_hash.c
@@ -432,14 +432,17 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
goto error;
/* Save protocol header to rss_meta. */
- *meta = rss_meta_ptr;
- ((struct rss_meta *)*meta)->pkt_hdr = ((struct rss_type_match_hdr *)
+ rss_meta_ptr->pkt_hdr = ((struct rss_type_match_hdr *)
(pattern_match_item->meta))->hdr_mask;
/* Check rss action. */
- ret = ice_hash_parse_action(pattern_match_item, actions, meta, error);
+ ret = ice_hash_parse_action(pattern_match_item, actions,
+ (void **)&rss_meta_ptr, error);
+
error:
- if (ret)
+ if (!ret && meta)
+ *meta = rss_meta_ptr;
+ else
rte_free(rss_meta_ptr);
rte_free(pattern_match_item);
@@ -503,12 +506,10 @@ ice_hash_create(struct ice_adapter *ad,
out:
flow->rule = filter_ptr;
- rte_free(meta);
return 0;
error:
rte_free(filter_ptr);
- rte_free(meta);
return -rte_errno;
}
diff --git a/drivers/net/ice/ice_switch_filter.c b/drivers/net/ice/ice_switch_filter.c
index 4a9356b31..8a2ba8aa6 100644
--- a/drivers/net/ice/ice_switch_filter.c
+++ b/drivers/net/ice/ice_switch_filter.c
@@ -249,13 +249,10 @@ ice_switch_create(struct ice_adapter *ad,
}
rte_free(list);
- rte_free(meta);
return 0;
error:
rte_free(list);
- rte_free(meta);
-
return -rte_errno;
}
@@ -1088,10 +1085,13 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad,
"Invalid input action");
goto error;
}
- *meta = sw_meta_ptr;
- ((struct sw_meta *)*meta)->list = list;
- ((struct sw_meta *)*meta)->lkups_num = lkups_num;
- ((struct sw_meta *)*meta)->rule_info = rule_info;
+
+ if (meta) {
+ *meta = sw_meta_ptr;
+ ((struct sw_meta *)*meta)->list = list;
+ ((struct sw_meta *)*meta)->lkups_num = lkups_num;
+ ((struct sw_meta *)*meta)->rule_info = rule_info;
+ }
rte_free(pattern_match_item);
return 0;
--
2.13.6
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/ice: remove unnecessary variable
2020-03-03 4:18 [dpdk-dev] [PATCH v2] net/ice: remove unnecessary variable Qi Zhang
@ 2020-03-03 8:27 ` Ye Xiaolong
2020-03-03 8:35 ` Zhang, Qi Z
0 siblings, 1 reply; 3+ messages in thread
From: Ye Xiaolong @ 2020-03-03 8:27 UTC (permalink / raw)
To: Qi Zhang; +Cc: yahui.cao, simei.su, wei.zhao, dev, stable
Hi, Qi
On 03/03, Qi Zhang wrote:
>Remove unnecessary variable "meta" in ice_flow_create and
>ice_flow_validate, it should be defined when really be needed:
>its ice_parse_engine_create and ice_parse_engine_validate.
>
>In a validate opertion, a meta is not necessary be created during
>parser, so NULL will be parsed to low level engine and all engine's
>parse_pattern_action need to be modified properly.
>
>The the patch also fix a potentional memory leak in
s/fix/fixes
>ice_parse_engine_validate, since meta may not be freed.
>
>Fixes: 4e27d3ed02bd ("net/ice: fix flow API framework")
>Cc: stable@dpdk.org
>
>Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>---
>
>v2:
>- remove meta free to low level engine
>- allow parse meta = NULL to parse_pattern_action.
>
> drivers/net/ice/ice_fdir_filter.c | 3 ++-
> drivers/net/ice/ice_generic_flow.c | 24 ++++++++++--------------
> drivers/net/ice/ice_hash.c | 13 +++++++------
> drivers/net/ice/ice_switch_filter.c | 14 +++++++-------
> 4 files changed, 26 insertions(+), 28 deletions(-)
>
>diff --git a/drivers/net/ice/ice_fdir_filter.c b/drivers/net/ice/ice_fdir_filter.c
>index 5a791610f..6342b560c 100644
>--- a/drivers/net/ice/ice_fdir_filter.c
>+++ b/drivers/net/ice/ice_fdir_filter.c
>@@ -1966,7 +1966,8 @@ ice_fdir_parse(struct ice_adapter *ad,
> if (ret)
> goto error;
>
>- *meta = filter;
>+ if (meta)
>+ *meta = filter;
> error:
> rte_free(item);
> return ret;
>diff --git a/drivers/net/ice/ice_generic_flow.c b/drivers/net/ice/ice_generic_flow.c
>index 38ac799d8..6bcf28de8 100644
>--- a/drivers/net/ice/ice_generic_flow.c
>+++ b/drivers/net/ice/ice_generic_flow.c
>@@ -1375,7 +1375,6 @@ typedef struct ice_flow_engine * (*parse_engine_t)(struct ice_adapter *ad,
> struct ice_parser_list *parser_list,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
>- void **meta,
> struct rte_flow_error *error);
>
> void
>@@ -1713,11 +1712,11 @@ ice_parse_engine_create(struct ice_adapter *ad,
> struct ice_parser_list *parser_list,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
>- void **meta,
> struct rte_flow_error *error)
> {
> struct ice_flow_engine *engine = NULL;
> struct ice_flow_parser_node *parser_node;
>+ void *meta = NULL;
> void *temp;
>
> TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) {
>@@ -1726,7 +1725,7 @@ ice_parse_engine_create(struct ice_adapter *ad,
> if (parser_node->parser->parse_pattern_action(ad,
> parser_node->parser->array,
> parser_node->parser->array_len,
>- pattern, actions, meta, error) < 0)
>+ pattern, actions, &meta, error) < 0)
> continue;
>
> engine = parser_node->parser->engine;
>@@ -1737,7 +1736,7 @@ ice_parse_engine_create(struct ice_adapter *ad,
> continue;
> }
>
>- ret = engine->create(ad, flow, *meta, error);
>+ ret = engine->create(ad, flow, meta, error);
> if (ret == 0)
> return engine;
> else if (ret == -EEXIST)
>@@ -1752,7 +1751,6 @@ ice_parse_engine_validate(struct ice_adapter *ad,
> struct ice_parser_list *parser_list,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
>- void **meta,
> struct rte_flow_error *error)
> {
> struct ice_flow_engine *engine = NULL;
>@@ -1763,12 +1761,13 @@ ice_parse_engine_validate(struct ice_adapter *ad,
> if (parser_node->parser->parse_pattern_action(ad,
> parser_node->parser->array,
> parser_node->parser->array_len,
>- pattern, actions, meta, error) < 0)
>+ pattern, actions, NULL, error) < 0)
> continue;
>
> engine = parser_node->parser->engine;
> break;
> }
>+
Irrelevant change.
> return engine;
> }
>
>@@ -1779,7 +1778,6 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
> const struct rte_flow_item pattern[],
> const struct rte_flow_action actions[],
> struct ice_flow_engine **engine,
>- void **meta,
> parse_engine_t ice_parse_engine,
> struct rte_flow_error *error)
> {
>@@ -1814,7 +1812,7 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
> return ret;
>
> *engine = ice_parse_engine(ad, flow, &pf->rss_parser_list,
>- pattern, actions, meta, error);
>+ pattern, actions, error);
> if (*engine != NULL)
> return 0;
>
>@@ -1822,11 +1820,11 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
> case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR_ONLY:
> case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR:
> *engine = ice_parse_engine(ad, flow, &pf->dist_parser_list,
>- pattern, actions, meta, error);
>+ pattern, actions, error);
> break;
> case ICE_FLOW_CLASSIFY_STAGE_PERMISSION:
> *engine = ice_parse_engine(ad, flow, &pf->perm_parser_list,
>- pattern, actions, meta, error);
>+ pattern, actions, error);
> break;
> default:
> return -EINVAL;
>@@ -1845,11 +1843,10 @@ ice_flow_validate(struct rte_eth_dev *dev,
> const struct rte_flow_action actions[],
> struct rte_flow_error *error)
> {
>- void *meta;
> struct ice_flow_engine *engine;
>
> return ice_flow_process_filter(dev, NULL, attr, pattern, actions,
>- &engine, &meta, ice_parse_engine_validate, error);
>+ &engine, ice_parse_engine_validate, error);
> }
>
> static struct rte_flow *
>@@ -1863,7 +1860,6 @@ ice_flow_create(struct rte_eth_dev *dev,
> struct rte_flow *flow = NULL;
> int ret;
> struct ice_flow_engine *engine = NULL;
>- void *meta;
>
> flow = rte_zmalloc("ice_flow", sizeof(struct rte_flow), 0);
> if (!flow) {
>@@ -1874,7 +1870,7 @@ ice_flow_create(struct rte_eth_dev *dev,
> }
>
> ret = ice_flow_process_filter(dev, flow, attr, pattern, actions,
>- &engine, &meta, ice_parse_engine_create, error);
>+ &engine, ice_parse_engine_create, error);
> if (ret < 0)
> goto free_flow;
> flow->engine = engine;
>diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
>index d891538bd..b37da9920 100644
>--- a/drivers/net/ice/ice_hash.c
>+++ b/drivers/net/ice/ice_hash.c
>@@ -432,14 +432,17 @@ ice_hash_parse_pattern_action(__rte_unused struct ice_adapter *ad,
> goto error;
>
> /* Save protocol header to rss_meta. */
>- *meta = rss_meta_ptr;
>- ((struct rss_meta *)*meta)->pkt_hdr = ((struct rss_type_match_hdr *)
>+ rss_meta_ptr->pkt_hdr = ((struct rss_type_match_hdr *)
> (pattern_match_item->meta))->hdr_mask;
>
> /* Check rss action. */
>- ret = ice_hash_parse_action(pattern_match_item, actions, meta, error);
>+ ret = ice_hash_parse_action(pattern_match_item, actions,
>+ (void **)&rss_meta_ptr, error);
>+
> error:
>- if (ret)
>+ if (!ret && meta)
>+ *meta = rss_meta_ptr;
>+ else
> rte_free(rss_meta_ptr);
> rte_free(pattern_match_item);
>
>@@ -503,12 +506,10 @@ ice_hash_create(struct ice_adapter *ad,
>
> out:
> flow->rule = filter_ptr;
>- rte_free(meta);
> return 0;
>
> error:
> rte_free(filter_ptr);
>- rte_free(meta);
> return -rte_errno;
> }
>
>diff --git a/drivers/net/ice/ice_switch_filter.c b/drivers/net/ice/ice_switch_filter.c
>index 4a9356b31..8a2ba8aa6 100644
>--- a/drivers/net/ice/ice_switch_filter.c
>+++ b/drivers/net/ice/ice_switch_filter.c
>@@ -249,13 +249,10 @@ ice_switch_create(struct ice_adapter *ad,
> }
>
> rte_free(list);
>- rte_free(meta);
> return 0;
>
> error:
> rte_free(list);
>- rte_free(meta);
>-
> return -rte_errno;
> }
>
>@@ -1088,10 +1085,13 @@ ice_switch_parse_pattern_action(struct ice_adapter *ad,
> "Invalid input action");
> goto error;
> }
>- *meta = sw_meta_ptr;
>- ((struct sw_meta *)*meta)->list = list;
>- ((struct sw_meta *)*meta)->lkups_num = lkups_num;
>- ((struct sw_meta *)*meta)->rule_info = rule_info;
>+
>+ if (meta) {
>+ *meta = sw_meta_ptr;
>+ ((struct sw_meta *)*meta)->list = list;
>+ ((struct sw_meta *)*meta)->lkups_num = lkups_num;
>+ ((struct sw_meta *)*meta)->rule_info = rule_info;
>+ }
If meta is NULL for ice_parse_engine_validate case, do we need free sw_meta_ptr
explicitly in switch like we free(rss_meta_ptr) in hash? And also free the list?
And for case meta is not NULL, who is responsible for freeing it eventually?
Thanks,
Xiaolong
> rte_free(pattern_match_item);
>
> return 0;
>--
>2.13.6
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/ice: remove unnecessary variable
2020-03-03 8:27 ` Ye Xiaolong
@ 2020-03-03 8:35 ` Zhang, Qi Z
0 siblings, 0 replies; 3+ messages in thread
From: Zhang, Qi Z @ 2020-03-03 8:35 UTC (permalink / raw)
To: Ye, Xiaolong; +Cc: Cao, Yahui, Su, Simei, Zhao, Wei, dev, stable
> -----Original Message-----
> From: Ye, Xiaolong <xiaolong.ye@intel.com>
> Sent: Tuesday, March 3, 2020 4:27 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Cao, Yahui <yahui.cao@intel.com>; Su, Simei <simei.su@intel.com>; Zhao,
> Wei <wei.zhao@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH v2] net/ice: remove unnecessary variable
>
> Hi, Qi
>
> On 03/03, Qi Zhang wrote:
> >Remove unnecessary variable "meta" in ice_flow_create and
> >ice_flow_validate, it should be defined when really be needed:
> >its ice_parse_engine_create and ice_parse_engine_validate.
> >
> >In a validate opertion, a meta is not necessary be created during
> >parser, so NULL will be parsed to low level engine and all engine's
> >parse_pattern_action need to be modified properly.
> >
> >The the patch also fix a potentional memory leak in
>
> s/fix/fixes
>
> >ice_parse_engine_validate, since meta may not be freed.
> >
> >Fixes: 4e27d3ed02bd ("net/ice: fix flow API framework")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> >---
> >
> >v2:
> >- remove meta free to low level engine
> >- allow parse meta = NULL to parse_pattern_action.
> >
> > drivers/net/ice/ice_fdir_filter.c | 3 ++-
> > drivers/net/ice/ice_generic_flow.c | 24 ++++++++++--------------
> > drivers/net/ice/ice_hash.c | 13 +++++++------
> > drivers/net/ice/ice_switch_filter.c | 14 +++++++-------
> > 4 files changed, 26 insertions(+), 28 deletions(-)
> >
> >diff --git a/drivers/net/ice/ice_fdir_filter.c
> >b/drivers/net/ice/ice_fdir_filter.c
> >index 5a791610f..6342b560c 100644
> >--- a/drivers/net/ice/ice_fdir_filter.c
> >+++ b/drivers/net/ice/ice_fdir_filter.c
> >@@ -1966,7 +1966,8 @@ ice_fdir_parse(struct ice_adapter *ad,
> > if (ret)
> > goto error;
> >
> >- *meta = filter;
> >+ if (meta)
> >+ *meta = filter;
> > error:
> > rte_free(item);
> > return ret;
> >diff --git a/drivers/net/ice/ice_generic_flow.c
> >b/drivers/net/ice/ice_generic_flow.c
> >index 38ac799d8..6bcf28de8 100644
> >--- a/drivers/net/ice/ice_generic_flow.c
> >+++ b/drivers/net/ice/ice_generic_flow.c
> >@@ -1375,7 +1375,6 @@ typedef struct ice_flow_engine *
> (*parse_engine_t)(struct ice_adapter *ad,
> > struct ice_parser_list *parser_list,
> > const struct rte_flow_item pattern[],
> > const struct rte_flow_action actions[],
> >- void **meta,
> > struct rte_flow_error *error);
> >
> > void
> >@@ -1713,11 +1712,11 @@ ice_parse_engine_create(struct ice_adapter *ad,
> > struct ice_parser_list *parser_list,
> > const struct rte_flow_item pattern[],
> > const struct rte_flow_action actions[],
> >- void **meta,
> > struct rte_flow_error *error)
> > {
> > struct ice_flow_engine *engine = NULL;
> > struct ice_flow_parser_node *parser_node;
> >+ void *meta = NULL;
> > void *temp;
> >
> > TAILQ_FOREACH_SAFE(parser_node, parser_list, node, temp) { @@
> -1726,7
> >+1725,7 @@ ice_parse_engine_create(struct ice_adapter *ad,
> > if (parser_node->parser->parse_pattern_action(ad,
> > parser_node->parser->array,
> > parser_node->parser->array_len,
> >- pattern, actions, meta, error) < 0)
> >+ pattern, actions, &meta, error) < 0)
> > continue;
> >
> > engine = parser_node->parser->engine; @@ -1737,7 +1736,7 @@
> >ice_parse_engine_create(struct ice_adapter *ad,
> > continue;
> > }
> >
> >- ret = engine->create(ad, flow, *meta, error);
> >+ ret = engine->create(ad, flow, meta, error);
> > if (ret == 0)
> > return engine;
> > else if (ret == -EEXIST)
> >@@ -1752,7 +1751,6 @@ ice_parse_engine_validate(struct ice_adapter *ad,
> > struct ice_parser_list *parser_list,
> > const struct rte_flow_item pattern[],
> > const struct rte_flow_action actions[],
> >- void **meta,
> > struct rte_flow_error *error)
> > {
> > struct ice_flow_engine *engine = NULL; @@ -1763,12 +1761,13 @@
> >ice_parse_engine_validate(struct ice_adapter *ad,
> > if (parser_node->parser->parse_pattern_action(ad,
> > parser_node->parser->array,
> > parser_node->parser->array_len,
> >- pattern, actions, meta, error) < 0)
> >+ pattern, actions, NULL, error) < 0)
> > continue;
> >
> > engine = parser_node->parser->engine;
> > break;
> > }
> >+
>
> Irrelevant change.
>
> > return engine;
> > }
> >
> >@@ -1779,7 +1778,6 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
> > const struct rte_flow_item pattern[],
> > const struct rte_flow_action actions[],
> > struct ice_flow_engine **engine,
> >- void **meta,
> > parse_engine_t ice_parse_engine,
> > struct rte_flow_error *error)
> > {
> >@@ -1814,7 +1812,7 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
> > return ret;
> >
> > *engine = ice_parse_engine(ad, flow, &pf->rss_parser_list,
> >- pattern, actions, meta, error);
> >+ pattern, actions, error);
> > if (*engine != NULL)
> > return 0;
> >
> >@@ -1822,11 +1820,11 @@ ice_flow_process_filter(struct rte_eth_dev *dev,
> > case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR_ONLY:
> > case ICE_FLOW_CLASSIFY_STAGE_DISTRIBUTOR:
> > *engine = ice_parse_engine(ad, flow, &pf->dist_parser_list,
> >- pattern, actions, meta, error);
> >+ pattern, actions, error);
> > break;
> > case ICE_FLOW_CLASSIFY_STAGE_PERMISSION:
> > *engine = ice_parse_engine(ad, flow, &pf->perm_parser_list,
> >- pattern, actions, meta, error);
> >+ pattern, actions, error);
> > break;
> > default:
> > return -EINVAL;
> >@@ -1845,11 +1843,10 @@ ice_flow_validate(struct rte_eth_dev *dev,
> > const struct rte_flow_action actions[],
> > struct rte_flow_error *error)
> > {
> >- void *meta;
> > struct ice_flow_engine *engine;
> >
> > return ice_flow_process_filter(dev, NULL, attr, pattern, actions,
> >- &engine, &meta, ice_parse_engine_validate, error);
> >+ &engine, ice_parse_engine_validate, error);
> > }
> >
> > static struct rte_flow *
> >@@ -1863,7 +1860,6 @@ ice_flow_create(struct rte_eth_dev *dev,
> > struct rte_flow *flow = NULL;
> > int ret;
> > struct ice_flow_engine *engine = NULL;
> >- void *meta;
> >
> > flow = rte_zmalloc("ice_flow", sizeof(struct rte_flow), 0);
> > if (!flow) {
> >@@ -1874,7 +1870,7 @@ ice_flow_create(struct rte_eth_dev *dev,
> > }
> >
> > ret = ice_flow_process_filter(dev, flow, attr, pattern, actions,
> >- &engine, &meta, ice_parse_engine_create, error);
> >+ &engine, ice_parse_engine_create, error);
> > if (ret < 0)
> > goto free_flow;
> > flow->engine = engine;
> >diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
> >index d891538bd..b37da9920 100644
> >--- a/drivers/net/ice/ice_hash.c
> >+++ b/drivers/net/ice/ice_hash.c
> >@@ -432,14 +432,17 @@ ice_hash_parse_pattern_action(__rte_unused
> struct ice_adapter *ad,
> > goto error;
> >
> > /* Save protocol header to rss_meta. */
> >- *meta = rss_meta_ptr;
> >- ((struct rss_meta *)*meta)->pkt_hdr = ((struct rss_type_match_hdr *)
> >+ rss_meta_ptr->pkt_hdr = ((struct rss_type_match_hdr *)
> > (pattern_match_item->meta))->hdr_mask;
> >
> > /* Check rss action. */
> >- ret = ice_hash_parse_action(pattern_match_item, actions, meta, error);
> >+ ret = ice_hash_parse_action(pattern_match_item, actions,
> >+ (void **)&rss_meta_ptr, error);
> >+
> > error:
> >- if (ret)
> >+ if (!ret && meta)
> >+ *meta = rss_meta_ptr;
> >+ else
> > rte_free(rss_meta_ptr);
> > rte_free(pattern_match_item);
> >
> >@@ -503,12 +506,10 @@ ice_hash_create(struct ice_adapter *ad,
> >
> > out:
> > flow->rule = filter_ptr;
> >- rte_free(meta);
> > return 0;
> >
> > error:
> > rte_free(filter_ptr);
> >- rte_free(meta);
> > return -rte_errno;
> > }
> >
> >diff --git a/drivers/net/ice/ice_switch_filter.c
> >b/drivers/net/ice/ice_switch_filter.c
> >index 4a9356b31..8a2ba8aa6 100644
> >--- a/drivers/net/ice/ice_switch_filter.c
> >+++ b/drivers/net/ice/ice_switch_filter.c
> >@@ -249,13 +249,10 @@ ice_switch_create(struct ice_adapter *ad,
> > }
> >
> > rte_free(list);
> >- rte_free(meta);
> > return 0;
> >
> > error:
> > rte_free(list);
> >- rte_free(meta);
> >-
> > return -rte_errno;
> > }
> >
> >@@ -1088,10 +1085,13 @@ ice_switch_parse_pattern_action(struct
> ice_adapter *ad,
> > "Invalid input action");
> > goto error;
> > }
> >- *meta = sw_meta_ptr;
> >- ((struct sw_meta *)*meta)->list = list;
> >- ((struct sw_meta *)*meta)->lkups_num = lkups_num;
> >- ((struct sw_meta *)*meta)->rule_info = rule_info;
> >+
> >+ if (meta) {
> >+ *meta = sw_meta_ptr;
> >+ ((struct sw_meta *)*meta)->list = list;
> >+ ((struct sw_meta *)*meta)->lkups_num = lkups_num;
> >+ ((struct sw_meta *)*meta)->rule_info = rule_info;
> >+ }
>
> If meta is NULL for ice_parse_engine_validate case, do we need free
> sw_meta_ptr explicitly in switch like we free(rss_meta_ptr) in hash? And also
> free the list?
Yes, we need.
>
>
> And for case meta is not NULL, who is responsible for freeing it eventually?
Seems I forgot to rollback all the meta data free in create ops.
Thanks
Qi
>
> Thanks,
> Xiaolong
>
> > rte_free(pattern_match_item);
> >
> > return 0;
> >--
> >2.13.6
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-03 8:35 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-03 4:18 [dpdk-dev] [PATCH v2] net/ice: remove unnecessary variable Qi Zhang
2020-03-03 8:27 ` Ye Xiaolong
2020-03-03 8:35 ` Zhang, Qi Z
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).