[RFC] Net device proposal #33
Conversation
Use [patch] sections in the workspace Cargo.toml file to redirect dependencies to custom repos/branches that are not upstream yet. Also switched to abort on panic for both debug and release builds. Enabled lto and codegen-unit = 1 for release builds (and they take longer now). Signed-off-by: Alexandru Agache <[email protected]>
The virtio device implementations are going to be added to a local devices crate, so renaming the module with the same name within vmm to serial (because there's only a serial device there for the time being). Going foward, we should move the serial console to the devices crate as well. Signed-off-by: Alexandru Agache <[email protected]>
Signed-off-by: Alexandru Agache <[email protected]>
Applied some changes to the vmm to support the creation of block devices. There's not cmdline support to configure them yet so there's one hardcoded block device for demo purposes for now. It will always attempt to open a file named 'disk.ext4` from the current working directory when running the VMM. Signed-off-by: Alexandru Agache <[email protected]>
Signed-off-by: Alexandru Agache <[email protected]>
Signed-off-by: Alexandru Agache <[email protected]>
| Ok(count as u32) | ||
| } | ||
|
|
||
| pub fn process_txq(&mut self) -> result::Result<(), Error> { |
There was a problem hiding this comment.
I was wandering if the TX can be done in a async fashion for performance reasons. Something like:
- do an async write on the tap without copying the packets from vm memory to the vmm;
- on the write completion notification mark the descriptor as used and signal the used queue.
Since this is tap device you shouldn't care of scatter gather.
There was a problem hiding this comment.
It's def helpful to skip as many copies as possible. We need to add some more support in various places to make that work safely & properly, but it's something we clearly want to do. If by async you also mean somehow submitting the requests to write/read data for each frame without waiting for them to complete in-between, then things become more nuanced. Overall, I imagine that doesn't provide a clear positive impact in this case, but would be interesting to explore at some point.
| // found in the THIRD-PARTY file. | ||
|
|
||
| // The following are manually copied from crosvm/firecracker. In the latter, they can be found as | ||
| // part of the `net_gen` local crate. We should figure out how to proceed going forward (i.e. |
There was a problem hiding this comment.
I think the best thing to do would be to regenerate these with bindgen and document steps to generate them.
There was a problem hiding this comment.
Yeah, this is going to be a part of the plan around adding tap support as a reusable component. For now these have been brought over here to enable the network implementation, and we'll stop using them locally as soon as that's possible.
| let queues = vec![Queue::new(args.mem.clone(), QUEUE_MAX_SIZE); 2]; | ||
| // TODO: We'll need a minimal config space to support setting an explicit MAC addr | ||
| // on the guest interface at least. We use an empty one for now. | ||
| let config_space = Vec::new(); |
There was a problem hiding this comment.
Should this reserve capacity for the MAC?
There was a problem hiding this comment.
It's not required until we actually add the support and advertise that feature.
| // found in the THIRD-PARTY file. | ||
|
|
||
| // The following are manually copied from crosvm/firecracker. In the latter, they can be found as | ||
| // part of the `net_gen` local crate. We should figure out how to proceed going forward (i.e. |
There was a problem hiding this comment.
Yeah, this is going to be a part of the plan around adding tap support as a reusable component. For now these have been brought over here to enable the network implementation, and we'll stop using them locally as soon as that's possible.
| let queues = vec![Queue::new(args.mem.clone(), QUEUE_MAX_SIZE); 2]; | ||
| // TODO: We'll need a minimal config space to support setting an explicit MAC addr | ||
| // on the guest interface at least. We use an empty one for now. | ||
| let config_space = Vec::new(); |
There was a problem hiding this comment.
It's not required until we actually add the support and advertise that feature.
| Ok(count as u32) | ||
| } | ||
|
|
||
| pub fn process_txq(&mut self) -> result::Result<(), Error> { |
There was a problem hiding this comment.
It's def helpful to skip as many copies as possible. We need to add some more support in various places to make that work safely & properly, but it's something we clearly want to do. If by async you also mean somehow submitting the requests to write/read data for each frame without waiting for them to complete in-between, then things become more nuanced. Overall, I imagine that doesn't provide a clear positive impact in this case, but would be interesting to explore at some point.
|
Closing after the creation of #49. |
This is similar to #32 (and adds on top of it) for net devices. There's a hardcoded net device that tries to open the
vmtap35iface, until things get integrated with the user interface. The tap backend implementation is the one from Firecracker, and we should add one in rust-vmm as well.