| Message ID | 20230130185757.9512-1-a.heider@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [iwinfo] nl80211: restore iterating over all devices in nl80211_phy2ifname() | expand |
On Mon, Jan 30, 2023 at 07:57:57PM +0100, Andre Heider wrote: > This reverts to the earlier behaviour, because: > * phys can be renamed, which breaks hardcoding "phy%d" > * /sys/class/ieee80211/*/device can return networks of other phys, > since "device" is a symlink which can have multiple phys > > The earlier behaviour fixes both points. > > Fixes: 6194aaf0 "nl80211: simplify iterating over phy's devices" > Fixes: #11902 > Signed-off-by: Andre Heider <a.heider@gmail.com> > Tested-by: Olcay Korkmaz <nuke_mania@hotmail.com> I remember we had some good stats of the timing improvement of 6194aaf05244241d2f51c471a43584f8d80b64ce. Does this reintroduce the additional slowdown? Should we add this as a fallback if the interface is not found using the expected way? > --- > iwinfo_nl80211.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c > index 5bc2f51..50bb8f0 100644 > --- a/iwinfo_nl80211.c > +++ b/iwinfo_nl80211.c > @@ -825,13 +825,15 @@ static char * nl80211_phy2ifname(const char *ifname) > > memset(nif, 0, sizeof(nif)); > > - snprintf(buffer, sizeof(buffer), > - "/sys/class/ieee80211/phy%d/device/net", phyidx); > - > - if ((d = opendir(buffer)) != NULL) > + if ((d = opendir("/sys/class/net")) != NULL) > { > while ((e = readdir(d)) != NULL) > { > + snprintf(buffer, sizeof(buffer), > + "/sys/class/net/%s/phy80211/index", e->d_name); > + if (nl80211_readint(buffer) != phyidx) > + continue; > + > snprintf(buffer, sizeof(buffer), > "/sys/class/net/%s/ifindex", e->d_name); > cifidx = nl80211_readint(buffer); > -- > 2.39.0 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
On 30/01/2023 23:18, Christian Marangi wrote: > On Mon, Jan 30, 2023 at 07:57:57PM +0100, Andre Heider wrote: >> This reverts to the earlier behaviour, because: >> * phys can be renamed, which breaks hardcoding "phy%d" >> * /sys/class/ieee80211/*/device can return networks of other phys, >> since "device" is a symlink which can have multiple phys >> >> The earlier behaviour fixes both points. >> >> Fixes: 6194aaf0 "nl80211: simplify iterating over phy's devices" >> Fixes: #11902 >> Signed-off-by: Andre Heider <a.heider@gmail.com> >> Tested-by: Olcay Korkmaz <nuke_mania@hotmail.com> > > I remember we had some good stats of the timing improvement of > 6194aaf05244241d2f51c471a43584f8d80b64ce. Does this reintroduce the > additional slowdown? Should we add this as a fallback if the interface > is not found using the expected way? With a HEAD of 6194aaf, yes, quite some slowdowns were fixed. While 6194aaf itself does iterate over less, this patch isn't a performance hit, maybe just above measurable. We can't use this as a fallback, as it does the wrong thing. It assumes things that I though were safe to assume, but alas... :) Cheers
diff --git a/iwinfo_nl80211.c b/iwinfo_nl80211.c index 5bc2f51..50bb8f0 100644 --- a/iwinfo_nl80211.c +++ b/iwinfo_nl80211.c @@ -825,13 +825,15 @@ static char * nl80211_phy2ifname(const char *ifname) memset(nif, 0, sizeof(nif)); - snprintf(buffer, sizeof(buffer), - "/sys/class/ieee80211/phy%d/device/net", phyidx); - - if ((d = opendir(buffer)) != NULL) + if ((d = opendir("/sys/class/net")) != NULL) { while ((e = readdir(d)) != NULL) { + snprintf(buffer, sizeof(buffer), + "/sys/class/net/%s/phy80211/index", e->d_name); + if (nl80211_readint(buffer) != phyidx) + continue; + snprintf(buffer, sizeof(buffer), "/sys/class/net/%s/ifindex", e->d_name); cifidx = nl80211_readint(buffer);