Bug 413211 - Wired connection default settings are misleading
Summary: Wired connection default settings are misleading
Status: RESOLVED FIXED
Alias: None
Product: plasma-nm
Classification: Plasma
Component: editor (show other bugs)
Version: master
Platform: Neon Linux
: NOR grave
Target Milestone: ---
Assignee: Jan Grulich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-19 21:09 UTC by Dmitry
Modified: 2019-12-05 15:28 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.18.0
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry 2019-10-19 21:09:32 UTC
SUMMARY
Settings editor applet shows and saves hardcoded default ethernet settings for connections without negotioation/speed/duplex explicitly saved. It leads to saving connections with implicit (autonegotiation => auto 1Gb/s Full) as explicit (no auto negotiation 100Mb/s Half).


STEPS TO REPRODUCE
1. Stop NetworkManager
2. Remove all wired connections in /etc/NetowrkManager/system-connections (replicates clear distribution installation
3. Start NetowrkManager
4. Ensure you have a wired connections
5. Check connection settings via ethtool
6. Edit anything (e.g. DNS/VPN) in the newly created connection settings
7. Save settings
8. Disconnect and reconnect ethernet (via UI/CLI/physically)


OBSERVED RESULT
Connection become 100 Mb/s Half duplex no auto negotiation

EXPECTED RESULT
Connection stay whatever it was (presumably autonegotiation => auto 1Gb/s Full)

SOFTWARE/OS VERSIONS
Linux: 4.15+ (presumably any)
Distribution: OpenSUSE, Arch, Fedora (presumably any)
KDE Plasma Version: 5.16/5.17 (presumably any)
NetworkManager: 1.18/1.20 
Hardware: r8169/e1000e NIC (including qemu and virtualbox) (presumably any)

ADDITIONAL INFORMATION
Bug comes from interpreting {speed: 0, duplex: UnknownDuplex} at libs/editor/settings/wiredconnectionwidget.cpp:63 (WiredConnectionWidget::loadConfig) as HalfDuplex 100 Mb/s from hardcoded defaults.

SEVERITY
Note, that ANY change of ANY implicitly created wired connection on ANY stock system leads to switching from default ethernet settings to rare and unexpected mode for virtually no reason. This behaviour is obscure and may turn regular users ot from using Linux/KDE due to slower "Internet"
Comment 1 Jan Grulich 2019-10-20 17:39:36 UTC
Git commit 14f7ab5e3c686f85de57c8845bedab54ca7cb71a by Jan Grulich.
Committed on 20/10/2019 at 17:39.
Pushed by grulich into branch 'Plasma/5.17'.

Wired connection: default to Full duplex when duplex is not set

M  +2    -1    libs/editor/settings/wiredconnectionwidget.cpp

https://commits.kde.org/plasma-nm/14f7ab5e3c686f85de57c8845bedab54ca7cb71a
Comment 2 Dmitry 2019-10-20 22:50:35 UTC
Proposed fix does not fix the issue itself, just mitigates the most extreme case.
Behaviour is still broken due to:

1. Configuration is still changes implicitly:
  * before editing: NetworkManager does not set ethernet mode
  * after editing: NetworkManager sets 100 Mb/s FullDuples
2. In some cases (1) leads to UX regression
  * it makes connection over gigabit link slower
  * it makes connection over 100 HD link no longer working
3. In some rare cases it leads to faulty condition
  * in case of (https://bugzilla.kernel.org/show_bug.cgi?id=205067) it's impossible to configure/preserve via GUI a persistently working gigabit connection.

The only fix I hae ideas about is implementing this as a separate mode of a connection like it works in XFCE, where not only "Manual" and "Auto" modes are present, but also "Ignored", which essentially is the described above mode.

Implementing this fix may require adding come code into networkmanager-qt as plasma-nm uses it to interact with NetworkManager.
Comment 3 Jan Grulich 2019-10-21 07:33:26 UTC
(In reply to Dmitry from comment #2)
> Proposed fix does not fix the issue itself, just mitigates the most extreme
> case.
> Behaviour is still broken due to:
> 
> 1. Configuration is still changes implicitly:
>   * before editing: NetworkManager does not set ethernet mode
>   * after editing: NetworkManager sets 100 Mb/s FullDuples
> 2. In some cases (1) leads to UX regression
>   * it makes connection over gigabit link slower
>   * it makes connection over 100 HD link no longer working
> 3. In some rare cases it leads to faulty condition
>   * in case of (https://bugzilla.kernel.org/show_bug.cgi?id=205067) it's
> impossible to configure/preserve via GUI a persistently working gigabit
> connection.
> 
> The only fix I hae ideas about is implementing this as a separate mode of a
> connection like it works in XFCE, where not only "Manual" and "Auto" modes
> are present, but also "Ignored", which essentially is the described above
> mode.
> 
> Implementing this fix may require adding come code into networkmanager-qt as
> plasma-nm uses it to interact with NetworkManager.

I just checked nm-connection-editor and they indeed have additional option, which is "Ignore", but in the code it doesn't make any difference. If the link negotiation is set to "Ignore" or "Automatic", it sets speed to 0 and duplex to unknown type, which is the same what we do in plasma-nm.
Comment 4 Dmitry 2019-10-22 07:33:13 UTC
(In reply to Jan Grulich from comment #3)
> I just checked nm-connection-editor and they indeed have additional option,
> which is "Ignore", but in the code it doesn't make any difference. If the
> link negotiation is set to "Ignore" or "Automatic", it sets speed to 0 and
> duplex to unknown type, which is the same what we do in plasma-nm.

So, my bad, I've made wrong assumptions from networkmanager-qt code.
However, the case is when no value is explicitly set in the connection beforehands, plasma-nm will apply UI defaults for speed/dulpex when user saves any other parameter e.g. DNS servers.

Described above case is present on the video:
https://youtu.be/8UOu76c-fNk 
Test was performed on a clear Plasma, Arch Linux (latest stable to 2019.10.22 morning)
Comment 5 Jan Grulich 2019-10-22 13:57:56 UTC
I opened a review request: https://phabricator.kde.org/D24866
Comment 6 Dennis Schridde 2019-11-11 07:31:53 UTC
I am coming here after, with Beniamino Galvani <bgalvani@redhat.com>'s help in https://bugzilla.redhat.com/show_bug.cgi?id=1765490#c13, it became clear that KDE Plasma NetworkManager interprets the data from NetworkManager wrongly:

$ nmcli connection show $conn_uuid
802-3-ethernet.speed:                   0
802-3-ethernet.duplex:                  --
802-3-ethernet.auto-negotiate:          no

shows up as Auto Negotiate: No, Speed: 100Mbps, Duplex: Half in the KDE Plasma NetworkManager connection configuration dialogue.

According to him, it *should* mean: Auto Negotiate: No, Speed: Link Default, Duplex: Link Default.
Comment 7 Fabian Vogt 2019-11-20 09:55:42 UTC
(In reply to Dennis Schridde from comment #6)
> I am coming here after, with Beniamino Galvani <bgalvani@redhat.com>'s help
> in https://bugzilla.redhat.com/show_bug.cgi?id=1765490#c13, it became clear
> that KDE Plasma NetworkManager interprets the data from NetworkManager
> wrongly:
> 
> $ nmcli connection show $conn_uuid
> 802-3-ethernet.speed:                   0
> 802-3-ethernet.duplex:                  --
> 802-3-ethernet.auto-negotiate:          no
> 
> shows up as Auto Negotiate: No, Speed: 100Mbps, Duplex: Half in the KDE
> Plasma NetworkManager connection configuration dialogue.
> 
> According to him, it *should* mean: Auto Negotiate: No, Speed: Link Default,
> Duplex: Link Default.

There's another layer of weirdness above that, as nmcli edit describe says:

> When TRUE, enforce auto-negotiation of speed and duplex mode.
> If "speed" and "duplex" properties are both specified, only that single mode
> will be advertised and accepted during the link auto-negotiation process:
> this works only for BASE-T 802.3 specifications and is useful for enforcing
> gigabits modes, as in these cases link negotiation is mandatory. When FALSE,
> "speed" and "duplex" properties should be both set or link configuration will
> be skipped.

I did some tests and if either .speed or .duplex are not set, "auto-negotiate: no" has no effect.
Currently the KCM shows that as "100MiB/s Full-Duplex" and saving that has weird effects.
Comment 8 Jan Grulich 2019-11-29 11:54:02 UTC
This should all be solved with the review I opened while ago, feel free to review it:

Review: https://phabricator.kde.org/D24866
Comment 9 Jan Grulich 2019-12-05 14:43:01 UTC
Git commit 8b9eb87388cc521b9f0dcc9e81e9d565674a2529 by Jan Grulich.
Committed on 05/12/2019 at 14:42.
Pushed by grulich into branch 'master'.

Wired setting: improve handling of link negotiation

Summary:
Adds option to ignore link-negotiation, also uses a combobox, similar to what
nm-connection-editor is using, to give a choice only with sane values for speed.

Test Plan:
I tried to configure all possible combinations, all of them were saved correctly and loaded afterwards.

Screenshot of updated wired setting:
{F7648938}

Reviewers: #plasma, #vdg, davidedmundson

Reviewed By: #plasma, #vdg, davidedmundson

Subscribers: davidedmundson, devurandom, ngraham, dvalter, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D24866

M  +83   -121  libs/editor/settings/ui/wiredconnectionwidget.ui
M  +47   -27   libs/editor/settings/wiredconnectionwidget.cpp
M  +11   -0    libs/editor/settings/wiredconnectionwidget.h

https://commits.kde.org/plasma-nm/8b9eb87388cc521b9f0dcc9e81e9d565674a2529