Message ID | 20250218020948.160643-11-sdf@fomichev.me (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: Hold netdev instance lock during ndo operations | expand |
On Mon, 17 Feb 2025 18:09:46 -0800 Stanislav Fomichev wrote: > A lot of selftests are using dummy module, convert it to netdev > instance lock to expand the test coverage. I think the next version should be ready for merging. What should we do with this patch? Can we add a bool inside struct net_device to opt-in for the ndo locking, without having to declare empty ops? I think more drivers could benefit from it that way.
On 02/18, Jakub Kicinski wrote: > On Mon, 17 Feb 2025 18:09:46 -0800 Stanislav Fomichev wrote: > > A lot of selftests are using dummy module, convert it to netdev > > instance lock to expand the test coverage. > > I think the next version should be ready for merging. > What should we do with this patch? > Can we add a bool inside struct net_device to opt-in > for the ndo locking, without having to declare empty > ops? I think more drivers could benefit from it that way. Awesome, will drop this patch and add another one with a bool opt-in! LMK if you prefer other name or a better comment: @@ -2456,6 +2456,12 @@ struct net_device { */ bool up; + /** + * @request_ops_lock: request the core to run all @netdev_ops and + * @ethtool_ops under the @lock. + */ + bool request_ops_lock; + /** * @lock: netdev-scope lock, protects a small selection of fields. * Should always be taken using netdev_lock() / netdev_unlock() helpers.
On Tue, 18 Feb 2025 20:56:40 -0800 Stanislav Fomichev wrote: > On 02/18, Jakub Kicinski wrote: > > On Mon, 17 Feb 2025 18:09:46 -0800 Stanislav Fomichev wrote: > > > A lot of selftests are using dummy module, convert it to netdev > > > instance lock to expand the test coverage. > > > > I think the next version should be ready for merging. > > What should we do with this patch? > > Can we add a bool inside struct net_device to opt-in > > for the ndo locking, without having to declare empty > > ops? I think more drivers could benefit from it that way. > > Awesome, will drop this patch and add another one with a bool opt-in! > > LMK if you prefer other name or a better comment: > > @@ -2456,6 +2456,12 @@ struct net_device { > */ > bool up; > > + /** > + * @request_ops_lock: request the core to run all @netdev_ops and > + * @ethtool_ops under the @lock. > + */ > + bool request_ops_lock; > + > /** > * @lock: netdev-scope lock, protects a small selection of fields. > * Should always be taken using netdev_lock() / netdev_unlock() helpers. > Sure, SGTM
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig index 1fd5acdc73c6..ba8ac1ce6fd5 100644 --- a/drivers/net/Kconfig +++ b/drivers/net/Kconfig @@ -59,6 +59,7 @@ config BONDING config DUMMY tristate "Dummy net driver support" + select NET_SHAPER help This is essentially a bit-bucket device (i.e. traffic you send to this device is consigned into oblivion) with a configurable IP diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c index 005d79975f3b..52d68246dc11 100644 --- a/drivers/net/dummy.c +++ b/drivers/net/dummy.c @@ -38,6 +38,7 @@ #include <linux/moduleparam.h> #include <linux/rtnetlink.h> #include <linux/net_tstamp.h> +#include <net/net_shaper.h> #include <net/rtnetlink.h> #include <linux/u64_stats_sync.h> @@ -82,6 +83,41 @@ static int dummy_change_carrier(struct net_device *dev, bool new_carrier) return 0; } +static int dummy_shaper_set(struct net_shaper_binding *binding, + const struct net_shaper *shaper, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static int dummy_shaper_del(struct net_shaper_binding *binding, + const struct net_shaper_handle *handle, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static int dummy_shaper_group(struct net_shaper_binding *binding, + int leaves_count, const struct net_shaper *leaves, + const struct net_shaper *root, + struct netlink_ext_ack *extack) +{ + return 0; +} + +static void dummy_shaper_cap(struct net_shaper_binding *binding, + enum net_shaper_scope scope, unsigned long *flags) +{ + *flags = ULONG_MAX; +} + +static const struct net_shaper_ops dummy_shaper_ops = { + .set = dummy_shaper_set, + .delete = dummy_shaper_del, + .group = dummy_shaper_group, + .capabilities = dummy_shaper_cap, +}; + static const struct net_device_ops dummy_netdev_ops = { .ndo_init = dummy_dev_init, .ndo_start_xmit = dummy_xmit, @@ -90,6 +126,7 @@ static const struct net_device_ops dummy_netdev_ops = { .ndo_set_mac_address = eth_mac_addr, .ndo_get_stats64 = dummy_get_stats64, .ndo_change_carrier = dummy_change_carrier, + .net_shaper_ops = &dummy_shaper_ops, }; static const struct ethtool_ops dummy_ethtool_ops = {
A lot of selftests are using dummy module, convert it to netdev instance lock to expand the test coverage. Cc: Saeed Mahameed <saeed@kernel.org> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- drivers/net/Kconfig | 1 + drivers/net/dummy.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+)