ethtool: do not set unavailable or never_changed bits

diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c
index ac21ef0f61a82924494cbbffad6b47e4d9f735e3..59b1bd86f0852cd2c815f0743ff20e115d37a393 100644 (file)
--- a/src/shared/ethtool-util.c
+++ b/src/shared/ethtool-util.c
@@ -427,30 +427,31 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde
         return 0;
 }
 
-static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, struct ethtool_gstrings **ret) {
+static int get_stringset(int ethtool_fd, const char *ifname, enum ethtool_stringset stringset_id, struct ethtool_gstrings **ret) {
         _cleanup_free_ struct ethtool_gstrings *strings = NULL;
         struct {
                 struct ethtool_sset_info info;
                 uint32_t space;
         } buffer = {
-                .info = {
-                        .cmd = ETHTOOL_GSSET_INFO,
-                        .sset_mask = UINT64_C(1) << stringset_id,
-                },
+                .info.cmd = ETHTOOL_GSSET_INFO,
+                .info.sset_mask = UINT64_C(1) << stringset_id,
         };
-        unsigned len;
+        struct ifreq ifr = {
+                .ifr_data = (void*) &buffer,
+        };
+        uint32_t len;
 
         assert(ethtool_fd >= 0);
-        assert(ifr);
+        assert(ifname);
         assert(ret);
 
-        ifr->ifr_data = (void *) &buffer.info;
+        strscpy(ifr.ifr_name, sizeof(ifr.ifr_name), ifname);
 
-        if (ioctl(ethtool_fd, SIOCETHTOOL, ifr) < 0)
+        if (ioctl(ethtool_fd, SIOCETHTOOL, &ifr) < 0)
                 return -errno;
 
-        if (!buffer.info.sset_mask)
-                return -EINVAL;
+        if (buffer.info.sset_mask == 0)
+                return -EOPNOTSUPP;
 
 #pragma GCC diagnostic push
 #if HAVE_ZERO_LENGTH_BOUNDS
@@ -458,8 +459,10 @@ static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, st
 #endif
         len = buffer.info.data[0];
 #pragma GCC diagnostic pop
+        if (len == 0)
+                return -EOPNOTSUPP;
 
-        strings = malloc0(sizeof(struct ethtool_gstrings) + len * ETH_GSTRING_LEN);
+        strings = malloc0(offsetof(struct ethtool_gstrings, data) + len * ETH_GSTRING_LEN);
         if (!strings)
                 return -ENOMEM;
 
@@ -467,47 +470,92 @@ static int get_stringset(int ethtool_fd, struct ifreq *ifr, int stringset_id, st
         strings->string_set = stringset_id;
         strings->len = len;
 
-        ifr->ifr_data = (void *) strings;
+        ifr.ifr_data = (void*) strings;
 
-        if (ioctl(ethtool_fd, SIOCETHTOOL, ifr) < 0)
+        if (ioctl(ethtool_fd, SIOCETHTOOL, &ifr) < 0)
                 return -errno;
 
         *ret = TAKE_PTR(strings);
+        return 0;
+}
+
+static int get_features(int ethtool_fd, const char *ifname, uint32_t n_features, struct ethtool_gfeatures **ret) {
+        _cleanup_free_ struct ethtool_gfeatures *gfeatures = NULL;
+        struct ifreq ifr;
+
+        assert(ethtool_fd >= 0);
+        assert(ifname);
+        assert(ret);
+        assert(n_features > 0);
+
+        gfeatures = malloc0(offsetof(struct ethtool_gfeatures, features) +
+                            DIV_ROUND_UP(n_features, 32U) * sizeof(gfeatures->features[0]));
+        if (!gfeatures)
+                return -ENOMEM;
+
+        gfeatures->cmd = ETHTOOL_GFEATURES;
+        gfeatures->size = DIV_ROUND_UP(n_features, 32U);
+
+        ifr = (struct ifreq) {
+                .ifr_data = (void*) gfeatures,
+        };
+        strscpy(ifr.ifr_name, sizeof(ifr.ifr_name), ifname);
+
+        if (ioctl(ethtool_fd, SIOCETHTOOL, &ifr) < 0)
+                return -errno;
 
+        *ret = TAKE_PTR(gfeatures);
         return 0;
 }
 
 static int set_features_bit(
                 const struct ethtool_gstrings *strings,
+                const struct ethtool_gfeatures *gfeatures,
+                struct ethtool_sfeatures *sfeatures,
                 const char *feature,
-                bool flag,
-                struct ethtool_sfeatures *sfeatures) {
+                int flag) {
+
         bool found = false;
+        int r = -ENODATA;
 
         assert(strings);
-        assert(feature);
+        assert(gfeatures);
         assert(sfeatures);
+        assert(feature);
+
+        if (flag < 0)
+                return 0;
+
+        for (uint32_t i = 0; i < strings->len; i++) {
+                uint32_t block, mask;
 
-        for (size_t i = 0; i < strings->len; i++)
-                if (streq((char *) &strings->data[i * ETH_GSTRING_LEN], feature) ||
-                    (endswith(feature, "-") && startswith((char *) &strings->data[i * ETH_GSTRING_LEN], feature))) {
-                        size_t block, bit;
+                if (!strneq((const char*) &strings->data[i * ETH_GSTRING_LEN], feature, ETH_GSTRING_LEN) &&
+                    !(endswith(feature, "-") && startswith((const char*) &strings->data[i * ETH_GSTRING_LEN], feature)))
+                        continue;
 
-                        block = i / 32;
-                        bit = i % 32;
+                block = i / 32;
+                mask = UINT32_C(1) << (i % 32);
 
-                        sfeatures->features[block].valid |= 1 << bit;
-                        SET_FLAG(sfeatures->features[block].requested, 1 << bit, flag);
-                        found = true;
+                if (!FLAGS_SET(gfeatures->features[block].available, mask) ||
+                    FLAGS_SET(gfeatures->features[block].never_changed, mask)) {
+                        r = -EOPNOTSUPP;
+                        continue;
                 }
 
-        return found ? 0 : -ENODATA;
+                sfeatures->features[block].valid |= mask;
+                SET_FLAG(sfeatures->features[block].requested, mask, flag);
+
+                found = true;
+        }
+
+        return found ? 0 : r;
 }
 
 int ethtool_set_features(int *ethtool_fd, const char *ifname, const int features[static _NET_DEV_FEAT_MAX]) {
         _cleanup_free_ struct ethtool_gstrings *strings = NULL;
-        struct ethtool_sfeatures *sfeatures;
-        struct ifreq ifr = {};
+        _cleanup_free_ struct ethtool_gfeatures *gfeatures = NULL;
+        _cleanup_free_ struct ethtool_sfeatures *sfeatures = NULL;
+        struct ifreq ifr;
         bool have = false;
         int r;
 
@@ -528,26 +576,32 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int features
         if (r < 0)
                 return r;
 
-        strscpy(ifr.ifr_name, sizeof(ifr.ifr_name), ifname);
+        r = get_stringset(*ethtool_fd, ifname, ETH_SS_FEATURES, &strings);
+        if (r < 0)
+                return log_debug_errno(r, "ethtool: could not get ethtool feature strings: %m");
 
-        r = get_stringset(*ethtool_fd, &ifr, ETH_SS_FEATURES, &strings);
+        r = get_features(*ethtool_fd, ifname, strings->len, &gfeatures);
         if (r < 0)
-                return log_debug_errno(r, "ethtool: could not get ethtool features for %s", ifname);
+                return log_debug_errno(r, "ethtool: could not get ethtool features for %s: %m", ifname);
+
+        sfeatures = malloc0(offsetof(struct ethtool_sfeatures, features) +
+                            DIV_ROUND_UP(strings->len, 32U) * sizeof(sfeatures->features[0]));
+        if (!sfeatures)
+                return log_oom_debug();
 
-        sfeatures = alloca0(sizeof(struct ethtool_sfeatures) + DIV_ROUND_UP(strings->len, 32U) * sizeof(sfeatures->features[0]));
         sfeatures->cmd = ETHTOOL_SFEATURES;
         sfeatures->size = DIV_ROUND_UP(strings->len, 32U);
 
-        for (size_t i = 0; i < _NET_DEV_FEAT_MAX; i++)
-                if (features[i] >= 0) {
-                        r = set_features_bit(strings, netdev_feature_table[i], features[i], sfeatures);
-                        if (r < 0) {
-                                log_debug_errno(r, "ethtool: could not find feature, ignoring: %s", netdev_feature_table[i]);
-                                continue;
-                        }
-                }
+        for (size_t i = 0; i < _NET_DEV_FEAT_MAX; i++) {
+                r = set_features_bit(strings, gfeatures, sfeatures, netdev_feature_table[i], features[i]);
+                if (r < 0)
+                        log_debug_errno(r, "ethtool: could not set feature %s for %s, ignoring: %m", netdev_feature_table[i], ifname);
+        }
 
-        ifr.ifr_data = (void *) sfeatures;
+        ifr = (struct ifreq) {
+                .ifr_data = (void*) sfeatures,
+        };
+        strscpy(ifr.ifr_name, sizeof(ifr.ifr_name), ifname);
 
         if (ioctl(*ethtool_fd, SIOCETHTOOL, &ifr) < 0)
                 return log_debug_errno(errno, "ethtool: could not set ethtool features for %s", ifname);