[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c193cbf3-58e5-41bd-855d-07a9c08e283d@redhat.com>
Date: Tue, 3 Sep 2024 11:55:06 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>
Cc: David Ahern <dsahern@...nel.org>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Andrew Lunn <andrew@...n.ch>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
nex.sw.ncis.osdt.itp.upstreaming@...el.com, netdev@...r.kernel.org,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Hannes Reinecke <hare@...e.de>
Subject: Re: [PATCH net-next v5 4/5] netdev_features: convert NETIF_F_FCOE_MTU
to dev->fcoe_mtu
CC: Martin and Hannes, to raise awareness that the core networking
changes in here will introduce a small delta in the scsi subsystem, too.
On 8/29/24 14:33, Alexander Lobakin wrote:
> Ability to handle maximum FCoE frames of 2158 bytes can never be changed
> and thus more of an attribute, not a toggleable feature.
> Move it from netdev_features_t to "cold" priv flags (bitfield bool) and
> free yet another feature bit.
>
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> ---
> .../networking/net_cachelines/net_device.rst | 1 +
> include/linux/netdev_features.h | 6 ++----
> include/linux/netdevice.h | 2 ++
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_fcoe.c | 6 ++----
> drivers/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 ++++-------
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 4 ++--
> drivers/scsi/fcoe/fcoe.c | 4 ++--
> net/8021q/vlan_dev.c | 1 +
> net/ethtool/common.c | 1 -
> 12 files changed, 20 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
> index e65ffdfc9e0a..c3bbf101a887 100644
> --- a/Documentation/networking/net_cachelines/net_device.rst
> +++ b/Documentation/networking/net_cachelines/net_device.rst
> @@ -167,6 +167,7 @@ unsigned:1 threaded -
> unsigned_long:1 see_all_hwtstamp_requests
> unsigned_long:1 change_proto_down
> unsigned_long:1 netns_local
> +unsigned_long:1 fcoe_mtu
> struct_list_head net_notifier_list
> struct_macsec_ops* macsec_ops
> struct_udp_tunnel_nic_info* udp_tunnel_nic_info
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index d5a3836f4793..37af2c6e7caf 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -58,7 +58,7 @@ enum {
>
> NETIF_F_FCOE_CRC_BIT, /* FCoE CRC32 */
> NETIF_F_SCTP_CRC_BIT, /* SCTP checksum offload */
> - NETIF_F_FCOE_MTU_BIT, /* Supports max FCoE MTU, 2158 bytes*/
> + __UNUSED_NETIF_F_37,
> NETIF_F_NTUPLE_BIT, /* N-tuple filters supported */
> NETIF_F_RXHASH_BIT, /* Receive hashing offload */
> NETIF_F_RXCSUM_BIT, /* Receive checksumming offload */
> @@ -105,7 +105,6 @@ enum {
> #define __NETIF_F(name) __NETIF_F_BIT(NETIF_F_##name##_BIT)
>
> #define NETIF_F_FCOE_CRC __NETIF_F(FCOE_CRC)
> -#define NETIF_F_FCOE_MTU __NETIF_F(FCOE_MTU)
> #define NETIF_F_FRAGLIST __NETIF_F(FRAGLIST)
> #define NETIF_F_FSO __NETIF_F(FSO)
> #define NETIF_F_GRO __NETIF_F(GRO)
> @@ -210,8 +209,7 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
> #define NETIF_F_ALL_TSO (NETIF_F_TSO | NETIF_F_TSO6 | \
> NETIF_F_TSO_ECN | NETIF_F_TSO_MANGLEID)
>
> -#define NETIF_F_ALL_FCOE (NETIF_F_FCOE_CRC | NETIF_F_FCOE_MTU | \
> - NETIF_F_FSO)
> +#define NETIF_F_ALL_FCOE (NETIF_F_FCOE_CRC | NETIF_F_FSO)
>
> /* List of features with software fallbacks. */
> #define NETIF_F_GSO_SOFTWARE (NETIF_F_ALL_TSO | NETIF_F_GSO_SCTP | \
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index a698e2402420..ca5f0dda733b 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1969,6 +1969,7 @@ enum netdev_reg_state {
> * HWTSTAMP_SOURCE_NETDEV
> * @change_proto_down: device supports setting carrier via IFLA_PROTO_DOWN
> * @netns_local: interface can't change network namespaces
> + * @fcoe_mtu: device supports maximum FCoE MTU, 2158 bytes
> *
> * @net_notifier_list: List of per-net netdev notifier block
> * that follow this device when it is moved
> @@ -2363,6 +2364,7 @@ struct net_device {
> unsigned long see_all_hwtstamp_requests:1;
> unsigned long change_proto_down:1;
> unsigned long netns_local:1;
> + unsigned long fcoe_mtu:1;
>
> struct list_head net_notifier_list;
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_fcoe.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_fcoe.c
> index 33b2c0c45509..f6f745f5c022 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_fcoe.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_fcoe.c
> @@ -81,8 +81,7 @@ int cxgb_fcoe_enable(struct net_device *netdev)
>
> netdev->features |= NETIF_F_FCOE_CRC;
> netdev->vlan_features |= NETIF_F_FCOE_CRC;
> - netdev->features |= NETIF_F_FCOE_MTU;
> - netdev->vlan_features |= NETIF_F_FCOE_MTU;
> + netdev->fcoe_mtu = true;
>
> netdev_features_change(netdev);
>
> @@ -112,8 +111,7 @@ int cxgb_fcoe_disable(struct net_device *netdev)
>
> netdev->features &= ~NETIF_F_FCOE_CRC;
> netdev->vlan_features &= ~NETIF_F_FCOE_CRC;
> - netdev->features &= ~NETIF_F_FCOE_MTU;
> - netdev->vlan_features &= ~NETIF_F_FCOE_MTU;
> + netdev->fcoe_mtu = false;
>
> netdev_features_change(netdev);
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
> index e85f7d2e8810..f2709b10c2e5 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_dcb_nl.c
> @@ -317,7 +317,7 @@ static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
> int max_frame = adapter->netdev->mtu + ETH_HLEN + ETH_FCS_LEN;
>
> #ifdef IXGBE_FCOE
> - if (adapter->netdev->features & NETIF_F_FCOE_MTU)
> + if (adapter->netdev->fcoe_mtu)
> max_frame = max(max_frame, IXGBE_FCOE_JUMBO_FRAME_SIZE);
> #endif
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> index 18d63c8c2ff4..955dced844a9 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
> @@ -858,7 +858,7 @@ int ixgbe_fcoe_enable(struct net_device *netdev)
>
> /* enable FCoE and notify stack */
> adapter->flags |= IXGBE_FLAG_FCOE_ENABLED;
> - netdev->features |= NETIF_F_FCOE_MTU;
> + netdev->fcoe_mtu = true;
> netdev_features_change(netdev);
>
> /* release existing queues and reallocate them */
> @@ -898,7 +898,7 @@ int ixgbe_fcoe_disable(struct net_device *netdev)
>
> /* disable FCoE and notify stack */
> adapter->flags &= ~IXGBE_FLAG_FCOE_ENABLED;
> - netdev->features &= ~NETIF_F_FCOE_MTU;
> + netdev->fcoe_mtu = false;
>
> netdev_features_change(netdev);
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> index 0ee943db3dc9..16fa621ce0ff 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
> @@ -981,7 +981,7 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
> set_bit(__IXGBE_RX_CSUM_UDP_ZERO_ERR, &ring->state);
>
> #ifdef IXGBE_FCOE
> - if (adapter->netdev->features & NETIF_F_FCOE_MTU) {
> + if (adapter->netdev->fcoe_mtu) {
> struct ixgbe_ring_feature *f;
> f = &adapter->ring_feature[RING_F_FCOE];
> if ((rxr_idx >= f->offset) &&
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 8057cef61f39..8b8404d8c946 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -5079,7 +5079,7 @@ static void ixgbe_configure_dcb(struct ixgbe_adapter *adapter)
> netif_set_tso_max_size(adapter->netdev, 32768);
>
> #ifdef IXGBE_FCOE
> - if (adapter->netdev->features & NETIF_F_FCOE_MTU)
> + if (adapter->netdev->fcoe_mtu)
> max_frame = max(max_frame, IXGBE_FCOE_JUMBO_FRAME_SIZE);
> #endif
>
> @@ -5136,8 +5136,7 @@ static int ixgbe_hpbthresh(struct ixgbe_adapter *adapter, int pb)
>
> #ifdef IXGBE_FCOE
> /* FCoE traffic class uses FCOE jumbo frames */
> - if ((dev->features & NETIF_F_FCOE_MTU) &&
> - (tc < IXGBE_FCOE_JUMBO_FRAME_SIZE) &&
> + if (dev->fcoe_mtu && tc < IXGBE_FCOE_JUMBO_FRAME_SIZE &&
> (pb == ixgbe_fcoe_get_tc(adapter)))
> tc = IXGBE_FCOE_JUMBO_FRAME_SIZE;
> #endif
> @@ -5197,8 +5196,7 @@ static int ixgbe_lpbthresh(struct ixgbe_adapter *adapter, int pb)
>
> #ifdef IXGBE_FCOE
> /* FCoE traffic class uses FCOE jumbo frames */
> - if ((dev->features & NETIF_F_FCOE_MTU) &&
> - (tc < IXGBE_FCOE_JUMBO_FRAME_SIZE) &&
> + if (dev->fcoe_mtu && tc < IXGBE_FCOE_JUMBO_FRAME_SIZE &&
> (pb == netdev_get_prio_tc_map(dev, adapter->fcoe.up)))
> tc = IXGBE_FCOE_JUMBO_FRAME_SIZE;
> #endif
> @@ -11096,8 +11094,7 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> NETIF_F_FCOE_CRC;
>
> netdev->vlan_features |= NETIF_F_FSO |
> - NETIF_F_FCOE_CRC |
> - NETIF_F_FCOE_MTU;
> + NETIF_F_FCOE_CRC;
> }
> #endif /* IXGBE_FCOE */
> if (adapter->flags2 & IXGBE_FLAG2_RSC_CAPABLE)
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index fcfd0a075eee..e71715f5da22 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -495,7 +495,7 @@ static int ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 max_frame, u32 vf
> int err = 0;
>
> #ifdef CONFIG_FCOE
> - if (dev->features & NETIF_F_FCOE_MTU)
> + if (dev->fcoe_mtu)
> pf_max_frame = max_t(int, pf_max_frame,
> IXGBE_FCOE_JUMBO_FRAME_SIZE);
>
> @@ -857,7 +857,7 @@ static void ixgbe_set_vf_rx_tx(struct ixgbe_adapter *adapter, int vf)
> int pf_max_frame = dev->mtu + ETH_HLEN;
>
> #if IS_ENABLED(CONFIG_FCOE)
> - if (dev->features & NETIF_F_FCOE_MTU)
> + if (dev->fcoe_mtu)
> pf_max_frame = max_t(int, pf_max_frame,
> IXGBE_FCOE_JUMBO_FRAME_SIZE);
> #endif /* CONFIG_FCOE */
> diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
> index f1429f270170..39aec710660c 100644
> --- a/drivers/scsi/fcoe/fcoe.c
> +++ b/drivers/scsi/fcoe/fcoe.c
> @@ -722,7 +722,7 @@ static int fcoe_netdev_config(struct fc_lport *lport, struct net_device *netdev)
> * will return 0, so do this first.
> */
> mfs = netdev->mtu;
> - if (netdev->features & NETIF_F_FCOE_MTU) {
> + if (netdev->fcoe_mtu) {
> mfs = FCOE_MTU;
> FCOE_NETDEV_DBG(netdev, "Supports FCOE_MTU of %d bytes\n", mfs);
> }
> @@ -1863,7 +1863,7 @@ static int fcoe_device_notification(struct notifier_block *notifier,
> case NETDEV_CHANGE:
> break;
> case NETDEV_CHANGEMTU:
> - if (netdev->features & NETIF_F_FCOE_MTU)
> + if (netdev->fcoe_mtu)
> break;
> mfs = netdev->mtu - (sizeof(struct fcoe_hdr) +
> sizeof(struct fcoe_crc_eof));
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index 3ca485537d77..09b46b057ab2 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -571,6 +571,7 @@ static int vlan_dev_init(struct net_device *dev)
>
> dev->features |= dev->hw_features;
> dev->lltx = true;
> + dev->fcoe_mtu = true;
> netif_inherit_tso_max(dev, real_dev);
> if (dev->features & NETIF_F_VLAN_FEATURES)
> netdev_warn(real_dev, "VLAN features are set incorrectly. Q-in-Q configurations may not work correctly.\n");
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index ca8e64162104..00f93c58b319 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -50,7 +50,6 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
>
> [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
> [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp",
> - [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu",
> [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter",
> [NETIF_F_RXHASH_BIT] = "rx-hashing",
> [NETIF_F_RXCSUM_BIT] = "rx-checksum",
Powered by blists - more mailing lists