2020-06-20 04:15:32

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH net 0/3] net: ethtool: netdev_features_strings[] cleanup

This little series adds the last forgotten feature string for
NETIF_F_GSO_TUNNEL_REMCSUM and attempts to prevent such losses
in future.

Patches 2-3 seem more like net-next candidates rather than net-fixes,
but if we don't stop it now, no guarantees that it won't happen again
soon.

I was thinking about some kind of static assertion to have an early
prevention mechanism for this, but the existing of 2 intended holes
(former NO_CSUM and UFO) makes this problematic, at least at first
sight.

Alexander Lobakin (3):
net: ethtool: add missing string for NETIF_F_GSO_TUNNEL_REMCSUM
net: ethtool: fix indentation of netdev_features_strings
net: ethtool: sync netdev_features_strings order with enum
netdev_features

net/ethtool/common.c | 133 ++++++++++++++++++++++++-------------------
1 file changed, 74 insertions(+), 59 deletions(-)

--
2.27.0



2020-06-20 04:26:27

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH net 3/3] net: ethtool: sync netdev_features_strings order with enum netdev_features

The ordering of netdev_features_strings[] makes no sense when it comes
to user interaction, as list of features in `ethtool -k` input is sorted
according to the corresponding bit's position.
Instead, it *does* make sense when it comes to adding new netdev_features
or modifying existing ones. We have at least 2 occasions of forgetting to
add the strings for newly introduced features, and one of them existed
since 3.1x times till now.

Let's keep this stringtable sorted according to bit's position in enum
netdev_features, as this simplifies both reading and modification of the
source code and can help not to miss or forget anything.

Signed-off-by: Alexander Lobakin <[email protected]>
---
net/ethtool/common.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index c8e3fce6e48d..24f35d47832d 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -8,25 +8,25 @@
const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
[NETIF_F_SG_BIT] = "tx-scatter-gather",
[NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
+
+ /* __UNUSED_NETIF_F_1 - deprecated */
+
[NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
[NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
[NETIF_F_HIGHDMA_BIT] = "highdma",
[NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
[NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
-
[NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
[NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
- [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
- [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
- [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
[NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
[NETIF_F_GSO_BIT] = "tx-generic-segmentation",
[NETIF_F_LLTX_BIT] = "tx-lockless",
[NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
[NETIF_F_GRO_BIT] = "rx-gro",
- [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
[NETIF_F_LRO_BIT] = "rx-lro",

+ /* NETIF_F_GSO_SHIFT = NETIF_F_TSO_BIT */
+
[NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
[NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
[NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
@@ -43,9 +43,14 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
[NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
[NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
[NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
+
+ /* NETIF_F_GSO_UDP_BIT - deprecated */
+
[NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
[NETIF_F_GSO_FRAGLIST_BIT] = "tx-gso-list",

+ /* NETIF_F_GSO_LAST = NETIF_F_GSO_FRAGLIST_BIT */
+
[NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
[NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp",
[NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu",
@@ -56,16 +61,25 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
[NETIF_F_LOOPBACK_BIT] = "loopback",
[NETIF_F_RXFCS_BIT] = "rx-fcs",
[NETIF_F_RXALL_BIT] = "rx-all",
+ [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
+ [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
+ [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
[NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
+
[NETIF_F_HW_TC_BIT] = "hw-tc-offload",
[NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
[NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
[NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
- [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
[NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
[NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
+
+ [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
+ [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
[NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
+
[NETIF_F_HW_MACSEC_BIT] = "macsec-hw-offload",
+
+ /* NETDEV_FEATURE_COUNT */
};

const char
--
2.27.0


2020-06-20 04:28:48

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH net 2/3] net: ethtool: fix indentation of netdev_features_strings

The current indentation is an absolute mess of tabs, spaces and their
mixes in different proportions. Convert it all to plain tabs and move
assignment operation char to the right, which is the most commonly
used style in Linux code.

Signed-off-by: Alexander Lobakin <[email protected]>
---
net/ethtool/common.c | 120 +++++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index aaecfc916a4d..c8e3fce6e48d 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -6,66 +6,66 @@
#include "common.h"

const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
- [NETIF_F_SG_BIT] = "tx-scatter-gather",
- [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
- [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
- [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
- [NETIF_F_HIGHDMA_BIT] = "highdma",
- [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
- [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
-
- [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
- [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
- [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
- [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
- [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
- [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
- [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
- [NETIF_F_LLTX_BIT] = "tx-lockless",
- [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
- [NETIF_F_GRO_BIT] = "rx-gro",
- [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
- [NETIF_F_LRO_BIT] = "rx-lro",
-
- [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
- [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
- [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
- [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation",
- [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
- [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation",
- [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation",
- [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation",
- [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation",
- [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation",
- [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
- [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
- [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
- [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
- [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
- [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
- [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
- [NETIF_F_GSO_FRAGLIST_BIT] = "tx-gso-list",
-
- [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",
- [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
- [NETIF_F_LOOPBACK_BIT] = "loopback",
- [NETIF_F_RXFCS_BIT] = "rx-fcs",
- [NETIF_F_RXALL_BIT] = "rx-all",
- [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
- [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
- [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
- [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
- [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
- [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
- [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
- [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
- [NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
- [NETIF_F_HW_MACSEC_BIT] = "macsec-hw-offload",
+ [NETIF_F_SG_BIT] = "tx-scatter-gather",
+ [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
+ [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
+ [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
+ [NETIF_F_HIGHDMA_BIT] = "highdma",
+ [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
+ [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
+
+ [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
+ [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
+ [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
+ [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
+ [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
+ [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
+ [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
+ [NETIF_F_LLTX_BIT] = "tx-lockless",
+ [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
+ [NETIF_F_GRO_BIT] = "rx-gro",
+ [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
+ [NETIF_F_LRO_BIT] = "rx-lro",
+
+ [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
+ [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
+ [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
+ [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation",
+ [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
+ [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation",
+ [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation",
+ [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation",
+ [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation",
+ [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation",
+ [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
+ [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
+ [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
+ [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
+ [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
+ [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
+ [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
+ [NETIF_F_GSO_FRAGLIST_BIT] = "tx-gso-list",
+
+ [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",
+ [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
+ [NETIF_F_LOOPBACK_BIT] = "loopback",
+ [NETIF_F_RXFCS_BIT] = "rx-fcs",
+ [NETIF_F_RXALL_BIT] = "rx-all",
+ [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
+ [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
+ [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
+ [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
+ [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
+ [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
+ [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
+ [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
+ [NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
+ [NETIF_F_HW_MACSEC_BIT] = "macsec-hw-offload",
};

const char
--
2.27.0


2020-06-20 04:30:32

by Alexander Lobakin

[permalink] [raw]
Subject: [PATCH net 1/3] net: ethtool: add missing string for NETIF_F_GSO_TUNNEL_REMCSUM

Commit e585f2363637 ("udp: Changes to udp_offload to support remote
checksum offload") added new GSO type and a corresponding netdev
feature, but missed Ethtool's 'netdev_features_strings' table.
Give it a name so it will be exposed to userspace and become available
for manual configuration.

Fixes: e585f2363637 ("udp: Changes to udp_offload to support remote
checksum offload")
Signed-off-by: Alexander Lobakin <[email protected]>
---
net/ethtool/common.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 47f63526818e..aaecfc916a4d 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -40,6 +40,7 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
[NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
[NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
[NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
+ [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
[NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
[NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
[NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
--
2.27.0


2020-06-21 00:49:09

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net 0/3] net: ethtool: netdev_features_strings[] cleanup


Please submit this again, I have two copies in my inbox and I have no idea
what is different between them.

Also, in some of your patches you cut the Fixes: tag into mutliple lines
please do not do that. The Fixes: tag line should be one single line no
matter how long it is.

Thank you.

2020-06-22 22:04:49

by Michal Kubecek

[permalink] [raw]
Subject: Re: [PATCH net 1/3] net: ethtool: add missing string for NETIF_F_GSO_TUNNEL_REMCSUM

On Fri, Jun 19, 2020 at 06:49:46PM +0000, Alexander Lobakin wrote:
> Commit e585f2363637 ("udp: Changes to udp_offload to support remote
> checksum offload") added new GSO type and a corresponding netdev
> feature, but missed Ethtool's 'netdev_features_strings' table.
> Give it a name so it will be exposed to userspace and become available
> for manual configuration.
>
> Fixes: e585f2363637 ("udp: Changes to udp_offload to support remote
> checksum offload")
> Signed-off-by: Alexander Lobakin <[email protected]>

Reviewed-by: Michal Kubecek <[email protected]>

> ---
> net/ethtool/common.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index 47f63526818e..aaecfc916a4d 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -40,6 +40,7 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
> [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
> [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
> [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
> + [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
> [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
> [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
> [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
> --
> 2.27.0
>
>


Attachments:
(No filename) (1.34 kB)
signature.asc (499.00 B)
Download all attachments

2020-06-22 22:07:48

by Michal Kubecek

[permalink] [raw]
Subject: Re: [PATCH net 2/3] net: ethtool: fix indentation of netdev_features_strings

On Fri, Jun 19, 2020 at 06:50:06PM +0000, Alexander Lobakin wrote:
> The current indentation is an absolute mess of tabs, spaces and their
> mixes in different proportions. Convert it all to plain tabs and move
> assignment operation char to the right, which is the most commonly
> used style in Linux code.
>
> Signed-off-by: Alexander Lobakin <[email protected]>

Reviewed-by: Michal Kubecek <[email protected]>

But this is a pure cleanup so it should rather go to net-next.

Michal

> ---
> net/ethtool/common.c | 120 +++++++++++++++++++++----------------------
> 1 file changed, 60 insertions(+), 60 deletions(-)
>
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index aaecfc916a4d..c8e3fce6e48d 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -6,66 +6,66 @@
> #include "common.h"
>
> const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
> - [NETIF_F_SG_BIT] = "tx-scatter-gather",
> - [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
> - [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
> - [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
> - [NETIF_F_HIGHDMA_BIT] = "highdma",
> - [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
> - [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
> -
> - [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
> - [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
> - [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
> - [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
> - [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
> - [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
> - [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
> - [NETIF_F_LLTX_BIT] = "tx-lockless",
> - [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> - [NETIF_F_GRO_BIT] = "rx-gro",
> - [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> - [NETIF_F_LRO_BIT] = "rx-lro",
> -
> - [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
> - [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
> - [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
> - [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation",
> - [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
> - [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation",
> - [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation",
> - [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation",
> - [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation",
> - [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation",
> - [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
> - [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
> - [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
> - [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
> - [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
> - [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
> - [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
> - [NETIF_F_GSO_FRAGLIST_BIT] = "tx-gso-list",
> -
> - [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",
> - [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
> - [NETIF_F_LOOPBACK_BIT] = "loopback",
> - [NETIF_F_RXFCS_BIT] = "rx-fcs",
> - [NETIF_F_RXALL_BIT] = "rx-all",
> - [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
> - [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
> - [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
> - [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
> - [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
> - [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> - [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> - [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
> - [NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
> - [NETIF_F_HW_MACSEC_BIT] = "macsec-hw-offload",
> + [NETIF_F_SG_BIT] = "tx-scatter-gather",
> + [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
> + [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
> + [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
> + [NETIF_F_HIGHDMA_BIT] = "highdma",
> + [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
> + [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
> +
> + [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
> + [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
> + [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
> + [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
> + [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
> + [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
> + [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
> + [NETIF_F_LLTX_BIT] = "tx-lockless",
> + [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> + [NETIF_F_GRO_BIT] = "rx-gro",
> + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> + [NETIF_F_LRO_BIT] = "rx-lro",
> +
> + [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
> + [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
> + [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
> + [NETIF_F_TSO_MANGLEID_BIT] = "tx-tcp-mangleid-segmentation",
> + [NETIF_F_TSO6_BIT] = "tx-tcp6-segmentation",
> + [NETIF_F_FSO_BIT] = "tx-fcoe-segmentation",
> + [NETIF_F_GSO_GRE_BIT] = "tx-gre-segmentation",
> + [NETIF_F_GSO_GRE_CSUM_BIT] = "tx-gre-csum-segmentation",
> + [NETIF_F_GSO_IPXIP4_BIT] = "tx-ipxip4-segmentation",
> + [NETIF_F_GSO_IPXIP6_BIT] = "tx-ipxip6-segmentation",
> + [NETIF_F_GSO_UDP_TUNNEL_BIT] = "tx-udp_tnl-segmentation",
> + [NETIF_F_GSO_UDP_TUNNEL_CSUM_BIT] = "tx-udp_tnl-csum-segmentation",
> + [NETIF_F_GSO_PARTIAL_BIT] = "tx-gso-partial",
> + [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
> + [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
> + [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
> + [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
> + [NETIF_F_GSO_FRAGLIST_BIT] = "tx-gso-list",
> +
> + [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",
> + [NETIF_F_NOCACHE_COPY_BIT] = "tx-nocache-copy",
> + [NETIF_F_LOOPBACK_BIT] = "loopback",
> + [NETIF_F_RXFCS_BIT] = "rx-fcs",
> + [NETIF_F_RXALL_BIT] = "rx-all",
> + [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
> + [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
> + [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
> + [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
> + [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
> + [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> + [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> + [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
> + [NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
> + [NETIF_F_HW_MACSEC_BIT] = "macsec-hw-offload",
> };
>
> const char
> --
> 2.27.0
>
>


Attachments:
(No filename) (7.16 kB)
signature.asc (499.00 B)
Download all attachments

2020-06-22 22:14:31

by Michal Kubecek

[permalink] [raw]
Subject: Re: [PATCH net 3/3] net: ethtool: sync netdev_features_strings order with enum netdev_features

On Fri, Jun 19, 2020 at 06:39:59PM +0000, Alexander Lobakin wrote:
> The ordering of netdev_features_strings[] makes no sense when it comes
> to user interaction, as list of features in `ethtool -k` input is sorted
> according to the corresponding bit's position.
> Instead, it *does* make sense when it comes to adding new netdev_features
> or modifying existing ones. We have at least 2 occasions of forgetting to
> add the strings for newly introduced features, and one of them existed
> since 3.1x times till now.
>
> Let's keep this stringtable sorted according to bit's position in enum
> netdev_features, as this simplifies both reading and modification of the
> source code and can help not to miss or forget anything.
>
> Signed-off-by: Alexander Lobakin <[email protected]>

Reviewed-by: Michal Kubecek <[email protected]>

This would also rather belong to net-next, IMHO.

Michal

> ---
> net/ethtool/common.c | 26 ++++++++++++++++++++------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> index c8e3fce6e48d..24f35d47832d 100644
> --- a/net/ethtool/common.c
> +++ b/net/ethtool/common.c
> @@ -8,25 +8,25 @@
> const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
> [NETIF_F_SG_BIT] = "tx-scatter-gather",
> [NETIF_F_IP_CSUM_BIT] = "tx-checksum-ipv4",
> +
> + /* __UNUSED_NETIF_F_1 - deprecated */
> +
> [NETIF_F_HW_CSUM_BIT] = "tx-checksum-ip-generic",
> [NETIF_F_IPV6_CSUM_BIT] = "tx-checksum-ipv6",
> [NETIF_F_HIGHDMA_BIT] = "highdma",
> [NETIF_F_FRAGLIST_BIT] = "tx-scatter-gather-fraglist",
> [NETIF_F_HW_VLAN_CTAG_TX_BIT] = "tx-vlan-hw-insert",
> -
> [NETIF_F_HW_VLAN_CTAG_RX_BIT] = "rx-vlan-hw-parse",
> [NETIF_F_HW_VLAN_CTAG_FILTER_BIT] = "rx-vlan-filter",
> - [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
> - [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
> - [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
> [NETIF_F_VLAN_CHALLENGED_BIT] = "vlan-challenged",
> [NETIF_F_GSO_BIT] = "tx-generic-segmentation",
> [NETIF_F_LLTX_BIT] = "tx-lockless",
> [NETIF_F_NETNS_LOCAL_BIT] = "netns-local",
> [NETIF_F_GRO_BIT] = "rx-gro",
> - [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> [NETIF_F_LRO_BIT] = "rx-lro",
>
> + /* NETIF_F_GSO_SHIFT = NETIF_F_TSO_BIT */
> +
> [NETIF_F_TSO_BIT] = "tx-tcp-segmentation",
> [NETIF_F_GSO_ROBUST_BIT] = "tx-gso-robust",
> [NETIF_F_TSO_ECN_BIT] = "tx-tcp-ecn-segmentation",
> @@ -43,9 +43,14 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
> [NETIF_F_GSO_TUNNEL_REMCSUM_BIT] = "tx-tunnel-remcsum-segmentation",
> [NETIF_F_GSO_SCTP_BIT] = "tx-sctp-segmentation",
> [NETIF_F_GSO_ESP_BIT] = "tx-esp-segmentation",
> +
> + /* NETIF_F_GSO_UDP_BIT - deprecated */
> +
> [NETIF_F_GSO_UDP_L4_BIT] = "tx-udp-segmentation",
> [NETIF_F_GSO_FRAGLIST_BIT] = "tx-gso-list",
>
> + /* NETIF_F_GSO_LAST = NETIF_F_GSO_FRAGLIST_BIT */
> +
> [NETIF_F_FCOE_CRC_BIT] = "tx-checksum-fcoe-crc",
> [NETIF_F_SCTP_CRC_BIT] = "tx-checksum-sctp",
> [NETIF_F_FCOE_MTU_BIT] = "fcoe-mtu",
> @@ -56,16 +61,25 @@ const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN] = {
> [NETIF_F_LOOPBACK_BIT] = "loopback",
> [NETIF_F_RXFCS_BIT] = "rx-fcs",
> [NETIF_F_RXALL_BIT] = "rx-all",
> + [NETIF_F_HW_VLAN_STAG_TX_BIT] = "tx-vlan-stag-hw-insert",
> + [NETIF_F_HW_VLAN_STAG_RX_BIT] = "rx-vlan-stag-hw-parse",
> + [NETIF_F_HW_VLAN_STAG_FILTER_BIT] = "rx-vlan-stag-filter",
> [NETIF_F_HW_L2FW_DOFFLOAD_BIT] = "l2-fwd-offload",
> +
> [NETIF_F_HW_TC_BIT] = "hw-tc-offload",
> [NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
> [NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
> [NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
> - [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
> [NETIF_F_HW_TLS_RX_BIT] = "tls-hw-rx-offload",
> +
> + [NETIF_F_GRO_HW_BIT] = "rx-gro-hw",
> + [NETIF_F_HW_TLS_RECORD_BIT] = "tls-hw-record",
> [NETIF_F_GRO_FRAGLIST_BIT] = "rx-gro-list",
> +
> [NETIF_F_HW_MACSEC_BIT] = "macsec-hw-offload",
> +
> + /* NETDEV_FEATURE_COUNT */
> };
>
> const char
> --
> 2.27.0
>
>


Attachments:
(No filename) (4.29 kB)
signature.asc (499.00 B)
Download all attachments