Hi Eelco,
Thanks for reviewing this patchset again. I will address them next week.
Thanks,
Chris
On 11/5/2021 8:18 PM, Eelco Chaudron wrote:
On 21 Oct 2021, at 10:00, 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 <[email protected]>
Reviewed-by: Eli Britstein <[email protected]>
---
lib/automake.mk | 2 ++
lib/dpif-offload-provider.h | 40 ++++++++++++++++++++++++++++++++++
lib/dpif-offload.c | 43 +++++++++++++++++++++++++++++++++++++
lib/dpif-provider.h | 4 +++-
4 files changed, 88 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-offload-provider.h b/lib/dpif-offload-provider.h
new file mode 100644
index 000000000..b5eb4ab04
--- /dev/null
+++ b/lib/dpif-offload-provider.h
@@ -0,0 +1,40 @@
+/*
+ * 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 {
+ /* 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);
+};
Not sure why you removed the init and destroy functions, as the offload API
should be like a class (where multiple could exist), and init/destroy should be
called when you register the offload class.
Guess more comments will be in patch 5.
+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..44e79882f
--- /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->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->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..15a0d2b63 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
@@ -35,6 +36,7 @@ extern "C" {
* This structure should be treated as opaque by dpif implementations. */
struct dpif {
const struct dpif_class *dpif_class;
+ const struct dpif_offload_api *offload_api;
char *base_name;
char *full_name;
uint8_t netflow_engine_type;
--
2.30.2
Adding the previous conversation about the API in general, so Ilya or who ever
reviews this can follow up:
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.
EC> To me, it's also not clear how we would continue from here, are there any
plans to move all offload stuff to the offload provider? If so, in what time frame?
CM> I assume this question is for Ilya. If it is for me, I don't have the
answer now. 😅
CM> Maybe we can open another thread to discuss it.
EC> Guess it’s for both you and Ilya. In some of the conversations with the
CCed people, this new offload provider was discussed, and I remember it was
mentioned that it only makes sense if there was a plan to follow through with the
migration.
EC> I’m afraid if we add the framework in this patch and it will not be
followed up, it will cause confusion in the future.
CM> OK, I see. We'll also discuss the plan internally.
====
CM> The main idea of dpif-offload layer is for some offload actions that are
not netdev based, but dpif based, like sFlow. And metering is another example that
can leverage on it.
CM> All other offload stuff can be moved to dpif-offload layer. But that's not
the main aim for dpif-offload.
CM> I think the plan is out of scope of this patchset.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
