Blob Blame History Raw
From 78e57b79c8790448412acca41e5d4495366305a6 Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Wed, 18 Aug 2021 16:41:11 +0900
Subject: [PATCH] udev: make RxChannels= or friends also accept "max"

Follow-up for 406041b7de767316674eb6a2f98ad466577ce8a4.

Also, this makes
- the settings accept an empty string,
- if the specified value is too large, also use the advertised maximum
  value.
- mention the range of the value in the man page.
---
 man/systemd.link.xml                 |  49 ++------
 src/shared/ethtool-util.c            | 170 ++++++++++-----------------
 src/shared/ethtool-util.h            |  36 +++---
 src/udev/net/link-config-gperf.gperf |  16 +--
 4 files changed, 90 insertions(+), 181 deletions(-)

diff --git a/man/systemd.link.xml b/man/systemd.link.xml
index fd744ebaed..dfb02073b2 100644
--- a/man/systemd.link.xml
+++ b/man/systemd.link.xml
@@ -710,58 +710,27 @@
       </varlistentry>
       <varlistentry>
         <term><varname>RxChannels=</varname></term>
-        <listitem>
-          <para>Sets the number of receive channels (a number between 1 and 4294967295) .</para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
         <term><varname>TxChannels=</varname></term>
-        <listitem>
-          <para>Sets the number of transmit channels (a number between 1 and 4294967295).</para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
         <term><varname>OtherChannels=</varname></term>
-        <listitem>
-          <para>Sets the number of other channels (a number between 1 and 4294967295).</para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
         <term><varname>CombinedChannels=</varname></term>
         <listitem>
-          <para>Sets the number of combined set channels (a number between 1 and 4294967295).</para>
+          <para>Specifies the number of receive, transmit, other, or combined channels, respectively.
+          Takes an unsigned integer in the range 1…4294967295 or <literal>max</literal>. If set to
+          <literal>max</literal>, the advertised maximum value of the hardware will be used. When
+          unset, the number will not be changed. Defaults to unset.</para>
         </listitem>
       </varlistentry>
       <varlistentry>
         <term><varname>RxBufferSize=</varname></term>
-        <listitem>
-          <para>Takes an integer or <literal>max</literal>. Specifies the maximum number of pending packets
-          in the NIC receive buffer. When unset, the kernel's default will be used. If set to
-          <literal>max</literal>, the hardware's advertised maximum size will be used.</para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
         <term><varname>RxMiniBufferSize=</varname></term>
-        <listitem>
-          <para>Takes an integer or <literal>max</literal>. Specifies the maximum number of pending packets
-          in the NIC mini receive buffer. When unset, the kernel's default will be used. If set to
-          <literal>max</literal>, the hardware's advertised maximum size will be used.</para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
         <term><varname>RxJumboBufferSize=</varname></term>
-        <listitem>
-          <para>Takes an integer or <literal>max</literal>. Specifies the maximum number of pending packets
-          in the NIC jumbo receive buffer. When unset, the kernel's default will be used. If set to
-          <literal>max</literal>, the hardware's advertised maximum size will be used.</para>
-        </listitem>
-      </varlistentry>
-      <varlistentry>
         <term><varname>TxBufferSize=</varname></term>
         <listitem>
-          <para>Takes an integer or <literal>max</literal>. Specifies the maximum number of pending packets
-          in the NIC transmit buffer. When unset, the kernel's default will be used. If set to
-          <literal>max</literal>, the hardware's advertised maximum size will be used.</para>
+          <para>Specifies the maximum number of pending packets in the NIC receive buffer, mini receive
+          buffer, jumbo receive buffer, or transmit buffer, respectively. Takes an unsigned integer in
+          the range 1…4294967295 or <literal>max</literal>. If set to <literal>max</literal>, the
+          advertised maximum value of the hardware will be used. When unset, the number will not be
+          changed. Defaults to unset.</para>
         </listitem>
       </varlistentry>
       <varlistentry>
diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c
index ed251ec8dd..2d41d861ba 100644
--- a/src/shared/ethtool-util.c
+++ b/src/shared/ethtool-util.c
@@ -329,6 +329,17 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et
                 dest = _v;                             \
         } while(false)
 
+#define UPDATE_WITH_MAX(dest, max, val, updated)       \
+        do {                                           \
+                typeof(dest) _v = (val);               \
+                typeof(dest) _max = (max);             \
+                if (_v == 0 || _v > _max)              \
+                        _v = _max;                     \
+                if (dest != _v)                        \
+                        updated = true;                \
+                dest = _v;                             \
+        } while(false)
+
 int ethtool_set_wol(int *ethtool_fd, const char *ifname, uint32_t wolopts) {
         struct ethtool_wolinfo ecmd = {
                 .cmd = ETHTOOL_GWOL,
@@ -382,10 +393,10 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde
         assert(ifname);
         assert(ring);
 
-        if (!ring->rx_pending_set &&
-            !ring->rx_mini_pending_set &&
-            !ring->rx_jumbo_pending_set &&
-            !ring->tx_pending_set)
+        if (!ring->rx.set &&
+            !ring->rx_mini.set &&
+            !ring->rx_jumbo.set &&
+            !ring->tx.set)
                 return 0;
 
         r = ethtool_connect(ethtool_fd);
@@ -398,25 +409,17 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde
         if (r < 0)
                 return -errno;
 
-        if (ring->rx_pending_set)
-                UPDATE(ecmd.rx_pending,
-                       ring->rx_pending == 0 ? ecmd.rx_max_pending : ring->rx_pending,
-                       need_update);
+        if (ring->rx.set)
+                UPDATE_WITH_MAX(ecmd.rx_pending, ecmd.rx_max_pending, ring->rx.value, need_update);
 
-        if (ring->rx_mini_pending_set)
-                UPDATE(ecmd.rx_mini_pending,
-                       ring->rx_mini_pending == 0 ? ecmd.rx_mini_max_pending : ring->rx_mini_pending,
-                       need_update);
+        if (ring->rx_mini.set)
+                UPDATE_WITH_MAX(ecmd.rx_mini_pending, ecmd.rx_mini_max_pending, ring->rx_mini.value, need_update);
 
-        if (ring->rx_jumbo_pending_set)
-                UPDATE(ecmd.rx_jumbo_pending,
-                       ring->rx_jumbo_pending == 0 ? ecmd.rx_jumbo_max_pending : ring->rx_jumbo_pending,
-                       need_update);
+        if (ring->rx_jumbo.set)
+                UPDATE_WITH_MAX(ecmd.rx_jumbo_pending, ecmd.rx_jumbo_max_pending, ring->rx_jumbo.value, need_update);
 
-        if (ring->tx_pending_set)
-                UPDATE(ecmd.tx_pending,
-                       ring->tx_pending == 0 ? ecmd.tx_max_pending : ring->tx_pending,
-                       need_update);
+        if (ring->tx.set)
+                UPDATE_WITH_MAX(ecmd.tx_pending, ecmd.tx_max_pending, ring->tx.value, need_update);
 
         if (!need_update)
                 return 0;
@@ -832,10 +835,10 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha
         assert(ifname);
         assert(channels);
 
-        if (!channels->rx_count_set &&
-            !channels->tx_count_set &&
-            !channels->other_count_set &&
-            !channels->combined_count_set)
+        if (!channels->rx.set &&
+            !channels->tx.set &&
+            !channels->other.set &&
+            !channels->combined.set)
                 return 0;
 
         r = ethtool_connect(fd);
@@ -848,17 +851,17 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha
         if (r < 0)
                 return -errno;
 
-        if (channels->rx_count_set)
-                UPDATE(ecmd.rx_count, channels->rx_count, need_update);
+        if (channels->rx.set)
+                UPDATE_WITH_MAX(ecmd.rx_count, ecmd.max_rx, channels->rx.value, need_update);
 
-        if (channels->tx_count_set)
-                UPDATE(ecmd.tx_count, channels->tx_count, need_update);
+        if (channels->tx.set)
+                UPDATE_WITH_MAX(ecmd.tx_count, ecmd.max_tx, channels->tx.value, need_update);
 
-        if (channels->other_count_set)
-                UPDATE(ecmd.other_count, channels->other_count, need_update);
+        if (channels->other.set)
+                UPDATE_WITH_MAX(ecmd.other_count, ecmd.max_other, channels->other.value, need_update);
 
-        if (channels->combined_count_set)
-                UPDATE(ecmd.combined_count, channels->combined_count, need_update);
+        if (channels->combined.set)
+                UPDATE_WITH_MAX(ecmd.combined_count, ecmd.max_combined, channels->combined.value, need_update);
 
         if (!need_update)
                 return 0;
@@ -917,57 +920,6 @@ int ethtool_set_flow_control(int *fd, const char *ifname, int rx, int tx, int au
         return 0;
 }
 
-int config_parse_channel(
-                const char *unit,
-                const char *filename,
-                unsigned line,
-                const char *section,
-                unsigned section_line,
-                const char *lvalue,
-                int ltype,
-                const char *rvalue,
-                void *data,
-                void *userdata) {
-
-        netdev_channels *channels = data;
-        uint32_t k;
-        int r;
-
-        assert(filename);
-        assert(section);
-        assert(lvalue);
-        assert(rvalue);
-        assert(data);
-
-        r = safe_atou32(rvalue, &k);
-        if (r < 0) {
-                log_syntax(unit, LOG_WARNING, filename, line, r,
-                           "Failed to parse channel value for %s=, ignoring: %s", lvalue, rvalue);
-                return 0;
-        }
-        if (k < 1) {
-                log_syntax(unit, LOG_WARNING, filename, line, 0,
-                           "Invalid %s= value, ignoring: %s", lvalue, rvalue);
-                return 0;
-        }
-
-        if (streq(lvalue, "RxChannels")) {
-                channels->rx_count = k;
-                channels->rx_count_set = true;
-        } else if (streq(lvalue, "TxChannels")) {
-                channels->tx_count = k;
-                channels->tx_count_set = true;
-        } else if (streq(lvalue, "OtherChannels")) {
-                channels->other_count = k;
-                channels->other_count_set = true;
-        } else if (streq(lvalue, "CombinedChannels")) {
-                channels->combined_count = k;
-                channels->combined_count_set = true;
-        }
-
-        return 0;
-}
-
 int config_parse_advertise(
                 const char *unit,
                 const char *filename,
@@ -1023,7 +975,7 @@ int config_parse_advertise(
         }
 }
 
-int config_parse_nic_buffer_size(
+int config_parse_ring_buffer_or_channel(
                 const char *unit,
                 const char *filename,
                 unsigned line,
@@ -1035,7 +987,7 @@ int config_parse_nic_buffer_size(
                 void *data,
                 void *userdata) {
 
-        netdev_ring_param *ring = data;
+        u32_opt *dst = data;
         uint32_t k;
         int r;
 
@@ -1045,36 +997,32 @@ int config_parse_nic_buffer_size(
         assert(rvalue);
         assert(data);
 
-        if (streq(rvalue, "max"))
-                k = 0;
-        else {
-                r = safe_atou32(rvalue, &k);
-                if (r < 0) {
-                        log_syntax(unit, LOG_WARNING, filename, line, r,
-                                "Failed to parse interface buffer value, ignoring: %s", rvalue);
-                        return 0;
-                }
-                if (k < 1) {
-                        log_syntax(unit, LOG_WARNING, filename, line, 0,
-                                "Invalid %s= value, ignoring: %s", lvalue, rvalue);
-                        return 0;
-                }
+        if (isempty(rvalue)) {
+                dst->value = 0;
+                dst->set = false;
+                return 0;
+        }
+
+        if (streq(rvalue, "max")) {
+                dst->value = 0;
+                dst->set = true;
+                return 0;
         }
 
-        if (streq(lvalue, "RxBufferSize")) {
-                ring->rx_pending = k;
-                ring->rx_pending_set = true;
-        } else if (streq(lvalue, "RxMiniBufferSize")) {
-                ring->rx_mini_pending = k;
-                ring->rx_mini_pending_set = true;
-        } else if (streq(lvalue, "RxJumboBufferSize")) {
-                ring->rx_jumbo_pending = k;
-                ring->rx_jumbo_pending_set = true;
-        } else if (streq(lvalue, "TxBufferSize")) {
-                ring->tx_pending = k;
-                ring->tx_pending_set = true;
+        r = safe_atou32(rvalue, &k);
+        if (r < 0) {
+                log_syntax(unit, LOG_WARNING, filename, line, r,
+                           "Failed to parse %s=, ignoring: %s", lvalue, rvalue);
+                return 0;
+        }
+        if (k < 1) {
+                log_syntax(unit, LOG_WARNING, filename, line, 0,
+                           "Invalid %s= value, ignoring: %s", lvalue, rvalue);
+                return 0;
         }
 
+        dst->value = k;
+        dst->set = true;
         return 0;
 }
 
diff --git a/src/shared/ethtool-util.h b/src/shared/ethtool-util.h
index aea131914e..8fdbdec39a 100644
--- a/src/shared/ethtool-util.h
+++ b/src/shared/ethtool-util.h
@@ -57,30 +57,23 @@ struct ethtool_link_usettings {
         } link_modes;
 };
 
+typedef struct u32_opt {
+        uint32_t value; /* a value of 0 indicates the hardware advertised maximum should be used.*/
+        bool set;
+} u32_opt;
+
 typedef struct netdev_channels {
-        uint32_t rx_count;
-        uint32_t tx_count;
-        uint32_t other_count;
-        uint32_t combined_count;
-
-        bool rx_count_set;
-        bool tx_count_set;
-        bool other_count_set;
-        bool combined_count_set;
+        u32_opt rx;
+        u32_opt tx;
+        u32_opt other;
+        u32_opt combined;
 } netdev_channels;
 
 typedef struct netdev_ring_param {
-        /* For any of the 4 following settings, a value of 0 indicates the hardware advertised maximum should
-         * be used. */
-        uint32_t rx_pending;
-        uint32_t rx_mini_pending;
-        uint32_t rx_jumbo_pending;
-        uint32_t tx_pending;
-
-        bool rx_pending_set;
-        bool rx_mini_pending_set;
-        bool rx_jumbo_pending_set;
-        bool tx_pending_set;
+        u32_opt rx;
+        u32_opt rx_mini;
+        u32_opt rx_jumbo;
+        u32_opt tx;
 } netdev_ring_param;
 
 int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret);
@@ -111,6 +104,5 @@ enum ethtool_link_mode_bit_indices ethtool_link_mode_bit_from_string(const char
 CONFIG_PARSER_PROTOTYPE(config_parse_duplex);
 CONFIG_PARSER_PROTOTYPE(config_parse_wol);
 CONFIG_PARSER_PROTOTYPE(config_parse_port);
-CONFIG_PARSER_PROTOTYPE(config_parse_channel);
 CONFIG_PARSER_PROTOTYPE(config_parse_advertise);
-CONFIG_PARSER_PROTOTYPE(config_parse_nic_buffer_size);
+CONFIG_PARSER_PROTOTYPE(config_parse_ring_buffer_or_channel);
diff --git a/src/udev/net/link-config-gperf.gperf b/src/udev/net/link-config-gperf.gperf
index e2f07d758b..d0190da5cb 100644
--- a/src/udev/net/link-config-gperf.gperf
+++ b/src/udev/net/link-config-gperf.gperf
@@ -58,15 +58,15 @@ Link.TCP6SegmentationOffload,          config_parse_tristate,                 0,
 Link.UDPSegmentationOffload,           config_parse_warn_compat,              DISABLED_LEGACY,               0
 Link.GenericReceiveOffload,            config_parse_tristate,                 0,                             offsetof(LinkConfig, features[NET_DEV_FEAT_GRO])
 Link.LargeReceiveOffload,              config_parse_tristate,                 0,                             offsetof(LinkConfig, features[NET_DEV_FEAT_LRO])
-Link.RxChannels,                       config_parse_channel,                  0,                             offsetof(LinkConfig, channels)
-Link.TxChannels,                       config_parse_channel,                  0,                             offsetof(LinkConfig, channels)
-Link.OtherChannels,                    config_parse_channel,                  0,                             offsetof(LinkConfig, channels)
-Link.CombinedChannels,                 config_parse_channel,                  0,                             offsetof(LinkConfig, channels)
+Link.RxChannels,                       config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, channels.rx)
+Link.TxChannels,                       config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, channels.tx)
+Link.OtherChannels,                    config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, channels.other)
+Link.CombinedChannels,                 config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, channels.combined)
 Link.Advertise,                        config_parse_advertise,                0,                             offsetof(LinkConfig, advertise)
-Link.RxBufferSize,                     config_parse_nic_buffer_size,          0,                             offsetof(LinkConfig, ring)
-Link.RxMiniBufferSize,                 config_parse_nic_buffer_size,          0,                             offsetof(LinkConfig, ring)
-Link.RxJumboBufferSize,                config_parse_nic_buffer_size,          0,                             offsetof(LinkConfig, ring)
-Link.TxBufferSize,                     config_parse_nic_buffer_size,          0,                             offsetof(LinkConfig, ring)
+Link.RxBufferSize,                     config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, ring.rx)
+Link.RxMiniBufferSize,                 config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, ring.rx_mini)
+Link.RxJumboBufferSize,                config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, ring.rx_jumbo)
+Link.TxBufferSize,                     config_parse_ring_buffer_or_channel,   0,                             offsetof(LinkConfig, ring.tx)
 Link.RxFlowControl,                    config_parse_tristate,                 0,                             offsetof(LinkConfig, rx_flow_control)
 Link.TxFlowControl,                    config_parse_tristate,                 0,                             offsetof(LinkConfig, tx_flow_control)
 Link.AutoNegotiationFlowControl,       config_parse_tristate,                 0,                             offsetof(LinkConfig, autoneg_flow_control)
-- 
2.31.1