On Wed, Mar 11, 2026 at 2:06 AM Adrian Moreno via dev < [email protected]> wrote:
> Netdev flags are queried multiple times per poll loop. Sending a GETLINK > command for each can be very inefficient if there is RTNL lock > contention. > > Cache the result of the query as we do with other netdev attributes. > > Signed-off-by: Adrian Moreno <[email protected]> > Hi Adrian, I think this this a good idea, but have an implementation question below. > --- > lib/netdev-linux.c | 15 ++++++++++++--- > lib/tnl-ports.c | 2 +- > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c > index 66153bf49..e6127240b 100644 > --- a/lib/netdev-linux.c > +++ b/lib/netdev-linux.c > @@ -265,6 +265,7 @@ enum { > VALID_DRVINFO = 1 << 6, > VALID_FEATURES = 1 << 7, > VALID_NUMA_ID = 1 << 8, > + VALID_FLAGS = 1 << 9, > }; > > /* Linux 4.4 introduced the ability to skip the internal stats gathering > @@ -830,7 +831,7 @@ netdev_linux_run(const struct netdev_class > *netdev_class OVS_UNUSED) > > ovs_mutex_lock(&netdev->mutex); > update_flags_local(netdev); > - netdev_linux_changed(netdev, netdev->ifi_flags, 0); > + netdev_linux_changed(netdev, netdev->ifi_flags, > VALID_FLAGS); > ovs_mutex_unlock(&netdev->mutex); > > netdev_close(netdev_); > @@ -910,6 +911,7 @@ netdev_linux_update__(struct netdev_linux *dev, > > dev->ifindex = change->if_index; > dev->cache_valid |= VALID_IFINDEX; > + dev->cache_valid |= VALID_FLAGS; > dev->get_ifindex_error = 0; > dev->present = true; > } else { > @@ -3849,7 +3851,11 @@ static int > update_flags_local(struct netdev_linux *netdev) > OVS_REQUIRES(netdev->mutex) > { > - return get_flags(&netdev->up, &netdev->ifi_flags); > + int error = get_flags(&netdev->up, &netdev->ifi_flags); > + if (!error) { > + netdev->cache_valid |= VALID_FLAGS; > + } > + return error; > } > > static int > @@ -3888,7 +3894,8 @@ netdev_linux_update_flags(struct netdev *netdev_, > enum netdev_flags off, > 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)) { > + if (!(netdev->cache_valid & VALID_FLAGS) && > + netdev_linux_update_via_netlink(netdev)) { > error = update_flags_local(netdev); > } > *old_flagsp = iff_to_nd_flags(netdev->ifi_flags); > @@ -6955,6 +6962,8 @@ netdev_linux_update_via_netlink(struct netdev_linux > *netdev) > netdev->ifi_flags = change->ifi_flags; > changed = true; > } > + netdev->cache_valid |= VALID_FLAGS; > + > if (change->mtu && change->mtu != netdev->mtu) { > netdev->mtu = change->mtu; > netdev->cache_valid |= VALID_MTU; > diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c > index 56119b300..67b6ebcf7 100644 > --- a/lib/tnl-ports.c > +++ b/lib/tnl-ports.c > @@ -402,13 +402,13 @@ insert_ipdev__(struct netdev *dev, > > ip_dev = xzalloc(sizeof *ip_dev); > ip_dev->dev = netdev_ref(dev); > - ip_dev->change_seq = netdev_get_change_seq(dev); > error = netdev_get_etheraddr(ip_dev->dev, &ip_dev->mac); > if (error) { > goto err_free_ipdev; > } > ip_dev->addr = addr; > ip_dev->n_addr = n_addr; > + ip_dev->change_seq = netdev_get_change_seq(dev); > Why is this change needed here? It doesn't seem related to flags caching. -M > ovs_strlcpy(ip_dev->dev_name, netdev_get_name(dev), sizeof > ip_dev->dev_name); > ovs_list_insert(&addr_list, &ip_dev->node); > map_insert_ipdev(ip_dev); > -- > 2.53.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
