Re: [PATCH net-next v5 0/5] netdev_features: start cleaning netdev_features_t up

From: Paolo Abeni
Date: Tue Sep 03 2024 - 05:28:39 EST

Next message: Thomas Hellström: "Re: [git pull] drm fixes for 6.11-rc6"
Previous message: zhangjiao2: "[PATCH] tools/counter: close fd when exit"
Next in thread: patchwork-bot+netdevbpf: "Re: [PATCH net-next v5 0/5] netdev_features: start cleaning netdev_features_t up"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 8/29/24 14:33, Alexander Lobakin wrote:

NETDEV_FEATURE_COUNT is currently 64, which means we can't add any new
features as netdev_features_t is u64.
As per several discussions, instead of converting netdev_features_t to
a bitmap, which would mean A LOT of changes, we can try cleaning up
netdev feature bits.
There's a bunch of bits which don't really mean features, rather device
attributes/properties that can't be changed via Ethtool in any of the
drivers. Such attributes can be moved to netdev private flags without
losing any functionality.

Start converting some read-only netdev features to private flags from
the ones that are most obvious, like lockless Tx, inability to change
network namespace etc. I was able to reduce NETDEV_FEATURE_COUNT from
64 to 60, which mean 4 free slots for new features. There are obviously
more read-only features to convert, such as highDMA, "challenged VLAN",
HSR (4 bits) - this will be done in subsequent series.

Please note that currently netdev features are not uAPI/ABI by any means.
Ethtool passes their names and bits to the userspace separately and there
are no hardcoded names/bits in the userspace, so that new Ethtool could
work on older kernels and vice versa.
This, however, isn't true for Ethtools < 3.4. I haven't changed the bit
positions of the already existing features and instead replaced the freed
bits with stubs. But it's anyway theoretically possible that Ethtools
older than 2011 will break. I hope no currently supported distros supply
such an ancient version.
Shell scripts also most likely won't break since the removed bits were
always read-only, meaning nobody would try touching them from a script.

Alexander Lobakin (5):
netdevice: convert private flags > BIT(31) to bitfields
netdev_features: convert NETIF_F_LLTX to dev->lltx
netdev_features: convert NETIF_F_NETNS_LOCAL to dev->netns_local
netdev_features: convert NETIF_F_FCOE_MTU to dev->fcoe_mtu
netdev_features: remove NETIF_F_ALL_FCOE

.../networking/net_cachelines/net_device.rst | 7 +++-
Documentation/networking/netdev-features.rst | 15 -------
Documentation/networking/netdevices.rst | 4 +-
Documentation/networking/switchdev.rst | 4 +-
drivers/net/ethernet/tehuti/tehuti.h | 2 +-
include/linux/netdev_features.h | 16 ++-----
include/linux/netdevice.h | 42 +++++++++++++------
drivers/net/amt.c | 4 +-
drivers/net/bareudp.c | 2 +-
drivers/net/bonding/bond_main.c | 8 ++--
drivers/net/dummy.c | 3 +-
drivers/net/ethernet/adi/adin1110.c | 2 +-
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 3 +-
.../net/ethernet/chelsio/cxgb4/cxgb4_fcoe.c | 6 +--
.../net/ethernet/freescale/dpaa/dpaa_eth.c | 3 +-
.../net/ethernet/freescale/dpaa2/dpaa2-eth.c | 3 +-
.../net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 4 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 11 ++---
.../net/ethernet/intel/ixgbe/ixgbe_sriov.c | 4 +-
.../ethernet/marvell/prestera/prestera_main.c | 3 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/en_rep.c | 3 +-
.../net/ethernet/mellanox/mlxsw/spectrum.c | 6 ++-
.../ethernet/microchip/lan966x/lan966x_main.c | 2 +-
.../net/ethernet/netronome/nfp/nfp_net_repr.c | 3 +-
drivers/net/ethernet/pasemi/pasemi_mac.c | 5 ++-
.../net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 2 +-
drivers/net/ethernet/rocker/rocker_main.c | 3 +-
drivers/net/ethernet/sfc/ef100_rep.c | 4 +-
drivers/net/ethernet/tehuti/tehuti.c | 4 +-
drivers/net/ethernet/ti/cpsw_new.c | 3 +-
drivers/net/ethernet/toshiba/spider_net.c | 3 +-
drivers/net/geneve.c | 2 +-
drivers/net/gtp.c | 2 +-
drivers/net/hamradio/bpqether.c | 2 +-
drivers/net/ipvlan/ipvlan_main.c | 3 +-
drivers/net/loopback.c | 4 +-
drivers/net/macsec.c | 4 +-
drivers/net/macvlan.c | 6 ++-
drivers/net/net_failover.c | 4 +-
drivers/net/netkit.c | 3 +-
drivers/net/nlmon.c | 4 +-
drivers/net/ppp/ppp_generic.c | 2 +-
drivers/net/rionet.c | 2 +-
drivers/net/team/team_core.c | 8 ++--
drivers/net/tun.c | 5 ++-
drivers/net/veth.c | 2 +-
drivers/net/vrf.c | 4 +-
drivers/net/vsockmon.c | 4 +-
drivers/net/vxlan/vxlan_core.c | 5 ++-
drivers/net/wireguard/device.c | 2 +-
drivers/scsi/fcoe/fcoe.c | 4 +-
drivers/staging/octeon/ethernet.c | 2 +-
lib/test_bpf.c | 3 +-
net/8021q/vlan_dev.c | 10 +++--
net/8021q/vlanproc.c | 4 +-
net/batman-adv/soft-interface.c | 5 ++-
net/bridge/br_device.c | 6 ++-
net/core/dev.c | 8 ++--
net/core/dev_ioctl.c | 9 ++--
net/core/net-sysfs.c | 3 +-
net/core/rtnetlink.c | 2 +-
net/dsa/user.c | 3 +-
net/ethtool/common.c | 3 --
net/hsr/hsr_device.c | 12 +++---
net/ieee802154/6lowpan/core.c | 2 +-
net/ieee802154/core.c | 10 ++---
net/ipv4/ip_gre.c | 4 +-
net/ipv4/ip_tunnel.c | 2 +-
net/ipv4/ip_vti.c | 2 +-
net/ipv4/ipip.c | 2 +-
net/ipv4/ipmr.c | 2 +-
net/ipv6/ip6_gre.c | 7 ++--
net/ipv6/ip6_tunnel.c | 4 +-
net/ipv6/ip6mr.c | 2 +-
net/ipv6/sit.c | 4 +-
net/l2tp/l2tp_eth.c | 2 +-
net/openvswitch/vport-internal_dev.c | 11 ++---
net/wireless/core.c | 10 ++---
net/xfrm/xfrm_interface_core.c | 2 +-
tools/testing/selftests/net/forwarding/README | 2 +-
83 files changed, 208 insertions(+), 194 deletions(-)

---
From v4[0]:
* don't remove the freed feature bits completely and replace them with
stubs to keep the original bit positions of the already present
features (Jakub);
* mention potential Ethtool < 3.4 breakage (Eric).

From v3[1]:
* 0001: fix kdoc for priv_flags_fast (it doesn't support describing
struct_groups()s yet) (Jakub);
* 0006: fix subject prefix (make it consistent with the rest).

From v2[2]:
* rebase on top of the latest net-next;
* 0003: don't remove the paragraph saying "LLTX is deprecated for real
HW drivers" (Willem);
* 0006: new, remove %NETIF_F_ALL_FCOE used only 2 times in 1 file
(Jakub);
* no functional changes.

From v1[3]:
* split bitfield priv flags into "hot" and "cold", leave the first
placed where the old ::priv_flags is and move the rest down next
to ::threaded (Jakub);
* document all the changes in Documentation/networking/net_cachelines/
net_device.rst;
* #3: remove the "-1 cacheline on Tx" paragraph, not really true (Eric).

From RFC[4]:
* drop:
* IFF_LOGICAL (as (LLTX | IFF_NO_QUEUE)) - will be discussed later;
* NETIF_F_HIGHDMA conversion - requires priv flags inheriting etc.,
maybe later;
* NETIF_F_VLAN_CHALLENGED conversion - same as above;
* convert existing priv_flags > BIT(31) to bitfield booleans and define
new flags the same way (Jakub);
* mention a couple times that netdev features are not uAPI/ABI by any
means (Andrew).

[0] https://lore.kernel.org/netdev/20240821150700.1760518-1-aleksander.lobakin@xxxxxxxxx
[1] https://lore.kernel.org/netdev/20240808152757.2016725-1-aleksander.lobakin@xxxxxxxxx
[2] https://lore.kernel.org/netdev/20240703150342.1435976-1-aleksander.lobakin@xxxxxxxxx
[3] https://lore.kernel.org/netdev/20240625114432.1398320-1-aleksander.lobakin@xxxxxxxxx
[4] https://lore.kernel.org/netdev/20240405133731.1010128-1-aleksander.lobakin@xxxxxxxxx

I think we are better off merging this series not too far into the release cycle. My understanding is that the points raised on the previous revisions have been addressed so I'll go over this a last time and eventually merge it soon.

Thanks,

Paolo

Next message: Thomas Hellström: "Re: [git pull] drm fixes for 6.11-rc6"
Previous message: zhangjiao2: "[PATCH] tools/counter: close fd when exit"
Next in thread: patchwork-bot+netdevbpf: "Re: [PATCH net-next v5 0/5] netdev_features: start cleaning netdev_features_t up"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]