From 11a288e84652e291b70ead4e2cb2fbfd519668d0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 21 May 2021 05:30:28 +0900 Subject: [PATCH 1/8] ethtool-util: downgrade log level This just hides issue #19625. It will be fixed in later commits. --- src/shared/ethtool-util.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 654d36a83a2..e6e7da66ae9 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -158,15 +158,14 @@ assert_cc((ELEMENTSOF(ethtool_link_mode_bit_table)-1) / 32 < N_ADVERTISE); DEFINE_STRING_TABLE_LOOKUP(ethtool_link_mode_bit, enum ethtool_link_mode_bit_indices); -static int ethtool_connect_or_warn(int *ret, bool warn) { +static int ethtool_connect(int *ret) { int fd; assert_return(ret, -EINVAL); fd = socket_ioctl_fd(); if (fd < 0) - return log_full_errno(warn ? LOG_WARNING: LOG_DEBUG, fd, - "ethtool: could not create control socket: %m"); + return log_debug_errno(fd, "ethtool: could not create control socket: %m"); *ret = fd; @@ -188,7 +187,7 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { assert(ret); if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -230,7 +229,7 @@ int ethtool_get_link_info( assert(ifname); if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, false); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -282,7 +281,7 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et ethtool_fd = &fd; if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, false); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -324,7 +323,7 @@ int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duple return 0; if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -385,7 +384,7 @@ int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol) { return 0; if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -475,7 +474,7 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde assert(ring); if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -608,7 +607,7 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature assert(features); if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); + r = ethtool_connect(ethtool_fd); if (r < 0) return r; } @@ -617,7 +616,7 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature r = get_stringset(*ethtool_fd, &ifr, ETH_SS_FEATURES, &strings); if (r < 0) - return log_warning_errno(r, "ethtool: could not get ethtool features for %s", ifname); + return log_debug_errno(r, "ethtool: could not get ethtool features for %s", ifname); sfeatures = alloca0(sizeof(struct ethtool_sfeatures) + DIV_ROUND_UP(strings->len, 32U) * sizeof(sfeatures->features[0])); sfeatures->cmd = ETHTOOL_SFEATURES; @@ -627,7 +626,7 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature if (features[i] != -1) { r = set_features_bit(strings, netdev_feature_table[i], features[i], sfeatures); if (r < 0) { - log_warning_errno(r, "ethtool: could not find feature, ignoring: %s", netdev_feature_table[i]); + log_debug_errno(r, "ethtool: could not find feature, ignoring: %s", netdev_feature_table[i]); continue; } } @@ -636,7 +635,7 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); if (r < 0) - return log_warning_errno(r, "ethtool: could not set ethtool features for %s", ifname); + return log_debug_errno(r, "ethtool: could not set ethtool features for %s", ifname); return 0; } @@ -842,12 +841,12 @@ int ethtool_set_glinksettings( assert(advertise); if (autonegotiation != AUTONEG_DISABLE && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { - log_info("ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable."); + log_debug("ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable."); return 0; } if (*fd < 0) { - r = ethtool_connect_or_warn(fd, true); + r = ethtool_connect(fd); if (r < 0) return r; } @@ -858,7 +857,7 @@ int ethtool_set_glinksettings( if (r < 0) { r = get_gset(*fd, &ifr, &u); if (r < 0) - return log_warning_errno(r, "ethtool: Cannot get device settings for %s : %m", ifname); + return log_debug_errno(r, "ethtool: Cannot get device settings for %s : %m", ifname); } if (speed > 0) @@ -885,7 +884,7 @@ int ethtool_set_glinksettings( else r = set_sset(*fd, &ifr, u); if (r < 0) - return log_warning_errno(r, "ethtool: Cannot set device settings for %s: %m", ifname); + return log_debug_errno(r, "ethtool: Cannot set device settings for %s: %m", ifname); return r; } @@ -905,7 +904,7 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha assert(channels); if (*fd < 0) { - r = ethtool_connect_or_warn(fd, true); + r = ethtool_connect(fd); if (r < 0) return r; } @@ -961,7 +960,7 @@ int ethtool_set_flow_control(int *fd, const char *ifname, int rx, int tx, int au assert(ifname); if (*fd < 0) { - r = ethtool_connect_or_warn(fd, true); + r = ethtool_connect(fd); if (r < 0) return r; } From 4f5040313dd9ac737816c992e0d7e4e94dca36b4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 May 2021 04:07:01 +0900 Subject: [PATCH 2/8] ethtool-util: make ethtool_connect() open fd only when it is not opened --- src/shared/ethtool-util.c | 92 ++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 55 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index e6e7da66ae9..da47aff7201 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -158,17 +158,20 @@ assert_cc((ELEMENTSOF(ethtool_link_mode_bit_table)-1) / 32 < N_ADVERTISE); DEFINE_STRING_TABLE_LOOKUP(ethtool_link_mode_bit, enum ethtool_link_mode_bit_indices); -static int ethtool_connect(int *ret) { +static int ethtool_connect(int *ethtool_fd) { int fd; - assert_return(ret, -EINVAL); + assert(ethtool_fd); + + /* This does nothing if already connected. */ + if (*ethtool_fd >= 0) + return 0; fd = socket_ioctl_fd(); if (fd < 0) return log_debug_errno(fd, "ethtool: could not create control socket: %m"); - *ret = fd; - + *ethtool_fd = fd; return 0; } @@ -186,11 +189,9 @@ int ethtool_get_driver(int *ethtool_fd, const char *ifname, char **ret) { assert(ifname); assert(ret); - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -228,11 +229,9 @@ int ethtool_get_link_info( assert(ethtool_fd); assert(ifname); - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -279,12 +278,9 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et if (!ethtool_fd) ethtool_fd = &fd; - - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -322,11 +318,9 @@ int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duple if (speed == 0 && duplex == _DUP_INVALID) return 0; - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -383,11 +377,9 @@ int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol) { if (wol == _WOL_INVALID) return 0; - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -473,11 +465,9 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde assert(ifname); assert(ring); - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -606,11 +596,9 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature assert(ifname); assert(features); - if (*ethtool_fd < 0) { - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -845,11 +833,9 @@ int ethtool_set_glinksettings( return 0; } - if (*fd < 0) { - r = ethtool_connect(fd); - if (r < 0) - return r; - } + r = ethtool_connect(fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -903,11 +889,9 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha assert(ifname); assert(channels); - if (*fd < 0) { - r = ethtool_connect(fd); - if (r < 0) - return r; - } + r = ethtool_connect(fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -959,11 +943,9 @@ int ethtool_set_flow_control(int *fd, const char *ifname, int rx, int tx, int au assert(fd); assert(ifname); - if (*fd < 0) { - r = ethtool_connect(fd); - if (r < 0) - return r; - } + r = ethtool_connect(fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); From 4323046c5fa9fc235af65ad759540abcde62244a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 21 May 2021 05:32:17 +0900 Subject: [PATCH 3/8] ethtool-util: move and rebreak comments --- src/shared/ethtool-util.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index da47aff7201..11e69929813 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -805,12 +805,6 @@ static int set_sset(int fd, struct ifreq *ifr, const struct ethtool_link_usettin return 0; } -/* If autonegotiation is disabled, the speed and duplex represent the fixed link - * mode and are writable if the driver supports multiple link modes. If it is - * enabled then they are read-only. If the link is up they represent the negotiated - * link mode; if the link is down, the speed is 0, %SPEED_UNKNOWN or the highest - * enabled speed and @duplex is %DUPLEX_UNKNOWN or the best enabled duplex mode. - */ int ethtool_set_glinksettings( int *fd, const char *ifname, @@ -828,6 +822,12 @@ int ethtool_set_glinksettings( assert(ifname); assert(advertise); + /* If autonegotiation is disabled, the speed and duplex represent the fixed link mode and are + * writable if the driver supports multiple link modes. If it is enabled then they are + * read-only. If the link is up they represent the negotiated link mode; if the link is down, + * the speed is 0, %SPEED_UNKNOWN or the highest enabled speed and @duplex is %DUPLEX_UNKNOWN + * or the best enabled duplex mode. */ + if (autonegotiation != AUTONEG_DISABLE && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { log_debug("ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable."); return 0; From c8e644b116ef1fe10aec8827d12643a6effaca6a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 21 May 2021 06:03:14 +0900 Subject: [PATCH 4/8] ethtool-util: disable autonegotiation when speed, duplex, or port is specified And if autonegotiation is enabled, speed, duplex, and port are ignored. Fixes #19625. --- src/shared/ethtool-util.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 11e69929813..81d8e56544d 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -828,9 +828,16 @@ int ethtool_set_glinksettings( * the speed is 0, %SPEED_UNKNOWN or the highest enabled speed and @duplex is %DUPLEX_UNKNOWN * or the best enabled duplex mode. */ - if (autonegotiation != AUTONEG_DISABLE && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { - log_debug("ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable."); - return 0; + if (speed > 0 || duplex >= 0 || port >= 0) { + if (autonegotiation == AUTONEG_ENABLE || !memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { + log_debug("ethtool: autonegotiation is enabled, ignoring speed, duplex, or port settings."); + speed = 0; + duplex = _DUP_INVALID; + port = _NET_DEV_PORT_INVALID; + } else { + log_debug("ethtool: setting speed, duplex, or port, disabling autonegotiation."); + autonegotiation = AUTONEG_DISABLE; + } } r = ethtool_connect(fd); From ba1030594e7533df341b01468b86bf21cd8eca12 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 May 2021 03:15:16 +0900 Subject: [PATCH 5/8] ethtool-util: do not touch anything if nothing is requested And update settings only when it is necessary. --- src/shared/ethtool-util.c | 254 +++++++++++++++++++------------------- 1 file changed, 130 insertions(+), 124 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 81d8e56544d..4b70ee451a0 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -302,6 +302,14 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et return 0; } +#define UPDATE(dest, val, updated) \ + do { \ + typeof(val) _v = (val); \ + if (dest != _v) \ + updated = true; \ + dest = _v; \ + } while(false) + int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duplex duplex) { struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, @@ -315,7 +323,7 @@ int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duple assert(ethtool_fd); assert(ifname); - if (speed == 0 && duplex == _DUP_INVALID) + if (speed == 0 && duplex < 0) return 0; r = ethtool_connect(ethtool_fd); @@ -328,35 +336,21 @@ int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duple if (r < 0) return -errno; - if (ethtool_cmd_speed(&ecmd) != speed) { + if (speed > 0) { + need_update = need_update || ethtool_cmd_speed(&ecmd) != speed; ethtool_cmd_speed_set(&ecmd, speed); - need_update = true; } - switch (duplex) { - case DUP_HALF: - if (ecmd.duplex != DUPLEX_HALF) { - ecmd.duplex = DUPLEX_HALF; - need_update = true; - } - break; - case DUP_FULL: - if (ecmd.duplex != DUPLEX_FULL) { - ecmd.duplex = DUPLEX_FULL; - need_update = true; - } - break; - default: - break; - } + if (duplex >= 0) + UPDATE(ecmd.duplex, duplex, need_update); - if (need_update) { - ecmd.cmd = ETHTOOL_SSET; + if (!need_update) + return 0; - r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - } + ecmd.cmd = ETHTOOL_SSET; + r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); + if (r < 0) + return -errno; return 0; } @@ -465,6 +459,12 @@ 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) + return 0; + r = ethtool_connect(ethtool_fd); if (r < 0) return r; @@ -475,33 +475,25 @@ int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netde if (r < 0) return -errno; - if (ring->rx_pending_set && ecmd.rx_pending != ring->rx_pending) { - ecmd.rx_pending = ring->rx_pending; - need_update = true; - } + if (ring->rx_pending_set) + UPDATE(ecmd.rx_pending, ring->rx_pending, need_update); - if (ring->rx_mini_pending_set && ecmd.rx_mini_pending != ring->rx_mini_pending) { - ecmd.rx_mini_pending = ring->rx_mini_pending; - need_update = true; - } + if (ring->rx_mini_pending_set) + UPDATE(ecmd.rx_mini_pending, ring->rx_mini_pending, need_update); - if (ring->rx_jumbo_pending_set && ecmd.rx_jumbo_pending != ring->rx_jumbo_pending) { - ecmd.rx_jumbo_pending = ring->rx_jumbo_pending; - need_update = true; - } + if (ring->rx_jumbo_pending_set) + UPDATE(ecmd.rx_jumbo_pending, ring->rx_jumbo_pending, need_update); - if (ring->tx_pending_set && ecmd.tx_pending != ring->tx_pending) { - ecmd.tx_pending = ring->tx_pending; - need_update = true; - } + if (ring->tx_pending_set) + UPDATE(ecmd.tx_pending, ring->tx_pending, need_update); - if (need_update) { - ecmd.cmd = ETHTOOL_SRINGPARAM; + if (!need_update) + return 0; - r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - } + ecmd.cmd = ETHTOOL_SRINGPARAM; + r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); + if (r < 0) + return -errno; return 0; } @@ -816,12 +808,17 @@ int ethtool_set_glinksettings( _cleanup_free_ struct ethtool_link_usettings *u = NULL; struct ifreq ifr = {}; + bool changed = false; int r; assert(fd); assert(ifname); assert(advertise); + if (autonegotiation < 0 && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE) && + speed == 0 && duplex < 0 && port < 0) + return 0; + /* If autonegotiation is disabled, the speed and duplex represent the fixed link mode and are * writable if the driver supports multiple link modes. If it is enabled then they are * read-only. If the link is up they represent the negotiated link mode; if the link is down, @@ -854,24 +851,32 @@ int ethtool_set_glinksettings( } if (speed > 0) - u->base.speed = DIV_ROUND_UP(speed, 1000000); + UPDATE(u->base.speed, DIV_ROUND_UP(speed, 1000000), changed); - if (duplex != _DUP_INVALID) - u->base.duplex = duplex; + if (duplex >= 0) + UPDATE(u->base.duplex, duplex, changed); - if (port != _NET_DEV_PORT_INVALID) - u->base.port = port; + if (port >= 0) + UPDATE(u->base.port, port, changed); if (autonegotiation >= 0) - u->base.autoneg = autonegotiation; + UPDATE(u->base.autoneg, autonegotiation, changed); if (!memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { - u->base.autoneg = AUTONEG_ENABLE; + UPDATE(u->base.autoneg, AUTONEG_ENABLE, changed); + + changed = changed || + memcmp(&u->link_modes.advertising, advertise, sizeof(uint32_t) * N_ADVERTISE) != 0 || + !memeqzero((uint8_t*) &u->link_modes.advertising + sizeof(uint32_t) * N_ADVERTISE, + ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES - sizeof(uint32_t) * N_ADVERTISE); memcpy(&u->link_modes.advertising, advertise, sizeof(uint32_t) * N_ADVERTISE); memzero((uint8_t*) &u->link_modes.advertising + sizeof(uint32_t) * N_ADVERTISE, ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NBYTES - sizeof(uint32_t) * N_ADVERTISE); } + if (!changed) + return 0; + if (u->base.cmd == ETHTOOL_GLINKSETTINGS) r = set_slinksettings(*fd, &ifr, u); else @@ -896,6 +901,12 @@ 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) + return 0; + r = ethtool_connect(fd); if (r < 0) return r; @@ -906,33 +917,25 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha if (r < 0) return -errno; - if (channels->rx_count_set && ecmd.rx_count != channels->rx_count) { - ecmd.rx_count = channels->rx_count; - need_update = true; - } + if (channels->rx_count_set) + UPDATE(ecmd.rx_count, channels->rx_count, need_update); - if (channels->tx_count_set && ecmd.tx_count != channels->tx_count) { - ecmd.tx_count = channels->tx_count; - need_update = true; - } + if (channels->tx_count_set) + UPDATE(ecmd.tx_count, channels->tx_count, need_update); - if (channels->other_count_set && ecmd.other_count != channels->other_count) { - ecmd.other_count = channels->other_count; - need_update = true; - } + if (channels->other_count_set) + UPDATE(ecmd.other_count, channels->other_count, need_update); - if (channels->combined_count_set && ecmd.combined_count != channels->combined_count) { - ecmd.combined_count = channels->combined_count; - need_update = true; - } + if (channels->combined_count_set) + UPDATE(ecmd.combined_count, channels->combined_count, need_update); - if (need_update) { - ecmd.cmd = ETHTOOL_SCHANNELS; + if (!need_update) + return 0; - r = ioctl(*fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - } + ecmd.cmd = ETHTOOL_SCHANNELS; + r = ioctl(*fd, SIOCETHTOOL, &ifr); + if (r < 0) + return -errno; return 0; } @@ -950,6 +953,9 @@ int ethtool_set_flow_control(int *fd, const char *ifname, int rx, int tx, int au assert(fd); assert(ifname); + if (rx < 0 && tx < 0 && autoneg < 0) + return 0; + r = ethtool_connect(fd); if (r < 0) return r; @@ -960,42 +966,38 @@ int ethtool_set_flow_control(int *fd, const char *ifname, int rx, int tx, int au if (r < 0) return -errno; - if (rx >= 0 && ecmd.rx_pause != (uint32_t) rx) { - ecmd.rx_pause = rx; - need_update = true; - } + if (rx >= 0) + UPDATE(ecmd.rx_pause, (uint32_t) rx, need_update); - if (tx >= 0 && ecmd.tx_pause != (uint32_t) tx) { - ecmd.tx_pause = tx; - need_update = true; - } + if (tx >= 0) + UPDATE(ecmd.tx_pause, (uint32_t) tx, need_update); - if (autoneg >= 0 && ecmd.autoneg != (uint32_t) autoneg) { - ecmd.autoneg = autoneg; - need_update = true; - } + if (autoneg >= 0) + UPDATE(ecmd.autoneg, (uint32_t) autoneg, need_update); - if (need_update) { - ecmd.cmd = ETHTOOL_SPAUSEPARAM; + if (!need_update) + return 0; - r = ioctl(*fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - } + ecmd.cmd = ETHTOOL_SPAUSEPARAM; + r = ioctl(*fd, SIOCETHTOOL, &ifr); + if (r < 0) + return -errno; 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) { +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; @@ -1035,16 +1037,18 @@ int config_parse_channel(const char *unit, return 0; } -int config_parse_advertise(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) { +int config_parse_advertise( + 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) { + uint32_t *advertise = data; const char *p; int r; @@ -1089,16 +1093,18 @@ int config_parse_advertise(const char *unit, } } -int config_parse_nic_buffer_size(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) { +int config_parse_nic_buffer_size( + 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_ring_param *ring = data; uint32_t k; int r; From a7994dd3ab43bbd01424d40ddaef5c1ff57530da Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 21 May 2021 06:22:37 +0900 Subject: [PATCH 6/8] udev/net: drop fallback logic to set speed ethtool_set_glinksettings() already fallback to use ETHTOOL_GSET/ETHTOOL_SSET commands when ETHTOOL_GLINKSETTINGS/ETHTOOL_SLINKSETTINGS are not supported. --- src/udev/net/link-config.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 3685c9d2280..dc6d437718a 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -308,28 +308,30 @@ static int link_config_apply_ethtool_settings(int *ethtool_fd, const link_config config->autonegotiation, config->advertise, config->speed, config->duplex, config->port); if (r < 0) { - if (config->port != _NET_DEV_PORT_INVALID) - log_device_warning_errno(device, r, "Could not set port '%s', ignoring: %m", port_to_string(config->port)); + if (config->autonegotiation >= 0) + log_device_warning_errno(device, r, "Could not %s auto negotiation, ignoring: %m", + enable_disable(config->autonegotiation)); if (!eqzero(config->advertise)) - log_device_warning_errno(device, r, "Could not set advertise mode, ignoring: %m"); /* TODO: include modes in the log message. */ + log_device_warning_errno(device, r, "Could not set advertise mode, ignoring: %m"); - if (config->speed) { - unsigned speed = DIV_ROUND_UP(config->speed, 1000000); - if (r == -EOPNOTSUPP) { - r = ethtool_set_speed(ethtool_fd, name, speed, config->duplex); - if (r < 0) - log_device_warning_errno(device, r, "Could not set speed to %uMbps, ignoring: %m", speed); - } - } + if (config->speed > 0) + log_device_warning_errno(device, r, "Could not set speed to %"PRIu64"Mbps, ignoring: %m", + DIV_ROUND_UP(config->speed, 1000000)); - if (config->duplex != _DUP_INVALID) - log_device_warning_errno(device, r, "Could not set duplex to %s, ignoring: %m", duplex_to_string(config->duplex)); + if (config->duplex >= 0) + log_device_warning_errno(device, r, "Could not set duplex to %s, ignoring: %m", + duplex_to_string(config->duplex)); + + if (config->port >= 0) + log_device_warning_errno(device, r, "Could not set port to '%s', ignoring: %m", + port_to_string(config->port)); } r = ethtool_set_wol(ethtool_fd, name, config->wol); if (r < 0) - log_device_warning_errno(device, r, "Could not set WakeOnLan to %s, ignoring: %m", wol_to_string(config->wol)); + log_device_warning_errno(device, r, "Could not set WakeOnLan to %s, ignoring: %m", + wol_to_string(config->wol)); r = ethtool_set_features(ethtool_fd, name, config->features); if (r < 0) From 80662eecbf39e218a68e60b7b7b2cf5abd331b49 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 May 2021 04:23:51 +0900 Subject: [PATCH 7/8] udev/net: drop unnecessary conditions --- src/udev/net/link-config.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index dc6d437718a..3d2f39f63b0 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -337,23 +337,17 @@ static int link_config_apply_ethtool_settings(int *ethtool_fd, const link_config if (r < 0) log_device_warning_errno(device, r, "Could not set offload features, ignoring: %m"); - if (config->channels.rx_count_set || config->channels.tx_count_set || config->channels.other_count_set || config->channels.combined_count_set) { - r = ethtool_set_channels(ethtool_fd, name, &config->channels); - if (r < 0) - log_device_warning_errno(device, r, "Could not set channels, ignoring: %m"); - } + r = ethtool_set_channels(ethtool_fd, name, &config->channels); + if (r < 0) + log_device_warning_errno(device, r, "Could not set channels, ignoring: %m"); - if (config->ring.rx_pending_set || config->ring.rx_mini_pending_set || config->ring.rx_jumbo_pending_set || config->ring.tx_pending_set) { - r = ethtool_set_nic_buffer_size(ethtool_fd, name, &config->ring); - if (r < 0) - log_device_warning_errno(device, r, "Could not set ring buffer, ignoring: %m"); - } + r = ethtool_set_nic_buffer_size(ethtool_fd, name, &config->ring); + if (r < 0) + log_device_warning_errno(device, r, "Could not set ring buffer, ignoring: %m"); - if (config->rx_flow_control >= 0 || config->tx_flow_control >= 0 || config->autoneg_flow_control >= 0) { - r = ethtool_set_flow_control(ethtool_fd, name, config->rx_flow_control, config->tx_flow_control, config->autoneg_flow_control); - if (r < 0) - log_device_warning_errno(device, r, "Could not set flow control, ignoring: %m"); - } + r = ethtool_set_flow_control(ethtool_fd, name, config->rx_flow_control, config->tx_flow_control, config->autoneg_flow_control); + if (r < 0) + log_device_warning_errno(device, r, "Could not set flow control, ignoring: %m"); return 0; } From 3ee316bd7ba1827e916d9cdf3f03db75df2622fc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 May 2021 04:37:54 +0900 Subject: [PATCH 8/8] ethtool-util: drop unused function --- src/shared/ethtool-util.c | 45 --------------------------------------- src/shared/ethtool-util.h | 1 - 2 files changed, 46 deletions(-) diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 4b70ee451a0..9506af8b8b2 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -310,51 +310,6 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et dest = _v; \ } while(false) -int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duplex duplex) { - struct ethtool_cmd ecmd = { - .cmd = ETHTOOL_GSET, - }; - struct ifreq ifr = { - .ifr_data = (void*) &ecmd, - }; - bool need_update = false; - int r; - - assert(ethtool_fd); - assert(ifname); - - if (speed == 0 && duplex < 0) - return 0; - - r = ethtool_connect(ethtool_fd); - if (r < 0) - return r; - - strscpy(ifr.ifr_name, IFNAMSIZ, ifname); - - r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - - if (speed > 0) { - need_update = need_update || ethtool_cmd_speed(&ecmd) != speed; - ethtool_cmd_speed_set(&ecmd, speed); - } - - if (duplex >= 0) - UPDATE(ecmd.duplex, duplex, need_update); - - if (!need_update) - return 0; - - ecmd.cmd = ETHTOOL_SSET; - r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - - return 0; -} - int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol) { struct ethtool_wolinfo ecmd = { .cmd = ETHTOOL_GWOL, diff --git a/src/shared/ethtool-util.h b/src/shared/ethtool-util.h index 11e2906681a..3c031cda439 100644 --- a/src/shared/ethtool-util.h +++ b/src/shared/ethtool-util.h @@ -99,7 +99,6 @@ int ethtool_get_link_info(int *ethtool_fd, const char *ifname, int *ret_autonegotiation, uint64_t *ret_speed, Duplex *ret_duplex, NetDevPort *ret_port); int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct ether_addr *ret); -int ethtool_set_speed(int *ethtool_fd, const char *ifname, unsigned speed, Duplex duplex); int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol); int ethtool_set_nic_buffer_size(int *ethtool_fd, const char *ifname, const netdev_ring_param *ring); int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *features);