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.

Sign up for GitHub

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

Jump to bottom

Handling encoding issues on netplan status #521

Merged
merged 5 commits into from
Sep 27, 2024
Merged

Handling encoding issues on netplan status #521

merged 5 commits into from
Sep 27, 2024

Conversation

daniloegea
Copy link
Contributor

@daniloegea daniloegea commented Sep 25, 2024 •
edited
Loading

Description

This PR addresses some issues with netplan status:

  1. SSID is not displayed with newer systemd
    On Ubuntu Noble for example, netplan status will not show the SSID for wifi interfaces.
    The problem was likely caused by systemd/systemd@8fff78a.

  2. The output of external programs was broken if non-ascii characters are present.

  3. Netdef IDs were not being extracted correctly from file names when non-ascii characters were escaped.

  4. Retrieving the SSID via networkctl wasn't producing the readable SSID string.
    To verify the enigmatic trick used in the commit state/SSID: handle non-ascii SSIDs:

ssid = '\\303\\241\\303\\251\\303\\255\\303\\263\\303\\272\\302\\242\\302\\242\\302\\242\\302\\243\\302\\243\\302\\243'
ssid2 = ssid.encode('latin1').decode('unicode-escape').encode('latin1').decode('utf-8')
print(ssid2)

The output should be áéíóú¢¢¢£££.

The line 1 above is what we receive from networkctl and we are currently displaying it as the SSID in netplan status.

  1. On some devices networkctl and iw will not retrieve the SSID and "(null)" will be displayed. This problem was fixed for Network Manager connections (where the SSID is retrieved via nmcli) but is still there when networkd is the backend.
    Example: ● 3: wlp2s0 wifi/"(null)" UP (NetworkManager: wlp2s0)

Checklist

  • Runs make check successfully.
  • Retains code coverage (make check-coverage).
  • New/changed keys in YAML format are documented.
  • (Optional) Adds example YAML for new feature.
  • (Optional) Closes an open bug in Launchpad.

@daniloegea daniloegea marked this pull request as ready for review September 26, 2024 13:47
@daniloegea daniloegea requested a review from slyon September 26, 2024 13:47
slyon
slyon approved these changes Sep 26, 2024
View reviewed changes
Copy link
Collaborator

@slyon slyon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM. With a few (non-critical) inline comments.

netplan_cli/cli/state.py Outdated
Comment on lines 63 to 65
# Used for wifi testing
'ieee80211_radiotap': 'wifi',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: Could you please provide some instruction of how this can be used for wifi testing? Either as part of the PR description or a code comment. I cannot see it being used in the automated tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry. This is the device type for the device hwsim0 created by the mac80211_hwsim module.
I used it to test my changes and noticed warnings from netplan status.

This is how networkctl sees it: 7 hwsim0 ieee80211_radiotap off unmanaged

I added it to the list to avoid this warning from netplan status:

~# netplan status
Unknown device type: ieee80211_radiotap
Unknown device type: ieee80211_radiotap
     Online state: online
...

I'll improve the comment.

netplan_cli/cli/state.py
key = r'^Wi-?Fi access point: (.*) \(.*\)'
if match := re.match(key, line):
ssid = match.group(1)
# TODO: Find a better way to retrieve the SSID
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick (non-blocking): We should try getting this information from the networkctl --json=short output instead.

IIRC, this was not implemented back when we first wanted to use it in netplan status, but I can see it on systems using systemd v255+ today: networkctl --json=short status wlp0s20f3 | jq | grep SSID

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes it's not available. The output below is from one of my laptops:

danilo@danilo-Aspire-ES1-572:~$ networkctl --json=short status wlp2s0 | jq | grep SSID
  "BSSID": [
danilo@danilo-Aspire-ES1-572:~$

On another laptop it will give me the SSID though. I suspect it might be some limitation of the driver, I don't really know.
That's the reason why I changed it to get it from Network Manager when NM is the backend.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And in these cases we will show it like this on netplan status:

●  5: wlan2 wifi/"(null)" UP (networkd: wlan2)

@daniloegea
status: fix SSID parsing
81e251b 
The networkctl output was changed from "WiFi access point" to "Wi-Fi
access point" as part of systemd commit
8fff78a1dded105e1ee87bc66e29ef2fd61bf8c9.

Use a regex that matches both formats to keep it compatible with older
versions of networkd.
@daniloegea
cli: set LC_ALL to C.UTF-8
69bf0cf 
Non-ascii characters returned from external programs (such as nmcli) are
being replaced by "?".
@daniloegea
state/Interface: improve netdef ID handling with NetworkManager
2410c62 
The ID is extracted from the file name. For Wi-Fi connections, the SSID
will also be present in the file name and we need to remove it in order
to extract the netdef ID. When non-ascii characters are
present in the SSID, NM will represent it using a byte sequence
separated with ;. The semicolons are escaped in the file name when we
emit the .nmconnection file.

In order to find and remove the SSID from the file name, so we can get
the netdef ID, we need to convert it to the same representation.

To detect this situation we check if the SSID is present in the file
name. If it's not, it likely because the SSID contains non-ascii
characters.
@daniloegea
state/SSID: handle non-ascii SSIDs
313e132 
This workaround is intended to properly decode non-ascii SSIDs returned
by networkctl.

In such a case, a sequence of bytes (in octal notation) will be returned
and we were displaying this string in netplan status.
@daniloegea
state: get the SSID from NM if it's the backend
f933e58 
networkctl might return "(null)" as the SSID value.

Use Network Manager to get the SSID when it's the backend.
@daniloegea daniloegea merged commit ff65879 into canonical:main Sep 27, 2024
16 checks passed
@slyon slyon added the stable Might be merged in a stable branch label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slyon slyon slyon approved these changes

Assignees
No one assigned
Labels
stable Might be merged in a stable branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@daniloegea @slyon