Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 11 Apr 2024 12:28:08 +0200 | Subject | Re: [PATCH RFC net-next 7/7] netdev_features: convert NETIF_F_FCOE_MTU to IFF_FCOE_MTU | From | Alexander Lobakin <> |
| |
From: Jakub Kicinski <kuba@kernel.org> Date: Mon, 8 Apr 2024 19:38:06 -0700
> On Fri, 5 Apr 2024 15:37:31 +0200 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 netdev_priv_flags and free one more >> feature bit. > >> @@ -1700,6 +1701,7 @@ enum netdev_priv_flags { >> IFF_NETNS_LOCAL = BIT_ULL(35), >> IFF_HIGHDMA = BIT_ULL(36), >> IFF_VLAN_CHALLENGED = BIT_ULL(37), >> + IFF_FCOE_MTU = BIT_ULL(38), >> IFF_LOGICAL = IFF_NO_QUEUE | IFF_LLTX, >> IFF_ONE_FOR_ALL = IFF_HIGHDMA | IFF_VLAN_CHALLENGED, >> IFF_ALL_FOR_ALL = IFF_XMIT_DST_RELEASE | > > Any reason not to make it a bitfield? I haven't looked at the longer > patches but this one seems to be used like a basic bool.
This whole enum could be made as bitfields, should we convert it? Would be a big patch tho ._.
> >> diff --git a/net/ethtool/common.c b/net/ethtool/common.c >> index 2de4dd5a30de..71e36e1a1b15 100644 >> --- a/net/ethtool/common.c >> +++ b/net/ethtool/common.c >> @@ -47,7 +47,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", > > > But this definitely _is_ a uAPI change, right?
Why?
> Some analysis why this is fine and why avoiding the problem isn't worth > it in the cover letter would be great. > >> [NETIF_F_NTUPLE_BIT] = "rx-ntuple-filter", >> [NETIF_F_RXHASH_BIT] = "rx-hashing", >> [NETIF_F_RXCSUM_BIT] = "rx-checksum",
Thanks, Olek
| ![\](/images/icornerr.gif) |