On 10/18/2021 11:14 PM, Eelco Chaudron wrote:

On 18 Oct 2021, at 14:03, Chris Mi wrote:

Hi Eelco,

On 10/15/2021 9:42 PM, Eelco Chaudron wrote:
Small comments inline, and Ilya please take a look at the first comment/request.

//Eelco


On 12 Oct 2021, at 10:19, 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>
---
<SNIP>

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;
  From previous revisions:

| 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.

  From my side, this looks wrong as there is a direct relation between dpif and 
dpif-offload. I would assume you should be able to pick a specific one, or what 
else would have stopped us from adding the 
dpif_offload_sflow_recv_wait()/dpif_offload_sflow_recv() directly in the dpif.
I'll think about it. If I understand correctly, it should be something like 
this, right?

struct dpif {
     const struct dpif_class *dpif_class;
     const struct dpif_offload_api *dpif_offload_api;
...
Yes, this is what I was referring to.
OK, got it.

Thanks,
Chris
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to