Message-ID: <05e7f7bb8573a32d81e09fbb5744d77d01292d51.camel@redhat.com>
Subject: Re: [PATCH net-next v3 2/5] vxlan: Expose helper vxlan_build_gbp_hdr
From:   Paolo Abeni <pabeni@redhat.com>
To:     Jakub Kicinski <kuba@kernel.org>,
        Simon Horman <simon.horman@corigine.com>
Cc:     Gavin Li <gavinl@nvidia.com>, davem@davemloft.net,
        edumazet@google.com, roopa@nvidia.com,
        eng.alaamohamedsoliman.am@gmail.com, bigeasy@linutronix.de,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        gavi@nvidia.com, roid@nvidia.com, maord@nvidia.com,
        saeedm@nvidia.com
Date:   Tue, 21 Feb 2023 08:38:17 +0100
In-Reply-To: <20230220123021.448dc1a0@kernel.org>
References: <20230217033925.160195-1-gavinl@nvidia.com>
         <20230217033925.160195-3-gavinl@nvidia.com> <Y/KHWxQWqyFbmi9Y@corigine.com>
         <b0f07723-893a-5158-2a95-6570d3a0481c@nvidia.com>
         <Y/MV1JFn4NuptO9q@corigine.com>
         <c8fcebb5-4eba-71c8-e20c-cd7afd7e0d98@nvidia.com>
         <Y/NMH2QRKoUpdNef@corigine.com> <20230220123021.448dc1a0@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Mon, 2023-02-20 at 12:30 -0800, Jakub Kicinski wrote:
> On Mon, 20 Feb 2023 11:31:59 +0100 Simon Horman wrote:
> > On Mon, Feb 20, 2023 at 03:15:20PM +0800, Gavin Li wrote:
> > > > Right. But what I was really wondering is if the definition
> > > > of the function could stay in drivers/net/vxlan/vxlan_core.c,
> > > > without being static. And have a declaration in include/net/vxlan.h=
 =20
> > >=20
> > > Tried that the first time the function was called by driver code. It =
would
> > > introduce dependency in linking between the driver and the kernel mod=
ule.
> > >=20
> > > Do you think it's OK to have such dependency? =20
> >=20
> > IMHO, yes. But others may feel differently.
> >=20
> > I do wonder if any performance overhead of a non-inline function
> > also needs to be considered.
>=20
> Do you recall any details of why Hannes broke the dependency in the
> first place?

IIRC it was that was a cleanup thing, so that setup not using vxlan
does not load the module (and the related deps chain) for no reasons.

Cheers,

Paolo

> Commit b7aade15485a ("vxlan: break dependency with netdev drivers")
> Maybe we should stick to the static inline, it doesn't look too
> large/terrible?

IMHO static inline is good enough here.

Thanks,

Paolo