smoltcp
smoltcp copied to clipboard
(Incomplete) Implement `join_multicast_group()` for IPv6
This is a preliminary and incomplete start to adding support for IPv6 multicast groups. This PR only adds joining groups via sending an initial MLDv2 report packet; it doesn't support resending the change report, leaving groups, responding to queries, etc. I wanted to open this PR first both to keep the review burden lighter and to get any feedback on the implementation that could affect implementing the remaining features.
On that last point, there are a few things in this PR that I'm not thrilled with and would be happy for any suggestions. I think all of them derive from difficulty crafting a fully-formed IpPacket to pass to dispatch_ip(). For the MLDv2 report, the specific difficulties and paths I took to deal with them were:
- We are required to set the router alert option in an ipv6 hop-by-hop header, which requires us to be able to craft a packet with a hop-by-hop header in the first place! I added a new case to
IpPacket(IpPacket::HopByHopIcmpv6), but that doesn't feel quite right. Ipv6HopByHopRepr'soptionsfield requires a borrowed slice. I couldn't see an easy way to get from a pair ofIpv6OptionReprs to a borrowed slice, so I punted on this and definedIpv6HopByHopRepr::MLDV2_ROUTER_ALERTas a constant.- Similarly,
MldRepr::Reportrequires a borrowed slice for its data (the address records); I couldn't see an easy way to get from anMldAddressRecordReprto that borrowed data, so I added anMldRepr::ReportRecordReprscase which holds a slice ofMldAddressRecordRepr. This definitely seems wrong. It also might be still have similar problems as the borrowed byte slice when we need to craft packets with multiple address records in them.
In addition to the unit tests present, I've tested this locally on a microcontroller running smoltcp, and I've confirmed I can communicate with it via a multicast address through the switch it's connected to, so I'm reasonably confident that what's here is correct (albeit incomplete and a little hacky, as noted above).
re the difficulties: Yes, IpPacket is not "scaling" well, this happens with every new feature added. Your solutions look reasonable. (I've been thinking for a while about refactoring it so that all the packet emitters (sockets, and the IGMP code) gets access to the buffer and write their packets there directly, but that has other downsides...)
From a quick look, the PR looks OK. Could you rebase+squash?
friendly ping @jgallagher :)