diff --git a/src/shared/ethtool-util.c b/src/shared/ethtool-util.c index 654d36a83a2..9506af8b8b2 100644 --- a/src/shared/ethtool-util.c +++ b/src/shared/ethtool-util.c @@ -158,18 +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_or_warn(int *ret, bool warn) { +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_full_errno(warn ? LOG_WARNING: LOG_DEBUG, fd, - "ethtool: could not create control socket: %m"); - - *ret = fd; + return log_debug_errno(fd, "ethtool: could not create control socket: %m"); + *ethtool_fd = fd; return 0; } @@ -187,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_or_warn(ethtool_fd, true); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -229,11 +229,9 @@ int ethtool_get_link_info( assert(ethtool_fd); assert(ifname); - if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, false); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -280,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_or_warn(ethtool_fd, false); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -307,66 +302,13 @@ int ethtool_get_permanent_macaddr(int *ethtool_fd, const char *ifname, struct et return 0; } -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 == _DUP_INVALID) - return 0; - - if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); - if (r < 0) - return r; - } - - strscpy(ifr.ifr_name, IFNAMSIZ, ifname); - - r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - - if (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 (need_update) { - ecmd.cmd = ETHTOOL_SSET; - - r = ioctl(*ethtool_fd, SIOCETHTOOL, &ifr); - if (r < 0) - return -errno; - } - - return 0; -} +#define UPDATE(dest, val, updated) \ + do { \ + typeof(val) _v = (val); \ + if (dest != _v) \ + updated = true; \ + dest = _v; \ + } while(false) int ethtool_set_wol(int *ethtool_fd, const char *ifname, WakeOnLan wol) { struct ethtool_wolinfo ecmd = { @@ -384,11 +326,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_or_warn(ethtool_fd, true); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -474,11 +414,15 @@ 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_or_warn(ethtool_fd, true); - if (r < 0) - return r; - } + 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; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -486,33 +430,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; } @@ -607,17 +543,15 @@ int ethtool_set_features(int *ethtool_fd, const char *ifname, const int *feature assert(ifname); assert(features); - if (*ethtool_fd < 0) { - r = ethtool_connect_or_warn(ethtool_fd, true); - if (r < 0) - return r; - } + r = ethtool_connect(ethtool_fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); 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 +561,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 +570,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; } @@ -818,12 +752,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, @@ -835,22 +763,38 @@ 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 != AUTONEG_DISABLE && memeqzero(advertise, sizeof(uint32_t) * N_ADVERTISE)) { - log_info("ethtool: autonegotiation is unset or enabled, the speed and duplex are not writable."); + 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, + * the speed is 0, %SPEED_UNKNOWN or the highest enabled speed and @duplex is %DUPLEX_UNKNOWN + * or the best enabled duplex mode. */ + + 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; + } } - if (*fd < 0) { - r = ethtool_connect_or_warn(fd, true); - if (r < 0) - return r; - } + r = ethtool_connect(fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -858,34 +802,42 @@ 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) - 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 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; } @@ -904,11 +856,15 @@ int ethtool_set_channels(int *fd, const char *ifname, const netdev_channels *cha assert(ifname); assert(channels); - if (*fd < 0) { - r = ethtool_connect_or_warn(fd, true); - if (r < 0) - return r; - } + 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; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -916,33 +872,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; } @@ -960,11 +908,12 @@ 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_or_warn(fd, true); - if (r < 0) - return r; - } + if (rx < 0 && tx < 0 && autoneg < 0) + return 0; + + r = ethtool_connect(fd); + if (r < 0) + return r; strscpy(ifr.ifr_name, IFNAMSIZ, ifname); @@ -972,42 +921,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; @@ -1047,16 +992,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; @@ -1101,16 +1048,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; 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); diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 3685c9d2280..3d2f39f63b0 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -308,50 +308,46 @@ 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) 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; }