Bug 1388613 - [RFE] Allow setting the MTU of mobile broadband connections in NetworkManager
Summary: [RFE] Allow setting the MTU of mobile broadband connections in NetworkManager
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: NetworkManager
Version: 7.3
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: rc
: ---
Assignee: Beniamino Galvani
QA Contact: Desktop QE
URL:
Whiteboard:
Depends On:
Blocks: 1393481
TreeView+ depends on / blocked
 
Reported: 2016-10-25 18:34 UTC by suresh kumar
Modified: 2017-08-01 09:19 UTC (History)
9 users (show)

Fixed In Version: NetworkManager-1.8.0-0.4.rc1.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-08-01 09:19:37 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2017:2299 normal SHIPPED_LIVE Moderate: NetworkManager and libnl3 security, bug fix and enhancement update 2017-08-01 12:40:28 UTC

Comment 2 Beniamino Galvani 2016-10-26 11:37:03 UTC
Pushed branch bg/wwan-mtu-rh1388613 for review.

Comment 3 Thomas Haller 2016-10-26 14:42:30 UTC
+    guint32 mtu = 0;
+              g_object_get (setting, "mtu", &mtu, NULL);

mtu should be guint.


rest lgtm

Comment 4 Beniamino Galvani 2016-10-27 07:58:44 UTC
(In reply to Thomas Haller from comment #3)
> +    guint32 mtu = 0;
> +              g_object_get (setting, "mtu", &mtu, NULL);
> 
> mtu should be guint.

Fixed, thanks.

Comment 5 Beniamino Galvani 2016-11-08 12:27:23 UTC
Repushed with an additional commit (device: don't call apply_mtu_from_config() when ipv4.method=disabled)

Comment 6 Thomas Haller 2016-11-08 14:01:06 UTC
lgtm




maybe: in a follow-up commit move apply_slave_mtu() inside where it's called.




maybe:

    else
         _LOGW (LOGD_IP4, "unhandled IPv4 config method '%s'; will fail", method);


should be just g_return_val_if_reached (ret); Seems we hit this line only in case of a bug, where we can just as well assert.

Comment 7 Beniamino Galvani 2016-11-15 17:31:24 UTC
(In reply to Thomas Haller from comment #6)
> lgtm
>  
> maybe: in a follow-up commit move apply_slave_mtu() inside where it's called.

I thinks it's nicer to have a separate function that does only one thing even if there is only one caller.

> maybe:
> 
>     else
>          _LOGW (LOGD_IP4, "unhandled IPv4 config method '%s'; will fail",
> method);
> 
> 
> should be just g_return_val_if_reached (ret); Seems we hit this line only in
> case of a bug, where we can just as well assert.

Updated, thanks.

Comment 10 Beniamino Galvani 2017-02-18 08:04:39 UTC
Rebased branch bg/wwan-mtu-rh1388613, please review.

Comment 11 Thomas Haller 2017-02-19 19:37:52 UTC
(In reply to Beniamino Galvani from comment #10)
> Rebased branch bg/wwan-mtu-rh1388613, please review.

pushed one fixup as a suggestion.

Rest lgtm

Comment 12 Beniamino Galvani 2017-02-20 08:38:10 UTC
(In reply to Thomas Haller from comment #11)
> (In reply to Beniamino Galvani from comment #10)
> > Rebased branch bg/wwan-mtu-rh1388613, please review.
> 
> pushed one fixup as a suggestion.

Looks good, squashed.

Merged to master:

https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=8b11c976baf9c40cc073a8193b5c89b16dd06137

Comment 15 errata-xmlrpc 2017-08-01 09:19:37 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2017:2299


Note You need to log in before you can comment on or make changes to this bug.