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

Interface configuration #19

Merged
merged 4 commits into from
Apr 16, 2024
Merged

Interface configuration #19

merged 4 commits into from
Apr 16, 2024

Conversation

rwestphal
Copy link
Member

Implement the following configuration options:

  • Interface IPv4/IPv6 addresses
  • Interface admin status
  • Interface MTU
  • VLAN subinterfaces

Example configuration (CLI):

interfaces interface eth1.10
 type ietf-if-extensions:ethSubInterface
 encapsulation dot1q-vlan outer-tag vlan-id 10
 parent-interface eth1
 !
 ipv4 mtu 1500
 ipv4 address 172.16.1.1
  prefix-length 24
!

Example configuration (JSON):

{
  "ietf-interfaces:interfaces": {
    "interface": [
      {
        "name": "eth1.10",
        "type": "ietf-if-extensions:ethSubInterface",
        "ietf-if-extensions:encapsulation": {
          "ietf-if-vlan-encapsulation:dot1q-vlan": {
            "outer-tag": {
              "vlan-id": 10
            }
          }
        },
        "ietf-if-extensions:parent-interface": "eth1",
        "ietf-ip:ipv4": {
          "mtu": 1500,
          "address": [
            {
              "ip": "172.16.1.1",
              "prefix-length": 24
            }
          ]
        }
      }
    ]
  }
}

Verification (kernel):

# ip -4 addr show dev eth1.10
6: eth1.10@eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    inet 172.16.1.1/24 brd 172.16.1.255 scope global eth1.10
       valid_lft forever preferred_lft forever
# ip -d link show dev eth1.10
6: eth1.10@eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT group default qlen 1000
    link/ether e2:9f:e6:d8:35:bb brd ff:ff:ff:ff:ff:ff promiscuity 0  allmulti 0 minmtu 0 maxmtu 65535 
    vlan protocol 802.1Q id 10 <REORDER_HDR> addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 tso_max_size 524280 tso_max_segs 65535 gro_max_size 65536

If the interface isn't available at the OS-level upon configuration, the configuration will be automatically applied once the interface becomes available. Similarly, VLAN subinterfaces are automatically created as soon as their parent interface is available.

Closes #11

@rwestphal rwestphal requested a review from frederic-loui April 13, 2024 23:55
Copy link

github-actions bot commented Apr 13, 2024

Test Results

    1 files  ±0    15 suites  ±0   0s ⏱️ ±0s
376 tests ±0  374 ✔️ ±0  2 💤 ±0  0 ±0 
380 runs  ±0  378 ✔️ ±0  2 💤 ±0  0 ±0 

Results for commit fb06e35. ± Comparison against base commit 8ad0dd1.

♻️ This comment has been updated with latest results.

@frederic-loui
Copy link
Member

Thanks for this PR !
Testing it right now. As a first comment interface description needs to have its behavior adjusted:

r1# configure
r1(config)# interfaces interface eth1
r1(config/interface[eth1])# description my description
% unknown command
r1(config/interface[eth1])# description my_description
r1(config/interface[eth1])# show candidate
!
interfaces interface eth1
 description my_description
 type iana-if-type:ethernetCsmacd
 ipv4
 !
 ipv4 address 1.2.3.1
  prefix-length 24
!
r1(config/interface[eth1])#
r1(config/interface[eth1])# description my description
% unknown command
r1(config/interface[eth1])# description "my description"
% unknown command
r1(config/interface[eth1])# description 'my description'
% unknown command
r1(config/interface[

A decision needs to be made :

  1. Description after description keyword ends at the end of the line.
    -> description my super long description
  2. Description is surrounded by double quote.
    -> description "my personal description"
  3. Description is surrounded by double quote.
    -> description 'my personal description'
  4. Or no in description which is not ideal.
    -> Docs needs to be updated and explicitly mention this.

Should a maximum length need to be set for description field.

@rwestphal
Copy link
Member Author

@frederic-loui Yeah that's a known limitation. The CLI is pretty primitive at this point, it doesn't support string values containing spaces. I think it would be a good idea to file an issue in the holo-cli repo to keep track of that problem.

@frederic-loui
Copy link
Member

Second comment: (using containerlab environment compiled with PR#19)

  • R6[holo] --- R7[freertr]
  • Configure R6[holo-router] Eth3@R6:
!
interfaces interface eth3
 description r6->r7
 type iana-if-type:ethernetCsmacd
 ipv4
 !
 ipv4 address 1.2.3.2
  prefix-length 24
!
  • interface is effectively configured at holo:
460: eth3@if461: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
    link/ether aa:c1:ab:e5:89:fd brd ff:ff:ff:ff:ff:ff link-netnsid 2
    inet 1.2.3.1/24 brd 1.2.3.255 scope global eth3
       valid_lft forever preferred_lft forever
    inet6 fe80::a8c1:abff:fee5:89fd/64 scope link
       valid_lft forever preferred_lft forever

--> First thing we notice is that the default MTU is set to 9500.

  • we set MTU to 1500:
r6(config)# show running
!
interfaces interface eth3
 description r6->r7
 type iana-if-type:ethernetCsmacd
 ipv4
 ipv4 mtu 1500
 !
 ipv4 address 1.2.3.1
  prefix-length 24
!
  • ping from freertr->r6 (same setup was working using iproute2 command ipv4 address configuration)
rare#ping 1.2.3.1 vrf CORE
pinging 1.2.3.1, src=null, vrf=CORE, cnt=5, len=64, df=false, tim=1000, gap=0, ttl=255, tos=0, sgt=0, flow=0, fill=0, alrt=-1, sweep=false, multi=false
.!!!!
result=80.0%, recv/sent/lost/err=4/5/1/0, took 1004, min/avg/max/dev rtt=0/0.2/1/0.1, ttl 64/64.0/64/0.0, tos 0/0.0/0/0.0
rare#

--> Shouldn't interface MTU default set to 1500 ?

@rwestphal
Copy link
Member Author

A decision needs to be made :

  1. Description after description keyword ends at the end of the line.
    -> description my super long description
  2. Description is surrounded by double quote.
    -> description "my personal description"
  3. Description is surrounded by double quote.
    -> description 'my personal description'
  4. Or no in description which is not ideal.
    -> Docs needs to be updated and explicitly mention this.

I think option #1 would be the way to go, that's how the "description" command works on most vendors.

Should a maximum length need to be set for description field.

Indeed, it would be good to define a maximum length. That can be done either using a YANG deviation or northbound validation callback. I'll think about that.

Shouldn't interface MTU default set to 1500 ?

Currently, if an explicit MTU isn't specified, the one learned from the kernel will be used. I think this behavior is good to accommodate users using holod solely for routing, and handling interface configuration separately.

@frederic-loui
Copy link
Member

no interface <INTERFACE-NAME> does not deconfigure the ip address.

Steps to reproduce: (example)

  • configure interface eth3 with ip address 1.2.3.6/24 at holo level:
holo(config)# show running
!
interfaces interface eth3
 type iana-if-type:ethernetCsmacd
 ipv4
 !
 ipv4 address 1.2.3.6
  prefix-length 24
!
  • at linux host level:
root@rt6:/# ip a show eth3
573: eth3@if574: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9500 qdisc noqueue state UP group default
    link/ether aa:c1:ab:c7:c8:98 brd ff:ff:ff:ff:ff:ff link-netnsid 3
    inet 1.2.3.6/24 brd 1.2.3.255 scope global eth3
       valid_lft forever preferred_lft forever
    inet6 fe80::a8c1:abff:fec7:c898/64 scope link
       valid_lft forever preferred_lft forever
  • ping test check from adjacent node
rare#ping 1.2.3.6 vrf CORE
pinging 1.2.3.6, src=null, vrf=CORE, cnt=5, len=64, df=false, tim=1000, gap=0, ttl=255, tos=0, sgt=0, flow=0, fill=0, alrt=-1, sweep=false, multi=false
.!!!!
result=80.0%, recv/sent/lost/err=4/5/1/0, took 1018, min/avg/max/dev rtt=1/1.0/1/0.0, ttl 64/64.0/64/0.0, tos 0/0.0/0/0.0
rare#
  • deconfigure interface eth3
holo(config)# no interfaces interface eth3
holo(config)# commit
% configuration committed successfully
holo(config)# show running
!
holo(config)#
  • Problem: interface eth3 is still configured
573: eth3@if574: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9500 qdisc noqueue state UP group default
    link/ether aa:c1:ab:c7:c8:98 brd ff:ff:ff:ff:ff:ff link-netnsid 3
    inet 1.2.3.6/24 brd 1.2.3.255 scope global eth3
       valid_lft forever preferred_lft forever
    inet6 fe80::a8c1:abff:fec7:c898/64 scope link
       valid_lft forever preferred_lft forever
  • Adjacent router still pinging:
rare#ping 1.2.3.6 vrf CORE
pinging 1.2.3.6, src=null, vrf=CORE, cnt=5, len=64, df=false, tim=1000, gap=0, ttl=255, tos=0, sgt=0, flow=0, fill=0, alrt=-1, sweep=false, multi=false
!!!!!
result=100.0%, recv/sent/lost/err=5/5/0/0, took 4, min/avg/max/dev rtt=0/0.6/1/0.2, ttl 64/64.0/64/0.0, tos 0/0.0/0/0.0
rare#

Example configuration (CLI):
```
interfaces interface eth-rt2
 type iana-if-type:ethernetCsmacd
 !
 ipv4 address 10.0.1.1
  prefix-length 24
!
```

Example configuration (JSON):
```
{
  "ietf-interfaces:interfaces": {
    "interface": [
      {
        "name": "eth-rt2",
        "type": "iana-if-type:ethernetCsmacd",
        "ietf-ip:ipv4": {
          "address": [
            {
              "ip": "10.0.1.1",
              "prefix-length": 24
            }
          ]
        }
      }
    }
  }
}
```

This commit also includes a refactoring to allow configuring interfaces
before they exist at the OS-level. Then, once the interface becomes
available, its pre-existing configuration is applied.

Signed-off-by: Renato Westphal <[email protected]>
Of particular note, the ietf-interfaces module has different leafs to
configure MTU for IPv4 and IPv6, but Linux supports a single MTU for
all L3 protocols. As such, a validation callback was added to ensure
the IPv4 and IPv6 MTUs are the same when both are configured.

Signed-off-by: Renato Westphal <[email protected]>
Example configuration (CLI):
```
[snip]
interfaces interface eth-rt2.10
 type ietf-if-extensions:ethSubInterface
 encapsulation dot1q-vlan outer-tag vlan-id 10
 parent-interface eth-rt2
!
```

Example configuration (JSON):
```
{
  "ietf-interfaces:interfaces": {
    "interface": [
      [snip]
      {
        "name": "eth-rt2.10",
        "type": "ietf-if-extensions:ethSubInterface",
        "ietf-if-extensions:encapsulation": {
          "ietf-if-vlan-encapsulation:dot1q-vlan": {
            "outer-tag": {
              "vlan-id": 10
            }
          }
        },
        "ietf-if-extensions:parent-interface": "eth-rt2"
      }
    ]
  }
}
```

Signed-off-by: Renato Westphal <[email protected]>
@rwestphal rwestphal force-pushed the interface-properties branch from 39dbaa5 to fb06e35 Compare April 15, 2024 20:25
@frederic-loui
Copy link
Member

frederic-loui commented Apr 16, 2024

  • Configuring IP
holo(config)# show running
!
interfaces interface eth3
 type iana-if-type:ethernetCsmacd
 ipv4
 !
 ipv4 address 1.2.3.6
  prefix-length 24
!
  • Before removing IP
655: eth3@if656: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9500 qdisc noqueue state UP group default
    link/ether aa:c1:ab:7a:4e:84 brd ff:ff:ff:ff:ff:ff link-netnsid 1
    inet 1.2.3.6/24 brd 1.2.3.255 scope global eth3
       valid_lft forever preferred_lft forever
    inet6 fe80::a8c1:abff:fe7a:4e84/64 scope link
       valid_lft forever preferred_lft forever
  • ping OK !
rare#ping 1.2.3.6 vrf CORE
pinging 1.2.3.6, src=null, vrf=CORE, cnt=5, len=64, df=false, tim=1000, gap=0, ttl=255, tos=0, sgt=0, flow=0, fill=0, alrt=-1, sweep=false, multi=false
.!!!!
result=80.0%, recv/sent/lost/err=4/5/1/0, took 1032, min/avg/max/dev rtt=0/1.0/2/0.4, ttl 64/64.0/64/0.0, tos 0/0.0/0/0.0
rare#
  • Removing IP
holo(config)# no interfaces interface eth3
holo(config)# commit
% configuration committed successfully
holo(config)# show running
!
  • Check eth3 from linux host perspective
root@rt6:/# ip add show eth3
655: eth3@if656: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9500 qdisc noqueue state UP group default
    link/ether aa:c1:ab:7a:4e:84 brd ff:ff:ff:ff:ff:ff link-netnsid 1
    inet6 fe80::a8c1:abff:fe7a:4e84/64 scope link
       valid_lft forever preferred_lft forever
root@rt6:/#

-> Test have also been done with IPv4 + 802.1q VLAN and IPv6 + 802.1q VLAN and passed !

@frederic-loui
Copy link
Member

frederic-loui commented Apr 16, 2024

Is this a desirable behavior ?

Other vendor tends:

  • to replace existing IPv4
  • or consider primary, secondary IPv4 address
holo(config/interface[eth3.666])# show running
!
interfaces interface eth3
 type iana-if-type:ethernetCsmacd
!
interfaces interface eth3.666
 type ietf-if-extensions:ethSubInterface
 parent-interface eth3
 ipv4
 !
 ipv4 address 1.2.3.6
  prefix-length 24
 !
 ipv4 address 5.6.7.6
  prefix-length 24
 !
 ipv4 address 8.9.10.6
  prefix-length 24
 ipv6
 !
 ipv6 address fd00:2200::6
  prefix-length 120
!
2: eth3.666@eth3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 9500 qdisc noqueue state UP group default qlen 1000
    link/ether aa:c1:ab:7a:4e:84 brd ff:ff:ff:ff:ff:ff
    inet 1.2.3.6/24 brd 1.2.3.255 scope global eth3.666
       valid_lft forever preferred_lft forever
    inet 5.6.7.6/24 brd 5.6.7.255 scope global eth3.666
       valid_lft forever preferred_lft forever
    inet 8.9.10.6/24 brd 8.9.10.255 scope global eth3.666
       valid_lft forever preferred_lft forever
    inet6 fd00:2200::6/120 scope global
       valid_lft forever preferred_lft forever
    inet6 fe80::a8c1:abff:fe7a:4e84/64 scope link
       valid_lft forever preferred_lft forever
root@rt6:/#

@rwestphal
Copy link
Member Author

@frederic-loui Yes, that's the desired behavior. ietf-ip models the list of IP addresses using a simple list, without distinguishing between primary and secondary addresses. The Linux kernel doesn't have the concept of primary and secondary addresses either.

All protocol implementations are prepared to handle interfaces with multiple addresses. In the case of OSPFv2, the first received address will be considered to be the primary address used for protocol operation.

@frederic-loui
Copy link
Member

frederic-loui commented Apr 16, 2024

all lgtm !

PS: Please let me know when/if holo-yang would switch to a new version number (0.4.1 ?) so that I can bump holo-cli dependency.

@frederic-loui frederic-loui merged commit 75d177d into master Apr 16, 2024
9 checks passed
@rwestphal
Copy link
Member Author

all lgtm !

PS: Please let me know when/if holo-yang would switch to a new version number (0.4.1 ?) so that I can bump holo-cli dependency.

Thanks, I had forgotten about that. Bumped to 0.4.1! It shouldn't be necessary to update holo-cli, as it currently depends on holo-yang = "0.4.0", which will include minor version updates such as 0.4.1.

@rwestphal rwestphal deleted the interface-properties branch July 6, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[holo-cli] possibility to configure interface properties
2 participants