Support multicast with sp-sim
#7545
Conversation
Previously the simulated SP did not truly support listening in a multicast group. This commit changes the network configuration for the simulated SP so that either "simulated" or "real" is explicitly configured: - if "simulated", then most of the current behaviour is preserved: listen on a bind address but don't join a multicast group. Note: the current behaviour when `multicast_addr` is supplied is incorrect because without the interface index matching the interface that is physically connected to MGS (or faux-mgs), it doesn't work. - if "real", then require all the necessary parameters to correctly configure joining a multicast group for an interface, and set it up.
| @@ -17,15 +17,64 @@ use std::path::Path; | |||
| use std::path::PathBuf; | |||
| use thiserror::Error; | |||
|
|
|||
| #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] | |||
| pub enum NetworkConfig { | |||
There was a problem hiding this comment.
Tiny nit - could we drop a #[serde(rename_all = "snake_case")] on this so the TOML keys are lowercase like the others in the config files?
| [[simulated_sps.sidecar.network_config]] | ||
| [simulated_sps.sidecar.network_config.Real] |
There was a problem hiding this comment.
| [[simulated_sps.sidecar.network_config]] | |
| [simulated_sps.sidecar.network_config.Real] | |
| [[simulated_sps.gimlet.network_config]] | |
| [simulated_sps.gimlet.network_config.Real] |
| [[simulated_sps.sidecar.network_config]] | ||
| [simulated_sps.sidecar.network_config.Real] |
There was a problem hiding this comment.
| [[simulated_sps.sidecar.network_config]] | |
| [simulated_sps.sidecar.network_config.Real] | |
| [[simulated_sps.gimlet.network_config]] | |
| [simulated_sps.gimlet.network_config.Real] |
| [simulated_sps.sidecar.network_config.Real] | ||
| bind_addr = "[::1]:33300" | ||
| multicast_addr = "ff15:0:1de::0" | ||
| multicast_interface = "net0" |
There was a problem hiding this comment.
It seems quite unfortunate that this file no longer works without replacing net0 / net1 with real interface names on whatever system this is being run. I'm not sure what to suggest; if we have to have this, we at least need to update the wicket README to tell people to edit these values before running sp-sim (but to what?).
Could we make multicast_interface optional and continue to pass index 0 to join_multicast_v6() if it isn't provided? Or if that is nonsensical, can we change this config to use simulated instead of real?
| } | ||
|
|
||
| let interface_index = | ||
| if_nametoindex(multicast_interface.as_str())?; |
There was a problem hiding this comment.
Can we add context with the interface name we tried to look up to this error? As written if the interface name doesn't exist, I get
sp-sim: ENODEV: No such device
| sock.join_multicast_v6(&multicast_addr, interface_index) | ||
| .with_context(|| { | ||
| format!( | ||
| "failed to join multicast group {} for index {}", |
There was a problem hiding this comment.
Maybe also include the interface name here?
Previously the simulated SP did not truly support listening in a multicast group. This commit changes the network configuration for the simulated SP so that either "simulated" or "real" is explicitly configured:
if "simulated", then most of the current behaviour is preserved: listen on a bind address but don't join a multicast group. Note: the current behaviour when
multicast_addris supplied is incorrect because without the interface index matching the interface that is physically connected to MGS (or faux-mgs), it doesn't work.if "real", then require all the necessary parameters to correctly configure joining a multicast group for an interface, and set it up.