[ovs-dev] [PATCH V3 02/19] netdev-offload-dpdk: Refactor action items freeing scheme
Eli Britstein
elibr at mellanox.com
Tue Dec 10 14:06:45 UTC 2019
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?
>
>> + }
>> }
>> -
>> - 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