virtio-net: Replace AsSliceU8 trait with zerocopy::AsBytes.

[joannejchen]

Changed VirtioNetHdr to derive from AsBytes instead of AsSliceU8 and changed related functions as necessary.

[stlankes]

Naive question. Are we able to remove https://github.com/hermitcore/libhermit-rs/blob/master/src/drivers/virtio/virtqueue/mod.rs#L533?

[mkroening]

Naive question. Are we able to remove https://github.com/hermitcore/libhermit-rs/blob/master/src/drivers/virtio/virtqueue/mod.rs#L533?

That's the goal. We might not be able to remove it completely due to size vs size minus padding differences, but at the core, yes. I'll have to look at the details regarding generics with zerocopy, though.

[mkroening]

Thanks for your PR! :)

I left some minor comments on your changes.

Regarding the bigger issue with FUSE: I'm currently on vacation without laptop, which makes it hard to me to get much insight quickly.
I have looked around a bit without testing anything but there are a few possible paths forward:

1. We split that issue off and merge this PR without the removing AsSliceU8 from FUSE. In this case, we should still aim to remove any uses outside of FUSE and move the trait to FUSE. Not sure if that is possible though.

2. You move forward with exploring possibilities of using zerocopy for FUSE. I won't be able to play around with things and give definitive judgements of the situation until the fifth of March, though, when I have access to my setup again and don't have to write this from my phone. Not sure if this fits with your deadlines, but we might be able to find a solution even before that. The fundamental problem is that zerocopy has to make sure our struct does not have any padding. Padding is not initialized and would not be valid for conversion to initialized bytes. Verifying this on a generic struct is difficult. Somehow, the generic field has to be constrained, so it does not contain any padding itself (easy, requiring the field to be AsBytes does this). Additionally, the field may not introduce any new padding to the outer struct by having a bigger alignment requirement than the previous fields (hard). I am not sure if these assessments make sense and are correct though, since I could not test them. Here are a few possible approaches for going forward:

   a. Try v0.7.0-alpha.3 and see if there is better support for generics.

   b. Implement support for this in zerocopy, if it does not exist in their main branch. This is probably out of scope for you, I would guess.

   c. Implement AsBytes manually. This is strongly discouraged by zerocopy, but still possible. We would have to dive into the details of zerocopy and all necessary conditions for implementing the trait as well as find a mechanism to correctly formulate any requirements to the inner type parameter. (const generics? Another manual marker trait?)

   d. Remove generics from the structs. While possible, this is probably not what we want.

Let me know which path you prefer. It's also possible to first do 1 and then do 2, to have something merged sooner than later.

[joannejchen]

Thanks for the extra information!

I think I would prefer to have 1 merged first (just for the school assignment) and I can do further research about how to approach 2 in the meantime (or someone else can also jump in if they have more knowledge regarding zerocopy).

My first commit in my branch (after addressing your comments) should be sufficient for 1 - VirtioNetHdr is the only other struct that implemented AsSliceU8 aside from FUSE, and I don't think we're able to yet move the trait entirely to FUSE because of other areas in the code that depend on it to use Cmd and Rsp.

I created another commit in my branch (though let me know if I should make this an entirely different pull request) trying to experiment some of the approaches for 2:
2a alone does not provide any additional support for generics, but it seems needed to be used with 2c because it has an additional trait "FromZeroes" that needs to be implemented in order for the structs to use the AsBytes function as_bytes_mut. So far, I have tried to naively manually implement the traits for Cmd and Rsp, but running into a panic in the CI at the moment: "panicked at 'called Result::unwrap() on an Err value: Write is to large for BufferToken!', src/drivers/fs/virtio_fs.rs:166:69".

2b does seem a little out of the scope for me.

As you stated, 2d does seem like it's probably not something we want. After naively testing it out, the issue seems to be that we are trying to use two non-auto traits when only one non-auto trait can be used and any additional traits should be auto. One way around this to remove generics might also be to create a new trait like "trait CmdTrait: FuseIn + core::fmt::Debug {}" and then use that, but this seems to just lead to a rabbit hole of other hacky workarounds compared to just using generics.

[mkroening]

Okay then let's merge the first commit and explore different approaches for FUSE later. Please remove the second commit from this PR and make sure there are no unrelated reformats in the Cargo.toml.

Then we can merge this PR.

Regarding formality: I would suggest to have this PR not close the issue, as it is not fully resolved. Does this work for you or does your assignment require successfully closing an issue?

[mkroening]

You can either open another WIP PR with your experimental approach or not, whatever you prefer. The issues are definitely interesting and I will have a look myself on this once I am back.

[joannejchen]

Ok, sounds good! I removed the experimental commit.

I don't need to have the issue entirely closed, so you can merge while leaving the issue open -- I removed the "fixes #" in my commit message, which I think should avoid my PR from auto-closing the issue.

[mkroening]

Looks good to me! 👍

bors r+

Thanks a lot for your efforts! :)

[bors]

Build succeeded:

• Build
• Check
• Check docs
• ci/gitlab/git.rwth-aachen.de
• Clippy
• Format
• Run RustyHermit (aarch64)
• Run RustyHermit (x86_64)
• Test