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

Add support for default route (IPv4 and IPv6) #2059

Merged
merged 6 commits into from
Mar 13, 2023

Conversation

thomasfire
Copy link
Contributor

@thomasfire thomasfire commented Mar 3, 2023

ARTIQ Pull Request

Description of Changes

Related Issue

Closes #1930
Closes #2033

Primarily cherry-picked from @mbirtwell 's #2033

Type of Changes

Type
βœ“ πŸ› Bug fix

Testing

  1. PC with two RJ-45 interfaces with Manjaro 22.03 installed, all firewalls disabled (including ip(6)tables) - further - the router
  2. One interface is connected to the office network, another is directly to the Kasli
  3. Configuration of IPv6 on the is as follows:
2: enp0s25: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 state UP qlen 1000
    inet6 2001:470:f891:1:bf12:49f8:e2f3:a057/64 scope global noprefixroute 
       valid_lft forever preferred_lft forever
    inet6 2001:470:f891:1::1/64 scope global 
       valid_lft forever preferred_lft forever
    inet6 fe80::80b1:7abb:eae6:423f/64 scope link noprefixroute 
       valid_lft forever preferred_lft forever
3: enp3s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 state UP qlen 1000
    inet6 2001:470:f891:1::2/64 scope global 
       valid_lft forever preferred_lft forever
    inet6 fe80::a057/64 scope link 
       valid_lft forever preferred_lft forever
  1. radvd.conf:
interface enp0s25
{
    MinRtrAdvInterval 3;
    MaxRtrAdvInterval 10;
    AdvSendAdvert on;
    AdvDefaultLifetime 300;
    route 2001:470:f891:2::/64 {
	AdvRouteLifetime 300;
	AdvRoutePreference high;
    };
};
  1. Kasli: artiq_mkfs -s ip6 2001:470:f891:2::42/64 -s ip 192.168.3.42/24 -s ipv4_default_route 192.168.3.0 -s ipv6_default_route fe80::a057 kasli.config

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

mbirtwell and others added 2 commits March 3, 2023 12:57
New config option "ipv4_default_route", which sets a static default route.
Support passing a CIDR as ip, e.g. ip = "192.168.1.2/24" the prefix defaults
to 0 same as before.
If ip mode is use dhcp, then get the default route from the DHCP config.
Signed-off-by: Egor Savkin <es@m-labs.hk>
ipv4_addr,
ipv6_ll_addr,
ipv6_addr,
ipv4_default_route: default_route,
Copy link
Member

@sbourdeauducq sbourdeauducq Mar 3, 2023

Choose a reason for hiding this comment

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

Inconsistent variable naming (again)

@sbourdeauducq
Copy link
Member

Testing?

@thomasfire
Copy link
Contributor Author

Testing?

Ongoing, that's why it's WIP

Copy link
Contributor

@mbirtwell mbirtwell left a comment

Choose a reason for hiding this comment

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

Thanks @thomasfire. That mostly looks good, just a couple of small comments.

};
let mut routes = [None; 1];
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be:

Suggested change
let mut routes = [None; 1];
let mut routes = [None; 2];

In case both an ipv4 and ipv6 default route is set?

Otherwise one of those unwraps below might panic.

Comment on lines 167 to 166
if let Some(default_routev6) = net_addresses.ipv6_default_route {
interface.routes_mut().add_default_ipv6_route(default_routev6).unwrap();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should probably be setting this even if ipv4 is set to use DHCP. There's no DHCPv6 support and I think that would be separate. (There doesn't seem to be DHCPv6 support in smoltcp yet).

Comment on lines 290 to 300
pub fn set_ipv6_address(&self, addr: &Ipv6Cidr) {
self.network.borrow_mut().update_ipv6_addr(addr)
}

pub fn set_ipv6_default_route(&self, addr: Ipv6Address) -> Result<Option<Route>, Error> {
Ok(self.network.borrow_mut().routes_mut().add_default_ipv6_route(addr)?)
}

pub fn remove_ipv6_default_route(&self) -> Option<Route> {
self.network.borrow_mut().routes_mut().remove_default_ipv6_route()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

These all appear to be unused. Personally I'd avoid adding functions until they were needed.

thomasfire and others added 3 commits March 8, 2023 17:10
Signed-off-by: Egor Savkin <es@m-labs.hk>
# Conflicts:
#	doc/manual/installing.rst
Need to set the prefix length of unspecified IP addresses to be 32 so that
they don't appear to be in the same subnet as valid IPs
@thomasfire thomasfire changed the title WIP: add default route Add support for default route (IPv4 and IPv6) Mar 8, 2023
Check that you can ping the device. If ping fails, check that the Ethernet link LED is ON - on Kasli, it is the LED next to the SFP0 connector. As a next step, look at the messages emitted on the UART during boot. Use a program such as flterm or PuTTY to connect to the device's serial port at 115200bps 8-N-1 and reboot the device. On Kasli, the serial port is on FTDI channel 2 with v1.1 hardware (with channel 0 being JTAG) and on FTDI channel 1 with v1.0 hardware. Note that on Windows you might need to install the `FTDI drivers <https://ftdichip.com/drivers/>`_ first.

If you want to use IPv6, the device also has a link-local address that corresponds to its EUI-64, and an additional arbitrary IPv6 address can be defined by using the ``ip6`` configuration key. All IPv4 and IPv6 addresses can be used at the same time.

If IPv6 ping fails, check that the IPv6 default route is set up correctly. In general, the default route should point to the global unicast address of the next-hop gateway/router.
Copy link
Member

Choose a reason for hiding this comment

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

Why is this specific to IPv6?

Copy link
Member

@sbourdeauducq sbourdeauducq Mar 11, 2023

Choose a reason for hiding this comment

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

Also this sounds like a general network issue - the manual does mention checking the Ethernet LED and where it is if ping fails because this is specific to our device, but the ARTIQ manual is not really the place for general network setup advice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed this from manual

$ artiq_flash -t [board] -V [variant] -f flash_storage.img storage start

For Kasli devices, flashing a MAC address is not necessary as they can obtain it from their EEPROM.
If you only want to access the kasli from the same subnet you might be able to
Copy link
Member

Choose a reason for hiding this comment

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

Kasli or "core device" since this is not even specific to Kasli
IP
"Might be able to"? You don't seem so sure. Is that code so buggy?

If the ip config field is not set, or set to "use_dhcp" then the device will attempt to obtain an IP address using
DHCP. If a static IP address is wanted, install OpenOCD as before, and flash the IP (and, if necessary, MAC) addresses
directly: ::
If the ip config field is not set, or set to "use_dhcp" then the device will
Copy link
Member

Choose a reason for hiding this comment

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

ip with backticks

Signed-off-by: Egor Savkin <es@m-labs.hk>
@sbourdeauducq sbourdeauducq merged commit 1ca09b9 into m-labs:master Mar 13, 2023
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.

Cannot access coredevice across subnets after DHCP feature merge
3 participants