pbr: T6430: Local IP rules targeting VRFs by name as well as route table IDs #3938

talmakion · 2024-08-03T10:02:54Z

Change Summary

My previous PR for simply extending the table ID range was rejected, so I've prepared another PR set closer to my own use case for this - "policy based route leaking" between VRFs. It may or may not line up with Bernhard's intention from the original ticket.

This PR extends policy local-route syntax to allow set vrf as well as set table.

The main PR for firewall-driven forward PBR has already been merged, this one covers local ip rule manipulation.

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/T6430

Related PR(s)

Component(s) name

pbr

Proposed changes

This is the policy local-route* part of T6430, manipulating ip rules, another PR covers firewall-backed policy route* for similar functionality
Local PBR (policy local-route*) can only target table IDs up to 200 and the previous PR to extend the range was rejected
PBR with this PR can now also target VRFs directly by name, working around targeting problems for VRF table IDs outside the overlapping 100-200 range
Validation ensures rules can't target both a table ID and a VRF name
Unlike the first PR, this one depends on ip rule aliasing for VRF symbolic names rather than looking up and manipulating table IDs from the conf script
Added a set of smoketests for policy local-route functionality
Relocated TestPolicyRoute.verify_rules() into VyOSUnitTestSHIM.TestCase, extended to allow lookups in other address families (IPv6 in the new tests). verify_rules() is used by original pbr and new lpbr smoketests in this PR.

How to test

Rules apply OK:

set policy local-route rule 4 inbound-interface eth0
set policy local-route rule 4 protocol udp
set policy local-route rule 4 fwmark 111
set policy local-route rule 4 destination address 198.51.100.0/24
set policy local-route rule 4 destination port 111
set policy local-route rule 4 source address 198.51.100.1
set policy local-route rule 4 source port 443
set policy local-route rule 4 set vrf MGT
set policy local-route6 rule 6 inbound-interface eth0
set policy local-route6 rule 6 protocol udp
set policy local-route6 rule 6 fwmark 111
set policy local-route6 rule 6 destination port 111
set policy local-route6 rule 6 source port 443
set policy local-route6 rule 6 set vrf MGT
set policy local-route6 rule 6 source address 2001:db8::1
set policy local-route6 rule 6 destination address 2001:db8::/64

# commit
[edit]
vyos@TEST-VYOS-LEFT#  ip rule show
4:      from 198.51.100.1 to 198.51.100.0/24 fwmark 0x6f iif eth0 ipproto udp sport 443 dport 111 lookup MGT
220:    from all lookup 220
1000:   from all lookup [l3mdev-table]
2000:   from all lookup [l3mdev-table] unreachable
32765:  from all lookup local
32766:  from all lookup main
32767:  from all lookup default
[edit]
vyos@TEST-VYOS-LEFT#  ip -6 rule show
6:      from 2001:db8::1 to 2001:db8::/64 fwmark 0x6f iif eth0 ipproto udp sport 443 dport 111 lookup MGT
220:    from all lookup 220
1000:   from all lookup [l3mdev-table]
2000:   from all lookup [l3mdev-table] unreachable
32765:  from all lookup local
32766:  from all lookup main
[edit]

Smoketest result

Since I changed the policy route smoketests and the parent shim, I ran through both smoketests to double check:

$ python3 /usr/libexec/vyos/tests/smoke/cli/test_policy_route.py 
test_pbr_group (__main__.TestPolicyRoute.test_pbr_group) ... ok
test_pbr_mark (__main__.TestPolicyRoute.test_pbr_mark) ... ok
test_pbr_mark_connection (__main__.TestPolicyRoute.test_pbr_mark_connection) ... ok
test_pbr_matching_criteria (__main__.TestPolicyRoute.test_pbr_matching_criteria) ... ok
test_pbr_table (__main__.TestPolicyRoute.test_pbr_table) ... ok
test_pbr_vrf (__main__.TestPolicyRoute.test_pbr_vrf) ... ok

----------------------------------------------------------------------
Ran 6 tests in 44.617s

OK
$ python3 /usr/libexec/vyos/tests/smoke/cli/test_policy_local-route.py 
test_local_pbr_matching_criteria (__main__.TestPolicyLocalRoute.test_local_pbr_matching_criteria) ... ok
test_local_pbr_rule_changes (__main__.TestPolicyLocalRoute.test_local_pbr_rule_changes) ... ok
test_local_pbr_rule_removal (__main__.TestPolicyLocalRoute.test_local_pbr_rule_removal) ... ok
test_local_pbr_target_vrf (__main__.TestPolicyLocalRoute.test_local_pbr_target_vrf) ... ok

----------------------------------------------------------------------
Ran 4 tests in 28.064s

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

github-actions · 2024-08-03T10:03:14Z

👍
No issues in PR Title / Commit Title

github-actions · 2024-08-03T10:03:32Z

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

talmakion · 2024-09-03T18:16:02Z

Rebased on current, it's been a little while...

Basic functional checks on policy local-route look good and smoketests still running OK:

$ python3 /usr/libexec/vyos/tests/smoke/cli/test_policy_local-route.py 
test_local_pbr_matching_criteria (__main__.TestPolicyLocalRoute.test_local_pbr_matching_criteria) ... ok
test_local_pbr_rule_changes (__main__.TestPolicyLocalRoute.test_local_pbr_rule_changes) ... ok
test_local_pbr_rule_removal (__main__.TestPolicyLocalRoute.test_local_pbr_rule_removal) ... ok
test_local_pbr_target_vrf (__main__.TestPolicyLocalRoute.test_local_pbr_target_vrf) ... ok

----------------------------------------------------------------------
Ran 4 tests in 27.523s

OK
$ python3 /usr/libexec/vyos/tests/smoke/cli/test_policy_route.py 
test_pbr_group (__main__.TestPolicyRoute.test_pbr_group) ... ok
test_pbr_mark (__main__.TestPolicyRoute.test_pbr_mark) ... ok
test_pbr_mark_connection (__main__.TestPolicyRoute.test_pbr_mark_connection) ... ok
test_pbr_matching_criteria (__main__.TestPolicyRoute.test_pbr_matching_criteria) ... ok
test_pbr_table (__main__.TestPolicyRoute.test_pbr_table) ... ok
test_pbr_vrf (__main__.TestPolicyRoute.test_pbr_vrf) ... ok

----------------------------------------------------------------------
Ran 6 tests in 44.219s

OK

sever-sever · 2024-09-04T05:51:04Z

I lost access to the router with this config:

set policy local-route rule 100 destination
set policy local-route rule 100 set vrf 'mgmt'
set policy local-route rule 100 source address '192.0.2.1'

set policy local-route rule 123 set vrf 'default'
set policy local-route rule 123 source address '0.0.0.0/0'
set vrf name mgmt table '123'

Generated rules:

vyos@r14:~$ ip rule show
100:	from 192.0.2.1 lookup mgmt
123:	from all lookup main
1000:	from all lookup [l3mdev-table]
2000:	from all lookup [l3mdev-table] unreachable
32765:	from all lookup local
32766:	from all lookup main
32767:	from all lookup default
vyos@r14:~$

Check:

vyos@r14:~$ ping 1.1.1.1
PING 1.1.1.1 (1.1.1.1) 56(84) bytes of data.
^C
--- 1.1.1.1 ping statistics ---
3 packets transmitted, 0 received, 100% packet loss, time 2074ms

So need to be careful with VRF default and to fix this I have to use lookup local (122) before

set policy local-route rule 100 destination
set policy local-route rule 100 set vrf 'mgmt'
set policy local-route rule 100 source address '192.0.2.1'
set policy local-route rule 122 set table 'local'
set policy local-route rule 122 source address '0.0.0.0/0'
set policy local-route rule 123 set vrf 'default'
set policy local-route rule 123 source address '0.0.0.0/0'


vyos@r14# run ping 1.1.1.1 count 1
PING 1.1.1.1 (1.1.1.1) 56(84) bytes of data.
64 bytes from 1.1.1.1: icmp_seq=1 ttl=58 time=8.77 ms

--- 1.1.1.1 ping statistics ---
1 packets transmitted, 1 received, 0% packet loss, time 0ms
rtt min/avg/max/mdev = 8.770/8.770/8.770/0.000 ms

sever-sever

Checked, works fine in my tests

talmakion · 2024-09-04T09:26:38Z

set policy local-route rule 123 set vrf 'default'
set policy local-route rule 123 source address '0.0.0.0/0'

I'd say this was always a bit dangerous in local-route, would it be worthwhile me adding a warning for rules matching v4/v6 default routes and loopbacks?

dmbaturin

I don't see any arguments against this, but this needs more reviews, of course.

HollyGurza · 2024-09-27T13:43:33Z

interface-definitions/policy_local-route.xml.in

@@ -113,6 +131,24 @@
                      </completionHelp>
                    </properties>
                  </leafNode>
+                  <leafNode name="vrf">


code is duplicate, can we move this to some file and use #include

* This is the `policy local-route*` part of T6430, manipulating ip rules, another PR covers firewall-backed `policy route*` for similar functionality * Local PBR (policy local-route*) can only target table IDs up to 200 and the previous PR to extend the range was rejected * PBR with this PR can now also target VRFs directly by name, working around targeting problems for VRF table IDs outside the overlapping 100-200 range * Validation ensures rules can't target both a table ID and a VRF name (internally they are handled the same) * Relocated TestPolicyRoute.verify_rules() into VyOSUnitTestSHIM.TestCase, extended to allow lookups in other address families (IPv6 in the new tests). verify_rules() is used by original pbr and new lpbr smoketests in this PR.

talmakion requested a review from a team as a code owner August 3, 2024 10:02

talmakion requested review from dmbaturin, sarthurdev, zdc, jestabro, sever-sever and fett0 August 3, 2024 10:02

github-actions bot added the current label Aug 3, 2024

github-actions bot assigned talmakion Aug 3, 2024

talmakion force-pushed the feature/T6430-local-pbr branch from d7dabbb to 3058707 Compare September 3, 2024 18:10

sever-sever approved these changes Sep 4, 2024

View reviewed changes

dmbaturin approved these changes Sep 11, 2024

View reviewed changes

HollyGurza reviewed Sep 27, 2024

View reviewed changes

talmakion and others added 2 commits October 7, 2024 17:15

xml: T6430: add re-usable vrf CLI node for firewall and pbr

9c291d1

c-po force-pushed the feature/T6430-local-pbr branch from 3058707 to 9c291d1 Compare October 7, 2024 15:19

c-po requested a review from HollyGurza October 7, 2024 15:19

c-po enabled auto-merge October 7, 2024 15:19

c-po merged commit e8c65fa into vyos:current Oct 7, 2024
15 of 17 checks passed

vyos deleted a comment from github-actions bot Oct 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pbr: T6430: Local IP rules targeting VRFs by name as well as route table IDs #3938

pbr: T6430: Local IP rules targeting VRFs by name as well as route table IDs #3938

talmakion commented Aug 3, 2024 •

edited

Loading

github-actions bot commented Aug 3, 2024 •

edited

Loading

github-actions bot commented Aug 3, 2024 •

edited

Loading

talmakion commented Sep 3, 2024

sever-sever commented Sep 4, 2024

sever-sever left a comment •

edited

Loading

talmakion commented Sep 4, 2024 •

edited

Loading

dmbaturin left a comment

HollyGurza Sep 27, 2024

pbr: T6430: Local IP rules targeting VRFs by name as well as route table IDs #3938

pbr: T6430: Local IP rules targeting VRFs by name as well as route table IDs #3938

Conversation

talmakion commented Aug 3, 2024 • edited Loading

Change Summary

Types of changes

Related Task(s)

Related PR(s)

Component(s) name

Proposed changes

How to test

Smoketest result

Checklist:

github-actions bot commented Aug 3, 2024 • edited Loading

github-actions bot commented Aug 3, 2024 • edited Loading

talmakion commented Sep 3, 2024

sever-sever commented Sep 4, 2024

sever-sever left a comment • edited Loading

Choose a reason for hiding this comment

talmakion commented Sep 4, 2024 • edited Loading

dmbaturin left a comment

Choose a reason for hiding this comment

HollyGurza Sep 27, 2024

Choose a reason for hiding this comment

talmakion commented Aug 3, 2024 •

edited

Loading

github-actions bot commented Aug 3, 2024 •

edited

Loading

github-actions bot commented Aug 3, 2024 •

edited

Loading

sever-sever left a comment •

edited

Loading

talmakion commented Sep 4, 2024 •

edited

Loading