[ovs-dev] [PATCH V2 11/14] netdev-offload-dpdk: Refactor offload rule creation
Eli Britstein
elibr at nvidia.com
Wed Feb 24 10:17:38 UTC 2021
On 2/24/2021 11:55 AM, Sriharsha Basavapatna wrote:
> On Wed, Feb 10, 2021 at 8:57 PM Eli Britstein <elibr at nvidia.com> wrote:
>> Refactor offload rule creation as a pre-step towards tunnel matching
>> that depend on the netdev.
>>
>> Signed-off-by: Eli Britstein <elibr at nvidia.com>
>> Reviewed-by: Gaetan Rivet <gaetanr at nvidia.com>
>> ---
>> lib/netdev-offload-dpdk.c | 106 ++++++++++++++++----------------------
>> 1 file changed, 45 insertions(+), 61 deletions(-)
>>
>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>> index 493cc9159..f6e668bff 100644
>> --- a/lib/netdev-offload-dpdk.c
>> +++ b/lib/netdev-offload-dpdk.c
>> @@ -1009,30 +1009,6 @@ add_flow_mark_rss_actions(struct flow_actions *actions,
>> add_flow_action(actions, RTE_FLOW_ACTION_TYPE_END, NULL);
>> }
>>
>> -static struct rte_flow *
>> -netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns,
>> - struct netdev *netdev,
>> - uint32_t flow_mark)
>> -{
>> - struct flow_actions actions = { .actions = NULL, .cnt = 0 };
>> - const struct rte_flow_attr flow_attr = {
>> - .group = 0,
>> - .priority = 0,
>> - .ingress = 1,
>> - .egress = 0
>> - };
>> - struct rte_flow_error error;
>> - struct rte_flow *flow;
>> -
>> - add_flow_mark_rss_actions(&actions, flow_mark, netdev);
>> -
>> - flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
>> - &actions, &error);
>> -
>> - free_flow_actions(&actions);
>> - return flow;
>> -}
>> -
>> static void
>> add_count_action(struct flow_actions *actions)
>> {
>> @@ -1509,27 +1485,48 @@ parse_flow_actions(struct netdev *netdev,
>> return 0;
>> }
>>
>> -static struct rte_flow *
>> -netdev_offload_dpdk_actions(struct netdev *netdev,
>> - struct flow_patterns *patterns,
>> - struct nlattr *nl_actions,
>> - size_t actions_len)
>> +static struct ufid_to_rte_flow_data *
>> +create_netdev_offload(struct netdev *netdev,
>> + const ovs_u128 *ufid,
>> + struct flow_patterns *flow_patterns,
>> + struct flow_actions *flow_actions,
>> + bool enable_full,
>> + uint32_t flow_mark)
>> {
>> - const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 };
>> - struct flow_actions actions = { .actions = NULL, .cnt = 0 };
>> + struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1, };
>> + struct flow_actions rss_actions = { .actions = NULL, .cnt = 0 };
>> + struct rte_flow_item *items = flow_patterns->items;
>> + struct ufid_to_rte_flow_data *flow_data = NULL;
>> + bool actions_offloaded = true;
>> struct rte_flow *flow = NULL;
>> struct rte_flow_error error;
>> - int ret;
>>
>> - ret = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
>> - if (ret) {
>> - goto out;
>> + if (enable_full) {
>> + flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
>> + flow_actions, &error);
>> }
>> - flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, patterns->items,
>> - &actions, &error);
>> -out:
>> - free_flow_actions(&actions);
>> - return flow;
>> +
>> + if (!flow) {
>> + /* If we failed to offload the rule actions fallback to MARK+RSS
>> + * actions.
>> + */
> A debug message might be useful here, when we fallback to mark/rss action ?
We can, but this is just a refactor commit, and this fallback is not
new. Add it anyway?
>> + actions_offloaded = false;
>> + flow_attr.transfer = 0;
>> + add_flow_mark_rss_actions(&rss_actions, flow_mark, netdev);
>> + flow = netdev_offload_dpdk_flow_create(netdev, &flow_attr, items,
>> + &rss_actions, &error);
>> + }
>> +
>> + if (flow) {
>> + flow_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
>> + actions_offloaded);
>> + VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
>> + netdev_get_name(netdev), flow,
>> + UUID_ARGS((struct uuid *) ufid));
>> + }
>> +
>> + free_flow_actions(&rss_actions);
> This free is needed only when we fallback to mark/rss offload, not otherwise.
OK.
>
>> + return flow_data;
>> }
>>
>> static struct ufid_to_rte_flow_data *
>> @@ -1541,9 +1538,9 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
>> struct offload_info *info)
>> {
>> struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
>> - struct ufid_to_rte_flow_data *flows_data = NULL;
>> - bool actions_offloaded = true;
>> - struct rte_flow *flow;
>> + struct flow_actions actions = { .actions = NULL, .cnt = 0 };
>> + struct ufid_to_rte_flow_data *flow_data = NULL;
>> + int err;
>>
>> if (parse_flow_match(&patterns, match)) {
>> VLOG_DBG_RL(&rl, "%s: matches of ufid "UUID_FMT" are not supported",
>> @@ -1551,28 +1548,15 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
>> goto out;
>> }
>>
>> - flow = netdev_offload_dpdk_actions(netdev, &patterns, nl_actions,
>> - actions_len);
>> - if (!flow) {
>> - /* If we failed to offload the rule actions fallback to MARK+RSS
>> - * actions.
>> - */
>> - flow = netdev_offload_dpdk_mark_rss(&patterns, netdev,
>> - info->flow_mark);
>> - actions_offloaded = false;
>> - }
>> + err = parse_flow_actions(netdev, &actions, nl_actions, actions_len);
>>
>> - if (!flow) {
>> - goto out;
>> - }
>> - flows_data = ufid_to_rte_flow_associate(ufid, netdev, flow,
>> - actions_offloaded);
>> - VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
>> - netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid));
>> + flow_data = create_netdev_offload(netdev, ufid, &patterns, &actions, !err,
>> + info->flow_mark);
>>
>> out:
>> free_flow_patterns(&patterns);
>> - return flows_data;
>> + free_flow_actions(&actions);
>> + return flow_data;
>> }
>>
>> static int
>> --
>> 2.28.0.546.g385c171
>>
More information about the dev
mailing list