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

pkg/tinydtls/sock_dtls: use ztimer_usec #17677

Merged
merged 2 commits into from
Feb 21, 2022

Conversation

fjmolinas
Copy link
Contributor

Contribution description

In #17564 tinydtls code was migrated to ztimer, but for some reason, sock_dtls was looked over, and not caught by the CI, it's now resurfaced with #17670, so this PR cleans this up.

Since sock timeous are in us this PR us using `ztimer_usec, only 32bits are required here.

I also uncrustified in a separate commit.

Testing procedure

I ran examples/dtls-sock between two native nodes, also change the timeouts from SOCK_NO_TIMEOUT to 5 * US_PER_SEC and saw it timeout after 5 second when only one native node was up:

  • timeout
main(): This is RIOT! (Version: 2022.04-devel-416-g092ca-pr_tinydtls_ztimer_cleanup)
DTLS sock example application
All up, running the shell now
> dtlsc fe80::4cb2:45ff:fe3a:8461 "secure data"
dtlsc fe80::4cb2:45ff:fe3a:8461 "secure data"
Error creating session: -110
>
  • success
main(): This is RIOT! (Version: 2022.04-devel-416-g092ca-pr_tinydtls_ztimer_cleanup)
DTLS sock example application
All up, running the shell now
> dtlss start
dtlss start
> ifconfig
ifconfig
Iface  5  HWaddr: 4E:B2:45:3A:84:61
          L2-PDU:1500  MTU:1500  HL:64  Source address length: 6
          Link type: wired
          inet6 addr: fe80::4cb2:45ff:fe3a:8461  scope: link  VAL
          inet6 group: ff02::1
          inet6 group: ff02::1:ff3a:8461

> New client connected
Received 11 bytes -- (echo)

main(): This is RIOT! (Version: 2022.04-devel-416-g092ca-pr_tinydtls_ztimer_cleanup)
DTLS sock example application
All up, running the shell now
> dtlsc fe80::4cb2:45ff:fe3a:8461 "secure data"
dtlsc fe80::4cb2:45ff:fe3a:8461 "secure data"
From [fe80::4cb2:45ff:fe3a:8461%5]:20220
Client got hint: hint
Connection to server successful
Sent DTLS message
Received 11 bytes: "secure data"
Terminating

Issues/PRs references

Cleanup from #17564
Needed for #17670

@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 18, 2022
@fjmolinas fjmolinas requested a review from kfessel February 18, 2022 12:34
@github-actions github-actions bot added the Area: pkg Area: External package ports label Feb 18, 2022
@kfessel
Copy link
Contributor

kfessel commented Feb 18, 2022

just checked is USEC is necessary for the timeout - sadly yes to keep the sock interface "happy"

the parameter needs it since our current sock interface uses usec timeout everywhere (connect, accept, recv, send ...)

checked if usec is needed in the sock interface - unlikely

unlikely: even though linux uses struct timeval and there for provides such a high resolution

  • FreeRTOS uses ms,
  • WINSOCK uses ms and
  • IBM is very specific in its zOS docs "While the timeval structure can be specified using mircosecond granularity, the internal TCP/IP timers used to implement this function have a granularity of approximately 100 milliseconds."

may be this could go with a similar solution to IBM for now, accept high granularity (usecs) but use an internal resolution of ms -> this PR could go for ZTIMER_MSEC and divide the values from the interface by a 1000

@fjmolinas
Copy link
Contributor Author

may be this could go with a similar solution to IBM for now, accept high granularity (usecs) but use an internal resolution of ms -> this PR could go for ZTIMER_MSEC and divide the values from the interface by a 1000

I feel this would be out of scope of this PR, this is would an API change for sock...

@fjmolinas
Copy link
Contributor Author

may be this could go with a similar solution to IBM for now, accept high granularity (usecs) but use an internal resolution of ms -> this PR could go for ZTIMER_MSEC and divide the values from the interface by a 1000

I feel this would be out of scope of this PR, this is would an API change for sock...

But otherwise I would agree on this, similar to what your are proposing in #16598

@fjmolinas
Copy link
Contributor Author

All green!

@fjmolinas fjmolinas merged commit 3e2b670 into RIOT-OS:master Feb 21, 2022
@fjmolinas
Copy link
Contributor Author

Thanks!

@fjmolinas fjmolinas deleted the pr_tinydtls_ztimer_cleanup branch February 21, 2022 07:27
@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants