[ovs-dev] [PATCH v14 3/7] dpif-offload-provider: Introduce dpif-offload-provider layer
Eelco Chaudron
echaudro at redhat.com
Mon Sep 6 10:56:14 UTC 2021
On 15 Jul 2021, at 8:01, Chris Mi wrote:
> Some offload actions require functionality that is not netdev
> based, but dpif. For example, sFlow action requires to create
> a psample netlink socket to receive the sampled packets from
> TC or kernel driver.
>
> Create dpif-offload-provider layer to support such actions.
>
> Signed-off-by: Chris Mi <cmi at nvidia.com>
> Reviewed-by: Eli Britstein <elibr at nvidia.com>
> ---
> lib/automake.mk | 2 ++
> lib/dpif-netdev.c | 1 +
> lib/dpif-netlink.c | 2 ++
> lib/dpif-offload-provider.h | 34 +++++++++++++++++++++++++++++
> lib/dpif-offload.c | 43 +++++++++++++++++++++++++++++++++++++
> lib/dpif-provider.h | 8 ++++++-
> lib/dpif.c | 10 +++++++++
> 7 files changed, 99 insertions(+), 1 deletion(-)
> create mode 100644 lib/dpif-offload-provider.h
> create mode 100644 lib/dpif-offload.c
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 3c9523c1a..dc865b0ef 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -123,6 +123,8 @@ lib_libopenvswitch_la_SOURCES = \
> lib/dpif-netdev-private.h \
> lib/dpif-netdev-perf.c \
> lib/dpif-netdev-perf.h \
> + lib/dpif-offload.c \
> + lib/dpif-offload-provider.h \
> lib/dpif-provider.h \
> lib/dpif.c \
> lib/dpif.h \
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 610949f36..35d73542b 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -8269,6 +8269,7 @@ const struct dpif_class dpif_netdev_class = {
> dpif_netdev_bond_add,
> dpif_netdev_bond_del,
> dpif_netdev_bond_stats_get,
> + NULL, /* dpif_offlod_api */
offlod -> offload
> };
>
> static void
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 39dc8300e..6a7defb95 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -34,6 +34,7 @@
>
> #include "bitmap.h"
> #include "dpif-netlink-rtnl.h"
> +#include "dpif-offload-provider.h"
> #include "dpif-provider.h"
> #include "fat-rwlock.h"
> #include "flow.h"
> @@ -4036,6 +4037,7 @@ const struct dpif_class dpif_netlink_class = {
> NULL, /* bond_add */
> NULL, /* bond_del */
> NULL, /* bond_stats_get */
> + NULL, /* dpif_offlod_api */
offlod -> offload
> };
>
> static int
> diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
> new file mode 100644
> index 000000000..97108402a
> --- /dev/null
> +++ b/lib/dpif-offload-provider.h
> @@ -0,0 +1,34 @@
> +/*
> + * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#ifndef DPIF_OFFLOAD_PROVIDER_H
> +#define DPIF_OFFLOAD_PROVIDER_H
> +
> +struct dpif;
> +struct dpif_offload_sflow;
> +
> +struct dpif_offload_api {
Can we add help details on each function below, preferably as in the netdev/dpif_class definitions? i.e., describe the function, its input parameters, and return value?
> + void (*init)(void);
Do we want to think ahead and allow init to fail? Other init callbacks return int.
> + void (*uninit)(void);
Don't really like the uninit() function name. I like destroy() as it sort of indicates we should no longer use this resource once called.
However, in other projects, they use deinit() a lot, but to me, it does not express the urgency to no use it after the call.
> + void (*sflow_recv_wait)(void);
> + int (*sflow_recv)(struct dpif_offload_sflow *sflow);
> +};
> +
> +void dpif_offload_sflow_recv_wait(const struct dpif *dpif);
> +int dpif_offload_sflow_recv(const struct dpif *dpif,
> + struct dpif_offload_sflow *sflow);
> +
> +#endif /* DPIF_OFFLOAD_PROVIDER_H */
> diff --git a/lib/dpif-offload.c b/lib/dpif-offload.c
> new file mode 100644
> index 000000000..842e05798
> --- /dev/null
> +++ b/lib/dpif-offload.c
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +#include <errno.h>
> +
> +#include "dpif-provider.h"
> +
> +void
> +dpif_offload_sflow_recv_wait(const struct dpif *dpif)
> +{
> + const struct dpif_offload_api *offload_api = dpif->dpif_class->offload_api;
> +
> + if (offload_api && offload_api->sflow_recv_wait) {
> + offload_api->sflow_recv_wait();
> + }
> +}
> +
> +int
> +dpif_offload_sflow_recv(const struct dpif *dpif,
> + struct dpif_offload_sflow *sflow)
> +{
> + const struct dpif_offload_api *offload_api = dpif->dpif_class->offload_api;
> +
> + if (offload_api && offload_api->sflow_recv) {
> + return offload_api->sflow_recv(sflow);
> + }
> +
> + return EOPNOTSUPP;
> +}
> diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
> index 59e0a3a9d..7fb93d69d 100644
> --- a/lib/dpif-provider.h
> +++ b/lib/dpif-provider.h
> @@ -22,8 +22,9 @@
> * exposed over OpenFlow as a single switch. Datapaths and the collections of
> * ports that they contain may be fixed or dynamic. */
>
> -#include "openflow/openflow.h"
> #include "dpif.h"
> +#include "dpif-offload-provider.h"
> +#include "openflow/openflow.h"
> #include "util.h"
>
> #ifdef __cplusplus
> @@ -633,6 +634,11 @@ struct dpif_class {
> * sufficient to store BOND_BUCKETS number of elements. */
> int (*bond_stats_get)(struct dpif *dpif, uint32_t bond_id,
> uint64_t *n_bytes);
> +
> + /* Some offload actions require functionality that is not netdev based,
> + * but dpif. Add dpif-offload-provider layer API to support such
> + * offload actions. */
> + const struct dpif_offload_api *offload_api;
Here you add the provider directly into the dpif class. Not sure if this is what Ilya had in mind. As in general, these get integrated into the dpif/netdev, not the class. Ilya can you comment on/review this?
> };
>
> extern const struct dpif_class dpif_netlink_class;
> diff --git a/lib/dpif.c b/lib/dpif.c
> index 26e8bfb7d..a0987988a 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -153,6 +153,10 @@ dp_register_provider__(const struct dpif_class *new_class)
> return error;
> }
>
> + if (new_class->offload_api && new_class->offload_api->init) {
> + new_class->offload_api->init();
> + }
> +
> registered_class = xmalloc(sizeof *registered_class);
> registered_class->dpif_class = new_class;
> registered_class->refcount = 0;
> @@ -183,6 +187,7 @@ static int
> dp_unregister_provider__(const char *type)
> {
> struct shash_node *node;
> + const struct dpif_class *dpif_class;
> struct registered_dpif_class *registered_class;
>
> node = shash_find(&dpif_classes, type);
> @@ -196,6 +201,11 @@ dp_unregister_provider__(const char *type)
> return EBUSY;
> }
>
> + dpif_class = registered_class->dpif_class;
> + if (dpif_class->offload_api && dpif_class->offload_api->uninit) {
> + dpif_class->offload_api->uninit();
> + }
> +
> shash_delete(&dpif_classes, node);
> free(registered_class);
>
> --
> 2.21.0
More information about the dev
mailing list