Skip to content

Minpoll maxpoll doc update#1478

Open
bsun-sudo wants to merge 3 commits intosonic-net:masterfrom
bsun-sudo:minpoll_maxpoll_doc_update
Open

Minpoll maxpoll doc update#1478
bsun-sudo wants to merge 3 commits intosonic-net:masterfrom
bsun-sudo:minpoll_maxpoll_doc_update

Conversation

@bsun-sudo
Copy link

This update is to add support of per NTP server minpoll and maxpoll configuration.

| Rev | Date | Author | Description |
|:---:|:----------:|:-----------------:|:----------------|
| 0.1 | 01/02/2023 | Yevhen Fastiuk 🇺🇦 | Initial version |
| 0.1 | 09/18/2023 | Bing Sun | Add NTP server minpoll and maxpoll |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bump it to 0.2

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines +72 to +73
| minpoll | minimum poll interval |
| maxpoll | maximum poll interval |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minimum -> Minimum
maximum -> Maximum

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

| key (ntpd) | none | All packets sent to and received from the server or peer are to include authentication fields encrypted using the specified key identifier with values from 1 to 65535, inclusive. The default is to include no encryption field. |
| version | 4 | Specifies the version number to be used for outgoing NTP packets. Versions 1-4 are the choices, with version 4 the default. |
| iburst | none | When the server is unreachable, send a burst of eight packets instead of the usual one. The packet spacing is normally 2 s; however, the spacing between the first two packets can be changed with the calldelay command to allow additional time for a modem or ISDN call to complete. This is designed to speed the initial synchronization acquisition with the server command and s addresses and when ntpd is started with the -q option. |
| minpoll | 6 | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some description for minpoll, even it is similar as maxpoll

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Comment on lines +1095 to +1096
must "(current()/../maxpoll > current())" {
error-message "maxpoll has to be larger than minpoll.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be the same?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. They can be the same. Modified the doc.

Comment on lines +1109 to +1110
must "(current()/../minpoll < current())" {
error-message "maxpoll has to be larger than minpoll.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same question here

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, they can be the same.

@fastiuk
Copy link
Contributor

fastiuk commented Nov 19, 2023

Looks good, just a couple of small comments

@venkatmahalingam
Copy link
Collaborator

@bsun-sudo looks good to me, please address the outstanding comments.

@bsun-sudo
Copy link
Author

All comments addressed.

@zhangyanzhao
Copy link
Collaborator

@bsun-sudo can you please help to add the code PR by referring to #806 ? Thanks.

@zhangyanzhao
Copy link
Collaborator

@bsun-sudo has this HLD been reviewed in community meeting? I can not find a record on my side. Thanks.

@zhangyanzhao
Copy link
Collaborator

code PR is not ready, move to backlog for future release

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

Labels

None yet

Projects

Status: MovedToBacklog

Development

Successfully merging this pull request may close these issues.

4 participants