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

Jump to bottom

networkmanager: 1.22.10 → 1.26.0 #87286

Merged
merged 3 commits into from Jul 18, 2020
Merged

networkmanager: 1.22.10 → 1.26.0 #87286

merged 3 commits into from Jul 18, 2020

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented May 8, 2020 •
edited

Motivation for this change

Finally support for OWE/Enhanced Open.
(Though we'd need hostapd 2.10 for that to work)

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/blob/1.24.0/NEWS

=============================================
NetworkManager-1.24
Overview of changes since NetworkManager-1.22
=============================================

* Add support for virtual routing and forwarding (VRF) interfaces.
* Add support for Opportunistic Wireless Encryption mode (OWE) for Wi-Fi networks.
* Add support for 31-bit prefixes on IPv4 point-to-point links according to
  RFC 3021.
* Drop dependencies for libpolkit-agent-1 and libpolkit-gobject-1.
* nmcli:
  - support setting removal via new command
    `nmcli connection modify $CON_NAME remove $setting`.
  - support backslash escape sequences for "vpn.data", "vpn.secrets",
    "bond.options", and "ethernet.s390-options".
* bridge: support new options "bridge.multicast-querier", "bridge.multicast-query-use-ifaddr",
  "bridge.multicast-router", "bridge.vlan-stats-enabled", "bridge.vlan-protocol",
  "bridge.group-address".
* IPv6 SLAAC: add support for "ipv6.ra-timeout" setting
* IPv6 DHCP: add support for "ipv6.dhcp-timeout" setting
* WWAN: NetworkManager now detects if a PIN-protected SIM card has been
  externally unlocked and automatically tries to activate a suitable
  connection on the modem.
* OVS:
  - add support for changing MTU of OVS interfaces.
  - remove length limitation for OVS Bridge, Patches and Interfaces
    (only Patch types) names.
* VPN: accept empty values for VPN data items and secrets.
* All nm-devices now expose the 'HwAddress' property via D-Bus.
* Slave devices now do not get created/activated if master is missing.
* Fixed multiple issues in the internal "nettools" DHCP client.
* Export NM_CAPABILITY_OVS capability on D-Bus and in libnm to
  indicate that the OVS plugin is loaded.
* Fixes for importing WireGuard profiles in nmcli and better handle
  configurations that enable ip4-auto-default-route with an explicit
  gateway.
* Various bug fixes and improvements.
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Phreedom @domenkozar @obadz @worldofpeace

@mweinelt mweinelt changed the base branch from master to staging May 8, 2020 19:46
@jtojnar
Copy link
Contributor

jtojnar commented May 8, 2020 •
edited

🤷‍♀️

  • Drop dependencies for libpolkit-agent-1 and libpolkit-gobject-1.

It still has a runtime dependency on polkit-agent-helper-1 and build-time dependencies on the pkg-config files.

Also do not like how easily it can fall through /usr:

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/365/diffs?commit_id=df1d214b2ea7dc65cc8c20658c66a97eb222569a#0cc1139e3347f573ae1feee5b73dbc8a8a21fcfa_500_501

@mweinelt
Copy link
Member Author

mweinelt commented May 8, 2020

Also do not like how easily it can fall through /usr:

I'm not too familiar with meson, but would something like this be fine?

diff --git a/meson.build b/meson.build
index a2d925a7e..23c85f128 100644
--- a/meson.build
+++ b/meson.build
@@ -522,7 +522,7 @@ polkit_agent_dep = dependency('polkit-agent-1', version: '>= 0.97', required : f
 if polkit_agent_dep.found()
        config_h.set_quoted('POLKIT_PACKAGE_PREFIX', polkit_agent_dep.get_pkgconfig_variable('prefix'))
 else
-       config_h.set_quoted('POLKIT_PACKAGE_PREFIX', '/usr')
+       config_h.set_quoted('POLKIT_PACKAGE_PREFIX', '@polkit@')
 endif

@jtojnar
Copy link
Contributor

jtojnar commented May 8, 2020

Yes, if you substituted it in the patch.

Though maybe removing the required : false would be even more reliable. Ideally, we would came up with a patch that can fail fast and be acceptable to upstream.

@mweinelt
Copy link
Member Author

mweinelt commented May 8, 2020 •
edited

I hope that works for you.

I'm not sure I can help coming up with a solution that would be acceptable upstream. I'm not even sure what the constraints would be.

@flokli flokli requested a review from jtojnar May 31, 2020 23:54
@worldofpeace worldofpeace self-assigned this Jul 8, 2020
@worldofpeace
Copy link
Contributor

I've bumped this to 1.24.2 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/1.24.2/NEWS

@worldofpeace worldofpeace changed the title networkmanager: 1.22.10 → 1.24.0 networkmanager: 1.22.10 → 1.24.2 Jul 8, 2020
@worldofpeace
Copy link
Contributor

@jtojnar Was there anything unresolved with this? This was in my queue, so I pleased to see an open PR 😄 and maybe merge it.

jtojnar
jtojnar reviewed Jul 8, 2020
View reviewed changes
pkgs/tools/networking/network-manager/fix-paths.patch Outdated
config_h.set_quoted('POLKIT_PACKAGE_PREFIX', polkit_agent_dep.get_pkgconfig_variable('prefix'))
else
- config_h.set_quoted('POLKIT_PACKAGE_PREFIX', '/usr')
+ config_h.set_quoted('POLKIT_PACKAGE_PREFIX', '@polkit@')
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, looking at the use case, this is not even correct.

We need to patch https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/blob/75bc21c4cf2c54e368d665302760cfe41b1b6814/clients/common/nm-polkit-listener.c#L555 to /run/wrappers/bin/polkit-agent-helper-1

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly, will fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've hardcoded it for now by not using POLKIT_PACKGE_PREFIX as suggested.
Now, about an upstream change, they can just have

option('polkit_package_prefix', type: 'string', value: '/usr', description: 'Prefix for the polkit package')

and just use that option value in the else. That way we could just put the path in mesonFlags?

Copy link
Contributor

Choose a reason for hiding this comment

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

An option for adjusting just the prefix would not help us much, we can already do as such using PKG_CONFIG_POLKIT_AGENT_1_PREFIX environment variable. The only part we share with the upstream path is the executable name:

-/usr/lib/polkit-1/polkit-agent-helper-1
+/run/wrappers/bin/polkit-agent-helper-1

@worldofpeace worldofpeace changed the title networkmanager: 1.22.10 → 1.24.2 networkmanager: 1.22.10 → 1.26.0 Jul 14, 2020
@jtojnar
Copy link
Contributor

jtojnar commented Jul 15, 2020 •
edited

Changes look mostly good but I am not sure what this means for us:

  • Add a new build option 'firewalld-zone'; when enabled,
    NetworkManager installs a firewalld zone for connection sharing and
    puts interfaces using IPv4 or IPv6 shared mode in this zone during
    activation. The option is enabled by default.
    Note that NetworkManager still calls to iptables to enable
    masquerading and open needed ports for DHCP and DNS. The new option
    is useful on systems using firewalld with the nftables backend,
    where the iptables rules would not be sufficient.

@jtojnar jtojnar mentioned this pull request Jul 16, 2020
Merged
10 tasks
@worldofpeace worldofpeace merged commit b8aa90c into NixOS:staging Jul 18, 2020
@worldofpeace
Copy link
Contributor

Thank you @mweinelt, this PR was useful to me ✨

@mweinelt
Copy link
Member Author

Thanks for taking the time to finish it!

@mweinelt mweinelt deleted the pr/nm branch July 19, 2020 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

@obadz obadz Awaiting requested review from obadz

@Phreedom Phreedom Awaiting requested review from Phreedom

@worldofpeace worldofpeace Awaiting requested review from worldofpeace

@domenkozar domenkozar Awaiting requested review from domenkozar

@andir andir Awaiting requested review from andir

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

Assignees

@worldofpeace worldofpeace

Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mweinelt @jtojnar @worldofpeace