| Message ID | 20191208132304.15521-3-elibr@mellanox.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | netdev datapath actions offload | expand |
On 08.12.2019 14:22, Eli Britstein wrote: > Action item data structures are pointed by rte_flow_action items. > Refactor the code to free the data structures when freeing the > rte_flow_action items, allowing simpler future actions simpler to add to > the code. > > Signed-off-by: Eli Britstein <elibr@mellanox.com> > --- > lib/netdev-offload-dpdk.c | 92 ++++++++++++++++++++++++++--------------------- > 1 file changed, 52 insertions(+), 40 deletions(-) > > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c > index a008e642f..c3b595a0a 100644 > --- a/lib/netdev-offload-dpdk.c > +++ b/lib/netdev-offload-dpdk.c > @@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type, > actions->cnt++; > } > > -struct action_rss_data { > - struct rte_flow_action_rss conf; > - uint16_t queue[0]; > -}; > - > -static struct action_rss_data * > -add_flow_rss_action(struct flow_actions *actions, > - struct netdev *netdev) > +static void > +free_flow_actions(struct flow_actions *actions) > { > int i; > - struct action_rss_data *rss_data; > - > - rss_data = xmalloc(sizeof *rss_data + > - netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); > - *rss_data = (struct action_rss_data) { > - .conf = (struct rte_flow_action_rss) { > - .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > - .level = 0, > - .types = 0, > - .queue_num = netdev_n_rxq(netdev), > - .queue = rss_data->queue, > - .key_len = 0, > - .key = NULL > - }, > - }; > > - /* Override queue array with default. */ > - for (i = 0; i < netdev_n_rxq(netdev); i++) { > - rss_data->queue[i] = i; > + for (i = 0; i < actions->cnt; i++) { > + if (actions->actions[i].conf) { > + free((void *)actions->actions[i].conf); Please, don't cast the argument. > + } > } > - > - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); > - > - return rss_data; > + free(actions->actions); > + actions->actions = NULL; > + actions->cnt = 0; > } > > struct flow_items { > @@ -572,6 +551,47 @@ parse_flow_match(struct flow_patterns *patterns, > return 0; > } > > +static void > +add_flow_mark_rss_actions(struct flow_actions *actions, > + uint32_t flow_mark, > + struct netdev *netdev) const struct netdev *netdev) > +{ > + struct rte_flow_action_mark *mark; > + struct action_rss_data { > + struct rte_flow_action_rss conf; > + uint16_t queue[0]; > + } *rss_data; It seems we need this: BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0); > + int i; > + > + mark = xzalloc(sizeof *mark); > + > + mark->id = flow_mark; > + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); > + > + rss_data = xmalloc(sizeof *rss_data + > + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); > + *rss_data = (struct action_rss_data) { > + .conf = (struct rte_flow_action_rss) { > + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > + .level = 0, > + .types = 0, > + .queue_num = netdev_n_rxq(netdev), > + .queue = rss_data->queue, > + .key_len = 0, > + .key = NULL > + }, > + }; > + > + /* Override queue array with default. */ > + for (i = 0; i < netdev_n_rxq(netdev); i++) { > + rss_data->queue[i] = i; > + } > + > + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); > + This empty line doesn't seem to be necessary. > + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL); > +} > + > static int > netdev_offload_dpdk_add_flow(struct netdev *netdev, > const struct match *match, > @@ -599,20 +619,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > goto out; > } > > - struct rte_flow_action_mark mark; > - struct action_rss_data *rss; > - > - mark.id = info->flow_mark; > - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); > - > - rss = add_flow_rss_action(&actions, netdev); > - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); > + add_flow_mark_rss_actions(&actions, info->flow_mark, netdev); > > flow = netdev_dpdk_rte_flow_create(netdev, &flow_attr, > patterns.items, > actions.actions, &error); > > - free(rss); > if (!flow) { > VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", > netdev_get_name(netdev), error.type, error.message); > @@ -625,7 +637,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, > > out: > free(patterns.items); > - free(actions.actions); > + free_flow_actions(&actions); > return ret; > } > >
On 12/10/2019 3:43 PM, Ilya Maximets wrote: > On 08.12.2019 14:22, Eli Britstein wrote: >> Action item data structures are pointed by rte_flow_action items. >> Refactor the code to free the data structures when freeing the >> rte_flow_action items, allowing simpler future actions simpler to add to >> the code. >> >> Signed-off-by: Eli Britstein <elibr@mellanox.com> >> --- >> lib/netdev-offload-dpdk.c | 92 ++++++++++++++++++++++++++--------------------- >> 1 file changed, 52 insertions(+), 40 deletions(-) >> >> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >> index a008e642f..c3b595a0a 100644 >> --- a/lib/netdev-offload-dpdk.c >> +++ b/lib/netdev-offload-dpdk.c >> @@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type, >> actions->cnt++; >> } >> >> -struct action_rss_data { >> - struct rte_flow_action_rss conf; >> - uint16_t queue[0]; >> -}; >> - >> -static struct action_rss_data * >> -add_flow_rss_action(struct flow_actions *actions, >> - struct netdev *netdev) >> +static void >> +free_flow_actions(struct flow_actions *actions) >> { >> int i; >> - struct action_rss_data *rss_data; >> - >> - rss_data = xmalloc(sizeof *rss_data + >> - netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); >> - *rss_data = (struct action_rss_data) { >> - .conf = (struct rte_flow_action_rss) { >> - .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >> - .level = 0, >> - .types = 0, >> - .queue_num = netdev_n_rxq(netdev), >> - .queue = rss_data->queue, >> - .key_len = 0, >> - .key = NULL >> - }, >> - }; >> >> - /* Override queue array with default. */ >> - for (i = 0; i < netdev_n_rxq(netdev); i++) { >> - rss_data->queue[i] = i; >> + for (i = 0; i < actions->cnt; i++) { >> + if (actions->actions[i].conf) { >> + free((void *)actions->actions[i].conf); > Please, don't cast the argument. The conf field is declared as "const void *" in DPDK. How can we avoid it? > >> + } >> } >> - >> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); >> - >> - return rss_data; >> + free(actions->actions); >> + actions->actions = NULL; >> + actions->cnt = 0; >> } >> >> struct flow_items { >> @@ -572,6 +551,47 @@ parse_flow_match(struct flow_patterns *patterns, >> return 0; >> } >> >> +static void >> +add_flow_mark_rss_actions(struct flow_actions *actions, >> + uint32_t flow_mark, >> + struct netdev *netdev) > const struct netdev *netdev) OK > >> +{ >> + struct rte_flow_action_mark *mark; >> + struct action_rss_data { >> + struct rte_flow_action_rss conf; >> + uint16_t queue[0]; >> + } *rss_data; > It seems we need this: > > BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0); OK > >> + int i; >> + >> + mark = xzalloc(sizeof *mark); >> + >> + mark->id = flow_mark; >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); >> + >> + rss_data = xmalloc(sizeof *rss_data + >> + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); >> + *rss_data = (struct action_rss_data) { >> + .conf = (struct rte_flow_action_rss) { >> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >> + .level = 0, >> + .types = 0, >> + .queue_num = netdev_n_rxq(netdev), >> + .queue = rss_data->queue, >> + .key_len = 0, >> + .key = NULL >> + }, >> + }; >> + >> + /* Override queue array with default. */ >> + for (i = 0; i < netdev_n_rxq(netdev); i++) { >> + rss_data->queue[i] = i; >> + } >> + >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); >> + > This empty line doesn't seem to be necessary. OK > >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL); >> +} >> + >> static int >> netdev_offload_dpdk_add_flow(struct netdev *netdev, >> const struct match *match, >> @@ -599,20 +619,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >> goto out; >> } >> >> - struct rte_flow_action_mark mark; >> - struct action_rss_data *rss; >> - >> - mark.id = info->flow_mark; >> - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); >> - >> - rss = add_flow_rss_action(&actions, netdev); >> - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); >> + add_flow_mark_rss_actions(&actions, info->flow_mark, netdev); >> >> flow = netdev_dpdk_rte_flow_create(netdev, &flow_attr, >> patterns.items, >> actions.actions, &error); >> >> - free(rss); >> if (!flow) { >> VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", >> netdev_get_name(netdev), error.type, error.message); >> @@ -625,7 +637,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >> >> out: >> free(patterns.items); >> - free(actions.actions); >> + free_flow_actions(&actions); >> return ret; >> } >> >>
On 10.12.2019 15:06, Eli Britstein wrote: > > On 12/10/2019 3:43 PM, Ilya Maximets wrote: >> On 08.12.2019 14:22, Eli Britstein wrote: >>> Action item data structures are pointed by rte_flow_action items. >>> Refactor the code to free the data structures when freeing the >>> rte_flow_action items, allowing simpler future actions simpler to add to >>> the code. >>> >>> Signed-off-by: Eli Britstein <elibr@mellanox.com> >>> --- >>> lib/netdev-offload-dpdk.c | 92 ++++++++++++++++++++++++++--------------------- >>> 1 file changed, 52 insertions(+), 40 deletions(-) >>> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>> index a008e642f..c3b595a0a 100644 >>> --- a/lib/netdev-offload-dpdk.c >>> +++ b/lib/netdev-offload-dpdk.c >>> @@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type, >>> actions->cnt++; >>> } >>> >>> -struct action_rss_data { >>> - struct rte_flow_action_rss conf; >>> - uint16_t queue[0]; >>> -}; >>> - >>> -static struct action_rss_data * >>> -add_flow_rss_action(struct flow_actions *actions, >>> - struct netdev *netdev) >>> +static void >>> +free_flow_actions(struct flow_actions *actions) >>> { >>> int i; >>> - struct action_rss_data *rss_data; >>> - >>> - rss_data = xmalloc(sizeof *rss_data + >>> - netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); >>> - *rss_data = (struct action_rss_data) { >>> - .conf = (struct rte_flow_action_rss) { >>> - .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>> - .level = 0, >>> - .types = 0, >>> - .queue_num = netdev_n_rxq(netdev), >>> - .queue = rss_data->queue, >>> - .key_len = 0, >>> - .key = NULL >>> - }, >>> - }; >>> >>> - /* Override queue array with default. */ >>> - for (i = 0; i < netdev_n_rxq(netdev); i++) { >>> - rss_data->queue[i] = i; >>> + for (i = 0; i < actions->cnt; i++) { >>> + if (actions->actions[i].conf) { >>> + free((void *)actions->actions[i].conf); >> Please, don't cast the argument. > The conf field is declared as "const void *" in DPDK. How can we avoid it? If it's here to remove the 'const', use explicit CONST_CAST instead. >> >>> + } >>> } >>> - >>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); >>> - >>> - return rss_data; >>> + free(actions->actions); >>> + actions->actions = NULL; >>> + actions->cnt = 0; >>> } >>> >>> struct flow_items { >>> @@ -572,6 +551,47 @@ parse_flow_match(struct flow_patterns *patterns, >>> return 0; >>> } >>> >>> +static void >>> +add_flow_mark_rss_actions(struct flow_actions *actions, >>> + uint32_t flow_mark, >>> + struct netdev *netdev) >> const struct netdev *netdev) > OK >> >>> +{ >>> + struct rte_flow_action_mark *mark; >>> + struct action_rss_data { >>> + struct rte_flow_action_rss conf; >>> + uint16_t queue[0]; >>> + } *rss_data; >> It seems we need this: >> >> BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0); > OK >> >>> + int i; >>> + >>> + mark = xzalloc(sizeof *mark); >>> + >>> + mark->id = flow_mark; >>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); >>> + >>> + rss_data = xmalloc(sizeof *rss_data + >>> + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); >>> + *rss_data = (struct action_rss_data) { >>> + .conf = (struct rte_flow_action_rss) { >>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>> + .level = 0, >>> + .types = 0, >>> + .queue_num = netdev_n_rxq(netdev), >>> + .queue = rss_data->queue, >>> + .key_len = 0, >>> + .key = NULL >>> + }, >>> + }; >>> + >>> + /* Override queue array with default. */ >>> + for (i = 0; i < netdev_n_rxq(netdev); i++) { >>> + rss_data->queue[i] = i; >>> + } >>> + >>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); >>> + >> This empty line doesn't seem to be necessary. > OK >> >>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL); >>> +} >>> + >>> static int >>> netdev_offload_dpdk_add_flow(struct netdev *netdev, >>> const struct match *match, >>> @@ -599,20 +619,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >>> goto out; >>> } >>> >>> - struct rte_flow_action_mark mark; >>> - struct action_rss_data *rss; >>> - >>> - mark.id = info->flow_mark; >>> - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); >>> - >>> - rss = add_flow_rss_action(&actions, netdev); >>> - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); >>> + add_flow_mark_rss_actions(&actions, info->flow_mark, netdev); >>> >>> flow = netdev_dpdk_rte_flow_create(netdev, &flow_attr, >>> patterns.items, >>> actions.actions, &error); >>> >>> - free(rss); >>> if (!flow) { >>> VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", >>> netdev_get_name(netdev), error.type, error.message); >>> @@ -625,7 +637,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >>> >>> out: >>> free(patterns.items); >>> - free(actions.actions); >>> + free_flow_actions(&actions); >>> return ret; >>> } >>> >>>
On 12/10/2019 4:09 PM, Ilya Maximets wrote: > On 10.12.2019 15:06, Eli Britstein wrote: >> On 12/10/2019 3:43 PM, Ilya Maximets wrote: >>> On 08.12.2019 14:22, Eli Britstein wrote: >>>> Action item data structures are pointed by rte_flow_action items. >>>> Refactor the code to free the data structures when freeing the >>>> rte_flow_action items, allowing simpler future actions simpler to add to >>>> the code. >>>> >>>> Signed-off-by: Eli Britstein <elibr@mellanox.com> >>>> --- >>>> lib/netdev-offload-dpdk.c | 92 ++++++++++++++++++++++++++--------------------- >>>> 1 file changed, 52 insertions(+), 40 deletions(-) >>>> >>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c >>>> index a008e642f..c3b595a0a 100644 >>>> --- a/lib/netdev-offload-dpdk.c >>>> +++ b/lib/netdev-offload-dpdk.c >>>> @@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type, >>>> actions->cnt++; >>>> } >>>> >>>> -struct action_rss_data { >>>> - struct rte_flow_action_rss conf; >>>> - uint16_t queue[0]; >>>> -}; >>>> - >>>> -static struct action_rss_data * >>>> -add_flow_rss_action(struct flow_actions *actions, >>>> - struct netdev *netdev) >>>> +static void >>>> +free_flow_actions(struct flow_actions *actions) >>>> { >>>> int i; >>>> - struct action_rss_data *rss_data; >>>> - >>>> - rss_data = xmalloc(sizeof *rss_data + >>>> - netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); >>>> - *rss_data = (struct action_rss_data) { >>>> - .conf = (struct rte_flow_action_rss) { >>>> - .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>>> - .level = 0, >>>> - .types = 0, >>>> - .queue_num = netdev_n_rxq(netdev), >>>> - .queue = rss_data->queue, >>>> - .key_len = 0, >>>> - .key = NULL >>>> - }, >>>> - }; >>>> >>>> - /* Override queue array with default. */ >>>> - for (i = 0; i < netdev_n_rxq(netdev); i++) { >>>> - rss_data->queue[i] = i; >>>> + for (i = 0; i < actions->cnt; i++) { >>>> + if (actions->actions[i].conf) { >>>> + free((void *)actions->actions[i].conf); >>> Please, don't cast the argument. >> The conf field is declared as "const void *" in DPDK. How can we avoid it? > If it's here to remove the 'const', use explicit CONST_CAST instead. OK > >>>> + } >>>> } >>>> - >>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); >>>> - >>>> - return rss_data; >>>> + free(actions->actions); >>>> + actions->actions = NULL; >>>> + actions->cnt = 0; >>>> } >>>> >>>> struct flow_items { >>>> @@ -572,6 +551,47 @@ parse_flow_match(struct flow_patterns *patterns, >>>> return 0; >>>> } >>>> >>>> +static void >>>> +add_flow_mark_rss_actions(struct flow_actions *actions, >>>> + uint32_t flow_mark, >>>> + struct netdev *netdev) >>> const struct netdev *netdev) >> OK >>>> +{ >>>> + struct rte_flow_action_mark *mark; >>>> + struct action_rss_data { >>>> + struct rte_flow_action_rss conf; >>>> + uint16_t queue[0]; >>>> + } *rss_data; >>> It seems we need this: >>> >>> BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0); >> OK >>>> + int i; >>>> + >>>> + mark = xzalloc(sizeof *mark); >>>> + >>>> + mark->id = flow_mark; >>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); >>>> + >>>> + rss_data = xmalloc(sizeof *rss_data + >>>> + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); >>>> + *rss_data = (struct action_rss_data) { >>>> + .conf = (struct rte_flow_action_rss) { >>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>>> + .level = 0, >>>> + .types = 0, >>>> + .queue_num = netdev_n_rxq(netdev), >>>> + .queue = rss_data->queue, >>>> + .key_len = 0, >>>> + .key = NULL >>>> + }, >>>> + }; >>>> + >>>> + /* Override queue array with default. */ >>>> + for (i = 0; i < netdev_n_rxq(netdev); i++) { >>>> + rss_data->queue[i] = i; >>>> + } >>>> + >>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); >>>> + >>> This empty line doesn't seem to be necessary. >> OK >>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL); >>>> +} >>>> + >>>> static int >>>> netdev_offload_dpdk_add_flow(struct netdev *netdev, >>>> const struct match *match, >>>> @@ -599,20 +619,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >>>> goto out; >>>> } >>>> >>>> - struct rte_flow_action_mark mark; >>>> - struct action_rss_data *rss; >>>> - >>>> - mark.id = info->flow_mark; >>>> - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); >>>> - >>>> - rss = add_flow_rss_action(&actions, netdev); >>>> - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); >>>> + add_flow_mark_rss_actions(&actions, info->flow_mark, netdev); >>>> >>>> flow = netdev_dpdk_rte_flow_create(netdev, &flow_attr, >>>> patterns.items, >>>> actions.actions, &error); >>>> >>>> - free(rss); >>>> if (!flow) { >>>> VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", >>>> netdev_get_name(netdev), error.type, error.message); >>>> @@ -625,7 +637,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, >>>> >>>> out: >>>> free(patterns.items); >>>> - free(actions.actions); >>>> + free_flow_actions(&actions); >>>> return ret; >>>> } >>>> >>>>
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index a008e642f..c3b595a0a 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -374,40 +374,19 @@ add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type, actions->cnt++; } -struct action_rss_data { - struct rte_flow_action_rss conf; - uint16_t queue[0]; -}; - -static struct action_rss_data * -add_flow_rss_action(struct flow_actions *actions, - struct netdev *netdev) +static void +free_flow_actions(struct flow_actions *actions) { int i; - struct action_rss_data *rss_data; - - rss_data = xmalloc(sizeof *rss_data + - netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); - *rss_data = (struct action_rss_data) { - .conf = (struct rte_flow_action_rss) { - .func = RTE_ETH_HASH_FUNCTION_DEFAULT, - .level = 0, - .types = 0, - .queue_num = netdev_n_rxq(netdev), - .queue = rss_data->queue, - .key_len = 0, - .key = NULL - }, - }; - /* Override queue array with default. */ - for (i = 0; i < netdev_n_rxq(netdev); i++) { - rss_data->queue[i] = i; + for (i = 0; i < actions->cnt; i++) { + if (actions->actions[i].conf) { + free((void *)actions->actions[i].conf); + } } - - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); - - return rss_data; + free(actions->actions); + actions->actions = NULL; + actions->cnt = 0; } struct flow_items { @@ -572,6 +551,47 @@ parse_flow_match(struct flow_patterns *patterns, return 0; } +static void +add_flow_mark_rss_actions(struct flow_actions *actions, + uint32_t flow_mark, + struct netdev *netdev) +{ + struct rte_flow_action_mark *mark; + struct action_rss_data { + struct rte_flow_action_rss conf; + uint16_t queue[0]; + } *rss_data; + int i; + + mark = xzalloc(sizeof *mark); + + mark->id = flow_mark; + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); + + rss_data = xmalloc(sizeof *rss_data + + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); + *rss_data = (struct action_rss_data) { + .conf = (struct rte_flow_action_rss) { + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, + .level = 0, + .types = 0, + .queue_num = netdev_n_rxq(netdev), + .queue = rss_data->queue, + .key_len = 0, + .key = NULL + }, + }; + + /* Override queue array with default. */ + for (i = 0; i < netdev_n_rxq(netdev); i++) { + rss_data->queue[i] = i; + } + + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); + + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL); +} + static int netdev_offload_dpdk_add_flow(struct netdev *netdev, const struct match *match, @@ -599,20 +619,12 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, goto out; } - struct rte_flow_action_mark mark; - struct action_rss_data *rss; - - mark.id = info->flow_mark; - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); - - rss = add_flow_rss_action(&actions, netdev); - add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); + add_flow_mark_rss_actions(&actions, info->flow_mark, netdev); flow = netdev_dpdk_rte_flow_create(netdev, &flow_attr, patterns.items, actions.actions, &error); - free(rss); if (!flow) { VLOG_ERR("%s: rte flow creat error: %u : message : %s\n", netdev_get_name(netdev), error.type, error.message); @@ -625,7 +637,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev, out: free(patterns.items); - free(actions.actions); + free_flow_actions(&actions); return ret; }
Action item data structures are pointed by rte_flow_action items. Refactor the code to free the data structures when freeing the rte_flow_action items, allowing simpler future actions simpler to add to the code. Signed-off-by: Eli Britstein <elibr@mellanox.com> --- lib/netdev-offload-dpdk.c | 92 ++++++++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 40 deletions(-)