[4/5] examples/l2fwd-cat: fix build on FreeBSD

Message ID 20190409092933.55356-5-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series small cleanup and fixes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson April 9, 2019, 9:29 a.m. UTC
  The definition of CPU_AND differs from Linux to BSD, so we need to use
RTE_CPU_AND instead.

Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample application for PQoS CAT and CDP")
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 examples/l2fwd-cat/cat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

David Marchand April 9, 2019, 9:56 a.m. UTC | #1
On Tue, Apr 9, 2019 at 11:30 AM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> The definition of CPU_AND differs from Linux to BSD, so we need to use
> RTE_CPU_AND instead.
>
> Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample application for PQoS
> CAT and CDP")
> Cc: stable@dpdk.org


This creates a dependency on backporting c3568ea37670 ("eal: restrict
control threads to startup CPU affinity") which introduced the RTE_CPU_AND
macro.
  
Bruce Richardson April 9, 2019, 10:03 a.m. UTC | #2
On Tue, Apr 09, 2019 at 11:56:04AM +0200, David Marchand wrote:
>    On Tue, Apr 9, 2019 at 11:30 AM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      The definition of CPU_AND differs from Linux to BSD, so we need to
>      use
>      RTE_CPU_AND instead.
>      Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample application for
>      PQoS CAT and CDP")
>      Cc: [2]stable@dpdk.org
> 
>    This creates a dependency on backporting c3568ea37670 ("eal: restrict
>    control threads to startup CPU affinity") which introduced the
>    RTE_CPU_AND macro.
>    --
>    David Marchand
> 
Shall I drop the stable reference from the v2, then?
  
David Marchand April 9, 2019, 10:34 a.m. UTC | #3
On Tue, Apr 9, 2019 at 12:03 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Tue, Apr 09, 2019 at 11:56:04AM +0200, David Marchand wrote:
> >    On Tue, Apr 9, 2019 at 11:30 AM Bruce Richardson
> >    <[1]bruce.richardson@intel.com> wrote:
> >
> >      The definition of CPU_AND differs from Linux to BSD, so we need to
> >      use
> >      RTE_CPU_AND instead.
> >      Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample application for
> >      PQoS CAT and CDP")
> >      Cc: [2]stable@dpdk.org
> >
> >    This creates a dependency on backporting c3568ea37670 ("eal: restrict
> >    control threads to startup CPU affinity") which introduced the
> >    RTE_CPU_AND macro.
> >    --
> >    David Marchand
> >
> Shall I drop the stable reference from the v2, then?
>

We can backport in 18.11, as I would expect c3568ea37670 to be backported.

The question is more what we want to do with 17.11.
We could backport only the macro bits from this patch if needed.
  
Bruce Richardson April 9, 2019, 10:40 a.m. UTC | #4
On Tue, Apr 09, 2019 at 12:34:06PM +0200, David Marchand wrote:
>    On Tue, Apr 9, 2019 at 12:03 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      On Tue, Apr 09, 2019 at 11:56:04AM +0200, David Marchand wrote:
>      >    On Tue, Apr 9, 2019 at 11:30 AM Bruce Richardson
>      >    <[1][2]bruce.richardson@intel.com> wrote:
>      >
>      >      The definition of CPU_AND differs from Linux to BSD, so we
>      need to
>      >      use
>      >      RTE_CPU_AND instead.
>      >      Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample
>      application for
>      >      PQoS CAT and CDP")
>      >      Cc: [2][3]stable@dpdk.org
>      >
>      >    This creates a dependency on backporting c3568ea37670 ("eal:
>      restrict
>      >    control threads to startup CPU affinity") which introduced the
>      >    RTE_CPU_AND macro.
>      >    --
>      >    David Marchand
>      >
>      Shall I drop the stable reference from the v2, then?
> 
>    We can backport in 18.11, as I would expect c3568ea37670 to be
>    backported.
>    The question is more what we want to do with 17.11.
>    We could backport only the macro bits from this patch if needed.
>    --
Yes. I'll leave the stable cc and then each maintainer can decide
themselves on backport. I'll include note in the patch (below cutline) to
call out dependency explicitly.

/Bruce
  
Luca Boccassi April 9, 2019, 10:40 a.m. UTC | #5
On Tue, 2019-04-09 at 12:34 +0200, David Marchand wrote:
> On Tue, Apr 9, 2019 at 12:03 PM Bruce Richardson <
> bruce.richardson@intel.com
> >
> wrote:
> 
> > On Tue, Apr 09, 2019 at 11:56:04AM +0200, David Marchand wrote:
> > >    On Tue, Apr 9, 2019 at 11:30 AM Bruce Richardson
> > >    <[1]
> > > bruce.richardson@intel.com
> > > > wrote:
> > > 
> > >      The definition of CPU_AND differs from Linux to BSD, so we
> > > need to
> > >      use
> > >      RTE_CPU_AND instead.
> > >      Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample
> > > application for
> > >      PQoS CAT and CDP")
> > >      Cc: [2]
> > > stable@dpdk.org
> > > 
> > > 
> > >    This creates a dependency on backporting c3568ea37670 ("eal:
> > > restrict
> > >    control threads to startup CPU affinity") which introduced the
> > >    RTE_CPU_AND macro.
> > >    --
> > >    David Marchand
> > > 
> > 
> > Shall I drop the stable reference from the v2, then?
> > 
> 
> We can backport in 18.11, as I would expect c3568ea37670 to be
> backported.
> 
> The question is more what we want to do with 17.11.
> We could backport only the macro bits from this patch if needed.

If c3568 is not destined for 17.11.x, if you are up for doing the extra
work you can send the macro-only change as an individual patch to 
stable@dpdk.org (do not cc dev) and use --subject-prefix='PATCH 17.11'
- alternatively, the 17.11 maintainer can simply opt to not pick up
this patch.
  
Luca Boccassi April 9, 2019, 10:40 a.m. UTC | #6
On Tue, 2019-04-09 at 10:29 +0100, Bruce Richardson wrote:
> The definition of CPU_AND differs from Linux to BSD, so we need to
> use
> RTE_CPU_AND instead.
> 
> Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample application for
> PQoS CAT and CDP")
> Cc: 
> stable@dpdk.org
> 
> 
> Signed-off-by: Bruce Richardson <
> bruce.richardson@intel.com
> >
> ---
>  examples/l2fwd-cat/cat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Luca Boccassi <bluca@debian.org>
  
David Marchand April 9, 2019, 11:09 a.m. UTC | #7
On Tue, Apr 9, 2019 at 12:40 PM Luca Boccassi <bluca@debian.org> wrote:

> On Tue, 2019-04-09 at 12:34 +0200, David Marchand wrote:
> > On Tue, Apr 9, 2019 at 12:03 PM Bruce Richardson <
> > bruce.richardson@intel.com
> > >
> > wrote:
> >
> > > On Tue, Apr 09, 2019 at 11:56:04AM +0200, David Marchand wrote:
> > > >    On Tue, Apr 9, 2019 at 11:30 AM Bruce Richardson
> > > >    <[1]
> > > > bruce.richardson@intel.com
> > > > > wrote:
> > > >
> > > >      The definition of CPU_AND differs from Linux to BSD, so we
> > > > need to
> > > >      use
> > > >      RTE_CPU_AND instead.
> > > >      Fixes: f6baccbc2b3b ("examples/l2fwd-cat: add sample
> > > > application for
> > > >      PQoS CAT and CDP")
> > > >      Cc: [2]
> > > > stable@dpdk.org
> > > >
> > > >
> > > >    This creates a dependency on backporting c3568ea37670 ("eal:
> > > > restrict
> > > >    control threads to startup CPU affinity") which introduced the
> > > >    RTE_CPU_AND macro.
> > > >    --
> > > >    David Marchand
> > > >
> > >
> > > Shall I drop the stable reference from the v2, then?
> > >
> >
> > We can backport in 18.11, as I would expect c3568ea37670 to be
> > backported.
> >
> > The question is more what we want to do with 17.11.
> > We could backport only the macro bits from this patch if needed.
>
> If c3568 is not destined for 17.11.x, if you are up for doing the extra
> work you can send the macro-only change as an individual patch to
> stable@dpdk.org (do not cc dev) and use --subject-prefix='PATCH 17.11'
> - alternatively, the 17.11 maintainer can simply opt to not pick up
> this patch.
>

Not hard to achieve from my pov, the relevant bits are just this, I can
send it if the patch is selected.

@@ -23,10 +23,18 @@
 #define LCORE_ID_ANY     UINT32_MAX       /**< Any lcore. */

 #if defined(__linux__)
-       typedef cpu_set_t rte_cpuset_t;
+typedef        cpu_set_t rte_cpuset_t;
+#define RTE_CPU_AND(dst, src1, src2) CPU_AND(dst, src1, src2)
 #elif defined(__FreeBSD__)
 #include <pthread_np.h>
-       typedef cpuset_t rte_cpuset_t;
+typedef cpuset_t rte_cpuset_t;
+#define RTE_CPU_AND(dst, src1, src2) do \
+{ \
+       cpuset_t tmp; \
+       CPU_COPY(src1, &tmp); \
+       CPU_AND(&tmp, src2); \
+       CPU_COPY(&tmp, dst); \
+} while (0)
 #endif

 /**
  

Patch

diff --git a/examples/l2fwd-cat/cat.c b/examples/l2fwd-cat/cat.c
index a6081e676..502c6b327 100644
--- a/examples/l2fwd-cat/cat.c
+++ b/examples/l2fwd-cat/cat.c
@@ -345,7 +345,7 @@  check_cpus_overlapping(void)
 
 	for (i = 0; i < m_config_count; i++) {
 		for (j = i + 1; j < m_config_count; j++) {
-			CPU_AND(&mask,
+			RTE_CPU_AND(&mask,
 				&m_config[i].cpumask,
 				&m_config[j].cpumask);