diff --git a/openhcl/underhill_core/src/emuplat/netvsp.rs b/openhcl/underhill_core/src/emuplat/netvsp.rs index 8fc7577891..4f64d60175 100644 --- a/openhcl/underhill_core/src/emuplat/netvsp.rs +++ b/openhcl/underhill_core/src/emuplat/netvsp.rs @@ -506,52 +506,63 @@ impl HclNetworkVFManagerWorker { /// /// On return, the worker will no longer treat the VF as offered, regardless /// of whether notification or revoke operations encountered errors. - async fn try_notify_guest_and_revoke_vtl0_vf(&mut self, bus_control: &Vtl0Bus) { + /// + /// If the VTL2 device is not `Present`, skip HWC calls to move the MAC filter. + /// + /// If no vPCI bus control is available (neither `Present` nor `HiddenPresent`), then VTL0 VF revoke is skipped. + async fn try_notify_guest_and_revoke_vtl0_vf( + &mut self, + bus_control: &Vtl0Bus, + vtl2_device_state: &Vtl2DeviceState, + ) { if !self.guest_state.is_offered_to_guest().await { return; } + let vtl2_present = matches!(vtl2_device_state, Vtl2DeviceState::Present); let vtl2_vfid = vtl2_vfid_from_bus_control(&self.vtl2_bus_control); // Make removal request a no-op by setting offered to false. The actual removal will be done at the end of this // method. *self.guest_state.offered_to_guest.lock().await = false; // Give the network stack a chance to prepare for the removal. - if let Err(err) = self + let guest_notification_result = self .send_vf_state_change_notifications() .instrument(tracing::info_span!( "sending VTL0 VF removal notice", vtl2_vfid, vtl0_bus = %bus_control)) - .await - { + .await; + if let Err(err) = &guest_notification_result { tracing::error!( vtl2_vfid, err = err.as_ref() as &dyn std::error::Error, "Notify VTL0 VF removal" ); - // Force data path to VTL2 on error. - if let Err(err) = - futures::future::join_all(self.endpoint_controls.iter_mut().map(async |control| { - let endpoint = control - .disconnect() - .await - .context("failed to disconnect endpoint")?; - if let Some(endpoint) = endpoint { - if let Err(err) = endpoint.set_data_path_to_guest_vf(false).await { - tracing::error!( - vtl2_vfid, - err = err.as_ref() as &dyn std::error::Error, - "Failed to force data path to synthetic" - ); + if vtl2_present { + // Force data path to VTL2 on error. + if let Err(err) = futures::future::join_all(self.endpoint_controls.iter_mut().map( + async |control| { + let endpoint = control + .disconnect() + .await + .context("failed to disconnect endpoint")?; + if let Some(endpoint) = endpoint { + if let Err(err) = endpoint.set_data_path_to_guest_vf(false).await { + tracing::error!( + vtl2_vfid, + err = err.as_ref() as &dyn std::error::Error, + "Failed to force data path to synthetic" + ); + } + control + .connect(endpoint) + .context("failed to reconnect endpoint")?; } - control - .connect(endpoint) - .context("failed to reconnect endpoint")?; - } - Ok::<(), anyhow::Error>(()) - })) + Ok::<(), anyhow::Error>(()) + }, + )) .instrument(tracing::info_span!( "forcing datapath to synthetic", vtl2_vfid @@ -559,34 +570,50 @@ impl HclNetworkVFManagerWorker { .await .into_iter() .collect::, _>>() - { - tracing::error!( - vtl2_vfid, - err = err.as_ref() as &dyn std::error::Error, - "Failed forcing endpoint to switch data path" - ); + { + tracing::error!( + vtl2_vfid, + err = err.as_ref() as &dyn std::error::Error, + "Failed forcing endpoint to switch data path" + ); + } } - // Explicitly update save state mac filter settings in case of errors. + } + // Explicitly update save state mac filter settings in case of errors or + // if VTL2 is no longer present. + if !vtl2_present || guest_notification_result.is_err() { for direction_to_vtl0 in &mut *self.save_state.direction_to_vtl0.lock() { *direction_to_vtl0 = Some(false); } } - if let Err(err) = { - let vpci_bus_control = if let Vtl0Bus::Present(current_bus) = bus_control { - current_bus - } else { - let Vtl0Bus::Present(current_bus) = &self.vtl0_bus_control else { - unreachable!(); - }; - current_bus - }; + // Pick a `Present` vPCI bus control to revoke against: try the caller-supplied + // bus first, then fall back to the worker's current bus. + let vpci_bus_control = match bus_control { + Vtl0Bus::Present(current_bus) | Vtl0Bus::HiddenPresent(current_bus) => { + Some(current_bus) + } + _ => match &self.vtl0_bus_control { + Vtl0Bus::Present(current_bus) | Vtl0Bus::HiddenPresent(current_bus) => { + Some(current_bus) + } + _ => None, + }, + }; - self.revoke_vtl0_vf(vpci_bus_control).await - } { - tracing::error!( + if let Some(vpci_bus_control) = vpci_bus_control { + if let Err(err) = self.revoke_vtl0_vf(vpci_bus_control).await { + tracing::error!( + vtl2_vfid, + vtl0_bus = %self.vtl0_bus_control, + revoke_vtl0_vfid = vfid_from_guid(&vpci_bus_control.instance_id()), + err = err.as_ref() as &dyn std::error::Error, + "Failed to revoke VTL0 VF" + ); + } + } else { + tracing::info!( vtl2_vfid, - err = err.as_ref() as &dyn std::error::Error, - "Failed to revoke VTL0 VF" + "VTL0 VF offered, but vPCI bus is either not present or hidden; skipping revoke" ); } } @@ -654,8 +681,16 @@ impl HclNetworkVFManagerWorker { /// Assumes the worker is not in shutdown. /// On return, `guest_state.offered_to_guest` is true only if the offer /// RPC succeeds; otherwise the worker state is left unchanged. - async fn add_vtl0_vf(&mut self) { + async fn add_vtl0_vf(&mut self, vtl2_device_state: &Vtl2DeviceState) { let vtl2_vfid = vtl2_vfid_from_bus_control(&self.vtl2_bus_control); + if !matches!(vtl2_device_state, Vtl2DeviceState::Present) { + tracing::info!( + vtl2_vfid, + vtl2_device_state = ?vtl2_device_state, + "VTL2 device not present; skipping VTL0 VF offer" + ); + return; + } if !self.guest_state.is_offered_to_guest().await && self.guest_state.vtl0_vfid().await.is_some() { @@ -713,12 +748,13 @@ impl HclNetworkVFManagerWorker { let old_bus_control = std::mem::replace(&mut self.vtl0_bus_control, Vtl0Bus::HiddenNotPresent); if matches!(old_bus_control, Vtl0Bus::Present(_)) { - if matches!(vtl2_device_state, Vtl2DeviceState::Present) { - *self.guest_state.vtl0_vfid.lock().await = - vtl0_vfid_from_bus_control(&self.vtl0_bus_control); - self.try_notify_guest_and_revoke_vtl0_vf(&old_bus_control) - .await; - } + *self.guest_state.vtl0_vfid.lock().await = + vtl0_vfid_from_bus_control(&self.vtl0_bus_control); + self.try_notify_guest_and_revoke_vtl0_vf( + &old_bus_control, + vtl2_device_state, + ) + .await; let Vtl0Bus::Present(bus_control) = old_bus_control else { unreachable!(); }; @@ -808,37 +844,59 @@ impl HclNetworkVFManagerWorker { Vtl0Bus::Present(_) | Vtl0Bus::HiddenPresent(_) ); assert!(is_present != bus_control.is_some()); + let is_offered_to_guest = self.guest_state.is_offered_to_guest().await; tracing::info!( vtl2_vfid = vtl2_vfid_from_bus_control(&self.vtl2_bus_control), - vtl0_vfid = vtl0_vfid_from_bus_control(&self.vtl0_bus_control), + vtl0_bus = %self.vtl0_bus_control, present = bus_control.is_some(), + vtl2_device_state = ?vtl2_device_state, + is_offered_to_guest, "VTL0 VF device change" ); - if matches!(&self.vtl0_bus_control, Vtl0Bus::HiddenNotPresent) { - self.vtl0_bus_control = Vtl0Bus::HiddenPresent(bus_control.unwrap()) - } else if matches!(&self.vtl0_bus_control, Vtl0Bus::HiddenPresent(_)) { - self.vtl0_bus_control = Vtl0Bus::HiddenNotPresent; - } else if matches!(vtl2_device_state, Vtl2DeviceState::Present) { - let bus_control = bus_control - .map(Vtl0Bus::Present) - .unwrap_or(Vtl0Bus::NotPresent); - *self.guest_state.vtl0_vfid.lock().await = vtl0_vfid_from_bus_control(&bus_control); - let old_bus_control = std::mem::replace(&mut self.vtl0_bus_control, bus_control); - match self.vtl0_bus_control { - Vtl0Bus::Present(_) => self.notify_vtl0_vf_arrival(), - Vtl0Bus::NotPresent => { - self.try_notify_guest_and_revoke_vtl0_vf(&old_bus_control) + // The reaction to a VTL0 VF add/remove depends on both the current + // VTL0 bus visibility and backing VTL2 device state. + match (*vtl2_device_state, &self.vtl0_bus_control) { + // The VF is hidden from the guest (e.g. prepared for hibernate). + // Store the new backing state; it will be surfaced to the guest + // if/when it is unhidden. + (_, Vtl0Bus::HiddenNotPresent) => { + self.vtl0_bus_control = Vtl0Bus::HiddenPresent(bus_control.unwrap()); + } + (_, Vtl0Bus::HiddenPresent(_)) => { + self.vtl0_bus_control = Vtl0Bus::HiddenNotPresent; + } + // The VTL2 device is present; apply the VTL0 change immediately. + (Vtl2DeviceState::Present, _) => { + let bus_control = bus_control + .map(Vtl0Bus::Present) + .unwrap_or(Vtl0Bus::NotPresent); + *self.guest_state.vtl0_vfid.lock().await = + vtl0_vfid_from_bus_control(&bus_control); + let old_bus_control = + std::mem::replace(&mut self.vtl0_bus_control, bus_control); + match self.vtl0_bus_control { + Vtl0Bus::Present(_) => self.notify_vtl0_vf_arrival(), + Vtl0Bus::NotPresent => { + self.try_notify_guest_and_revoke_vtl0_vf( + &old_bus_control, + vtl2_device_state, + ) .await + } + _ => unreachable!(), } - _ => unreachable!(), } - } else { // When the VTL2 device is restored, the VTL0 update will be applied. - assert_eq!(*self.guest_state.offered_to_guest.lock().await, false); - assert!(self.guest_state.vtl0_vfid.lock().await.is_none()); - self.vtl0_bus_control = bus_control - .map(Vtl0Bus::Present) - .unwrap_or(Vtl0Bus::NotPresent); + ( + Vtl2DeviceState::Reconfiguring + | Vtl2DeviceState::Missing + | Vtl2DeviceState::DeviceEnumerated, + _, + ) => { + self.vtl0_bus_control = bus_control + .map(Vtl0Bus::Present) + .unwrap_or(Vtl0Bus::NotPresent); + } } }) .await @@ -1044,7 +1102,7 @@ impl HclNetworkVFManagerWorker { vtl0_vfid = vtl0_vfid_from_bus_control(&self.vtl0_bus_control), "VTL0 VF being removed as a result of VF Reconfiguration." ); - self.try_notify_guest_and_revoke_vtl0_vf(&Vtl0Bus::NotPresent) + self.try_notify_guest_and_revoke_vtl0_vf(&Vtl0Bus::NotPresent, vtl2_device_state) .await; } @@ -1168,7 +1226,7 @@ impl HclNetworkVFManagerWorker { vtl0_vfid = vtl0_vfid_from_bus_control(&self.vtl0_bus_control), "VTL0 VF being removed as a result of VTL2 VF revoke." ); - self.try_notify_guest_and_revoke_vtl0_vf(&Vtl0Bus::NotPresent) + self.try_notify_guest_and_revoke_vtl0_vf(&Vtl0Bus::NotPresent, vtl2_device_state) .await; } @@ -1346,7 +1404,7 @@ impl HclNetworkVFManagerWorker { continue; } - self.add_vtl0_vf() + self.add_vtl0_vf(&vtl2_device_state) .instrument(tracing::info_span!("add vtl0 vf", vtl2_vfid)) .await; }