Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

network/tasks/detected_network.yml: can_be_ap grep failed on absence of regex #3323

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

holta
Copy link
Member

@holta holta commented Jul 28, 2022

@holta holta added the bug label Jul 28, 2022
@holta holta added this to the 8.0 milestone Jul 28, 2022
@holta
Copy link
Member Author

holta commented Jul 28, 2022

Quickly tested and Ed in the UK will now further test after merge.

@holta holta merged commit 2efd7c0 into iiab:master Jul 28, 2022
@holta
Copy link
Member Author

holta commented Jul 28, 2022

  1. Thank you to Ed from Zimbabwe (@EMG70) for surfacing this bug!

  2. There are related issues he'll be posting about on another ticket — as he tries to get IIAB working on his Dell Inspiron 1750 laptop — trying to get any kind of WiFi learning hotspot working (internal WiFi ideally, but USB WiFi dongle if necessary!?)

@jvonau
Copy link
Contributor

jvonau commented Jul 28, 2022

Now was iw preinstalled by the os when the first failure occurred?
#3070 f55f4f3
4d5143e

@holta
Copy link
Member Author

holta commented Jul 28, 2022

was iw preinstalled by the os when the first failure occurred?

Not by the OS per se, but I believe that yes iw was already pre-installed — by roles/1-prep here:

- name: Install ~12 network/wifi/related packages + Squid if necessary + configure /etc/sysctl.conf -- full configuration LATER in 'network', after Stage 9
include_tasks: roles/network/tasks/install.yml
when: network_install and network_installed is undefined

Which ran:

- name: 'Install 11 network packages: avahi-daemon, hostapd, iproute2, iptables-persistent, iw, libnss-mdns, netmask, net-tools, rfkill, wireless-tools, wpasupplicant -- later used by https://github.com/iiab/iiab/tree/master/roles/network'
package:
name:
- avahi-daemon # 97kB download: RasPiOS (and package libnss-mnds, below) install this regardless -- holdover from the XO days and used to advertise ssh/admin-console being available via avahi-daemon -- used with https://github.com/iiab/iiab/blob/master/roles/network/tasks/avahi.yml
#- avahi-discover # 46kB download: 2021-07-27: Commented out long ago
- hostapd # 764kB download: IEEE 802.11 AP and IEEE 802.1X/WPA/WPA2/EAP Authenticator -- has its service masked out of the box, and only used when IIAB's network roles detects the presence of WiFi and an AP is desired
#- inetutils-syslogd # 240kB download: 2021-07-27: Error logging facility -- holdover from the XO days, journalctl has replaced this in newer distros
- iproute2 # 902kB download: RasPiOS installs this regardless -- the new networking and traffic control tools, meant to replace net-tools
- iptables-persistent # 12kB download: Boot-time loader for netfilter rules, iptables (firewall) plugin -- however Netfilter / nftables is ever moving forward so keep an eye on it!
- iw # 97kB download: RasPiOS installs this regardless -- configure Linux wireless devices -- hard dependence for ap0 creation, SEE https://github.com/iiab/iiab/blob/master/roles/network/templates/hostapd/iiab-clone-wifi.service.j2
- libnss-mdns # 27kB download: RasPiOS (and package avahi-daemon, above) install this regardless -- client-side library -- provides name resolution via mDNS (Multicast DNS) using Zeroconf/Bonjour e.g. Avahi
- netmask # 25kB download: Handy utility -- helps determine network masks
- net-tools # 248kB download: RasPiOS installs this regardless -- @jvonau suggests possibly deleting this...unless oldtimers really want these older commands in iiab-diagnostics output?
- rfkill # 87kB download: RasPiOS installs this regardless -- enable & disable wireless devices
- wireless-tools # 112kB download: RasPiOS installs this regardless -- manipulate Linux Wireless Extensions
- wpasupplicant # 1188kB download: RasPiOS installs this regardless -- client library for connections to a WiFi AP
state: present

@holta
Copy link
Member Author

holta commented Jul 28, 2022

Please move the discussion to his actual ticket if possible?

@jvonau
Copy link
Contributor

jvonau commented Jul 29, 2022

was iw preinstalled by the os when the first failure occurred?

Not by the OS per se, but I believe that yes iw was already pre-installed — by roles/1-prep here:

That doesn't answer the question of why the stanza failed on the first pass.. Then at some point recorded can_be_ap as True

@holta
Copy link
Member Author

holta commented Jul 29, 2022

why the stanza failed on the first pass

Recap, of what's explained in this PR itself:

Our Ansible logic was just wrong (prior to this PR).

As it failed when attempted on any laptop (machine) like the one @EMG70 is attempting to get working with IIAB — e.g. when 'AP' (or anything like it) is not contained anywhere in the output of iw list

In short, the logic that we had in place was just wrong, failing to account for the above kind of situation — when the shell command fails — as a result of grep return code of 1.

As mentioned earlier today: this failure occurs regardless of the whatever regex grep is scanning for. Specifically: when the regular expression (whatever regex) is not found in the output of iw list

@jvonau
Copy link
Contributor

jvonau commented Jul 29, 2022

That is why there was ' | wc -l' which was vetted against the live test of subject #3057 and later changed with f55f4f3 but was not cherry-picked into the tree then 4d5143e that I suspect was not tested against non-AP capable hardware... back over to you to fix..

@holta
Copy link
Member Author

holta commented Jul 29, 2022

Indeed the earlier regular expression had serious errors (counting the wrong kinds of lines until about May, which is now cleaned up).

I agree a simple simple existence test is what's really needed (no counting, no fancy corner cases, just a Dead Simple yes-or-no presence test which things like grep -q were designed for).

So the code is not the most elegant as is — but so be it at least it's finally fixed yay.

Thanks go to @EMG70 for uncovering this.

@jvonau
Copy link
Contributor

jvonau commented Aug 1, 2022

Indeed the earlier regular expression had serious errors (counting the wrong kinds of lines until about May, which is now cleaned up).

You have never shown any supporting evidence or issues reported to support that claim.

@holta
Copy link
Member Author

holta commented Aug 1, 2022

Erroneous results were in fact extremely common, caused be strings that happen to contain the letters 'AP' within another word.

As documented here:

# shell: iw list | grep -v AP: | grep AP | wc -l # False positives 'EAP' etc

This is the reason the regex was much improved around May (of this year).

(Apparently EAP = Extensible Authentication Protocol)

@jvonau
Copy link
Contributor

jvonau commented Aug 2, 2022

In the face of the lack of presented evidence that statement is hearsay and nothing more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants