Skip to content

Conversation

@JKRhb
Copy link
Collaborator

@JKRhb JKRhb commented Dec 30, 2020

Inspired by the discussion in this issue in the original RIOT repository, this PR is a draft for adding a pseudomodule with the intention of including the source address in CoAP packets.

So far only the submodule definition and the packet field itself have been added. As there are still things that may have to be discussed I opened this PR as a draft. If an actual solution for the problem emerges from this PR, its origin branch could probably also be merged into the RIOT main branch.

@JKRhb JKRhb requested a review from Citrullin December 30, 2020 18:38
@JKRhb JKRhb marked this pull request as draft December 30, 2020 18:40
@Citrullin
Copy link
Owner

I thought about that one too. Need to do some research on the API. Maybe it is already possible to get the IP address and just store it in a global variable in order to use it within gcoap.

@JKRhb JKRhb requested review from Citrullin and removed request for Citrullin January 1, 2021 16:38
@JKRhb
Copy link
Collaborator Author

JKRhb commented Jan 1, 2021

After delving into the UDP sock API I tried to implement a solution using sock_aux_local which is used to insert the local IP address into coap packets. The code is still a bit rough as it took me a while to get a grasp of the existing codebase but it is maybe a foundation on which we can build upon.

Copy link
Collaborator Author

@JKRhb JKRhb left a comment

Choose a reason for hiding this comment

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

I highlighted some points that may require discussion below.

Comment on lines +112 to +114
ifneq (,$(filter nanocoap_pkt_ipv6_address,$(USEMODULE)))
USEMODULE += sock_aux_local
endif
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This adds sock_aux_local as a dependency for the newly defined pseudo-module.

Comment on lines +196 to +198
#ifdef MODULE_NANOCOAP_PKT_IPV6_ADDRESS
ipv6_addr_t source_address; /**< request source address */
#endif
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe a different datastructure could be used for storing the ip address. (I just noticed that this field should be renamed to "target address".)

Comment on lines -144 to 148
if (type & SOCK_ASYNC_MSG_RECV) {
ssize_t res = sock_udp_recv(sock, _listen_buf, sizeof(_listen_buf),
0, &remote);
ssize_t res = sock_udp_recv_aux(sock, _listen_buf, sizeof(_listen_buf),
0, &remote, &aux);
if (res <= 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is changing the signature fine or should a separate function be defined for the use of the pseudo-module?

Comment on lines +203 to +205
#if IS_USED(MODULE_NANOCOAP_PKT_IPV6_ADDRESS)
memcpy(&pdu.source_address, &aux->local.addr.ipv6, sizeof(ipv6_addr_t));
#endif
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't entirely sure if this is actually working but I found something similar in the existing codebase. In any case, compiling the gcoap example with the added submodule works but actual testing (perhaps in the context of our wot implementation) is still needed.

@JKRhb
Copy link
Collaborator Author

JKRhb commented Jul 27, 2022

Hmm, I think this PR is not relevant anymore, right?

@Citrullin
Copy link
Owner

Hmm, I think this PR is not relevant anymore, right?

I actually don't know. I didn't had the time to do a lot with RIOT... I forgot a lot at this point...
I let you decide. Also to merge. That probably makes things easier :D

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