Add support for IPv6 gateway, and use it to make examples IPv6-capable #207
Conversation
examples/ping.rs
Outdated
|
|
||
| macro_rules! get_icmp_pong { | ||
| ( $repr_type:ident, $repr:expr, $payload:expr, $waiting_queue:expr, $remote_addr:expr, | ||
| $timestamp:expr, $received:expr) => {{ |
examples/ping.rs
Outdated
| Ipv4Address, Icmpv4Repr, Icmpv4Packet}; | ||
| use smoltcp::iface::{NeighborCache, EthernetInterfaceBuilder}; | ||
| use smoltcp::socket::{SocketSet, IcmpSocket, IcmpSocketBuffer, IcmpPacketMetadata, IcmpEndpoint}; | ||
| use std::collections::HashMap; | ||
| use byteorder::{ByteOrder, NetworkEndian}; | ||
|
|
||
| macro_rules! send_icmp_ping { | ||
| ( $repr_type:ident, $packet_type:ident, $ident:expr, $seq_no:expr, $echo_payload:expr, | ||
| $socket:expr, $remote_addr:expr, $device_caps:expr ) => {{ |
There was a problem hiding this comment.
device_caps isn't used and what is the purpose of the double {?
There was a problem hiding this comment.
I forgot to remove device_caps after an intermediate version.
The first { is part of the macro_rules! syntax, the second one means it's a block.
There was a problem hiding this comment.
I forgot to remove device_caps after an intermediate version.
The first { is part of the macro_rules! syntax, the second one means it's a block.
There was a problem hiding this comment.
Ah right, because the body is essentially a rvalue.
| @@ -65,6 +65,8 @@ struct InterfaceInner<'b, 'c> { | |||
| ip_addrs: ManagedSlice<'c, IpCidr>, | |||
| #[cfg(feature = "proto-ipv4")] | |||
| ipv4_gateway: Option<Ipv4Address>, | |||
| #[cfg(feature = "proto-ipv6")] | |||
| ipv6_gateway: Option<Ipv6Address>, | |||
There was a problem hiding this comment.
This works for now, but we really need to add support for NDISC Router Advertisements and Solicitations. In IPv6 this should actually be default_routes or something like that, where some could have been statically assigned and some could have been dynamically acquired via a Router Advertisement.
There was a problem hiding this comment.
Done, I added support for routes, to both IPv4 and IPv6.
src/iface/ethernet.rs
Outdated
| neighbor_cache: NeighborCache<'b>, | ||
| ethernet_addr: EthernetAddress, | ||
| ip_addrs: ManagedSlice<'c, IpCidr>, | ||
| #[cfg(feature = "proto-ipv4")] | ||
| ipv4_gateway: Option<Ipv4Address>, |
There was a problem hiding this comment.
I really like the idea of working on routing, but anything having to do with routes should probably be a separate PR.
|
I could remove the commits with routes, and put them in a new PR after
you merged this one. Would that be ok?
|
Yeah, I think that would probably be better. |
|
done
|
examples/httpclient.rs
Outdated
| let ip_addrs = [IpCidr::new(IpAddress::v4(192, 168, 69, 1), 24)]; | ||
| let ip_addrs = [IpCidr::new(IpAddress::v4(192, 168, 69, 1), 24), | ||
| IpCidr::new(IpAddress::v6(0xfd80, 0, 0, 0, 0, 0, 0, 1), 64), | ||
| IpCidr::new(IpAddress::v6(0xfe80, 0, 0, 0, 0, 0, 0, 1), 64)]; |
There was a problem hiding this comment.
This line is repeated.
There was a problem hiding this comment.
No, one is link-local (fe80::), the other is not (fd80::)
examples/ping.rs
Outdated
| let src_ipv6 = IpAddress::v6(0xfd80, 0, 0, 0, 0, 0, 0, 1); | ||
| let ip_addrs = [IpCidr::new(IpAddress::v4(192, 168, 69, 1), 24), | ||
| IpCidr::new(src_ipv6, 64), | ||
| IpCidr::new(IpAddress::v6(0xfe80, 0, 0, 0, 0, 0, 0, 1), 64)]; |
There was a problem hiding this comment.
This is the same address as src_ipv6 right?
README.md
Outdated
| @@ -190,13 +190,19 @@ a specific user: | |||
| sudo ip tuntap add name tap0 mode tap user $USER | |||
| sudo ip link set tap0 up | |||
| sudo ip addr add 192.168.69.100/24 dev tap0 | |||
| sudo ip -6 addr add fe80::100/64 dev tap0 | |||
| sudo ip -6 addr add fd80::100/64 dev tap0 | |||
There was a problem hiding this comment.
Address listed twice (for real this time 😄)
There was a problem hiding this comment.
Ugh I'm wrong again! The fe -> fd keeps confusing me lol
There was a problem hiding this comment.
Do you think I should use a different prefix to make it clearer? (eg. fdaa::/64)
There was a problem hiding this comment.
Yeah, that would be great! Could you also squash your commits?
README.md
Outdated
| sudo ip -6 addr add fe80::100/64 dev tap0 | ||
| sudo ip -6 addr add fd80::100/64 dev tap0 | ||
| sudo ip -6 route fe80::/64 dev tap0 | ||
| sudo ip -6 route fd80::/64 dev tap0 |
There was a problem hiding this comment.
Same. This command also fails for me. Did you mean ip -6 route add default via ...?
There was a problem hiding this comment.
Oh, I forgot the add after route
examples/httpclient.rs
Outdated
| let ip_addrs = [IpCidr::new(IpAddress::v4(192, 168, 69, 1), 24)]; | ||
| let ip_addrs = [IpCidr::new(IpAddress::v4(192, 168, 69, 1), 24), | ||
| IpCidr::new(IpAddress::v6(0xfd80, 0, 0, 0, 0, 0, 0, 1), 64), | ||
| IpCidr::new(IpAddress::v6(0xfe80, 0, 0, 0, 0, 0, 0, 1), 64)]; |
|
Looks good once the |
|
@m-labs-homu r+ |
|
📌 Commit c08b8b5 has been approved by |
|
@dlrobertson Thank you for reviewing this! I am very happy with how the code turned out, and even more so that I have been freed to do something else :D |
@progval thanks for sticking with it! Sorry for criticizing you for duplicating lines so many times 😆 Also could you open up an RFC issue of sorts before you create a PR for multiple routes. I think some time spent discussing a design could be useful. |
|
☀️ Test successful - status-travis |
Also contains a fix to #205 (
Protocol::Icmp->Protocol::Icmpv6).Both examples were tested on my computer with both IP versions.
Instructions in the README should work, but they are a summary of a lot of trial-and-error to configure my computer so some command(s) might be missing.