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

ConnMan: use UseGatewaysAsTimeservers option #2818

Merged

Conversation

chrisnovakovic
Copy link
Contributor

I recently implemented runtime-configurable support for ConnMan's "use default gateway as timeserver" feature and had it accepted upstream (the salient commits are 20f20417 and 3ce40776). The default is for this behaviour to be disabled, as it's only useful in a tiny number of (broken) network setups and is almost always the wrong thing to do everywhere else. This new feature obsoletes LibreELEC's custom patch connman-04-do-not-add-default-gw-as-timeserver.patch, which hard-codes the same behaviour at compile-time.

This PR cherry-picks the upstream commits implementing this feature, disables it by setting UseGatewaysAsTimeservers=false in /etc/connman/main.conf (not strictly necessary, but useful in case upstream decides to change the default behaviour someday), and drops connman-04-do-not-add-default-gw-as-timeserver.patch (as it's no longer necessary). When ConnMan 1.37 is released and we upgrade to it, connman-00-upstream-main-Add-UseGatewaysAsTimeservers-option.patch and connman-00-upstream-timeserver-UseGatewaysAsTimeservers.patch can also be dropped.

@MilhouseVH
Copy link
Contributor

Thanks @chrisnovakovic - I'll include this in test builds for the next few days just to be on the safe side, even though it doesn't look like it's going to cause any problems.

@MilhouseVH
Copy link
Contributor

The connman-00-upstream-main-Add-UseGatewaysAsTimeservers-option.patch patch doesn't apply on connman-1.36:

  INSTALL    network (target)
      UNPACK   connman
          APPLY PATCH (common)   packages/network/connman/patches/connman-00-upstream-main-Add-UseGatewaysAsTimeservers-option.patch
patching file src/main.c
Hunk #1 FAILED at 82.
Hunk #2 FAILED at 102.
Hunk #3 FAILED at 123.
Hunk #4 FAILED at 144.
Hunk #5 succeeded at 431 with fuzz 1 (offset -10 lines).
Hunk #6 succeeded at 651 with fuzz 2 (offset -13 lines).
4 out of 6 hunks FAILED -- saving rejects to file src/main.c.rej
Makefile:9: recipe for target 'release' failed

@chrisnovakovic
Copy link
Contributor Author

chrisnovakovic commented Jul 7, 2018

@MilhouseVH You're quite right, sorry - I didn't think there had been any significant changes to main.c between 1.36 and that commit. I'll revise and recommit.

Upstream commits 20f20417 ("main: Add UseGatewaysAsTimeservers option")
and 3ce40776 ("timeserver: Use gateways as timeservers if
UseGatewaysAsTimeservers=true") change ConnMan's behaviour regarding the
use of default gateways as timeservers: this is now a configurable
option at run-time, with the default setting being to disable this
behaviour. These commits obsolete one of LibreELEC's custom patches,
connman-04-do-not-add-default-gw-as-timeserver.patch.

Prepare for the next stable release of ConnMan by cherry-picking these
commits from upstream (with minor modifications to commit 20f20417 so it
applies cleanly to 1.36) and using the new UseGatewaysAsTimeservers
option in /etc/connman/main.conf to forcefully disable the feature,
which matches the behaviour of the LibreELEC patch. The cherry-picked
upstream commits can be dropped when a new stable version of ConnMan is
released and LibreELEC upgrades to it.
@chrisnovakovic chrisnovakovic force-pushed the master-connman-gateway-timeserver branch 2 times, most recently from bdf22be to 3d9033f Compare July 8, 2018 01:16
@chrisnovakovic
Copy link
Contributor Author

This one applies cleanly and compiles correctly.

@MilhouseVH
Copy link
Contributor

Thanks, looks good now (builds fine on RPi/RPi2/Generic). I'll include for a few days in test builds, and if no problems we should be able to merge middle of next week.

@stefansaraev
Copy link
Contributor

lol

sorry for the spam, but I could not resist :D

@chrisnovakovic
Copy link
Contributor Author

Mistakes were made, let's leave it at that... 😉

@MilhouseVH
Copy link
Contributor

No reported issues so far, I reckon this can go in. Thanks @chrisnovakovic !

@MilhouseVH MilhouseVH merged commit ff9b270 into LibreELEC:master Jul 11, 2018
@chrisnovakovic chrisnovakovic deleted the master-connman-gateway-timeserver branch July 11, 2018 21:48
@chewitt
Copy link
Member

chewitt commented Mar 19, 2019

@chrisnovakovic I spotted this when working on some connman issues with one of their developers:

https://lists.01.org/pipermail/connman/2019-March/023277.html

should be resolved in the next connman bump

@chrisnovakovic
Copy link
Contributor Author

@chewitt Good spot, thanks! No idea how that happened...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants