Date:   Mon, 20 Feb 2023 11:31:59 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Gavin Li <gavinl@nvidia.com>
Cc:     davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.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
Subject: Re: [PATCH net-next v3 2/5] vxlan: Expose helper vxlan_build_gbp_hdr
Message-ID: <Y/NMH2QRKoUpdNef@corigine.com>
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>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <c8fcebb5-4eba-71c8-e20c-cd7afd7e0d98@nvidia.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SnNyTldpTG1PdlJqSERFSVV4Ti85RnRUT2U2WS9FQ3lSKzEyUnc3Y2RkamYv?=
 =?utf-8?B?aHNXbFVKNVJyc2tKT1NtV2Z6YzVzWU44aG4wSEpUeXdoYU1JV0RoSFI5dXVh?=
 =?utf-8?B?bGwyd0R5Mm12SFpOSXBVVmRCaFo0dkQrZXJhSjM3RU0wRElJN0pkUkZIdHYw?=
 =?utf-8?B?VmkzcTY1Z1NlRnNyNG1ra241QUE1cGNNcVhHZ3RQUndjL21jZHcveUw4UFJs?=
 =?utf-8?B?cVcvTjBRUERhcDNzNXVEdDJtWUR5eHlOY29xa0JtK1REMmYwOWJXalU4U0lj?=
 =?utf-8?B?RFlwS2x1QjBzNkliQWdzZ1hGcXVyNnVONmVNUEFLaHozKzZDcmhGRkx6SkJ5?=
 =?utf-8?B?ejJValQrVDBDaDE0S1BTK1UzMFRhNjhLeEQ4YVVHS3dYTVBYaDlQUXM1aFlK?=
 =?utf-8?B?K2VraC9PY1VDVUM1WFBmWHczMEVFWDRpREtTb0FjTDM1NnFyT1VIMk1jM2Fa?=
 =?utf-8?B?QWJ3VThtc2gvMWc5dGF5S295dnFSWEZkVGFnM0MyeDB3MU96S0N1NkVNcEhn?=
 =?utf-8?B?TTJTMWM1RlREbmsxWnlsRUF4Rm9WdEpieU5yOC9zOWlyYk9RUFQ3TzVkVFZi?=
 =?utf-8?B?c2ozeHp3SVpNZ1JwdXVLdHZLc2tOajVCaWN2d2h2Q3dXdldwUEhJVG1lYzFy?=
 =?utf-8?B?VlpKc3hDYXlYak5XemN6Tm9HU0VoYmRqejVjWjVFYVZBT0RHeUVFck91YmFC?=
 =?utf-8?B?WjRabVAwUjF2TnN1UTk2NHlsVlJSNnV6NnNVb3hualZBcW5Qd1lZSEpDSjlW?=
 =?utf-8?B?ZGxibGhkc2U5NGpEYzBvazE0THdPMExmSFV0WWwyMlM5WkJJazZlbGRRUm9W?=
 =?utf-8?B?Mm8zVUtwZ2FJSjd2ckZkZEYzbHRTWlJyNEZNYytVTjYvaEowcEdNWlNJam9j?=
 =?utf-8?B?SHRtT0k4VUNlS3VGbmE3Z21wanpwcTYwK0ZNU3pJUjVzM25UckFPa2prejFL?=
 =?utf-8?B?bHc3QUkrbWZXamtzVkczNjRRMmp4QUkrMHl4bVlQZ2hCUjZOaDhzVG5WRUc0?=
 =?utf-8?B?SC9rbHRnYmVuSndzU2lBWERuL0syYmFxQmt4RnZQNmwrQnlja1VWVzFzOG9L?=
 =?utf-8?B?cG9oQXdEMTVvY1o3TXV2N2tjUVdxR2VWN3FHL2pRendmK3BxYkZLTmNjT0hQ?=
 =?utf-8?B?OTdReHNMeGZPMmhUOVJJMWs1UGNDME0yMnh2b2piV2pyYlZ0Q0d2ZHVWcEN5?=
 =?utf-8?B?enU5S29TWXhSdklUdDNBZ0R2c25mdGVyNUtUUEdFSlNoYUlKR3owRzduOEJC?=
 =?utf-8?B?WUhnNXN1YmRQZ1VObExBNHZkc2ZOWmg2ZzNVUmVoc2F1M21KQVB3NTlKS0Ft?=
 =?utf-8?B?dUQ1eGQxdFVVek9RbVI0QzAxNGRCR0IzTnRnaHZoQzFFT1hpU1hjZHJKcW1V?=
 =?utf-8?B?TWQrRVd3akt0aWlobkVzQUNxVTFPODRWU0dWYWxJcG9OWHlRUFRYN2hjb0Fx?=
 =?utf-8?B?S2taek5VYW5WRVRUejFSY2RmVFgzSHR0MURNV1FEb1Z6Y05iQzU4ZzhkOHJq?=
 =?utf-8?B?cWhwbENlbW1qVHJCZVAvTSs1Z0p6NitnRis0QlRqT2VTQ3FSU2hCNnk4blpL?=
 =?utf-8?B?M3hYMWk2MXlLVnVaczBiREhUSStIV0xNbFYrWmZocFpPcDlEZmhXYmEvN1pL?=
 =?utf-8?B?bjF2UG5LdGMvVVpyQU9MbkhWN29nQWFLTGpQd0pVZXd2SEhwNmg5bk90dDhk?=
 =?utf-8?B?cDh3WDZoNG90SGRnQjNWd1hyWXdqWUJSeHJjZnp5bU5GUGYxKzN5MVFCTUlX?=
 =?utf-8?B?dHhnZ1ZRYjdlZmxQalRya1VJNVpMcldMb1BrOC91NklVTC94K2lVekpxck9m?=
 =?utf-8?B?cEJuTlJteUkxOFFkYWt4VHo0TjFkT2doRDUvT3FpV3pxb2NNTnNIbDFmUFZO?=
 =?utf-8?B?YXUwU2MvV2xnTGltaUNZN1hxb3pOMW55Y3B3QkZuMWczR2pESmhzd1VUdk11?=
 =?utf-8?B?UitzZEpNY2tWRHhDaXY3MWsvcit6R3A0OUxOazRvZUtINkxwMGZoS1RKWWpK?=
 =?utf-8?B?blovKzJxSitGZVFkdi9rMUV6eitaNFNUeVRxa0srTkVkK2JSU1NhS1pLSEJI?=
 =?utf-8?B?d0NZRktPYmEyMGU2Zm1ndUJKQldpWGYyQ2RmZC8zWFJzYmFscWpycG9meVEy?=
 =?utf-8?B?M0RtK0lnS1didzkxL1VjUlp6czJrSnUyek5FZWNxOEVHTEFZVWhrTGw5ZmVN?=
 =?utf-8?B?S1Nxdk1ud2Q1bkhRd1dmbmJuK0F5eU5zSjNudThWWDgyYUF1aGNhU0s0dEpT?=
 =?utf-8?B?WDJBTXEvU1hPYXBYZmpaTVdVUXh3PT0=?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 106420f8-4329-42e4-2405-08db132db69c
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Feb 2023 10:32:06.2605
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9BxjoMikMzyDvEsDrnfGTgx/69E+cQY+Vpx72t8aD86HLJttWtmxVzhKp9mo11QjnQOUeVG8Txorpyts2qTlSPGz0DTxM1KV4o25gT5eo3A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR13MB3738
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@nvidia.com>
> > > > > Reviewed-by: Gavi Teitz <gavi@nvidia.com>
> > > > > Reviewed-by: Roi Dayan <roid@nvidia.com>
> > > > > Reviewed-by: Maor Dickman <maord@nvidia.com>
> > > > > Acked-by: Saeed Mahameed <saeedm@nvidia.com>
> > > > 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.