Skip to content

dhcpv6: RFC4833 timezones#284

Closed
systemcrash wants to merge 1 commit intoopenwrt:masterfrom
systemcrash:tzdb
Closed

dhcpv6: RFC4833 timezones#284
systemcrash wants to merge 1 commit intoopenwrt:masterfrom
systemcrash:tzdb

Conversation

@systemcrash
Copy link
Contributor

ping @Alphix and @Noltari

Can you see anything that should be improved here? This option differs from the others in that it's not an interface option, but instead just an internal system_config thing. I made it 'opt in', so the user must specifically enable its usage, so if there are any gremlins through its usage, they can be disabled.

====

This implements RFC4833 - supplying timezone information to clients that request them. Both forms are possible, when timezone is configured in the uci system settings (it saves both forms to the config).

e.g.

config system
	option zonename 'America/Puerto Rico'
	option timezone 'AST4'

There is also an odhcpd flag to toggle their use, set in uci dhcp.

config odhcpd 'odhcpd'
	option enable_tzdb '1'

Once enabled, the options, when requested, are sent:

NEW_POSIX_TIMEZONE 41 // 'AST4'
NEW_TZDB_TIMEZONE 42 // 'America/Puerto_Rico'

Wireshark disassemble of options sent to client:

...
Time Zone Database
    Option: Time Zone Database (42)
    Length: 19
    TZ-database: America/Puerto_Rico
Time Zone
    Option: Time Zone (41)
    Length: 4
    Time-zone: AST4

@Alphix
Copy link
Contributor

Alphix commented Oct 21, 2025

Good stuff!

Two more things:

  • The README.md needs to be updated as well
  • I guess the init script needs to be updated to also list system as a reload trigger

@systemcrash systemcrash force-pushed the tzdb branch 3 times, most recently from 79a0f29 to 5299542 Compare October 21, 2025 19:14
@systemcrash
Copy link
Contributor Author

Good stuff!

Two more things:

* The `README.md` needs to be updated as well

* I guess [the init script](https://github.com/openwrt/openwrt/blob/f5fd7ef8886faadc654b627e5e4b4484c6ca31d7/package/network/services/odhcpd/files/odhcpd.init#L20) needs to be updated to also list `system` as a reload trigger

Potentially yes - but generally once a user sets a timezone, they rarely change it. Wondering if it's worth it, even though it's 'correct' :)

All done.

@Alphix
Copy link
Contributor

Alphix commented Oct 21, 2025

Good stuff!
Two more things:

* The `README.md` needs to be updated as well

* I guess [the init script](https://github.com/openwrt/openwrt/blob/f5fd7ef8886faadc654b627e5e4b4484c6ca31d7/package/network/services/odhcpd/files/odhcpd.init#L20) needs to be updated to also list `system` as a reload trigger

Potentially yes - but generally once a user sets a timezone, they rarely change it. Wondering if it's worth it, even though it's 'correct' :)

It was partially a reminder to self...I'm going to have to do something similar when I get around to updating the global DUID PR

@Alphix
Copy link
Contributor

Alphix commented Oct 21, 2025

The second version looks so much cleaner (IMHO)

@systemcrash
Copy link
Contributor Author

OK - sweet. Thanks for the pointers. Low fat. Way better than v0.

If there are no further comments, LGTM.

This implements RFC4833 - supplying timezone information to clients that
request them. Both forms are possible, when timezone is configured in
the uci system settings (the luci GUI saves both forms to the config).

e.g.
```
config system
	option zonename 'America/Puerto Rico'
	option timezone 'AST4'
```

There is also an odhcpd flag to disable their use, set in uci dhcp.

```
config odhcpd 'odhcpd'
	option enable_tzdb '0'
```

Once enabled, the options, when requested, are sent:
NEW_POSIX_TIMEZONE 41 // 'AST4'
NEW_TZDB_TIMEZONE 42 // 'America/Puerto_Rico'

Wireshark disassemble of options sent to client:

```
...
Time Zone Database
    Option: Time Zone Database (42)
    Length: 19
    TZ-database: America/Puerto_Rico
Time Zone
    Option: Time Zone (41)
    Length: 4
    Time-zone: AST4
```

Signed-off-by: Paul Donald <newtwen+github@gmail.com>
@Alphix
Copy link
Contributor

Alphix commented Oct 22, 2025

Purty. Still don't like struct sys_conf but not my call. LGTM ❤️

@systemcrash
Copy link
Contributor Author

Ready when you are @Noltari. I've tested the latest iteration on my production system and it performs as expected.

openwrt-bot pushed a commit that referenced this pull request Oct 22, 2025
This implements RFC4833 - supplying timezone information to clients that
request them. Both forms are possible, when timezone is configured in
the uci system settings (the luci GUI saves both forms to the config).

e.g.
```
config system
	option zonename 'America/Puerto Rico'
	option timezone 'AST4'
```

There is also an odhcpd flag to disable their use, set in uci dhcp.

```
config odhcpd 'odhcpd'
	option enable_tzdb '0'
```

Once enabled, the options, when requested, are sent:
NEW_POSIX_TIMEZONE 41 // 'AST4'
NEW_TZDB_TIMEZONE 42 // 'America/Puerto_Rico'

Wireshark disassemble of options sent to client:

```
...
Time Zone Database
    Option: Time Zone Database (42)
    Length: 19
    TZ-database: America/Puerto_Rico
Time Zone
    Option: Time Zone (41)
    Length: 4
    Time-zone: AST4
```

Signed-off-by: Paul Donald <newtwen+github@gmail.com>
Link: #284
Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
@Noltari
Copy link
Member

Noltari commented Oct 22, 2025

Merged, thanks @systemcrash!

@Noltari Noltari closed this Oct 22, 2025
@systemcrash systemcrash deleted the tzdb branch October 22, 2025 14:22
@Alphix
Copy link
Contributor

Alphix commented Oct 22, 2025

@Noltari out of curiosity...why are some PRs closed after merge (like this one) and some marked as merged?

@Noltari
Copy link
Member

Noltari commented Oct 22, 2025

@Noltari out of curiosity...why are some PRs closed after merge (like this one) and some marked as merged?

Normally Github will automatically close PRs if the same commits are added to the branch referenced by the PR.
Since this repository is a mirror of https://git.openwrt.org/?p=project/odhcpd.git, whenever I push there, @openwrt-bot will automatically push the same changes to Github. That's why it's always flagged as merged by @openwrt-bot.
However, in this specific case I forgot to push the PR branch first (my bad, sorry for that) and Github couldn't match the commits with the master branch ones.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants