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

T5333: Set prefix UD for PBR generated user-defined chain names #2069

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

sever-sever
Copy link
Member

Change Summary

We cannot use some specific names like POSTROUTING/PREROUTING as for PBR they overlap with VyOS defined chains
Chains autoconfigured by VyOS itself:

  chain VYOS_PBR_PREROUTING
  chain VYOS_PBR_POSTROUTING

If we try to use the chain name POSTROUTING it generates 2 chains with the same name
chain VYOS_PBR_POSTROUTING one is autoconfigured and the second is defined by the user

set policy route POSTROUTING rule 100

Add the user-defined (UD) prefix to separate user-defined names That allow to use of any user-defined names

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)

Component(s) name

pbr

Proposed changes

How to test

VyOS config:

set policy route POSTROUTING interface "eth1"
set policy route POSTROUTING rule 100 destination address '192.0.2.2'
set policy route POSTROUTING rule 100 set mark '200'
set policy route POSTROUTING rule 201 protocol 'icmp'
set policy route POSTROUTING rule 201 set mark '200'
set policy route POSTROUTING rule 201 source address '192.168.0.0/24'
set policy route POSTROUTING rule 300 destination address '192.0.2.5'
set policy route POSTROUTING rule 300 destination port '8888'
set policy route POSTROUTING rule 300 protocol 'tcp_udp'
set policy route POSTROUTING rule 300 set mark '300'

Before fix, chain VYOS_PBR_POSTROUTING defined 2 times it causes nft can't apply this rules:


vyos@r14# commit
[ policy route POSTROUTING ]
Failed to apply policy based routing

[[policy route POSTROUTING]] failed
Commit failed
[edit]
vyos@r14#
table ip vyos_mangle {
    chain VYOS_PBR_PREROUTING {
        type filter hook prerouting priority -150; policy accept;
        iifname { eth1 } counter jump VYOS_PBR_POSTROUTING
    }

    chain VYOS_PBR_POSTROUTING {
        type filter hook postrouting priority -150; policy accept;
    }

    chain VYOS_PBR_POSTROUTING {
        ip daddr 192.0.2.2 counter meta mark set 200 return comment "POSTROUTING-100"
        meta l4proto  icmp ip saddr 192.168.0.0/24 counter meta mark set 200 return comment "POSTROUTING-201"
        meta l4proto  {tcp, udp} ip daddr 192.0.2.5 th dport {8888} counter meta mark set 300 return comment "POSTROUTING-300"
    }


}


vyos@r14# sudo nft -f /run/nftables_policy.conf 
/run/nftables_policy.conf:9:39-58: Error: Could not process rule: Operation not supported
        iifname { eth1 } counter jump VYOS_PBR_POSTROUTING
                                      ^^^^^^^^^^^^^^^^^^^^
[edit]
vyos@r14#

After the fix we have auto-generated VYOS_PBR_POSTROUTING and user-defined VYOS_PBR_UD_POSTROUTING:

vyos@r14# sudo nft list table ip vyos_mangle
table ip vyos_mangle {
	chain VYOS_PBR_PREROUTING {
		type filter hook prerouting priority mangle; policy accept;
		iifname "eth1" counter packets 0 bytes 0 jump VYOS_PBR_UD_POSTROUTING
	}

	chain VYOS_PBR_POSTROUTING {
		type filter hook postrouting priority mangle; policy accept;
	}

	chain VYOS_PBR_UD_POSTROUTING {
		ip daddr 192.0.2.2 counter packets 0 bytes 0 meta mark set 0x000000c8 return comment "POSTROUTING-100"
		meta l4proto icmp ip saddr 192.168.0.0/24 counter packets 0 bytes 0 meta mark set 0x000000c8 return comment "POSTROUTING-201"
		meta l4proto { tcp, udp } ip daddr 192.0.2.5 th dport 8888 counter packets 0 bytes 0 meta mark set 0x0000012c return comment "POSTROUTING-300"
	}
}

Smoketest:

vyos@r14:~$ /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

----------------------------------------------------------------------
Ran 5 tests in 20.666s

OK
vyos@r14:~`

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

We cannot use some specific names like POSTROUTING/PREROUTING
as for PBR they overlaps with VyOS defined chains
Chains aftoconfigured by VyOS itself:
  chain VYOS_PBR_PREROUTING
  chain VYOS_PBR_POSTROUTING

If we try to use chain name "POSTROUTING" it generates 2 chains
with the same name "chain VYOS_PBR_POSTROUTING" one is
autoconfigured and the second defined by user

  set policy route POSTROUTING rule 100

Add the user-defined (UD) prefix to separate user defined names
That allows to use any user-defined names
@vyosbot vyosbot requested review from a team, dmbaturin, sarthurdev, zdc, jestabro and c-po and removed request for a team July 3, 2023 11:49
@c-po c-po merged commit f1b5075 into vyos:current Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants