smoltcp icon indicating copy to clipboard operation
smoltcp copied to clipboard

Add the up/down Device and Interface methods

Open dlrobertson opened this issue 4 years ago • 4 comments
trafficstars

Add up/down methods to Device and Interface for placing the underlying device in a up or down state. This interface will also be handy when adding support for MLDv2.

dlrobertson avatar Jun 17 '21 23:06 dlrobertson

Thanks for the PR! What's the intended use case?

It seems the new functionality is not yet used for anything, method calls are threaded through and exposed to the user. You can call custom methods on your particular Device with iface.device_mut().up() without needing to add them to the Device trait.

IMO we should add methods to Device when smoltcp itself needs them. Does MLDv2 require up/down?

Dirbaio avatar Jun 27 '21 07:06 Dirbaio

Does MLDv2 require up/down?

Not explicitly, but there will be requests that we might want to send when we start/stop.

It seems the new functionality is not yet used for anything, method calls are threaded through and exposed to the user. You can call custom methods on your particular Device with iface.device_mut().up() without needing to add them to the Device trait.

IMO we should add methods to Device when smoltcp itself needs them. Does MLDv2 require up/down?

Yeah, currently this isn't really much different than iface.device_mut.up(). You're 100% right about that. This doesn't have to complicate any existing device implementations, but is something that users would likely have to hand implement. I can't think of a use case where you would not want to implement a up/down method for your device.

dlrobertson avatar Jun 27 '21 23:06 dlrobertson

Thinking about it, there are 2 related but different things we want:

  • Actively set the device in an up/down state.
  • Ask whether the device is up/down.

A device may come up/down "on its own". For example, an Ethernet being plugged/unplugged, WiFi connecting/disconnecting.... Or it may come up/down due to us manually setting it up/down. I imagine an Ethernet driver may report as "up" when we've set it as up AND there's a cable plugged in.

so perhaps all smoltcp needs is the second? Just an fn is_up(&self) -> bool in the trait. This would be useful for DHCP, MLD, IGMP and others to know when the network comes up, to react accordingly.

The first can be done with device-specific methods, calling them through iface.device_mut(). Smoltcp will never need to call them, just the user, so it makes sense for them not to be in the trait.

Dirbaio avatar Sep 08 '21 21:09 Dirbaio

We discussed yesterday in the chat about the DHCP always needing 5s. It turned out to be that smoltcp tried to do DHCP DISCOVER while the link was down, and the 5s came from the retry cooldown.

Something like this would give smoltcp opportunity to act accordingly to the link state, as @Dirbaio pointed out. As it is not the user need to track the up/down state of the link and reset DHCP when the link goes down -> up.

Not sure if it's smoltcp's job to track this though, I'm not that familiar with its design.

korken89 avatar Oct 21 '21 08:10 korken89