Netvsp remove VF_ASSOCIATION_COMPLETION no-op packet #1494
Conversation
vm/devices/net/netvsp/src/lib.rs
Outdated
| @@ -1971,7 +1972,11 @@ fn parse_packet<'a, T: RingMem>( | |||
| IncomingPacket::Data(data) => data, | |||
| IncomingPacket::Completion(completion) => { | |||
| let data = if completion.transaction_id() == VF_ASSOCIATION_TRANSACTION_ID { | |||
There was a problem hiding this comment.
This block seems pretty complicated now and has a lot of rightward drift - is there any reasonable way to make this a subfunction, that maybe wouldn't have early returns? Something like parse_incoming_packet(...) -> Result<PacketData, Err>?
Tested manually on my lab machine, but let me look into a unit test. Update: Almost every netvsp unit test sends the VF Association Completion Message. (With the expectation that it's a no-op). |
|
Should there be a new test that does not send the packet to match the behavior this bug fixes? |
| // that it is safe to expose the AccelNet device to the guest. | ||
| // Some Linux guests lack support for inline completions; they do not send the packet. | ||
| // Since the completion packet is not sent by all guests, it gets ignored. | ||
| Err(PacketError::IgnoreVfAssociationCompletion) |
There was a problem hiding this comment.
I do not think it's a good idea to report expected behavior as an error that you then have to filter back out.
|
What problem are you trying to solve here? |
Ideally every Guest would send a VF_ASSOCIATION_COMPLETION packet and netvsp could use that to know when to explose the AccelNet device to the client. Some Linux Guests do not support inline completions, they don't send the completion packet. Since not all guests send the packet, the code is a no-op.