Re: [PATCH] hash: make gfni stubs inline

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Tue, Mar 05, 2024 at 11:14:45AM +0100, David Marchand wrote:
> On Mon, Mar 4, 2024 at 7:45 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > This reverts commit 07d836e5929d18ad6640ebae90dd2f81a2cafb71.
> >
> > Tyler found build issues with MSVC and the thash gfni stubs.
> > The problem would be link errors from missing symbols.
> 
> Trying to understand this link error.
> Does it come from the fact that rte_thash_gfni/rte_thash_gfni_bulk
> declarations are hidden under RTE_THASH_GFNI_DEFINED in
> rte_thash_gfni.h?
> 
> If so, why not always expose those two symbols unconditionnally and
> link with the stub only when ! RTE_THASH_GFNI_DEFINED.

So I don't have a lot of background of this lib.

I think we understand that we can't conditionally expose symbols. That's
what windows was picking up because it seems none of our CI's ever end
up with RTE_THASH_GFNI_DEFINED but my local test system did and failed.
(my experiments showed that Linux would complain too if it was defined)

If we always expose the symbols then as you point out we have to
conditionally link with the stub otherwise the inline (non-stub) will be
duplicate and build / link will fail.

I guess the part I don't understand with your suggestion is how we would
conditionally link with just the stub? We have to link with rte_hash to
get the rest of hash and the stub. I've probably missed something here.

Since we never had a release exposing the new symbols introduced by
Stephen in question my suggestion was that we just revert for 24.03 so
we don't end up with an ABI break later if we choose to solve the
problem without exports.

I don't know what else to do, but I think we need to decide for 24.03.

ty

> 
> -- 
> David Marchand