Re: [PATCH net-next v3 2/5] vxlan: Expose helper vxlan_build_gbp_hdr

[Simon Horman]

On Mon, Feb 20, 2023 at 03:15:20PM +0800, Gavin Li wrote:
> 
> On 2/20/2023 2:40 PM, Simon Horman wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Mon, Feb 20, 2023 at 10:05:00AM +0800, Gavin Li wrote:
> > > On 2/20/2023 4:32 AM, Simon Horman wrote:
> > > > External email: Use caution opening links or attachments
> > > > 
> > > > 
> > > > On Fri, Feb 17, 2023 at 05:39:22AM +0200, Gavin Li wrote:
> > > > > vxlan_build_gbp_hdr will be used by other modules to build gbp option in
> > > > > vxlan header according to gbp flags.
> > > > > 
> > > > > Signed-off-by: Gavin Li <gavinl@xxxxxxxxxx>
> > > > > Reviewed-by: Gavi Teitz <gavi@xxxxxxxxxx>
> > > > > Reviewed-by: Roi Dayan <roid@xxxxxxxxxx>
> > > > > Reviewed-by: Maor Dickman <maord@xxxxxxxxxx>
> > > > > Acked-by: Saeed Mahameed <saeedm@xxxxxxxxxx>
> > > > I do wonder if this needs to be a static inline function.
> > > > But nonetheless,
> > > Will get "unused-function" from gcc without "inline"
> > > 
> > > ./include/net/vxlan.h:569:13: warning: ‘vxlan_build_gbp_hdr’ defined but not
> > > used [-Wunused-function]
> > >   static void vxlan_build_gbp_hdr(struct vxlanhdr *vxh, const struct
> > > vxlan_metadata *md)
> > 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
> 
> Tried that the first time the function was called by driver code. It would
> introduce dependency in linking between the driver and the kernel module.
> 
> Do you think it's OK to have such dependency?

IMHO, yes. But others may feel differently.

I do wonder if any performance overhead of a non-inline function
also needs to be considered.