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

T6750: Add initial Segment Routing Traffic Engineering #4166

Draft
wants to merge 1 commit into
base: current
Choose a base branch
from

Conversation

Cheeze-It
Copy link
Contributor

@Cheeze-It Cheeze-It commented Oct 19, 2024

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T6750

Related PR(s)

None to my knowledge.

Component(s) name

mpls, segment routing, pathd

Proposed changes

The changes that I am adding here are adding the segment routing traffic engineering config
options within FRR. They are however very experimental as there are seemingly some problems
with using them. But this will add initial support for those configs in FRR.

How to test

Adding segment routing database import:

vyos@vyos# set protocols segment-routing traffic-engineering database-import-protocol isis
[edit]
vyos@vyos# compare
[protocols]
+ segment-routing {
+     traffic-engineering {
+         database-import-protocol {
+             isis
+         }
+     }
+ }

[edit]
vyos@vyos# commit
[edit]
vyos@vyos# exit
Warning: configuration changes have not been saved.
exit
vyos@vyos:~$ vtysh -c "show run"
Building configuration...

Current configuration:
!
frr version 9.1.2
frr defaults traditional
hostname vyos
log syslog
log facility local7
service integrated-vtysh-config
!
ip route 0.0.0.0/0 10.0.0.65
!
segment-routing
 traffic-eng
  mpls-te on
  mpls-te import isis
 exit
exit
!
rpki
exit
!
end
Adding a segment list here:

[edit]
vyos@vyos# set protocols segment-routing traffic-engineering segment-list testing-segment-list index value 0 mpls label 1000
[edit]
vyos@vyos# set protocols segment-routing traffic-engineering segment-list testing-segment-list index value 0 nai adjacency ipv4 source-identifier 192.168.0.1
[edit]
vyos@vyos# set protocols segment-routing traffic-engineering segment-list testing-segment-list index value 0 nai adjacency ipv4 destination-identifier 192.168.0.2
[edit]
vyos@vyos#
[edit]
vyos@vyos# compare
[protocols segment-routing traffic-engineering]
+ segment-list testing-segment-list {
+     index {
+         value 0 {
+             mpls {
+                 label "1000"
+             }
+             nai {
+                 adjacency {
+                     ipv4 {
+                         destination-identifier "192.168.0.2"
+                         source-identifier "192.168.0.1"
+                     }
+                 }
+             }
+         }
+     }
+ }

[edit]
vyos@vyos# commit
[edit]
vyos@vyos# exit
Warning: configuration changes have not been saved.
exit
vyos@vyos:~$ vtysh -c "show run"
Building configuration...

Current configuration:
!
frr version 9.1.2
frr defaults traditional
hostname vyos
log syslog
log facility local7
service integrated-vtysh-config
!
ip route 0.0.0.0/0 10.0.0.65
!
segment-routing
 traffic-eng
  mpls-te on
  mpls-te import isis
  segment-list testing-segment-list
   index 0 mpls label 1000 nai adjacency 192.168.0.1 192.168.0.2
  exit
 exit
exit
!
rpki
exit
!
end

Smoketest result

vyos@vyos:~$
vyos@vyos:~$ /usr/libexec/vyos/tests/smoke/cli/test_protocols_segment-routing.py
test_segment_routing_01_srv6 (__main__.TestProtocolsSegmentRouting.test_segment_routing_01_srv6) ... ok
test_segment_routing_02_srv6_sysctl (__main__.TestProtocolsSegmentRouting.test_segment_routing_02_srv6_sysctl) ... ok
test_segment_routing_03_srte_database (__main__.TestProtocolsSegmentRouting.test_segment_routing_03_srte_database) ... ok
test_segment_routing_04_mpls_label (__main__.TestProtocolsSegmentRouting.test_segment_routing_04_mpls_label) ... ok
test_segment_routing_05_mpls_label_and_adjacency (__main__.TestProtocolsSegmentRouting.test_segment_routing_05_mpls_label_and_adjacency) ... ok
test_segment_routing_06_mpls_label_and_prefix (__main__.TestProtocolsSegmentRouting.test_segment_routing_06_mpls_label_and_prefix) ... ok

----------------------------------------------------------------------
Ran 6 tests in 26.516s

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

I have not updated documentation yet but will do if PR is merged.

Copy link

github-actions bot commented Oct 19, 2024

👍
No issues in PR Title / Commit Title

Copy link

github-actions bot commented Oct 19, 2024

✅ No issues found in unused-imports check.. Please refer the workflow run

@Cheeze-It
Copy link
Contributor Author

Cheeze-It commented Oct 19, 2024

So I found a few....I guess bugs when it comes to this PR and per this part of FRR. So, normally when we delete the dict in the XML and Jinja2, it also deletes said configuration from FRR. But it seems in this case it does NOT which sucks. Here's an example:

Adding configuration is fine as can be seen in the initial PR notes. But deleting it doesn't work. For example:

vyos@vyos:~$ configure
[edit]
vyos@vyos# delete protocols segment-routing
[edit]
vyos@vyos# compare
[protocols]
- segment-routing {
-     traffic-engineering {
-         database-import-protocol {
-             isis
-         }
-         segment-list testing-segment-list {
-             index {
-                 value 0 {
-                     mpls {
-                         label "1000"
-                     }
-                     nai {
-                         adjacency {
-                             ipv4 {
-                                 destination-identifier "192.168.0.2"
-                                 source-identifier "192.168.0.1"
-                             }
-                         }
-                     }
-                 }
-             }
-         }
-     }
- }

[edit]
vyos@vyos# commit
[edit]
vyos@vyos# exit

But then when one checks FRR we get:

vyos# do show running-config
Building configuration...

Current configuration:
!
frr version 9.1.2
frr defaults traditional
hostname vyos
log syslog
log facility local7
service integrated-vtysh-config
!
ip route 0.0.0.0/0 10.0.0.65
!
segment-routing
 traffic-eng
  mpls-te on
  mpls-te import isis
  segment-list testing-segment-list
   index 0 mpls label 1000 nai adjacency 192.168.0.1 192.168.0.2
  exit
 exit
exit
!
rpki
exit
!
end

One has to manually remove the configuration:

vyos# configure terminal
vyos(config)# segment-routing
vyos(config-sr)# traffic-eng
vyos(config-sr-te)# no mpls-te
vyos(config-sr-te)# no segment-list testing-segment-list
vyos(config-sr-te)# end
vyos# show running-config
Building configuration...

Current configuration:
!
frr version 9.1.2
frr defaults traditional
hostname vyos
log syslog
log facility local7
service integrated-vtysh-config
!
ip route 0.0.0.0/0 10.0.0.65
!
segment-routing
 traffic-eng
 exit
exit
!
rpki
exit
!
end
vyos# exit

This really sucks, and I think this might be an FRR bug with frr-reload().

@Cheeze-It
Copy link
Contributor Author

A second possible bug I found might be with FRR CLI itself, and I think this might be affecting the first bug I found above.

So after running the smoketest we get a config that looks like this, even though it should be deleted:

vyos(config-sr-te)# do show running-config
Building configuration...

Current configuration:
!
frr version 9.1.2
frr defaults traditional
hostname vyos
log syslog
log facility local7
service integrated-vtysh-config
!
ip route 0.0.0.0/0 10.0.0.65
!
segment-routing
 traffic-eng
  mpls-te on
  mpls-te import isis
  segment-list smoketest-mpls-only-segment-list
   index 0 mpls label 500
  exit
  segment-list smoketest-mpls-with-adjacency-segment-list
   index 0 mpls label 1000 nai adjacency 2003::1 2003::2
  exit
  segment-list smoketest-mpls-with-prefix-segment-list
   index 0 mpls label 1500 nai prefix 2003::/120 algorithm 1
  exit
 exit
exit
!
rpki
exit
!
end

So when we try to delete stuff say entire stanzas (like say you want to delete all SR-TE at once), we get the following problem:

vyos(config-sr)# no
  output  Direct vtysh output to file
  srv6    Segment Routing SRv6

When we try to go into traffic engineering though we can:

vyos(config-sr)# traffic-eng
  <cr>

So to actually delete the segment lists, or the pathd segment lists we need to do the following:

vyos(config-sr)#
  end          End current mode and change to enable mode
  exit         Exit current mode and down to previous mode
  find         Find CLI command matching a regular expression
  list         Print command list
  no           Negate a command or set its defaults
  output       Direct vtysh output to file
  quit         Exit current mode and down to previous mode
  srv6         Segment-Routing SRv6 configuration
  traffic-eng  Configure SR traffic engineering

vyos(config-sr)# traffic-eng
vyos(config-sr-te)# no segment-list smoketest-mpls-only-segment-list
vyos(config-sr-te)# no segment-list smoketest-mpls-with-adjacency-segment-list
vyos(config-sr-te)# no segment-list smoketest-mpls-with-prefix-segment-list
vyos(config-sr-te)#
vyos(config-sr-te)# do show running-config
Building configuration...

Current configuration:
!
frr version 9.1.2
frr defaults traditional
hostname vyos
log syslog
log facility local7
service integrated-vtysh-config
!
ip route 0.0.0.0/0 10.0.0.65
!
segment-routing
 traffic-eng
  mpls-te on
  mpls-te import isis
 exit
exit
!
rpki
exit
!
end

So this definitely is another problem when trying to remove configurations from Segment Routing Traffic Engineering.

@Cheeze-It
Copy link
Contributor Author

I think we should hold this PR until FRR is able to fix the problems on their end first. Which makes me sad, but I think this is just the part of the experimental nature of all of this.

@Cheeze-It Cheeze-It changed the title T6750: Adding initial Segment Routing Traffic Engineering configurations T6750: Add initial Segment Routing Traffic Engineering Oct 19, 2024
Copy link

CI integration ❌ failed!

Details

CI logs

  • CLI Smoketests (no interfaces) ❌ failed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests ❌ failed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@sever-sever
Copy link
Member

sever-sever commented Oct 20, 2024

Can you set PR to draft until it will be ready?

@Cheeze-It Cheeze-It marked this pull request as draft October 20, 2024 05:04
@Cheeze-It
Copy link
Contributor Author

Can you set PR to draft until it will be ready?

Done :)

We can hold it until we move to FRR 10 and we can get FRR to look at said bugs/behaviors.

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants