On Wed, Dec 04, 2024 at 04:52:43PM +0000, Bruce Richardson wrote:
> On Wed, Dec 04, 2024 at 08:20:19AM -0800, Andre Muezerie wrote:
> > On Wed, Dec 04, 2024 at 08:56:35AM +0100, David Marchand wrote:
> > > Hello Andre,
> > >
> > > On Wed, Dec 4, 2024 at 3:20 AM Andre Muezerie
> > > <andre...@linux.microsoft.com> wrote:
> > > >
> > > > MSVC issues the warning below:
> > > >
> > > > ../lib/lpm/rte_lpm.c(297): warning C4013
> > > > '__atomic_store' undefined; assuming extern returning int
> > > > ../lib/lpm/rte_lpm.c(298): error C2065:
> > > > '__ATOMIC_RELAXED': undeclared identifier
> > > >
> > > > The fix is to use standard atomic_store_explicit() instead of
> > > > gcc specific __atomic_store().
> > > > atomic_store_explicit() was already being used in other parts
> > > > of DPDK and is compatible
> > > > with many compilers, including MSVC.
> > > >
> > > > Signed-off-by: Andre Muezerie <andre...@linux.microsoft.com>
> > >
> > > With this change, is there anything remaining that blocks this library
> > > compilation with MSVC?
> > > If not, please update meson.build so that CI can test lpm compilation
> > > with MSVC on this patch (and that will detect regressions once
> > > merged).
> > >
> > >
> > > --
> > > David Marchand
> >
> > Hi David,
> >
> > I'm eager to enable lpm to be compiled with MSVC. Even though
> > this was the last issue I observed for this lib on my machine,
> > lpm depends on hash, which depends on net, which depends on mbuf and
> > mbuf is not enabled for MSVC yet.
> >
> I was a bit curious about this dependency chain and decided to investigate
> a bit. The "weak link" in this chain appears to me to be the link between
> the hash library and the net library. Within the hash library, I believe
> only the thash functionality depends on net, for definitions of the ipv6
> headers and address fields.
>
> If we want to break that dependency (temporarily, since net is pretty much
> an essential DPDK lib), the following patch should work.
>
> Regards,
> /Bruce
>
> diff --git a/lib/hash/meson.build b/lib/hash/meson.build
> index e6cb1ebe3b..f9096edd67 100644
> --- a/lib/hash/meson.build
> +++ b/lib/hash/meson.build
> @@ -6,24 +6,34 @@ headers = files(
> 'rte_hash_crc.h',
> 'rte_hash.h',
> 'rte_jhash.h',
> - 'rte_thash.h',
> - 'rte_thash_gfni.h',
> )
> indirect_headers += files(
> 'rte_crc_arm64.h',
> 'rte_crc_generic.h',
> 'rte_crc_sw.h',
> 'rte_crc_x86.h',
> - 'rte_thash_x86_gfni.h',
> )
>
> sources = files(
> 'rte_cuckoo_hash.c',
> 'rte_hash_crc.c',
> 'rte_fbk_hash.c',
> +)
> +
> +deps = ['rcu']
> +
> +if dpdk_conf.has('RTE_LIB_NET')
> + headers += files(
> + 'rte_thash.h',
> + 'rte_thash_gfni.h',
> + )
> + indirect_headers += files(
> + 'rte_thash_x86_gfni.h',
> + )
> + sources += files(
> 'rte_thash.c',
> 'rte_thash_gfni.c',
> 'rte_thash_gf2_poly_math.c',
> -)
> -
> -deps = ['net', 'rcu']
> + )
> + deps += ['net']
> +endif
That's a great suggestion. Unfortunately hash also directly depends on
rcu, which is also not yet enabled for MSVC due to pending reviews.
Regards,
Andre Muezerie
