Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <e6de7c0b-97cc-090e-a331-dc566019f54e@intel.com>
Date:   Wed, 13 Sep 2023 11:58:30 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] net/core: Export dev_core_stats_rx_dropped_inc sets
Content-Language: en-US
To:     Yajun Deng <yajun.deng@linux.dev>
CC:     <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <horms@kernel.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20230911082016.3694700-1-yajun.deng@linux.dev>
 <9a1de9b3-b3cf-d26b-388e-c98294580bca@intel.com>
 <599cdff8-4865-3ade-0439-36e337891ca0@linux.dev>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <599cdff8-4865-3ade-0439-36e337891ca0@linux.dev>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?d2I3c2ZaRzJNSWxjTWpwZmdDWXlVekdIcktUTXRRbURvWnYxNlhrSk5GQjVh?=
 =?utf-8?B?bHBrMjRxbGZRcXV5ZjdyU2NmSmZ4N214TEJ3OTZicGRUNkhaVi85eGMxV2l0?=
 =?utf-8?B?ZHZzRW9ST2V2eElrN2xNdWN4eFlvalA5TzRGdWpPREp2cFExTGtQZTEvWFAy?=
 =?utf-8?B?OHpnUHl1WHlTVTVIM3orUjlIR3dnL3ZrcjAxc3hwd3YwRjhUMEpQQitnRTlj?=
 =?utf-8?B?VHhLY2E3MTRPV3lSTmU3TG42dmRFdGpkTjhhQTJFOFhSTGR4Nzd0eng0S290?=
 =?utf-8?B?RHVoTWRaMllwalRqTDROS2pDS3NqYzZ0dDBxejlMcTZrTWs4QWNEQ3NFU0V2?=
 =?utf-8?B?ZW1kYzNLbmRjWkU5YmRBeVhLYkFaZWlSd2V1VnRIUEVIa2MvLzh6TE5yeGtt?=
 =?utf-8?B?dWd2ZmpkaXkvYVdqUDU1TVFzNjdjaU1YeUpObkhSVUFsKzlXOTNuVEo5cm1j?=
 =?utf-8?B?ZzRJeXVhMlZYRllOZW1BV2xldWpsYUhtRWV6enVrTmpRb2RwVm1YT21EVTda?=
 =?utf-8?B?NUI0alFNVFJUTjNsN0VVZm5BamZCcG1BclErQnZUODBLRXIranNsYVpuUDdz?=
 =?utf-8?B?Y3llRGc3b05PQ1NlSytRRkNIRVR0QSt3Q2VEaUJYUklpS1E2eXp1TmhKSitX?=
 =?utf-8?B?OTlxa29ya2Fqcm1HQXA2MUxHaXZxeXVLUlEwZ1E1ZlZVeEl0dzVlQjRwVEll?=
 =?utf-8?B?NjVKMUlkanpHeUt0TmpVbGhPL3dWRFQ3V29KOG8vZTZ4ZTZUUVRrcjNnbWJy?=
 =?utf-8?B?V2lCeTZWNTNoNDg3MzhnVFNSbXhwYkYvNmcwYUR2Z2l0Q1lUTFVqL014MnJ6?=
 =?utf-8?B?anI5OHh2a3FuMDZINktiSyt2djBjbUwweDZabVdscjdRaUlaTm5xOTRpRHRU?=
 =?utf-8?B?aGVRK3pqakE1cVZjaHhYcVhRTzRLRHR1NTluVTg0MWY1NFZZR0lvMmN2b3Y1?=
 =?utf-8?B?ZmhCd0o0L2RpWkZhbjR2YlMxMG9VSTg5R0FVejZ5MS9La0J2OEFPeWcvMXNQ?=
 =?utf-8?B?dE96S3Y2OTRxNUhZYnVEaWJVZDg5MUVZQXpMTGFKcCtZbHFFREFpYWFDalk3?=
 =?utf-8?B?RjBGdCsrR2R5RXk3bisrR3RxZDFKeHoyOHV0NnJXUldIZ0NiSEh0OVJyT2VR?=
 =?utf-8?B?WnJHMkl3OGdaYnJzcEZ4aUM4ZHZCbVQwcDBXNnRGc3h4Q0t5WEVLamRMQjRZ?=
 =?utf-8?B?QUF2TFM2Slhyak1NT2tzZyttV0pncTgrcW4ydXZlb0FVRS9OYStGQS9URVpy?=
 =?utf-8?B?WG5TZ1FvZ1RCc3VPdS8vQUo3b1ZHNEJlekl1RjlSQ0luS0JOWVp2Q0phcFhh?=
 =?utf-8?B?cEhVMUpaQktIZmpSdFNFOER4cjZEMFJ1Mlp5RENYUU9LNUxoNW9vTU04UHdl?=
 =?utf-8?B?NVViRlcwYU51WE40NUdGTHhlN0FpTUY0RlRUbGNtK2l5bXhrMXh5eElLUFJM?=
 =?utf-8?B?MTBHbitYYUo1VUFaaDgxOEs5YVlNN04rTDFaYUJTeWxQVWt2UE5hV3paWjR2?=
 =?utf-8?B?Y3ZldlZRV3pWNVZPdWNBcXNtTm90dSt2bm02a2ZBeUhCenAwNTZ0YVV5cnIv?=
 =?utf-8?B?TkkxV0ZPWjhveGc0U3dIMWxQUXd6ai9UeS92OEpjNHRCY010RWIvSjRmK3VO?=
 =?utf-8?B?Q0VWemo2RW9xSjJieEMxSFRqbmhxMjdqemJwVi9ROEw1Mnl1aGpNYXZWUHdI?=
 =?utf-8?B?WlNCaFMreFdkQURGeS9SbU4rM3owNHVjL1M4eW5HS2dLMFNmaDNvZ0o3YlV0?=
 =?utf-8?B?R0JxblB1dTc2R0NRL0JYVDV3eXB5TmJvVmZpS3Y5L05uOGIxNzEybnFMaHlt?=
 =?utf-8?B?djdaVk54Q2hRWndjUnhKZE9hR1FqWFM1dThtQlZsVnJSdlQyeE1oVlFnNm90?=
 =?utf-8?B?T3E2Qk4xbXR4MGdBRGZVSmZNWXBqVGRaOCtYVkhIT2ExSUV4dUp3OTE2Z1lu?=
 =?utf-8?B?MVBjQ1hXSUlDR3B1N29LU3I1ZmFsdHVRSEZHZW1PeVhvd2M2R1RIYTZJN3cz?=
 =?utf-8?B?TklLN3h2SlV0RnI1RkhYTW1uSzU1ellBeHF2U2NhOHB3d1V5WGlReVFyUSs5?=
 =?utf-8?B?UVJQdjZaRkVZWHVqb2JTS2VwYnpVY2hYTk55YzZOTEI2TXk4RmlReHppQ1ZG?=
 =?utf-8?B?Uk9INkRXNHJkZkFIU2hBUnliUTVibmVMQjM3ZzRHRysyeERKWkxKb21nTFlm?=
 =?utf-8?B?ZHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8994a53f-d321-4088-ca8c-08dbb4401406
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Sep 2023 09:59:11.2616
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3HRf8a2FBz8ZDmY+87gW9VQJJUcgY84bTmsZuYQCdREA9LUA5Tdo+DVXhL4toZ9n9TpV9Otb2fMx86TGYdXLj7ZhC+bIZwJjphEMOGh6AkY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7115
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Wed, 13 Sep 2023 02:59:22 -0700 (PDT)

From: Yajun Deng <yajun.deng@linux.dev>
Date: Wed, 13 Sep 2023 10:08:08 +0800

> 
> On 2023/9/13 00:22, Alexander Lobakin wrote:
>> From: Yajun Deng <yajun.deng@linux.dev>
>> Date: Mon, 11 Sep 2023 16:20:16 +0800

[...]

>> EXPORT_SYMBOL_GPL(dev_core_stats_inc); // Why not GPL BTW?
> 
> This may be a better option.
> 
> Just because EXPORT_SYMBOL(netdev_core_stats_alloc) before,  but I think
> 
> EXPORT_SYMBOL_GPL is better.

Ah I see. BTW, if you will still define increment functions as
externals, there will be no reason to export netdev_core_stats_alloc()
or even make it non-static at all.

> 
>  
>> And then build inlines:
>>
>> #define DEV_CORE_STATS_INC(FIELD)				\
>> static inline void						\
>> dev_core_stats_##FIELD##_inc(struct net_device *dev)		\
>> {								\
>> 	dev_core_stats_inc(dev,					\
>> 		offsetof(struct net_device_core_stats, FIELD));	\
>> }
>>
>> DEV_CORE_STATS_INC(rx_dropped);
>> ...
>>
>> OR even just make them macros
>>
>> #define __DEV_CORE_STATS_INC(dev, field)			\
>> 	dev_core_stats_inc(dev,					\
>> 		offsetof(struct net_device_core_stats, field))
>>
>> #define dev_core_stats_rx_dropped_inc(dev)			\
>> 	__DEV_CORE_STATS_INC(dev, rx_dropped)
>> ...
> 
> I would like the former.  Keep it the same as before.

By "the former" you mean to build static inlines or externals? Seems
like the first one, but I got confused by your "the same as before" :D

> 
> 
>> Just don't copy that awful Thunderbird's line wrap and don't assume this
>> code builds and works and that is something finished/polished.
>>
>> You'll be able to trace functions and you'll be able to understand which
>> counter has been incremented by checking the second argument, i.e. the
>> field offset (IIRC tracing shows you arguments).
>> And that way you wouldn't geometrically increase the number of symbol
>> exports and deal with its consequences.
> I agree that.

Ok, after this one I guess you meant "I'd like to use your approach with
static inlines".

>>>  
>>>  /**
>>>   *	dev_get_stats	- get network device statistics
>> Thanks,
>> Olek

Thanks,
Olek