[ovs-dev] [PATCH V3 02/19] netdev-offload-dpdk: Refactor action items freeing scheme
Ilya Maximets
i.maximets at ovn.org
Tue Dec 10 14:09:07 UTC 2019
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 at 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;
>>> }
>>>
>>>
More information about the dev
mailing list