From 49e304e9fd3bbb6bdae6c659f9b48879ac184dcb Mon Sep 17 00:00:00 2001 From: Sven Rosenzweig Date: Tue, 26 May 2026 16:04:25 +0200 Subject: [PATCH] fix(iosxr): configure L2 and L3 MTU for different interface types Due to IOS-XR MTU inheritance behavior for subinterfaces and bundle members, the following rules are sufficient for our use case: - Physical interface: Configure L2 MTU and, if an IP address is present, configure L3 MTU. - Bundle interface: Configure only the L2 MTU. - Bundle member interface (physical): Do not configure MTU settings directly. L2 MTU is inherited from the bundle interface, and L3 MTU is configured on the corresponding subinterface. - Subinterface (physical or bundle): Configure only the L3 MTU, using a default value of 1500 bytes. In this commit, we assume a default L3 MTU of 1500 bytes. Cisco IOS-XR automatically adds: - 4 bytes for 802.1Q encapsulation - 8 bytes for 802.1ad (Q-in-Q) encapsulation This allows subinterfaces to operate correctly with the default L3 MTU. Official Cisco documentation: https://www.cisco.com/c/de_de/support/docs/ios-nx-os-software/ios-xr-software/116350-trouble-ios-xr-mtu-00.html Signed-off-by: Sven Rosenzweig --- internal/provider/cisco/iosxr/intf.go | 14 +++++- internal/provider/cisco/iosxr/intf_test.go | 4 +- internal/provider/cisco/iosxr/provider.go | 39 ++++++++------- .../provider/cisco/iosxr/provider_test.go | 47 +++++++++++++++++++ 4 files changed, 84 insertions(+), 20 deletions(-) diff --git a/internal/provider/cisco/iosxr/intf.go b/internal/provider/cisco/iosxr/intf.go index a08fb92de..7c62dd3ad 100644 --- a/internal/provider/cisco/iosxr/intf.go +++ b/internal/provider/cisco/iosxr/intf.go @@ -12,6 +12,16 @@ import ( "github.com/ironcore-dev/network-operator/internal/transport/gnmiext" ) +// DefaultLinkMTU is calculated based on the default MTU of 1500 bytes for routed interfaces on IOS-XR and the addition of 14 bytes for the L2 header. +// L2-payload + L2-header (1500 + 14) +const DefaultLinkMTU int32 = 1514 + +// DefaultL3MTU configures the maximum packet size of that protocol which includes the L3 header +// For subinterfaces automatically +// add 4 bytes for each VLAN tag configured on the sub-interface. +// add 8 bytes for a IEEE 802.1Q tunneling (QinQ) sub-interface. +const DefaultL3MTU int32 = 1500 + var ( bundleEtherRE = regexp.MustCompile(`^(Bundle-Ether|bundle-ether)(\d+)(\.\d+)?$`) physicalInterfaceRE = regexp.MustCompile(`^(TenGigE|TwentyFiveGigE|FortyGigE|HundredGigE|GigabitEthernet)(\d){1}(\/\d){2}(\/\d+){1}(.\d{1,5})?$`) @@ -96,7 +106,7 @@ type Statistics struct { type IPv4Network struct { Addresses AddressesIPv4 `json:"addresses"` - Mtu uint16 `json:"mtu,omitzero"` + MTU uint16 `json:"mtu,omitzero"` } type AddressesIPv4 struct { @@ -109,7 +119,7 @@ type Primary struct { } type IPv6Network struct { - Mtu uint16 `json:"mtu"` + MTU uint16 `json:"mtu"` Addresses AddressesIPv6 `json:"addresses"` } diff --git a/internal/provider/cisco/iosxr/intf_test.go b/internal/provider/cisco/iosxr/intf_test.go index 276fa9c6e..008facb2b 100644 --- a/internal/provider/cisco/iosxr/intf_test.go +++ b/internal/provider/cisco/iosxr/intf_test.go @@ -32,10 +32,10 @@ func init() { Netmask: "255.255.255.0", }, }, - Mtu: 1000, + MTU: 1000, }, IPv6Network: IPv6Network{ - Mtu: 2100, + MTU: 2100, Addresses: AddressesIPv6{ RegularAddresses: RegularAddresses{ RegularAddress: []RegularAddress{ diff --git a/internal/provider/cisco/iosxr/provider.go b/internal/provider/cisco/iosxr/provider.go index 23258ca67..67807d167 100644 --- a/internal/provider/cisco/iosxr/provider.go +++ b/internal/provider/cisco/iosxr/provider.go @@ -121,9 +121,20 @@ func (p *Provider) Reprovision(_ context.Context, conn *deviceutil.Connection) e return errors.New("IOS XR Provider does not support reprovisioning") } +// EnsureInterface configures the interface based on the provided request. +// MTU configuration rules: +// - Physical interface: +// Configure L2 MTU and, if an IP address is present, configure L3 MTU. +// - Bundle interface: +// Configure only the L2 MTU. +// - Bundle member interface (Physical): +// Do not configure MTU settings directly. +// L2 MTU is inherited from the bundle interface, and L3 MTU is configured +// on the corresponding subinterface. +// - Subinterface (physical or bundle): +// Configure only the L3 MTU, using a default value of 1500 bytes. func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInterfaceRequest) error { // TODO(sven-rosenweig): Make use of the VRF information in the request to assign the interface to the correct VRF. - // FIXME(sven-rosenweig): Use the ExtractOwnerFromInterfaceName function in the ValidateInterfaceName function name := req.Interface.Spec.Name if err := ValidateInterfaceName(name); err != nil { @@ -162,13 +173,11 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte } iface.IPv4Network = ipv4 - if req.Interface.Spec.MTU != 0 { - mtu, err := NewMTU(name, req.Interface.Spec.MTU) - if err != nil { - return err - } - iface.MTUs = mtu + mtu, err := NewMTU(name, req.Interface.Spec.MTU) + if err != nil { + return err } + iface.MTUs = mtu } // Make interface part of a bundle @@ -274,14 +283,6 @@ func (p *Provider) EnsureInterface(ctx context.Context, req *provider.EnsureInte iface.SubInterface.VlanIdentifier.VlanType = "vlan-type-dot1ad" } - if req.Interface.Spec.MTU != 0 { - mtu, err := NewMTU(name, req.Interface.Spec.MTU) - if err != nil { - return err - } - iface.MTUs = mtu - } - ipv4, err := NewIPv4(req.Interface.Spec.IPv4) if err != nil { return err @@ -303,8 +304,13 @@ func NewMTU(intName string, mtu int32) (MTUs, error) { message := "failed to extract MTU owner from interface name" + intName return MTUs{}, errors.New(message) } + + mtuValue := mtu + if mtu == 0 { + mtuValue = DefaultLinkMTU + } return MTUs{MTU: []MTU{{ - MTU: mtu, + MTU: mtuValue, Owner: string(owner), }}}, nil } @@ -329,6 +335,7 @@ func NewIPv4(ips *v1alpha1.InterfaceIPv4) (IPv4Network, error) { Netmask: netmask, }, }, + MTU: uint16(DefaultL3MTU), }, nil } diff --git a/internal/provider/cisco/iosxr/provider_test.go b/internal/provider/cisco/iosxr/provider_test.go index 49083fb8f..9c578c223 100644 --- a/internal/provider/cisco/iosxr/provider_test.go +++ b/internal/provider/cisco/iosxr/provider_test.go @@ -218,3 +218,50 @@ func Test_GetState(t *testing.T) { t.Fatalf("GetInterfaceStatus() expected OperStatus=true, got false") } } + +func Test_NewMTU(t *testing.T) { + tests := []struct { + name string + interfaceName string + mtu int32 + wantMTU int32 + wantErr bool + }{ + { + name: "empty MTU should return default link MTU", + interfaceName: "TwentyFiveGigE0/0/0/14", + mtu: 0, + wantMTU: DefaultLinkMTU, + wantErr: false, + }, + { + name: "jumbo MTU value of 9000", + interfaceName: "TwentyFiveGigE0/0/0/14", + mtu: 9000, + wantMTU: 9000, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := NewMTU(tt.interfaceName, tt.mtu) + if (err != nil) != tt.wantErr { + t.Errorf("NewMTU() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !tt.wantErr { + if len(got.MTU) == 0 { + t.Errorf("NewMTU() returned empty MTU slice") + return + } + if got.MTU[0].MTU != tt.wantMTU { + t.Errorf("NewMTU() MTU = %v, want %v", got.MTU[0].MTU, tt.wantMTU) + } + if got.MTU[0].Owner != "TwentyFiveGigE" { + t.Errorf("NewMTU() Owner = %v, want TwentyFiveGigE", got.MTU[0].Owner) + } + } + }) + } +}