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

gcoap_dns: various fixes [backport 2022.07] #18373

Merged

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Jul 26, 2022

Backport of #18367

Contribution description

While doing some more experiments with the current version of gcoap_dns, I noticed some bugs, specifically, when using block-wise transfer. This PR fixes those and also replaces some left-over printfs with DEBUG.

Testing procedure

Similar to the testing procedures in #16705:

$ sudo dist/tools/tapsetup/tapsetup
$ sudo ip addr add 2001:db8::1 dev tapbr0
$ sudo ip route add 2001:db8::/64 via "<native link-local>" dev tapbr0
$ pip install --upgrade git+https://github.com/miri64/aiodnsprox.git@exp  # blockwise does not work with upstream aiodnsprox yet
$ cat << EOF > config.yaml
dtls_credentials:
  client_identity: Client_identity
  psk: secretPSK
mock_dns_upstream:
  IN:
    A: 192.0.2.7
    AAAA: 2001:db8::7
$ aiodns-proxy -C config.yaml -c 2001:db8::1
RIOT_CONFIG_KCONFIG_USEMODULE_GCOAP_DNS=y \
   RIOT_CONFIG_KCONFIG_USEMODULE_GCOAP=y \
   RIOT_CONFIG_GCOAP_DNS_PDU_BUF_SIZE=100 \
   RIOT_CONFIG_GCOAP_DNS_BLOCK_SIZE=16 \
   RIOT_CONFIG_GCOAP_RESEND_BUFS_MAX=2 \
   make -C tests/gcoap_dns/ flash -j term
> ifconfig 7 add 2001:db8::2
ifconfig 7 add 2001:db8::2
success: added 2001:db8::2/64 to interface 7
> nib route add 7 default fe80::dc1a:a8ff:fe09:45b3
nib route add 7 default fe80::dc1a:a8ff:fe09:45b3
> uri coap://[2001:db8::1]/dns
uri coap://[2001:db8::1]/dns
Successfully added URI coap://[2001:db8::1]/dns
> query example.org inet6
query example.org inet6
Hostname example.org resolves to 2001:db8::7 (IPv6)

Wiershark also shows blockwise transfer for both request and response:

grafik

Without this fix query example.org inet6 will just fail with Unable to resolve query: Invalid argument (stemming from the wrongly reported -EINVAL).

Issues/PRs references

Follow-up on #16705

@miri64 miri64 added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jul 26, 2022
miri64 added 2 commits July 27, 2022 09:38
(cherry picked from commit 349a89c)
(cherry picked from commit 875411f)
@miri64 miri64 force-pushed the backport/2022.07/gcoap_dns/fix/various branch from 995d998 to dfbf597 Compare July 27, 2022 07:38
@miri64
Copy link
Member Author

miri64 commented Jul 27, 2022

Rebased to current master.

@miri64 miri64 enabled auto-merge July 27, 2022 13:12
@miri64 miri64 merged commit 4592a53 into RIOT-OS:2022.07-branch Jul 27, 2022
@miri64 miri64 deleted the backport/2022.07/gcoap_dns/fix/various branch July 27, 2022 13:54
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants