From: Yajun Deng <redacted>
Date: Wed, 13 Sep 2023 10:08:08 +0800
On 2023/9/13 00:22, Alexander Lobakin wrote:
quoted
From: Yajun Deng <redacted>
Date: Mon, 11 Sep 2023 16:20:16 +0800
[...]
quoted
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.
quoted
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
quoted
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".
quoted
quoted
/**
* dev_get_stats - get network device statistics
Thanks,
Olek
Thanks,
Olek
On 2023/9/13 17:58, Alexander Lobakin wrote:
From: Yajun Deng <redacted>
Date: Wed, 13 Sep 2023 10:08:08 +0800
quoted
On 2023/9/13 00:22, Alexander Lobakin wrote:
quoted
From: Yajun Deng <redacted>
Date: Mon, 11 Sep 2023 16:20:16 +0800
[...]
quoted
quoted
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.
quoted
quoted
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
quoted
quoted
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".
Finally, I give up this approach.
The new function dev_core_stats_inc() didn't called by external modules directly.
So EXPORT_SYMBOL_GPL(dev_core_stats_inc) can be removed by anyone.
quoted
quoted
quoted
/**
* dev_get_stats - get network device statistics
Thanks,
Olek
Thanks,
Olek
From: Yajun Deng <redacted>
Date: Thu, 14 Sep 2023 10:44:14 +0800
On 2023/9/13 17:58, Alexander Lobakin wrote:
quoted
From: Yajun Deng <redacted>
Date: Wed, 13 Sep 2023 10:08:08 +0800
quoted
On 2023/9/13 00:22, Alexander Lobakin wrote:
quoted
From: Yajun Deng <redacted>
Date: Mon, 11 Sep 2023 16:20:16 +0800
[...]
quoted
quoted
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.
quoted
quoted
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
quoted
quoted
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".
Finally, I give up this approach.
The new function dev_core_stats_inc() didn't called by external modules
directly.
If it's called via an inline or macro or whatever, it still needs to be
exported.
Double-check that modpost doesn't complain on allmodconfig build.
So EXPORT_SYMBOL_GPL(dev_core_stats_inc) can be removed by anyone.
That doesn't mean it won't be needed tomorrow.
And I don't feel like it's a good excuse to define 1 external function
per counter instead of 1 external + static inlines for the rest. It's
not only about the exports.
Esp. given that I wrote almost the whole code needed for it to work in
one of my previous replies.
If you don't want to do that, I could take it over xD
quoted
quoted
quoted
quoted
/**
* dev_get_stats - get network device statistics
Thanks,
Olek
Thanks,
Olek
Thanks,
Olek