Bug 1388613
| Summary: | [RFE] Allow setting the MTU of mobile broadband connections in NetworkManager | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | suresh kumar <surkumar> |
| Component: | NetworkManager | Assignee: | Beniamino Galvani <bgalvani> |
| Status: | CLOSED ERRATA | QA Contact: | Desktop QE <desktop-qa-list> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 7.3 | CC: | atragler, bgalvani, fgiudici, lrintel, rkhan, sukulkar, surkumar, thaller, vbenes |
| Target Milestone: | rc | Keywords: | FutureFeature |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | NetworkManager-1.8.0-0.4.rc1.el7 | Doc Type: | If docs needed, set a value |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2017-08-01 09:19:37 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 1393481 | ||
|
Comment 2
Beniamino Galvani
2016-10-26 11:37:03 UTC
+ 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 |