gnrc, nimble/ble: notify network layer of BLE connection events #21608
Conversation
If a new L2 connection was established, the node should be inserted in the neighbor cache. For 6LN nodes, NIB will never start the usual neighbor discovery process because it can directly resolve the link-layer address from the IPv6 address. Thus we have to manually add such nodes to the NC by building the IPv6 address from the L2 address. If a L2 connection closed, the node is removed again from the NC.
Iterate through all parents whose address match `addr`. In most cases there will only be a single parent, but if `GNRC_RPL_INSTANCES_NUMOF > 1` then one node can be parent in multiple DODAGs.
Handle netapi notification for discovered and unreachable neighbors. For newly discovered neighbors, send a DIS message to the node. For unreachable nodes trigger a timeout for RPL parents that match the address.
`gnrc_rpl` now receives a notification if a new node is reachable (i.e., connected) and sends a DIS. No need anymore to also do it in rpble.
| /** | ||
| * @brief Iterate over all parents in all DODAGs with @p IPv6 address. | ||
| * | ||
| * @param[in] idx Index to start searching from. | ||
| * @param[in] addr IPV6 address of the parent. | ||
| * @param[out] parent Pointer to the parent if one was found. Otherwise NULL. | ||
| * | ||
| * @return Index > 0 to continue next search from, if parent was found. | ||
| * @return -ENONENT if not found | ||
| */ | ||
| int gnrc_rpl_parent_iter_by_addr(ipv6_addr_t *addr, gnrc_rpl_parent_t **parent, int idx); | ||
|
|
There was a problem hiding this comment.
See #21586 (comment).
There was a problem hiding this comment.
Quoting from there for reviewer's convenience:
This function is a bit weird, but it is needed because, theoretically, if we have more than one RPL instance, the same node can be present as parent in multiple dodags, and thus appear multiple times in the gnrc_rpl_parents list.
We want to mark the parent as unreachable in all dodags.
I'm not knowledgeable enough with RPL to comment on this.
sys/include/net/gnrc/ipv6/nib/nc.h
Outdated
| /** | ||
| * @brief Adds an unmanaged neighbor entry to the NIB if the interface represents a 6LN node | ||
| * and the IPv6 address can be constructed from the L2 address @p l2addr. | ||
| * | ||
| * @param[in] iface The interface to the neighbor. | ||
| * @param[in] l2addr The neighbor's layer 2 address. | ||
| * @param[in] l2addr_len Length of @p l2addr. | ||
| * | ||
| * @return 0 on success. | ||
| * @return -ENOTSUP, if the interface does not represent a 6LN or when | ||
| * gnrc_netif_t::device_type of the iface does not support IID conversion. | ||
| * @return -EINVAL, when @p addr_len is invalid for the | ||
| * gnrc_netif_t::device_type of @p netif. | ||
| * @return -ENOMEM, if no space is left in neighbor cache. | ||
| */ | ||
| int gnrc_ipv6_nib_nc_try_set_6ln(unsigned iface, const uint8_t *l2addr, | ||
| size_t l2addr_len); |
There was a problem hiding this comment.
Only possible for 6LN nodes because otherwise we can't statically build an IPv6 address from the link-layer address. The IPv6 address is build as a reverse of the _resolve_addr_from_ipv6 that is used when resolving IPv6-> L2address, following the stateless address autoconfiguration section in the RFC.
There was a problem hiding this comment.
Thanks for your PR! Already discussed the design offline, so that looks fine by me. But maybe someone with more knowledge with GNRC and the NETAPI could also comment on the approach (@miri64 maybe?).
Rather high-level comments from my side as I'm not very familiar with GNRC and RPL.
| /** | ||
| * @brief Iterate over all parents in all DODAGs with @p IPv6 address. | ||
| * | ||
| * @param[in] idx Index to start searching from. | ||
| * @param[in] addr IPV6 address of the parent. | ||
| * @param[out] parent Pointer to the parent if one was found. Otherwise NULL. | ||
| * | ||
| * @return Index > 0 to continue next search from, if parent was found. | ||
| * @return -ENONENT if not found | ||
| */ | ||
| int gnrc_rpl_parent_iter_by_addr(ipv6_addr_t *addr, gnrc_rpl_parent_t **parent, int idx); | ||
|
|
There was a problem hiding this comment.
Quoting from there for reviewer's convenience:
This function is a bit weird, but it is needed because, theoretically, if we have more than one RPL instance, the same node can be present as parent in multiple dodags, and thus appear multiple times in the gnrc_rpl_parents list.
We want to mark the parent as unreachable in all dodags.
I'm not knowledgeable enough with RPL to comment on this.
Co-authored-by: mguetschow <mikolai.guetschow@tu-dresden.de>
|
Latest Murdock failure is unrelated I think? Edit: just saw #21621 (comment). |
Use `thread_flags` to acknowledge a received netnotify event as soon as the associated event data was read. This avoids potentialy deadlocks that could happen if the receiver's handling of the event involves an exclusive lock on `netreg`. Instead of using `msg_send_receive`, the sender now blocks on the thread flag. Add helper functions in `netnotify` that take care of parsing the different event data types and directly setting the ACK flag.
| /** | ||
| * @{ | ||
| * | ||
| * @file | ||
| */ |
There was a problem hiding this comment.
| /** | |
| * @{ | |
| * | |
| * @file | |
| */ |
If you don't add documentation, you don't need this either. Same below.
There was a problem hiding this comment.
Every file (even .c files) should have the according Doxygen header. So please add the according fields to all the relevant files.
There was a problem hiding this comment.
So please add the according fields to all the relevant files.
@crasbe what fields should the doxygen header have in this case?
Renamed module `netnotify` -> `netapi_notify`.
Extract `netapi_notify_l2_connection_t` as a whole, rather than each property separately. -> Makes it consistent with the sender API und requires less parameters.
Respect the receiver netreg entry `type`.
5423c6a to
a7d5ac9
Compare
Use mutex, counter and condvar to collect the exact count of acknowledgements for a notify event. With this, we can now block for all replies after dispatching to all receivers and after releasing the lock on netreg. This a) maintains the receivers priority order and b) prevents possible deadlocks that could happen if we hold the netreg lock while blocking for an ACK.
Contribution description
This PR solves two issues that have been previously discussed in #21580 and #21586:
Both issues require communication between different netapi layers and are related with each other, so I bundled them in one PR.
Cross-layer Communication with netapi
With the current netapi it is not possible to send arbitrary data between networking threads.
The API only allows to either dispatch packets to multiple threads that are registered in the netreg (
gnrc_netapi_dispatch), or to set/get options to/from a single thread whose PID is known (gnrc_netapi_{get,set}).In my use-case, however, a lower layer (BLE) wants to notify other layers higher up the stack (IPv6, RPL) of an event (e.g., connection established). Ideally, the lower layer shouldn't need to know if/ what higher layers are present, similarly to the case when packets are passed up/ down the stack.
netapi
notifyTo solve the above, I added a new netapi function
gnrc_netapi_notifythat dispatches an "notification"-event to all subscribed threads. Like ingnrc_netapi_dispatchthe target threads are queried from netreg, but withnotifythe sending is done synchronously by blocking for an ACK. The latter is needed because contrary to the packets that get allocated in thepktbuf, the notify event data is stack allocated and thus blocking is needed to ensure the data was read before the function returns. This is not ideal, but I wasn't able to come up with a better solution.Handling BLE connection events
The
notifyevent API is used:In 2. the notification is sent from the IPv6 thread rather than from BLE because address translation from l2addr->ipv6 address is solved on that layer.
Testing procedure
Tested on FIT IoT-Testlab with 9 nrf52dk nodes by using thegnrc_networkingexample +nimble_rpblemodule.The entries are added/ removed from the NIB neighbor cache as expected, and the RPL tree updates when connections between parents and children close.
Edit:
Sorry, misunderstood the section as "how did I test it". Actual test instructions:
USEMODULE += nimble_rpbleto thegnrc_networkingand run on min 2. nodes (I tested with nrf52dk)ifconfig 8 add 2001:db8:1::1/64&&rpl root 1 2001:db8:1::1.ble infoandrplcommands)nib neigh)Issues/PRs references
Replaces #21586.
Part of #21574.
Related to (and might solve) #21580.
Additional Nodes
Happy to split up the PR in separate sub-PRs if that's easier, but as already mentioned it all kinda relates to each other.
See self-review for discussion points.