On 10/1/2021 5:52 PM, Eelco Chaudron wrote:
See some small comments inline.
On 15 Sep 2021, at 14:43, 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 <c...@nvidia.com>
Reviewed-by: Eli Britstein <el...@nvidia.com>
---
lib/automake.mk | 2 ++
lib/dpif-netdev.c | 1 +
lib/dpif-netlink.c | 2 ++
lib/dpif-offload-provider.h | 47 +++++++++++++++++++++++++++++++++++++
lib/dpif-offload.c | 43 +++++++++++++++++++++++++++++++++
lib/dpif-provider.h | 8 ++++++-
lib/dpif.c | 10 ++++++++
7 files changed, 112 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 46f869a33..9259f57de 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -127,6 +127,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 4cd0e9ebd..1645b44b0 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -8801,6 +8801,7 @@ const struct dpif_class dpif_netdev_class = {
dpif_netdev_bond_add,
dpif_netdev_bond_del,
dpif_netdev_bond_stats_get,
+ NULL, /* dpif_offload_api */
};
static void
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 34fc04237..f546b48e5 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"
@@ -4340,6 +4341,7 @@ const struct dpif_class dpif_netlink_class = {
NULL, /* bond_add */
NULL, /* bond_del */
NULL, /* bond_stats_get */
+ NULL, /* dpif_offload_api */
};
static int
diff --git a/lib/dpif-offload-provider.h b/lib/dpif-offload-provider.h
new file mode 100644
index 000000000..75238c4cb
--- /dev/null
+++ b/lib/dpif-offload-provider.h
@@ -0,0 +1,47 @@
+/*
+ * 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;
+
+/* Datapath interface offload structure, to be defined by each implementation
+ * of a datapath interface.
+ */
+struct dpif_offload_api {
+ /* Called when the dpif provider is registered and right after dpif
+ * provider init function. */
+ void (*init)(void);
| EC> Do we want to think ahead and allow init to fail? Other init callbacks
return int.
| CM> We don't want to fail the whole offload if dpif offload fails to init. So
we don't check the return value.
Why? In theory, if dpif offload fails, all offload would fill as we could miss
vital functions? I think we should add it and check for it (even in the current
partial implementation can’t fail).
If kernel disables CONFIG_PSAMPLE, dpif offload init will fail. If
return error for it, that means we can't use any offload.
I think that's a regression. Customer will not be happy with it if they
upgrade OVS.
+
+ /* Free all dpif offload resources. */
+ void (*destroy)(void);
+
+ /* Arranges for the poll loop for an upcall handler to wake up when psample
+ * has a message queued to be received. */
+ void (*sflow_recv_wait)(void);
+
+ /* Polls for an upcall from psample for an upcall handler.
+ * Return 0 for success. */
+ 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 7e11b9697..ce20cdeb1 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
@@ -635,6 +636,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;
| EC> 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?
| CM> OK.
Guess we are still waiting for Ilya’s feedback on this…
Ilya,
If you have time, could you please take a look if this new design is ok?
Thanks,
Chris
};
extern const struct dpif_class dpif_netlink_class;
diff --git a/lib/dpif.c b/lib/dpif.c
index 8c4aed47b..13a3cd0d4 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->destroy) {
+ dpif_class->offload_api->destroy();
+ }
+
shash_delete(&dpif_classes, node);
free(registered_class);
--
2.27.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
