Currently, flags from an expected-to-be-local netdev are read using get_flags(), which is commonly called with &netdev->ifi_flags as argument.
Refactor it into its own function and rename "update_flags" to "modify_flags" to increase clarity. As an additional benefit, this patch makes "update_flags" actually report errors when the device is not accessible. Signed-off-by: Adrian Moreno <[email protected]> --- lib/netdev-linux.c | 36 +++++++++++++++++++++--------------- tests/system-interface.at | 2 ++ tests/system-tap.at | 5 ++++- tests/system-traffic.at | 3 ++- 4 files changed, 29 insertions(+), 17 deletions(-) diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 47faea8c6..66153bf49 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -564,7 +564,8 @@ static int netdev_linux_do_ethtool(const char *name, struct ethtool_cmd *, int cmd, const char *cmd_name); static int get_flags(const struct netdev *, unsigned int *flags); static int set_flags(const char *, unsigned int flags); -static int update_flags(struct netdev_linux *netdev, enum netdev_flags off, +static int update_flags_local(struct netdev_linux *); +static int modify_flags(struct netdev_linux *netdev, enum netdev_flags off, enum netdev_flags on, enum netdev_flags *old_flagsp) OVS_REQUIRES(netdev->mutex); static int get_ifindex(const struct netdev *, int *ifindexp); @@ -826,11 +827,10 @@ netdev_linux_run(const struct netdev_class *netdev_class OVS_UNUSED) SHASH_FOR_EACH (node, &device_shash) { struct netdev *netdev_ = node->data; struct netdev_linux *netdev = netdev_linux_cast(netdev_); - unsigned int flags; ovs_mutex_lock(&netdev->mutex); - get_flags(netdev_, &flags); - netdev_linux_changed(netdev, flags, 0); + update_flags_local(netdev); + netdev_linux_changed(netdev, netdev->ifi_flags, 0); ovs_mutex_unlock(&netdev->mutex); netdev_close(netdev_); @@ -991,7 +991,7 @@ netdev_linux_construct(struct netdev *netdev_) netdev_linux_set_ol(netdev_); } - error = get_flags(&netdev->up, &netdev->ifi_flags); + error = update_flags_local(netdev); if (error == ENODEV) { if (netdev->up.netdev_class != &netdev_internal_class) { /* The device does not exist, so don't allow it to be opened. */ @@ -1038,7 +1038,7 @@ netdev_linux_construct_tap(struct netdev *netdev_) } /* Create tap device. */ - get_flags(&netdev->up, &netdev->ifi_flags); + update_flags_local(netdev); if (ovsthread_once_start(&once)) { if (ioctl(netdev->tap_fd, TUNGETFEATURES, &up) == -1) { @@ -1907,7 +1907,7 @@ netdev_linux_set_etheraddr(struct netdev *netdev_, const struct eth_addr mac) /* Tap devices must be brought down before setting the address. */ if (is_tap_netdev(netdev_)) { - update_flags(netdev, NETDEV_UP, 0, &old_flags); + modify_flags(netdev, NETDEV_UP, 0, &old_flags); } error = set_etheraddr(netdev_get_name(netdev_), mac); if (!error || error == ENODEV) { @@ -1919,7 +1919,7 @@ netdev_linux_set_etheraddr(struct netdev *netdev_, const struct eth_addr mac) } if (is_tap_netdev(netdev_) && old_flags & NETDEV_UP) { - update_flags(netdev, 0, NETDEV_UP, &old_flags); + modify_flags(netdev, 0, NETDEV_UP, &old_flags); } exit: @@ -3846,7 +3846,14 @@ iff_to_nd_flags(unsigned int iff) } static int -update_flags(struct netdev_linux *netdev, enum netdev_flags off, +update_flags_local(struct netdev_linux *netdev) + OVS_REQUIRES(netdev->mutex) +{ + return get_flags(&netdev->up, &netdev->ifi_flags); +} + +static int +modify_flags(struct netdev_linux *netdev, enum netdev_flags off, enum netdev_flags on, enum netdev_flags *old_flagsp) OVS_REQUIRES(netdev->mutex) { @@ -3858,7 +3865,7 @@ update_flags(struct netdev_linux *netdev, enum netdev_flags off, new_flags = (old_flags & ~nd_to_iff_flags(off)) | nd_to_iff_flags(on); if (new_flags != old_flags) { error = set_flags(netdev_get_name(&netdev->up), new_flags); - get_flags(&netdev->up, &netdev->ifi_flags); + update_flags_local(netdev); } return error; @@ -3878,14 +3885,13 @@ netdev_linux_update_flags(struct netdev *netdev_, enum netdev_flags off, error = EOPNOTSUPP; goto exit; } - error = update_flags(netdev, off, on, old_flagsp); + error = modify_flags(netdev, off, on, old_flagsp); } else { /* Try reading flags over netlink, or fall back to ioctl. */ - if (!netdev_linux_update_via_netlink(netdev)) { - *old_flagsp = iff_to_nd_flags(netdev->ifi_flags); - } else { - error = update_flags(netdev, off, on, old_flagsp); + if (netdev_linux_update_via_netlink(netdev)) { + error = update_flags_local(netdev); } + *old_flagsp = iff_to_nd_flags(netdev->ifi_flags); } exit: diff --git a/tests/system-interface.at b/tests/system-interface.at index 20a882d1c..ea6753f30 100644 --- a/tests/system-interface.at +++ b/tests/system-interface.at @@ -17,6 +17,7 @@ AT_CHECK([ip link add ovs-veth0 type veth peer name ovs-veth1]) AT_CHECK([ovs-vsctl del-port br0 ovs-veth0]) OVS_TRAFFIC_VSWITCHD_STOP(["dnl +/failed to get flags for network device ovs-veth0/d /could not open network device ovs-veth0/d /cannot get .*STP status on nonexistent port/d /ethtool command .*on network device ovs-veth0 failed/d @@ -177,6 +178,7 @@ AT_CHECK([ovs-appctl dpctl/show | grep port], [0], [dnl OVS_TRAFFIC_VSWITCHD_STOP([" /could not open network device ovs-veth0 (No such device)/d + /failed to get flags for network device ovs-veth0: No such device/d "]) AT_CLEANUP diff --git a/tests/system-tap.at b/tests/system-tap.at index 03ec01270..fe237ad10 100644 --- a/tests/system-tap.at +++ b/tests/system-tap.at @@ -29,6 +29,9 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -W 2 10.1.1.2 | FORMAT_PING], [0], OVS_START_L7([at_ns1], [http]) NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([10.1.1.2]), [0], [ignore], [ignore]) -OVS_TRAFFIC_VSWITCHD_STOP(["/.*ethtool command ETHTOOL_G.*/d"]) +OVS_TRAFFIC_VSWITCHD_STOP(["dnl +/.*ethtool command ETHTOOL_G.*/d +/.*failed to get flags for network device/d +"]) AT_CLEANUP diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 8f4fdf8b1..007b00859 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -4297,7 +4297,8 @@ tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src= OVS_TRAFFIC_VSWITCHD_STOP(["dnl /ioctl(SIOCGIFINDEX) on .* device failed: No such device/d -/removing policing failed: No such device/d"]) +/removing policing failed: No such device/d +/failed to get flags for network device/d"]) AT_CLEANUP AT_SETUP([conntrack - ct_mark]) -- 2.53.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
