Re: [dpdk-dev] [dpdk-stable] [PATCH] net/mlx5: fix compilation for rdma-core v19

[Christian Ehrhardt <christian.ehrhardt@canonical.com>]

On Thu, Jul 12, 2018 at 12:57 PM Shahaf Shuler <shahafs@mellanox.com> wrote:

> Thursday, July 12, 2018 1:54 PM, Ori Kam:
> > Subject: RE: [PATCH] net/mlx5: fix compilation for rdma-core v19
> > >
> > > The flow counter support introduced by commit 9a761de8ea14 ("net/mlx5:
> > > flow counter support") was intend to work only with MLNX_OFED_4.3 as
> > > the upstream rdma-core libraries were lack such support.
> > >
> > > On rdma-core v19 the support for the flow counters was added but with
> > > different user APIs, hence causing compilation issues on the PMD.
> > >
> > > This patch fix the compilation errors by forcing the flow counters to
> > > be enabled only with MLNX_OFED APIs.
> > > Once MLNX_OFED and rdma-core APIs will be aligned, a proper patch to
> > > support the new API will be submitted.
> > >
> > > Fixes: 9a761de8ea14 ("net/mlx5: flow counter support")
> > > Cc: stable@dpdk.org


In regard to the stable submission of this I wanted to mention that there
are issues with this.
It correctly fixes the detection with rdma-core v19 and does no more
set HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT

Due to that the following code triggers:
#ifndef HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT
struct ibv_counter_set_init_attr {
       int dummy;
};
struct ibv_flow_spec_counter_action {
       int dummy;
};


But that makes compilation run into:

drivers/net/mlx5/mlx5_flow.c
/<<PKGBUILDDIR>>/drivers/net/mlx5/mlx5_flow.c:69:8: error: redefinition of
‘struct ibv_flow_spec_counter_action’
struct ibv_flow_spec_counter_action {
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /<<PKGBUILDDIR>>/drivers/net/mlx5/mlx5_flow.c:42:0:
/usr/include/infiniband/verbs.h:1563:8: note: originally defined here
struct ibv_flow_spec_counter_action {
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is due to the series starting with b42c000e3 "net/mlx5: remove flow
support" not being in the current stable releases.

For now I appended something like this to the Makefile
+               HAVE_IBV_FLOW_SPEC_COUNTER_ACTION \
+               infiniband/verbs.h \
+               type 'struct ibv_flow_spec_counter_action' \
+               $(AUTOCONF_OUTPUT)

And this to the type definition:

--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -66,9 +66,15 @@
struct ibv_counter_set_init_attr {
       int dummy;
};
+/* rdma-core v19 has no ibv_counter_set_init_attr, but it has
+ * ibv_flow_spec_counter_action which would conflict.
+ * Newer DPDK, doesn't have the issue due to the series starting with
+ * "net/mlx5: remove flow support" */
+#ifndef HAVE_IBV_FLOW_SPEC_COUNTER_ACTION
struct ibv_flow_spec_counter_action {
       int dummy;
};
+#endif
struct ibv_counter_set {
       int dummy;
};

That makes it build, but I wanted to make you aware that this will let all
stable maintainers into issues as the fix applies, but then later breaks
compilation (only if mlx is enabled).
It would be great if you could submit a v2 to stable@ which solves it the
way you'd prefer it to be done.

Kind Regards,
Christian

> > Cc: orika@mellanox.com
> > >
> > > Reported-by:Stephen Hemminger <stephen@networkplumber.org>
> > > Reported-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > > ---
> > >  drivers/net/mlx5/Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> > > index 9e274964b4..d86c6bbab9 100644
> > > --- a/drivers/net/mlx5/Makefile
> > > +++ b/drivers/net/mlx5/Makefile
> > > @@ -150,7 +150,7 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-
> > > config-h.sh
> > >     $Q sh -- '$<' '$@' \
> > >             HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT \
> > >             infiniband/verbs.h \
> > > -           enum IBV_FLOW_SPEC_ACTION_COUNT \
> > > +           type 'struct ibv_counter_set_init_attr' \
> > >             $(AUTOCONF_OUTPUT)
> > >     $Q sh -- '$<' '$@' \
> > >             HAVE_RDMA_NL_NLDEV \
> > > --
> > > 2.12.0
> >
> >
> > Acked-by: Ori Kam <orika@mellanox.com>
>
> Applied to next-net-mlx, thanks.
>
>

-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd