netvsp: handle RNDIS packet filter OID for stopping Rx #1873
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements handling for RNDIS packet filter OID to stop receiving packets when the VMBus channel is being closed. This addresses an issue where Linux netvsc cannot successfully close and reopen channels under heavy traffic, leading to "Ring buffer not empty after closing rndis" errors.
Key changes:
- Added
stop_rxstate tracking throughout the network stack - Implemented OID_GEN_CURRENT_PACKET_FILTER handling to set the stop_rx flag
- Modified packet reception logic to respect the stop_rx flag
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| saved_state.rs | Added stop_rx field to ReadyPrimary struct for state persistence |
| rndisprot.rs | Added OID_REQUEST constant and RndisPacketFilterOidValue type for RNDIS protocol support |
| lib.rs | Implemented stop_rx state management across channels, OID handling, and packet processing logic |
benhillis
added
backport_2411
| tracing::debug!(?oid, "oid set"); | ||
|
|
||
| let mut restart_endpoint = false; | ||
| let mut stop_rx = false; | ||
| match oid { | ||
| rndisprot::Oid::OID_GEN_CURRENT_PACKET_FILTER => { |
There was a problem hiding this comment.
Should we support reading the filter value back?
erfrimod
force-pushed
the
erfrimod/netvsp_rndis_packet_filter
branch
from
f6d5eb4 to
61c4cb6
Compare
…pdates subchannel packet filters.
vm/devices/net/netvsp/src/lib.rs
Outdated
| data: ProcessingData::new(), | ||
| }) | ||
| // Get the packet filter of the primary channel. | ||
| self.coordinator.state_mut().unwrap().workers[0] |
There was a problem hiding this comment.
Instead of getting this out of the primary channel, can we have the primary channel report the new packet filter to the coordinator and so instead get it out of the coordinator?
There was a problem hiding this comment.
There is a danger of losing the value since we could send the coordinator message and then get stopped / saved before actually processing it.
There was a problem hiding this comment.
I've removed the filter from the primary channel. I store it in the worker[0] netchannel. And when readystate is saved, that's where it gets read from. And when the coordinator updates the subchannels, it also reads from worker[0].
| UpdateGuestVfState, | ||
| guest_vf_state: bool, | ||
| /// Update the receive filter for all channels. | ||
| filter_state: bool, |
There was a problem hiding this comment.
i.e., maybe this can be Option<u32> so that we can store the packet filter in the coordinator itself.
| guest_vf_state: guest_vf, | ||
| filter_state: packet_filter, | ||
| })); | ||
| } else if let Some(CoordinatorMessage::Restart) = self.restart { |
There was a problem hiding this comment.
I guess this would have to be updated, too, to store the new packet filter.
…ate holds filter value.
vm/devices/net/netvsp/src/lib.rs
Outdated
| self.stop_workers().await; | ||
| let worker_0_packet_filter = | ||
| self.workers[0].state().unwrap().channel.packet_filter; | ||
| self.workers.iter_mut().for_each(|worker| { |
There was a problem hiding this comment.
nit: self.workers.iter_mut().skip(1).for_each(|worker| {
Linux netvsc sends an OID to stop receiving packets on vmbus channel close. Example scenarios: hibernation and MTU change. Prior to opening a new channel and processing the packets, netvsc checks that there are no pending packets. If there are, netvsc logs and error and is unable to recover. We observe the error: `hv_netvsc eth0: Ring buffer not empty after closing rndis` in the guest syslog. Modifying netvsp to handle the OID and stop processing RX traffic. This will allow for netvsc to successfully close and re-open the vmbus channel, even under heavy incoming traffic. --------- Co-authored-by: Sunil Muthuswamy <[email protected]>
Linux netvsc sends an OID to stop receiving packets on vmbus channel close. Example scenarios: hibernation and MTU change. Prior to opening a new channel and processing the packets, netvsc checks that there are no pending packets. If there are, netvsc logs and error and is unable to recover. We observe the error: `hv_netvsc eth0: Ring buffer not empty after closing rndis` in the guest syslog. Modifying netvsp to handle the OID and stop processing RX traffic. This will allow for netvsc to successfully close and re-open the vmbus channel, even under heavy incoming traffic. --------- Co-authored-by: Sunil Muthuswamy <[email protected]>
Linux netvsc sends an OID to stop receiving packets on vmbus channel close. Example scenarios: hibernation and MTU change. Prior to opening a new channel and processing the packets, netvsc checks that there are no pending packets. If there are, netvsc logs and error and is unable to recover. We observe the error:
hv_netvsc eth0: Ring buffer not empty after closing rndisin the guest syslog.Modifying netvsp to handle the OID and stop processing RX traffic. This will allow for netvsc to successfully close and re-open the vmbus channel, even under heavy incoming traffic.