[ovs-dev] [PATCH 5/5] netdev: Clean up class initialization.

[Ben Pfaff]

On Wed, Aug 01, 2018 at 03:23:38PM +0100, Ian Stokes wrote:
> On 7/12/2018 10:55 PM, Ben Pfaff wrote:
> >The macros are hard to read.  This makes it a little more readable.
> >
> 
> Thanks for this Ben, one minor comment below.
> 
> >Signed-off-by: Ben Pfaff <blp at ovn.org>
> >---
> >  configure.ac          |   1 +
> >  lib/netdev-dpdk.c     | 235 ++++++++++++----------------------
> >  lib/netdev-dummy.c    | 134 ++++++++------------
> >  lib/netdev-linux.c    | 340 +++++++++++++++++++-------------------------------
> >  lib/netdev-linux.h    |  18 +--
> >  lib/netdev-provider.h |   2 -
> >  lib/netdev-vport.c    | 223 +++++++++++++++------------------
> >  7 files changed, 369 insertions(+), 584 deletions(-)
> >
> >diff --git a/configure.ac b/configure.ac
> >index c89c607c7124..66281c4d6811 100644
> >--- a/configure.ac
> >+++ b/configure.ac
> >@@ -172,6 +172,7 @@ OVS_ENABLE_OPTION([-Wduplicated-cond])
> >  OVS_ENABLE_OPTION([-Qunused-arguments])
> >  OVS_ENABLE_OPTION([-Wshadow])
> >  OVS_ENABLE_OPTION([-Wno-null-pointer-arithmetic])
> >+OVS_ENABLE_OPTION([-Warray-bounds-pointer-arithmetic])
> >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused], [HAVE_WNO_UNUSED])
> >  OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter], [HAVE_WNO_UNUSED_PARAMETER])
> >  OVS_ENABLE_WERROR
> >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> >index 9bf21856075b..4de4cf116e92 100644
> >--- a/lib/netdev-dpdk.c
> >+++ b/lib/netdev-dpdk.c
> >@@ -4695,161 +4695,86 @@ netdev_dpdk_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
> >                                          ufid, rte_flow);
> >  }
> >-#define DPDK_FLOW_OFFLOAD_API                                 \
> >-    NULL,                   /* flow_flush */                  \
> >-    NULL,                   /* flow_dump_create */            \
> >-    NULL,                   /* flow_dump_destroy */           \
> >-    NULL,                   /* flow_dump_next */              \
> >-    netdev_dpdk_flow_put,                                     \
> >-    NULL,                   /* flow_get */                    \
> >-    netdev_dpdk_flow_del,                                     \
> >-    NULL                    /* init_flow_api */
> 
> Not sure if DPDK_FLOW_OFFLOAD_API should be completely removed, as I
> understand it the remaining offload functionality is currently being worked
> on with a view to enable full HW offload so they will be re-introduced in
> the future.
> 
> The macro could be moved from here to netdev-dpdk.h and then added to the
> NETDEV_DPDK_CLASS_BASE macro you introduce below (this would be similar to
> what is implemented for netdev-linux, and a more uniform approach across the
> netdevs).
> 
> Sugesh, you've been involved in the HW full offload work, do you have an
> opinion on this?

OK.  I folded in the following and reposted v2 as:
        https://patchwork.ozlabs.org/patch/957990/

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 917b3b82c88e..628d75a75024 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4715,8 +4715,7 @@ netdev_dpdk_flow_del(struct netdev *netdev, const ovs_u128 *ufid,
     .rxq_construct = netdev_dpdk_rxq_construct,             \
     .rxq_destruct = netdev_dpdk_rxq_destruct,               \
     .rxq_dealloc = netdev_dpdk_rxq_dealloc,                 \
-    .flow_put = netdev_dpdk_flow_put,                       \
-    .flow_del = netdev_dpdk_flow_del
+    DPDK_FLOW_OFFLOAD_API
 
 #define NETDEV_DPDK_CLASS_BASE                          \
     NETDEV_DPDK_CLASS_COMMON,                           \
diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h
index b7d02a77dc72..cc0501d6879a 100644
--- a/lib/netdev-dpdk.h
+++ b/lib/netdev-dpdk.h
@@ -25,6 +25,10 @@ struct dp_packet;
 
 #ifdef DPDK_NETDEV
 
+#define DPDK_FLOW_OFFLOAD_API                   \
+    .flow_put = netdev_dpdk_flow_put,           \
+    .flow_del = netdev_dpdk_flow_del
+
 void netdev_dpdk_register(void);
 void free_dpdk_buf(struct dp_packet *);