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