| Message ID | 20240829123340.789395-1-aleksander.lobakin@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | netdev_features: start cleaning netdev_features_t up | expand |
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@intel.com > [1] https://lore.kernel.org/netdev/20240808152757.2016725-1-aleksander.lobakin@intel.com > [2] https://lore.kernel.org/netdev/20240703150342.1435976-1-aleksander.lobakin@intel.com > [3] https://lore.kernel.org/netdev/20240625114432.1398320-1-aleksander.lobakin@intel.com > [4] https://lore.kernel.org/netdev/20240405133731.1010128-1-aleksander.lobakin@intel.com 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
Hello: This series was applied to netdev/net-next.git (main) by Paolo Abeni <pabeni@redhat.com>: On Thu, 29 Aug 2024 14:33:35 +0200 you 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. > > [...] Here is the summary with links: - [net-next,v5,1/5] netdevice: convert private flags > BIT(31) to bitfields (no matching commit) - [net-next,v5,2/5] netdev_features: convert NETIF_F_LLTX to dev->lltx (no matching commit) - [net-next,v5,3/5] netdev_features: convert NETIF_F_NETNS_LOCAL to dev->netns_local (no matching commit) - [net-next,v5,4/5] netdev_features: convert NETIF_F_FCOE_MTU to dev->fcoe_mtu (no matching commit) - [net-next,v5,5/5] netdev_features: remove NETIF_F_ALL_FCOE https://git.kernel.org/netdev/net-next/c/a61fec1c87be You are awesome, thank you!
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@intel.com [1] https://lore.kernel.org/netdev/20240808152757.2016725-1-aleksander.lobakin@intel.com [2] https://lore.kernel.org/netdev/20240703150342.1435976-1-aleksander.lobakin@intel.com [3] https://lore.kernel.org/netdev/20240625114432.1398320-1-aleksander.lobakin@intel.com [4] https://lore.kernel.org/netdev/20240405133731.1010128-1-aleksander.lobakin@intel.com