tsarevvv/nmstate
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

sriov: Fix SR-IOV enabling and use in single desire state

Browse Source 
Currently, we isolate SR-IOV PF changes out for `enable-and-use` use
case, but the second desire state contains no SR-IOV PF changes which
cause the verification stage does not wait on VF shows up again.

Instead of isolating PF changes out to apply first, we only clone PF
changes to apply first when VF count changed and has missing ethernet in
desire state. This means we will apply twice SR-IOV PF changes to ensure
the final apply also wait on VF shows up.

Unit test cases included.

Current integration test case `test_wait_sriov_vf_been_created` and
`test_enable_sriov_and_use_future_vf` have covered the use case.

Signed-off-by: Gris Ge <fge@redhat.com>
This commit is contained in:
Gris Ge 2023-02-13 14:14:35 +08:00
parent fb7ace9db3
commit 8f6801800d
10 changed files with 258 additions and 170 deletions

12
rust/src/lib/iface.rs

@ -238,13 +238,13 @@ impl<'de> Deserialize<'de> for UnknownInterface {
D: Deserializer<'de>,
{
let mut ret = UnknownInterface::default();
let v = serde_json::Value::deserialize(deserializer)?;
let mut v = serde_json::Map::deserialize(deserializer)?;
let mut base_value = serde_json::map::Map::new();
if let Some(n) = v.get("name") {
base_value.insert("name".to_string(), n.clone());
if let Some(n) = v.remove("name") {
base_value.insert("name".to_string(), n);
}
if let Some(s) = v.get("state") {
base_value.insert("state".to_string(), s.clone());
if let Some(s) = v.remove("state") {
base_value.insert("state".to_string(), s);
}
// The BaseInterface will only have name and state
// These two properties are also stored in `other` for serializing
@ -252,7 +252,7 @@ impl<'de> Deserialize<'de> for UnknownInterface {
serde_json::value::Value::Object(base_value),
)
.map_err(serde::de::Error::custom)?;
ret.other = v;
ret.other = serde_json::Value::Object(v);
Ok(ret)
}
}

12
rust/src/lib/ifaces/ethernet.rs

@ -166,17 +166,6 @@ pub struct VethConfig {
}
impl MergedInterfaces {
pub(crate) fn has_sriov_vf_changes(&self) -> bool {
self.kernel_ifaces.values().any(|i| {
if let Some(Interface::Ethernet(eth_iface)) = i.for_apply.as_ref() {
eth_iface.ethernet.as_ref().map(|e| e.sr_iov.is_some())
== Some(true)
} else {
false
}
})
}
// Raise error if new veth interface has no peer defined.
// Mark old veth peer as absent when veth changed its peer.
// Mark veth peer as absent also when veth is marked as absent.
@ -205,7 +194,6 @@ impl MergedInterfaces {
if eth_iface.veth.is_none()
&& !self.gen_conf_mode
&& !veth_peers.contains(&eth_iface.base.name.as_str())
&& !self.has_sriov_vf_changes()
{
return Err(NmstateError::new(
ErrorKind::InvalidArgument,

3
rust/src/lib/ifaces/inter_ifaces.rs

@ -488,6 +488,7 @@ impl MergedInterfaces {
desired.set_unknown_iface_to_eth()?;
desired.set_missing_port_to_eth();
} else {
desired.resolve_sriov_reference(&current)?;
desired.resolve_unknown_ifaces(&current)?;
}
@ -510,8 +511,6 @@ impl MergedInterfaces {
desired.unify_veth_and_eth();
current.unify_veth_and_eth();
desired.resolve_sriov_reference(&current)?;
for (iface_name, des_iface) in desired
.kernel_ifaces
.drain()

6
rust/src/lib/ifaces/sriov.rs

@ -200,11 +200,7 @@ impl Interfaces {
current: &Self,
) -> Result<(), NmstateError> {
let mut changed_iface_names: Vec<String> = Vec::new();
for iface in self
.kernel_ifaces
.values_mut()
.filter(|i| i.iface_type() == InterfaceType::Ethernet)
{
for iface in self.kernel_ifaces.values_mut() {
if let Some((pf_name, vf_id)) = parse_sriov_vf_naming(iface.name())?
{
if let Some(vf_iface_name) =

112
rust/src/lib/query_apply/ethernet.rs

@ -2,8 +2,7 @@
use crate::{
ErrorKind, EthernetConfig, EthernetInterface, Interface, InterfaceType,
Interfaces, MergedInterface, MergedNetworkState, NmstateError, SrIovConfig,
VethConfig,
Interfaces, NetworkState, NmstateError, SrIovConfig, VethConfig,
};
impl EthernetInterface {
@ -101,36 +100,78 @@ impl Interfaces {
}
}
impl MergedNetworkState {
// Return newly create MergedNetworkState containing only ethernet section
// of interface with SR-IOV PF changes.
// The self will have SR-IOV PF change removed.
pub(crate) fn isolate_sriov_conf_out(&mut self) -> Option<Self> {
let mut pf_ifaces: Vec<MergedInterface> = Vec::new();
impl NetworkState {
pub(crate) fn has_vf_count_change_and_missing_eth(
&self,
current: &Self,
) -> bool {
self.has_vf_count_change(current) && self.has_missing_eth(current)
}
for iface in self.interfaces.kernel_ifaces.values_mut().filter(|i| {
i.is_desired() && i.merged.iface_type() == InterfaceType::Ethernet
}) {
if let Some(Interface::Ethernet(apply_iface)) =
iface.for_apply.as_mut()
fn has_vf_count_change(&self, current: &Self) -> bool {
for iface in
self.interfaces.kernel_ifaces.values().filter(|i| i.is_up())
{
if let (
Interface::Ethernet(iface),
Some(Interface::Ethernet(cur_iface)),
) = (iface, current.interfaces.kernel_ifaces.get(iface.name()))
{
if let Some(true) =
apply_iface.ethernet.as_ref().map(|e| e.sr_iov.is_some())
{
if let Some(eth_conf) = apply_iface.ethernet.as_ref() {
let new_iface = EthernetInterface {
base: apply_iface.base.clone_name_type_only(),
ethernet: Some(eth_conf.clone()),
..Default::default()
};
if let Ok(new_merged_iface) = MergedInterface::new(
Some(Interface::Ethernet(new_iface)),
iface.current.clone(),
) {
pf_ifaces.push(new_merged_iface);
apply_iface.ethernet = None;
}
}
let pf_count = iface
.ethernet
.as_ref()
.and_then(|e| e.sr_iov.as_ref())
.and_then(|s| s.total_vfs);
let cur_pf_count = cur_iface
.ethernet
.as_ref()
.and_then(|e| e.sr_iov.as_ref())
.and_then(|s| s.total_vfs);
if pf_count.is_some() && pf_count != cur_pf_count {
return true;
}
}
}
false
}
fn has_missing_eth(&self, current: &Self) -> bool {
self.interfaces
.kernel_ifaces
.values()
.filter(|i| {
i.is_up()
&& (i.iface_type() == InterfaceType::Ethernet
|| i.iface_type() == InterfaceType::Unknown)
})
.any(|i| !current.interfaces.kernel_ifaces.contains_key(i.name()))
}
// Return newly create NetworkState containing only ethernet section of
// interface with SR-IOV PF changes.
pub(crate) fn get_sriov_pf_conf_state(&self) -> Option<Self> {
let mut pf_ifaces: Vec<Interface> = Vec::new();
for iface in self.interfaces.kernel_ifaces.values().filter_map(|i| {
if i.is_up() {
if let Interface::Ethernet(iface) = i {
Some(iface)
} else {
None
}
} else {
None
}
}) {
if let Some(true) =
iface.ethernet.as_ref().map(|e| e.sr_iov.is_some())
{
if let Some(eth_conf) = iface.ethernet.as_ref() {
pf_ifaces.push(Interface::Ethernet(EthernetInterface {
base: iface.base.clone_name_type_only(),
ethernet: Some(eth_conf.clone()),
..Default::default()
}));
}
}
}
@ -138,16 +179,9 @@ impl MergedNetworkState {
if pf_ifaces.is_empty() {
None
} else {
let mut pf_state = MergedNetworkState::default();
let mut pf_state = NetworkState::default();
for pf_iface in pf_ifaces {
pf_state.interfaces.insert_order.push((
pf_iface.merged.name().to_string(),
pf_iface.merged.iface_type(),
));
pf_state
.interfaces
.kernel_ifaces
.insert(pf_iface.merged.name().to_string(), pf_iface);
pf_state.interfaces.push(pf_iface);
}
Some(pf_state)
}

15
rust/src/lib/query_apply/inter_ifaces.rs

@ -110,21 +110,6 @@ fn verify_desire_absent_but_found_in_current(
}
impl MergedInterfaces {
pub(crate) fn state_for_apply(&self) -> Interfaces {
let mut ifaces = Interfaces::new();
for merged_iface in self
.kernel_ifaces
.values()
.chain(self.user_ifaces.values())
.filter(|i| i.is_changed())
{
if let Some(iface) = merged_iface.for_apply.as_ref() {
ifaces.push(iface.clone());
}
}
ifaces
}
pub(crate) fn verify(
&self,
current: &Interfaces,

105
rust/src/lib/query_apply/net_state.rs

@ -82,61 +82,82 @@ impl NetworkState {
/// Apply the `NetworkState`.
/// Only available for feature `query_apply`.
pub fn apply(&self) -> Result<(), NmstateError> {
let mut cur_net_state = NetworkState::new();
cur_net_state.set_kernel_only(self.kernel_only);
cur_net_state.set_include_secrets(true);
cur_net_state.retrieve_full()?;
let mut merged_state = MergedNetworkState::new(
self.clone(),
cur_net_state.clone(),
false,
self.memory_only,
)?;
let ifaces = merged_state.interfaces.state_for_apply();
ifaces.check_sriov_capability()?;
if ifaces.to_vec().len() >= MAX_SUPPORTED_INTERFACES {
if self.interfaces.kernel_ifaces.len()
+ self.interfaces.user_ifaces.len()
>= MAX_SUPPORTED_INTERFACES
{
log::warn!(
"Interfaces count exceeds the support limit {} in \
desired state",
MAX_SUPPORTED_INTERFACES,
);
}
if !self.kernel_only {
self.apply_with_nm_backend(&mut merged_state, &cur_net_state)
self.apply_with_nm_backend()
} else {
// TODO: Need checkpoint for kernel only mode
self.apply_without_nm_backend(&merged_state, &cur_net_state)
self.apply_without_nm_backend()
}
}
fn apply_with_nm_backend(
&self,
merged_state: &mut MergedNetworkState,
cur_net_state: &Self,
) -> Result<(), NmstateError> {
let pf_merged_state = merged_state.isolate_sriov_conf_out();
fn apply_with_nm_backend(&self) -> Result<(), NmstateError> {
let mut cur_net_state = NetworkState::new();
cur_net_state.set_kernel_only(self.kernel_only);
cur_net_state.set_include_secrets(true);
cur_net_state.retrieve_full()?;
// At this point, the `unknown` interface type is not resolved yet,
// hence when user want `enable-and-use` single-transaction for SR-IOV,
// they need define the interface type. It is overkill to do resolve at
// this point for this corner use case.
let pf_state =
if self.has_vf_count_change_and_missing_eth(&cur_net_state) {
self.get_sriov_pf_conf_state()
} else {
None
};
let timeout = self.timeout.unwrap_or(DEFAULT_ROLLBACK_TIMEOUT);
let checkpoint = nm_checkpoint_create(timeout)?;
log::info!("Created checkpoint {}", &checkpoint);
let verify_count = if pf_state.is_some() {
VERIFY_RETRY_COUNT_SRIOV
} else {
VERIFY_RETRY_COUNT
};
with_nm_checkpoint(&checkpoint, self.no_commit, || {
if let Some(pf_merged_state) = pf_merged_state {
if let Some(pf_state) = pf_state {
pf_state.interfaces.check_sriov_capability()?;
let pf_merged_state = MergedNetworkState::new(
pf_state,
cur_net_state.clone(),
false,
self.memory_only,
)?;
self.apply_with_nm_backend_and_under_checkpoint(
&pf_merged_state,
cur_net_state,
&cur_net_state,
&checkpoint,
VERIFY_RETRY_COUNT_SRIOV,
)?
verify_count,
)?;
// Refresh current state
cur_net_state.retrieve_full()?;
}
let merged_state = MergedNetworkState::new(
self.clone(),
cur_net_state.clone(),
false,
self.memory_only,
)?;
self.apply_with_nm_backend_and_under_checkpoint(
merged_state,
cur_net_state,
&merged_state,
&cur_net_state,
&checkpoint,
VERIFY_RETRY_COUNT,
verify_count,
)
})
}
@ -180,12 +201,20 @@ impl NetworkState {
})
}
fn apply_without_nm_backend(
&self,
merged_state: &MergedNetworkState,
cur_net_state: &Self,
) -> Result<(), NmstateError> {
nispor_apply(merged_state)?;
fn apply_without_nm_backend(&self) -> Result<(), NmstateError> {
let mut cur_net_state = NetworkState::new();
cur_net_state.set_kernel_only(self.kernel_only);
cur_net_state.set_include_secrets(true);
cur_net_state.retrieve_full()?;
let merged_state = MergedNetworkState::new(
self.clone(),
cur_net_state.clone(),
false,
self.memory_only,
)?;
nispor_apply(&merged_state)?;
if let Some(running_hostname) =
self.hostname.as_ref().and_then(|c| c.running.as_ref())
{

120
rust/src/lib/unit_tests/sriov.rs

@ -3,8 +3,7 @@
use crate::{
unit_tests::testlib::new_eth_iface, BridgePortVlanMode, ErrorKind,
EthernetConfig, EthernetDuplex, Interface, InterfaceType, Interfaces,
MergedInterfaces, MergedNetworkState, NetworkState, SrIovConfig,
SrIovVfConfig,
MergedInterfaces, NetworkState, SrIovConfig, SrIovVfConfig,
};
#[test]
@ -612,32 +611,10 @@ fn test_sriov_enable_and_use_in_single_yaml() {
)
.unwrap();
let current = serde_yaml::from_str::<NetworkState>(
r#"---
interfaces:
- name: eth1
type: ethernet
state: up
ethernet:
sr-iov:
total-vfs: 0
"#,
)
.unwrap();
let pf_state = desired.get_sriov_pf_conf_state().unwrap();
let mut merged_state =
MergedNetworkState::new(desired, current, false, false).unwrap();
let pf_state = merged_state.isolate_sriov_conf_out().unwrap();
if let Interface::Ethernet(pf_iface) = pf_state
.interfaces
.kernel_ifaces
.get("eth1")
.unwrap()
.for_apply
.as_ref()
.unwrap()
if let Interface::Ethernet(pf_iface) =
pf_state.interfaces.kernel_ifaces.get("eth1").unwrap()
{
let eth_conf = pf_iface.ethernet.as_ref().unwrap();
assert_eq!(eth_conf.auto_neg, Some(false));
@ -648,17 +625,80 @@ fn test_sriov_enable_and_use_in_single_yaml() {
} else {
panic!("Expecting Ethernet interface, got {:?}", pf_state);
}
if let Interface::Ethernet(second_pf_iface) = merged_state
.interfaces
.kernel_ifaces
.get("eth1")
.unwrap()
.for_apply
.as_ref()
.unwrap()
{
assert!(second_pf_iface.ethernet.is_none())
} else {
panic!("Expecting Ethernet interface, got {:?}", pf_state);
}
}
#[test]
fn test_sriov_has_vf_count_change_and_missing_eth() {
let desired = serde_yaml::from_str::<NetworkState>(
r#"---
interfaces:
- name: eth1
type: ethernet
state: up
ethernet:
speed: 10000
duplex: full
auto-negotiation: false
sr-iov:
total-vfs: 2
- name: eth1v0
type: ethernet
state: up
- name: eth1v1
type: ethernet
state: up
"#,
)
.unwrap();
let current = serde_yaml::from_str::<NetworkState>(
r#"---
interfaces:
- name: eth1
type: ethernet
state: up
ethernet:
speed: 10000
duplex: full
auto-negotiation: false
sr-iov:
total-vfs: 0
"#,
)
.unwrap();
assert!(desired.has_vf_count_change_and_missing_eth(&current));
}
#[test]
fn test_sriov_has_vf_count_change_and_missing_eth_pf_none() {
let desired = serde_yaml::from_str::<NetworkState>(
r#"---
interfaces:
- name: eth1
type: ethernet
state: up
ethernet:
speed: 10000
duplex: full
auto-negotiation: false
sr-iov:
total-vfs: 2
- name: eth1v0
state: up
- name: eth1v1
state: up
"#,
)
.unwrap();
let current = serde_yaml::from_str::<NetworkState>(
r#"---
interfaces:
- name: eth1
type: ethernet
state: up
"#,
)
.unwrap();
assert!(desired.has_vf_count_change_and_missing_eth(&current));
}

21
tests/integration/sriov_test.py

@ -17,13 +17,13 @@ from libnmstate.schema import LinuxBridge
from libnmstate.schema import OVSBridge
from .testlib import assertlib
from .testlib import statelib
from .testlib.bondlib import bond_interface
from .testlib.bridgelib import add_port_to_bridge
from .testlib.bridgelib import create_bridge_subtree_state
from .testlib.bridgelib import linux_bridge
from .testlib.ovslib import ovs_bridge
from .testlib.ovslib import ovs_bridge_bond
from .testlib.sriov import get_sriov_vf_names
MAC1 = "00:11:22:33:44:55"
MAC2 = "00:11:22:33:44:66"
@ -189,10 +189,8 @@ class TestSrIov:
try:
libnmstate.apply(desired_state)
assertlib.assert_state_match(desired_state)
current_state = statelib.show_only(
(f"{pf_name}v0", f"{pf_name}v1")
)
assert len(current_state[Interface.KEY]) == 2
vf_ifaces = get_sriov_vf_names(pf_name)
assert len(vf_ifaces) == 2
finally:
desired_state[Interface.KEY][0][
@ -215,20 +213,16 @@ class TestSrIov:
try:
libnmstate.apply(desired_state)
assertlib.assert_state_match(desired_state)
current_state = statelib.show_only(
(f"{pf_name}v0", f"{pf_name}v1")
)
assert len(current_state[Interface.KEY]) == 2
vf_ifaces = get_sriov_vf_names(pf_name)
assert len(vf_ifaces) == 2
desired_state[Interface.KEY][0][Ethernet.CONFIG_SUBTREE][
Ethernet.SRIOV_SUBTREE
][Ethernet.SRIOV.TOTAL_VFS] = 1
libnmstate.apply(desired_state)
assertlib.assert_state_match(desired_state)
current_state = statelib.show_only(
(f"{pf_name}v0", f"{pf_name}v1")
)
assert len(current_state[Interface.KEY]) == 1
vf_ifaces = get_sriov_vf_names(pf_name)
assert len(vf_ifaces) == 1
finally:
desired_state[Interface.KEY][0][
@ -438,6 +432,7 @@ class TestSrIov:
iface_infos = [
{
Interface.NAME: pf_name,
Interface.TYPE: InterfaceType.ETHERNET,
Interface.STATE: InterfaceState.UP,
Ethernet.CONFIG_SUBTREE: {
Ethernet.SRIOV_SUBTREE: {Ethernet.SRIOV.TOTAL_VFS: 2},

22
tests/integration/testlib/sriov.py Normal file

@ -0,0 +1,22 @@
# SPDX-License-Identifier: LGPL-2.1-or-later
import json
import libnmstate
from libnmstate.schema import Interface
from .cmdlib import exec_cmd
def get_sriov_vf_names(pf_name):
output = exec_cmd(f"ip -j -d link show {pf_name}".split())[1]
link_info = json.loads(output)[0]
macs = [
vf_info["address"].upper()
for vf_info in link_info.get("vfinfo_list", [])
]
return [
iface[Interface.NAME]
for iface in libnmstate.show()[Interface.KEY]
if iface[Interface.MAC] in macs
]