From 235458b72fe54feb97349db4b139babc80821e58 Mon Sep 17 00:00:00 2001
From: Gris Ge <fge@redhat.com>
Date: Fri, 4 Jun 2021 06:23:47 +0800
Subject: [PATCH 2/3] ethtool feature: tx-lockless is not changeable

Currently, the `tx-lockless` is shown as changeable, but it actually is
hard coded as not changeable by `NETIF_F_NEVER_CHANGE` in kernel.

The kernel netlink use `ETHTOOL_A_FEATURES_NOCHANGE` for hard coded
non-changeable features, it take priority over `ETHTOOL_A_FEATURES_HW`.

Integration test case included, but disable in CI as CI has no ethtool
netlink interface in kernel.

Signed-off-by: Gris Ge <fge@redhat.com>
---
 src/lib/ifaces/ethtool.rs | 19 +++++++++-
 src/lib/tests/dummy.rs    | 74 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/src/lib/ifaces/ethtool.rs b/src/lib/ifaces/ethtool.rs
index 7a5c08a..0697ad5 100644
--- a/src/lib/ifaces/ethtool.rs
+++ b/src/lib/ifaces/ethtool.rs
@@ -313,6 +313,14 @@ async fn dump_feature_infos(
             let mut fixed_features = HashMap::new();
             let mut changeable_features = HashMap::new();
 
+            for nla in &nlas {
+                if let FeatureAttr::NoChange(feature_bits) = nla {
+                    for FeatureBit { name, .. } in feature_bits {
+                        fixed_features.insert(name.to_string(), false);
+                    }
+                }
+            }
+
             for nla in nlas {
                 if let FeatureAttr::Header(hdrs) = nla {
                     iface_name = get_iface_name_from_header(&hdrs);
@@ -324,7 +332,16 @@ async fn dump_feature_infos(
                                 name,
                                 value: true,
                             } => {
-                                changeable_features.insert(name, false);
+                                // Dummy interface show `tx-lockless` is
+                                // changeable, but FeatureAttr::NoChange() says
+                                // otherwise. The kernel code
+                                // `NETIF_F_NEVER_CHANGE` shows `tx-lockless`
+                                // should never been changeable.
+                                if fixed_features.contains_key(&name) {
+                                    fixed_features.insert(name, false);
+                                } else {
+                                    changeable_features.insert(name, false);
+                                }
                             }
                             FeatureBit {
                                 index: _,
diff --git a/src/lib/tests/dummy.rs b/src/lib/tests/dummy.rs
index a0feed3..aab9ffd 100644
--- a/src/lib/tests/dummy.rs
+++ b/src/lib/tests/dummy.rs
@@ -40,6 +40,66 @@ const EXPECTED_IFACE_STATE: &str = r#"---
         preferred_lft: forever
   mac_address: "00:23:45:67:89:1a""#;
 
+const EXPECTED_ETHTOOL_FEATURE: &str = r#"---
+fixed:
+  esp-hw-offload: false
+  esp-tx-csum-hw-offload: false
+  fcoe-mtu: false
+  hw-tc-offload: false
+  l2-fwd-offload: false
+  loopback: false
+  macsec-hw-offload: false
+  netns-local: false
+  rx-all: false
+  rx-checksum: false
+  rx-fcs: false
+  rx-gro-hw: false
+  rx-hashing: false
+  rx-lro: false
+  rx-ntuple-filter: false
+  rx-udp_tunnel-port-offload: false
+  rx-vlan-filter: false
+  rx-vlan-hw-parse: false
+  rx-vlan-stag-filter: false
+  rx-vlan-stag-hw-parse: false
+  tls-hw-record: false
+  tls-hw-rx-offload: false
+  tls-hw-tx-offload: false
+  tx-checksum-fcoe-crc: false
+  tx-checksum-ipv4: false
+  tx-checksum-ipv6: false
+  tx-checksum-sctp: false
+  tx-esp-segmentation: false
+  tx-fcoe-segmentation: false
+  tx-gso-list: false
+  tx-gso-partial: false
+  tx-gso-robust: false
+  tx-lockless: true
+  tx-sctp-segmentation: false
+  tx-tunnel-remcsum-segmentation: false
+  tx-udp-segmentation: false
+  tx-vlan-hw-insert: false
+  tx-vlan-stag-hw-insert: false
+  vlan-challenged: false
+changeable:
+  highdma: true
+  rx-gro: true
+  rx-gro-list: false
+  tx-checksum-ip-generic: true
+  tx-generic-segmentation: true
+  tx-gre-csum-segmentation: true
+  tx-gre-segmentation: true
+  tx-ipxip4-segmentation: true
+  tx-ipxip6-segmentation: true
+  tx-nocache-copy: false
+  tx-scatter-gather-fraglist: true
+  tx-tcp-ecn-segmentation: true
+  tx-tcp-mangleid-segmentation: true
+  tx-tcp-segmentation: true
+  tx-tcp6-segmentation: true
+  tx-udp_tnl-csum-segmentation: true
+  tx-udp_tnl-segmentation: true"#;
+
 #[test]
 fn test_get_iface_dummy_yaml() {
     with_dummy_iface(|| {
@@ -54,6 +114,20 @@ fn test_get_iface_dummy_yaml() {
     });
 }
 
+#[test]
+#[ignore] // CI does not have ethtool_netlink kernel module yet
+fn test_get_iface_dummy_ethtool_feature() {
+    with_dummy_iface(|| {
+        let state = NetState::retrieve().unwrap();
+        let iface = &state.ifaces[IFACE_NAME];
+        assert_eq!(
+            serde_yaml::to_string(&iface.ethtool.as_ref().unwrap().features)
+                .unwrap(),
+            EXPECTED_ETHTOOL_FEATURE
+        );
+    });
+}
+
 fn with_dummy_iface<T>(test: T) -> ()
 where
     T: FnOnce() -> () + panic::UnwindSafe,
-- 
2.32.0