Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for user-defined network interface order (kvm only) #4452

Merged
merged 5 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 53 additions & 7 deletions docs/APP-CONNECTIVITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,59 @@ Network connectivity is enabled for an application by configuring one or more ne
These consists of [virtual network interfaces](#virtual-network-interfaces), connecting
application with [network instances](#network-instances), and directly assigned
[physical NIC](#physical-network-ports) or [SR-IOV virtual functions](#sr-iov-vfs).
The order of network adapter configurations inside `AppInstanceConfig` matters, because this
is the same order at which these network adapters will be presented to the application by the
Virtual Machine Manager (VMM).
This order is reflected in the PCI addresses of the adapters. It is important to note that VIFs
and direct assignments are configured in two separate lists. The VMM is configured to present
VIFs first, adhering to their configured order, followed by direct assignments in their respective
order. User cannot enforce direct assignment to appear before VIF.

### Interface order

To match an application's network interface with its logical representation from the configuration,
using MAC addresses is recommended. For virtual network interfaces, users can configure MAC
addresses for easier interface identification. Meanwhile, directly assigned NICs are passed
through to applications with their original MAC addresses unchanged.

However, since MAC addresses differ across a fleet of devices, having a predictable application
network interface order provides a simpler and more scalable matching solution.
But there are two main challenges related to application interface ordering, which make this method
less recommended:

#### 1. Interface Order Configuration Limitations

Virtual and directly assigned NICs are configured in two separate lists within `AppInstanceConfig`:
`Interfaces` (VIFs) and `Adapters` (direct assignments, including physical NICs and SR-IOV virtual
functions). Until EVE version 13.8.0, it was not possible to configure the order between items
of these two lists. Consequently, EVE would configure the VMM to present the VIFs first, followed
by direct assignments. While directly assigned NICs adhered to the `Adapters` list order, virtual
interfaces were ordered based on the lowest ACL rule ID assigned to each VIF. This caused undesired
behavior when ACL rule IDs (assigned by the controller) did not align with the user-defined VIF order.
Furthermore, VIFs without ACLs (rather useless interfaces since all traffic is blocked) were
represented with an ACL ID of 0 and always placed first in the interface list.

EVE version 13.8.0 addressed this by allowing users to define the desired position of each VIF and
directly assigned network devices. This is propagated from controller to EVE via the `InterfaceOrder`
fields with unique (integer) values across both the `Interfaces` and `Adapters` lists. To maintain
backward compatibility and avoid changing interface order in existing deployments, the user-defined
order is applied only if `VmConfig.EnforceNetworkInterfaceOrder` is set to true. Otherwise, EVE
defaults to the legacy behavior described above.

#### 2. Hypervisor Limitations

The second, more fundamental challenge lies in the hypervisor's limited ability to control the order
of network interfaces inside VM applications. The actual interface order depends on the application OS,
and EVE cannot guarantee the desired outcome.

For the KVM hypervisor, EVE attempts to enforce legacy or user-defined orders via the virtualized
PCI topology. Devices expected to appear earlier in the interface list are assigned lower PCI
addresses. This generally works for Linux-based systems, especially those using
the [systemd's naming scheme](https://www.freedesktop.org/software/systemd/man/latest/systemd.net-naming-scheme.html).
However, some limitations persist:

* Devices of different types (e.g., Wi-Fi adapters vs. Ethernet NICs) often receive distinct name
prefixes from the application OS, making reordering impossible.
* Multifunction PCI devices must have all functions connected to the same PCI bridge and cannot be
interleaved with other devices. If a user places a VIF or another NIC between functions
of a multifunction device (with `EnforceNetworkInterfaceOrder` enabled), EVE will return an error,
and the application will not be deployed.

For Xen and KubeVirt hypervisors, application interface order is undefined, and
`EnforceNetworkInterfaceOrder` is not yet supported.

### Physical network ports

Expand Down
65 changes: 53 additions & 12 deletions pkg/pillar/cmd/zedagent/parseconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,8 @@ func parseAppInstanceConfig(getconfigCtx *getconfigContext,
appInstance.FixedResources.EnableVncShimVM = cfgApp.Fixedresources.EnableVncShimVm
appInstance.FixedResources.VncDisplay = cfgApp.Fixedresources.VncDisplay
appInstance.FixedResources.VncPasswd = cfgApp.Fixedresources.VncPasswd
appInstance.FixedResources.EnforceNetworkInterfaceOrder =
cfgApp.Fixedresources.EnforceNetworkInterfaceOrder
appInstance.DisableLogs = cfgApp.Fixedresources.DisableLogs
appInstance.MetaDataType = types.MetaDataType(cfgApp.MetaDataType)
appInstance.Delay = time.Duration(cfgApp.StartDelayInSeconds) * time.Second
Expand Down Expand Up @@ -734,6 +736,9 @@ func parseAppInstanceConfig(getconfigCtx *getconfigContext,
} else if ioa.Type == types.IoCAN || ioa.Type == types.IoVCAN || ioa.Type == types.IoLCAN {
log.Functionf("Got CAN adapter")
}
if ioa.Type.IsNet() && appInstance.FixedResources.EnforceNetworkInterfaceOrder {
ioa.IntfOrder = adapter.GetInterfaceOrder()
}
appInstance.IoAdapterList = append(appInstance.IoAdapterList, ioa)
}
log.Functionf("Got adapters %v", appInstance.IoAdapterList)
Expand Down Expand Up @@ -2439,11 +2444,12 @@ func parseAppNetAdapterConfig(appInstance *types.AppInstanceConfig,
intfEnt.Name, adapterCfg.Error)
}
}
// sort based on intfOrder
// XXX remove? Debug?
if len(appInstance.AppNetAdapterList) > 1 {
log.Functionf("XXX pre sort %+v", appInstance.AppNetAdapterList)
}

// Sort based on IntfOrder. When EnforceNetworkInterfaceOrder is enabled, this is done
// only for troubleshooting purposes to make the pubsub messages containing application
// interface list easier to read. Interface order is still determined by the IntfOrder
// attribute, and that includes direct attachments, not based on the order of items
// inside AppNetAdapterList.
sort.Slice(appInstance.AppNetAdapterList[:],
func(i, j int) bool {
return appInstance.AppNetAdapterList[i].IntfOrder <
Expand Down Expand Up @@ -2490,8 +2496,6 @@ func parseAppNetAdapterConfigEntry(

adapterCfg := new(types.AppNetAdapterConfig)
adapterCfg.Name = intfEnt.Name
// XXX set adapterCfg.IntfOrder from API once available
var intfOrder int32
// Lookup NetworkInstance ID
networkInstanceEntry := lookupNetworkInstanceId(intfEnt.NetworkId,
cfgNetworkInstances)
Expand Down Expand Up @@ -2553,6 +2557,7 @@ func parseAppNetAdapterConfigEntry(
}
}

var aclIntfOrder uint32
adapterCfg.ACLs = make([]types.ACE, len(intfEnt.Acls))
for aclIdx, acl := range intfEnt.Acls {
aclCfg := new(types.ACE)
Expand All @@ -2561,9 +2566,11 @@ func parseAppNetAdapterConfigEntry(
aclCfg.Actions = make([]types.ACEAction,
len(acl.Actions))
aclCfg.RuleID = acl.Id
// XXX temporary until we get an intfOrder in the API
if intfOrder == 0 {
intfOrder = acl.Id
// When EnforceNetworkInterfaceOrder is disabled, we fall back to the previous
// interface ordering method, where virtual interfaces are ordered according to ACL
// IDs and direct attachments come after virtual interfaces.
if aclIntfOrder == 0 && acl.Id > 0 {
aclIntfOrder = uint32(acl.Id)
}
aclCfg.Name = acl.Name
aclCfg.Dir = types.ACEDirection(acl.Dir)
Expand All @@ -2587,8 +2594,11 @@ func parseAppNetAdapterConfigEntry(
}
adapterCfg.ACLs[aclIdx] = *aclCfg
}
// XXX set adapterCfg.IntfOrder from API once available
adapterCfg.IntfOrder = intfOrder
if cfgApp.Fixedresources.EnforceNetworkInterfaceOrder {
adapterCfg.IntfOrder = intfEnt.GetInterfaceOrder()
} else {
adapterCfg.IntfOrder = aclIntfOrder
}
adapterCfg.AccessVlanID = intfEnt.AccessVlanId
adapterCfg.AllowToDiscover = intfEnt.AllowToDiscover

Expand Down Expand Up @@ -2822,6 +2832,37 @@ func checkAndPublishAppInstanceConfig(getconfigCtx *getconfigContext,
config.Errors = append(config.Errors, err.Error())
}

// If EnforceNetworkInterfaceOrder is enabled, check that every network interface
// has unique order number.
OhmSpectator marked this conversation as resolved.
Show resolved Hide resolved
if config.FixedResources.EnforceNetworkInterfaceOrder {
intfOrderMap := make(map[uint32]string)
for _, adapter := range config.AppNetAdapterList {
if adapter2, duplicate := intfOrderMap[adapter.IntfOrder]; duplicate {
err := fmt.Errorf("virtual network adapter %s has the same interface "+
"order (%d) configured as adapter %s", adapter.Name, adapter.IntfOrder,
adapter2)
log.Error(err)
config.Errors = append(config.Errors, err.Error())
continue
}
intfOrderMap[adapter.IntfOrder] = adapter.Name
}
for _, adapter := range config.IoAdapterList {
if !adapter.Type.IsNet() {
continue
}
if adapter2, duplicate := intfOrderMap[adapter.IntfOrder]; duplicate {
err := fmt.Errorf("directly attached network adapter %s has the same "+
"interface order (%d) configured as adapter %s", adapter.Name,
adapter.IntfOrder, adapter2)
log.Error(err)
config.Errors = append(config.Errors, err.Error())
continue
}
intfOrderMap[adapter.IntfOrder] = adapter.Name
}
}

pub.Publish(key, config)
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/pillar/cmd/zedagent/reportinfo.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,8 +644,10 @@ func PublishDeviceInfoToZedCloud(ctx *zedagentContext, dest destinationBitset) {
// TODO: Enhance capability reporting with a bitmap-like approach for increased granularity.
// We report the snapshot capability despite the fact that we support snapshots only
// for file-based volumes. If a controller tries to make a snapshot of ZFS-based volume
// device returns a runtime error.
ReportDeviceInfo.ApiCapability = info.APICapability_API_CAPABILITY_ADAPTER_USER_LABELS
// device returns a runtime error. Similarly, we only support enforced application network
// interface order for the KVM hypervisor. If enabled for application deployed under Xen
// or Kubevirt hypervisor, EVE returns error and the application will not be started.
ReportDeviceInfo.ApiCapability = info.APICapability_API_CAPABILITY_ENFORCED_NET_INTERFACE_ORDER

// Report if there is a local override of profile
if ctx.getconfigCtx.sideController.currentProfile !=
Expand Down
2 changes: 1 addition & 1 deletion pkg/pillar/cmd/zedmanager/zedmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ func handleCreate(ctxArg interface{}, key string,
if len(config.Errors) > 0 {
// Combine all errors from Config parsing state and send them in Status
for i, errStr := range config.Errors {
allErrors += errStr
allErrors += errStr + "\n"
log.Errorf("App Instance %s-%s: Error(%d): %s",
config.DisplayName, config.UUIDandVersion.UUID, i, errStr)
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/pillar/cmd/zedrouter/appnetwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ func (z *zedrouter) prepareConfigForVIFs(config types.AppNetworkConfig,
}
adapterStatus.HostName = config.Key()
adapterStatus.MTU = netInstStatus.MTU
// Propagate IntfOrder from adapter down to VifConfig, which zedmanager then passes
// to domainmgr.
adapterStatus.VifConfig.VifOrder = adapterStatus.IntfOrder
guestIP, err := z.lookupOrAllocateIPv4ForVIF(
netInstStatus, *adapterStatus, status.UUIDandVersion.UUID)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/pillar/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ require (
github.com/jaypipes/ghw v0.8.0
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.5.0
github.com/lf-edge/edge-containers v0.0.0-20240207093504-5dfda0619b80
github.com/lf-edge/eve-api/go v0.0.0-20241125213346-b026f21b3e75
github.com/lf-edge/eve-api/go v0.0.0-20241202084032-46c0a58dc84c
github.com/lf-edge/eve-libs v0.0.0-20240614221125-6913ec4213f9
github.com/lf-edge/eve/pkg/kube/cnirpc v0.0.0-20240315102754-0f6d1f182e0d
github.com/lf-edge/go-qemu v0.0.0-20231121152149-4c467eda0c56
Expand Down
4 changes: 2 additions & 2 deletions pkg/pillar/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1432,8 +1432,8 @@ github.com/lestrrat-go/jwx v1.2.25/go.mod h1:zoNuZymNl5lgdcu6P7K6ie2QRll5HVfF4xw
github.com/lestrrat-go/option v1.0.0/go.mod h1:5ZHFbivi4xwXxhxY9XHDe2FHo6/Z7WWmtT7T5nBBp3I=
github.com/lf-edge/edge-containers v0.0.0-20240207093504-5dfda0619b80 h1:kiqB1Rk8fmWci0idN68azRDJfPxCivD3zNDddWZocFw=
github.com/lf-edge/edge-containers v0.0.0-20240207093504-5dfda0619b80/go.mod h1:4yXdumKdTzF0URMtxOl8Xnzdxnoy1QR+2dzfOr4CIZY=
github.com/lf-edge/eve-api/go v0.0.0-20241125213346-b026f21b3e75 h1:vEu+sJLajrsVrBOKSl9+rZZ2tD6kg4Pi4gGiNAT2Cx0=
github.com/lf-edge/eve-api/go v0.0.0-20241125213346-b026f21b3e75/go.mod h1:ot6MhAhBXapUDl/hXklaX4kY88T3uC4PTg0D2wD8DzA=
github.com/lf-edge/eve-api/go v0.0.0-20241202084032-46c0a58dc84c h1:axc1BfKlb6MmvbxPhCMW427b+Wxn7u/5PxdqbvYmm6k=
github.com/lf-edge/eve-api/go v0.0.0-20241202084032-46c0a58dc84c/go.mod h1:ot6MhAhBXapUDl/hXklaX4kY88T3uC4PTg0D2wD8DzA=
github.com/lf-edge/eve-libs v0.0.0-20240614221125-6913ec4213f9 h1:blXdD227+xDCsezw8gTlk5Kh0ONb5ZekkH4lLnV4J04=
github.com/lf-edge/eve-libs v0.0.0-20240614221125-6913ec4213f9/go.mod h1:bz9uyBFalnRnzXl6A4oAsO7CLJfW01ZR/sT0Frqa6o8=
github.com/lf-edge/eve/pkg/kube/cnirpc v0.0.0-20240315102754-0f6d1f182e0d h1:tUBb9M6u42LXwHAYHyh22wJeUUQlTpDkXwRXalpRqbo=
Expand Down
6 changes: 6 additions & 0 deletions pkg/pillar/hypervisor/kubevirt.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,12 @@ func (ctx kubevirtContext) Setup(status types.DomainStatus, config types.DomainC

logrus.Debugf("Setup called for Domain: %s, vmmode %v", domainName, config.VirtualizationMode)

if config.EnforceNetworkInterfaceOrder {
logrus.Errorf("Enforcing user-defined network interface order is not supported "+
"with the KubeVirt hypervisor. Ignoring EnforceNetworkInterfaceOrder flag "+
"for app %s", config.DisplayName)
}

if config.VirtualizationMode == types.NOHYPER {
if err := ctx.CreatePodConfig(domainName, config, status, diskStatusList, aa, file); err != nil {
return logError("failed to build kube pod config: %v", err)
Expand Down
Loading
Loading