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

T5654: policy: move local-route[6] to ip-rule[6] #3781

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

Conversation

nicolas-fort
Copy link
Contributor

Change Summary

Move policy [local-route | local-route6] to policy [ip-rule | ip-rule6]

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)

Related PR(s)

Component(s) name

policy

Proposed changes

How to test

Config valid on 1.3

vyos@upgrade-test:~$ show config commands | grep local
set policy local-route rule 11 inbound-interface 'eth2'
set policy local-route rule 11 set table '11'
set policy local-route rule 11 source '203.0.113.0/24'
set policy local-route6 rule 66 fwmark '1001'
set policy local-route6 rule 66 inbound-interface 'eth3'
set policy local-route6 rule 66 set table '66'
set policy local-route6 rule 66 source '2001:db8::66'
vyos@upgrade-test:~$ 
vyos@upgrade-test:~$ sudo ip rule
0:	from all lookup local
11:	from 203.0.113.0/24 iif eth2 lookup 11
32766:	from all lookup main
32767:	from all lookup default
vyos@upgrade-test:~$ sudo ip -6 rule
0:	from all lookup local
66:	from 2001:db8::66 fwmark 0x3e9 iif eth3 lookup 66
32766:	from all lookup main
vyos@upgrade-test:~$

And after migration

vyos@upgrade-test:~$ show config comm | grep ip
set policy ip-rule rule 11 inbound-interface 'eth2'
set policy ip-rule rule 11 set table '11'
set policy ip-rule rule 11 source address '203.0.113.0/24'
set policy ip-rule6 rule 66 fwmark '1001'
set policy ip-rule6 rule 66 inbound-interface 'eth3'
set policy ip-rule6 rule 66 set table '66'
set policy ip-rule6 rule 66 source address '2001:db8::66'
set system conntrack modules sip
vyos@upgrade-test:~$ sudo ip rule
0:	from all lookup local
11:	from 203.0.113.0/24 iif eth2 lookup 11
32766:	from all lookup main
32767:	from all lookup default
vyos@upgrade-test:~$ sudo ip -6 rule
0:	from all lookup local
66:	from 2001:db8::66 fwmark 0x3e9 iif eth3 lookup 66
32766:	from all lookup main
vyos@upgrade-test:~$ 

Smoketest result

test_policy.py ==> OK

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

Copy link

github-actions bot commented Jul 4, 2024

👍
No issues in PR Title / Commit Title

Copy link

github-actions bot commented Jul 4, 2024

CI integration 👍 passed!

Details

CI logs

  • 👍 passed CLI Smoketests returned:
  • 👍 passed Config tests returned:
  • 👍 passed RAID1 tests returned:

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this may reduce the confusion. But if we are opening the can of worms and writing a migration script anyway, maybe we should rethink all PBR syntax and see what else we can change?

We may want to change the approach to unify the "local-route" and "route" under a single subtree.

@nicolas-fort
Copy link
Contributor Author

Marking as draft as we need to discuss this internally, in order to analyze if more changes should be included regarding PBR and ip-rules

@nicolas-fort nicolas-fort marked this pull request as draft July 8, 2024 15:14
Copy link

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

Copy link

This PR is stale because it has been open 30 days with no activity. The PR will be reviewed by a maintainer and may be closed

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.

3 participants