Red Hat Bugzilla – Bug 1388613
[RFE] Allow setting the MTU of mobile broadband connections in NetworkManager
Last modified: 2017-08-01 05:19:37 EDT
Pushed branch bg/wwan-mtu-rh1388613 for review.
+ guint32 mtu = 0; + g_object_get (setting, "mtu", &mtu, NULL); mtu should be guint. rest lgtm
(In reply to Thomas Haller from comment #3) > + guint32 mtu = 0; > + g_object_get (setting, "mtu", &mtu, NULL); > > mtu should be guint. Fixed, thanks.
Repushed with an additional commit (device: don't call apply_mtu_from_config() when ipv4.method=disabled)
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.
(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.
Rebased branch bg/wwan-mtu-rh1388613, please review.
(In reply to Beniamino Galvani from comment #10) > Rebased branch bg/wwan-mtu-rh1388613, please review. pushed one fixup as a suggestion. Rest lgtm
(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
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