1. Objective
The objective is to enchance routing policies to organize them as set of
independent lists, here 'chains', and allow to select and traverse chains
depending on match conditions.
Every routing policy rule have assigned chain name, a type of string, or
an empty string. Rule actions have new action 'jump' with string argument
that allows to switch rule's traversal to given chain.
2. New traversal flow.
The flow starts rule's traversal for only those having an empty chain name,
according to their priorities. If match occurs on rule having the 'jump'
action, a traversal restarts for rules that belongs to that chain, again,
according to their priorities. Any other actions will stop router policy flow.
(i.e. 'jump' have a meaning of 'goto' but not 'call/return').
3. Change of utilities.
ovn-nbctl lr-policy-add accepts chain name as optional argument:
ovn-nbctl [--chain Name] lr-policy-add ...
If omitted, chain name will be empty, i.e. rule added will belong
to default flow.
ovn-nbctl lr-policy-add accepts new action 'jump':
ovn-nbctl lr-policy-add ... jump Name
ovn-nbctl lr-policy-list changes its output format:
to print rules grouped by chain name
to print jump actions
ovn-nbctl lr-policy-del accepts chain name as optional argument:
ovn-nbctl [--chain Name] lr-policy-del ...
If --chain is used:
Delete by LR name: Delete whole chain.
Delete by uuid: just delete by uuid.
Delete by priority: Delete rules of this chain and given
priority.
Delete by priority & match: Delete rules of this chain
having given priority and match.
Else (--chain is not used):
Delete by LR name: Delete all rules.
Delete by uuid: just delete by uuid.
Delete by priority: Delete rules of default chain
(i.e. having chain name '') and given
priority.
Delete by priority & match: Delete rules of default chain
having given priority and match.
4. Changes in NB DB schema
Logical_Router_Policy:
New field:
Name: chain
Type: string
New field:
Name: jump_chain
Type: string
Modified field:
Name: actions
Value: add enum value 'jump'
5. Change in the SB DB generation
1. Reserve (reuse) one 16 bit register reg9[16..31] to hold chain ID
numeric value designating current chain to traverse.
3. Generate router policy flow in the following manner:
lr_in_ip_routing_ecmp (not changed)
lr_in_policy
match: (reg9[16..31] == <chain_id>) && (northd generated expression)
action: for 'jump': reg9[16..31] = <jump_chain_id>; next(ingress,
lr_in_policy);
for other actions: northd generated expression
Signed-off-by: Aleksandr Smirnov <[email protected]>
---
northd/en-northd.c | 3 +-
northd/northd.c | 94 ++++++++++++++++++++-
northd/northd.h | 6 +-
ovn-nb.ovsschema | 9 +-
ovn-nb.xml | 22 ++++-
tests/ovn-nbctl.at | 54 ++++++++++--
tests/ovn-northd.at | 27 ++++++
tests/ovn.at | 93 +++++++++++++++++++++
utilities/ovn-nbctl.c | 187 +++++++++++++++++++++++++++---------------
9 files changed, 416 insertions(+), 79 deletions(-)
---
v2: Addressed Mark's comments.
v3: Rework implementation according to Mark's advice:
Move processing code to engine functions.
Extend error condition check.
Omit whole policy rule if chain name cannot be processed
or jump's action target chain is incorrect.
Change added DB fields to optional.
diff --git a/northd/en-northd.c b/northd/en-northd.c
index c7d1ebcb3..7cea8863c 100644
--- a/northd/en-northd.c
+++ b/northd/en-northd.c
@@ -276,7 +276,8 @@ en_route_policies_run(struct engine_node *node, void *data)
build_route_policies(od, &northd_data->lr_ports,
&bfd_data->bfd_connections,
&route_policies_data->route_policies,
- &route_policies_data->bfd_active_connections);
+ &route_policies_data->bfd_active_connections,
+ &route_policies_data->chain_ids);
}
engine_set_node_state(node, EN_UPDATED);
diff --git a/northd/northd.c b/northd/northd.c
index 542b40685..16e5b9762 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -195,6 +195,7 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2));
#define REG_SRC_IPV4 "reg5"
#define REG_SRC_IPV6 "xxreg1"
#define REG_DHCP_RELAY_DIP_IPV4 "reg2"
+#define REG_POLICY_CHAIN_ID "reg9[16..31]"
#define REG_ROUTE_TABLE_ID "reg7"
/* Registers used for pasing observability information for switches:
@@ -10499,7 +10500,10 @@ build_routing_policy_flow(struct lflow_table *lflows,
struct ovn_datapath *od,
out_port->lrp_networks.ea_s,
out_port->json_key,
is_ipv4);
-
+ } else if (!strcmp(rule->action, "jump")) {
+ ds_put_format(&actions, REG_POLICY_CHAIN_ID
+ "=%d; next(pipeline=ingress, table=%d);",
+ rp->jump_chain_id,
ovn_stage_get_table(S_ROUTER_IN_POLICY));
} else if (!strcmp(rule->action, "drop")) {
ds_put_cstr(&actions, debug_drop_action());
} else if (!strcmp(rule->action, "allow")) {
@@ -10509,7 +10513,13 @@ build_routing_policy_flow(struct lflow_table *lflows,
struct ovn_datapath *od,
}
ds_put_cstr(&actions, REG_ECMP_GROUP_ID" = 0; next;");
}
- ds_put_format(&match, "%s", rule->match);
+
+ if (rp->chain_id == -1) {
+ ds_put_format(&match, "%s", rule->match);
+ } else {
+ ds_put_format(&match, REG_POLICY_CHAIN_ID" == %d && (%s)",
+ rp->chain_id, rule->match);
+ }
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, rule->priority,
ds_cstr(&match), ds_cstr(&actions), stage_hint,
@@ -13647,11 +13657,40 @@ route_policies_lookup(struct hmap *route_policies,
size_t hash,
return NULL;
}
+static bool
+policy_chain_id(struct simap *chain_ids, const char *chain_name, uint32_t *id)
+{
+ if (chain_name && *chain_name) {
+ *id = simap_get(chain_ids, chain_name);
+ return true;
+ }
+
+ return false;
+}
+
+static void
+policy_chain_add(struct simap *chain_ids, const char *chain_name)
+{
+ uint32_t id = simap_count(chain_ids) + 1;
+
+ if (id == UINT16_MAX) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "Too many policy chains for Logical Router.");
+ return;
+ }
+
+ if (!simap_put(chain_ids, chain_name, id)) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "Policy chain id unexpectedly appeared");
+ }
+}
+
void
build_route_policies(struct ovn_datapath *od, const struct hmap *lr_ports,
const struct hmap *bfd_connections,
struct hmap *route_policies,
- struct hmap *bfd_active_connections)
+ struct hmap *bfd_active_connections,
+ struct simap *chain_ids)
{
struct route_policy *rp;
@@ -13661,10 +13700,55 @@ build_route_policies(struct ovn_datapath *od, const struct hmap *lr_ports,
}
}
+ /* Create chain numeric ids for policies with chain name set */
+ for (int i = 0; i < od->nbr->n_policies; i++) {
+ const struct nbrec_logical_router_policy *rule = od->nbr->policies[i];
+ uint32_t id;
+
+ if (policy_chain_id(chain_ids, rule->chain, &id) && id == 0) {
+ policy_chain_add(chain_ids, rule->chain);
+ }
+ }
+
for (int i = 0; i < od->nbr->n_policies; i++) {
const struct nbrec_logical_router_policy *rule = od->nbr->policies[i];
size_t n_valid_nexthops = 0;
char **valid_nexthops = NULL;
+ uint32_t chain_id = 0;
+ uint32_t jump_chain_id = 0;
+
+ /* Skip policy if chain name is set but id was not created above */
+ if (policy_chain_id(chain_ids, rule->chain, &chain_id)
+ && chain_id == 0) {
+ continue;
+ }
+
+ if (!strcmp(rule->action, "jump")) {
+ /* Skip policy if action is 'jump' but no target chain is set */
+ if (!policy_chain_id(chain_ids, rule->jump_chain, &jump_chain_id))
{
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl,
+ "Logical router: %s, policy action 'jump'"
+ " has empty target",
+ od->nbr->name);
+ continue;
+ }
+
+ /* Skip policy if action is 'jump' but target chain name
+ is not resolved to numeric id */
+ if (jump_chain_id == 0) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl,
+ "Logical router: %s, policy action 'jump'"
+ " follows to non-existent chain %s",
+ od->nbr->name, rule->jump_chain);
+ continue;
+ }
+ }
+
+ if (simap_is_empty(chain_ids)) {
+ chain_id = -1;
+ }
if (!strcmp(rule->action, "reroute")) {
size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1;
@@ -13695,6 +13779,8 @@ build_route_policies(struct ovn_datapath *od, const
struct hmap *lr_ports,
new_rp->n_valid_nexthops = n_valid_nexthops;
new_rp->valid_nexthops = valid_nexthops;
new_rp->nbr = od->nbr;
+ new_rp->chain_id = chain_id;
+ new_rp->jump_chain_id = jump_chain_id;
size_t hash = uuid_hash(&od->key);
rp = route_policies_lookup(route_policies, hash, new_rp);
@@ -18354,6 +18440,7 @@ route_policies_init(struct route_policies_data *data)
{
hmap_init(&data->route_policies);
hmap_init(&data->bfd_active_connections);
+ simap_init(&data->chain_ids);
}
void
@@ -18448,6 +18535,7 @@ route_policies_destroy(struct route_policies_data *data)
};
hmap_destroy(&data->route_policies);
__bfd_destroy(&data->bfd_active_connections);
+ simap_destroy(&data->chain_ids);
}
void
diff --git a/northd/northd.h b/northd/northd.h
index 68af275cb..ac81763ba 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -177,6 +177,8 @@ struct route_policy {
char **valid_nexthops;
const struct nbrec_logical_router *nbr;
bool stale;
+ uint32_t chain_id;
+ uint32_t jump_chain_id;
};
struct routes_data {
@@ -188,6 +190,7 @@ struct routes_data {
struct route_policies_data {
struct hmap route_policies;
struct hmap bfd_active_connections;
+ struct simap chain_ids;
};
struct bfd_data {
@@ -789,7 +792,8 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *,
struct northd_tracked_data *);
void build_route_policies(struct ovn_datapath *, const struct hmap *,
- const struct hmap *, struct hmap *, struct hmap *);
+ const struct hmap *, struct hmap *, struct hmap *,
+ struct simap *);
void bfd_table_sync(struct ovsdb_idl_txn *, const struct nbrec_bfd_table *,
const struct hmap *, const struct hmap *,
const struct hmap *, const struct hmap *,
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index e7aa6b2b1..23aa7ed41 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
- "version": "7.9.0",
- "cksum": "2414335430 38682",
+ "version": "7.9.1",
+ "cksum": "414526704 38879",
"tables": {
"NB_Global": {
"columns": {
@@ -524,10 +524,13 @@
"priority": {"type": {"key": {"type": "integer",
"minInteger": 0,
"maxInteger": 32767}}},
+ "chain": {"type": {"key": "string", "min": 0, "max": 1}},
"match": {"type": "string"},
"action": {"type": {
"key": {"type": "string",
- "enum": ["set", ["allow", "drop", "reroute"]]}}},
+ "enum": ["set",
+ ["allow", "drop", "reroute", "jump"]]}}},
+ "jump_chain": {"type": {"key": "string", "min": 0, "max": 1}},
"nexthop": {"type": {"key": "string", "min": 0, "max": 1}},
"nexthops": {"type": {
"key": "string", "min": 0, "max": "unlimited"}},
diff --git a/ovn-nb.xml b/ovn-nb.xml
index d82f9872b..f1df02043 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -3960,7 +3960,15 @@ or
<p>
The routing policy's priority. Rules with numerically higher priority
take precedence over those with lower. A rule is uniquely identified
- by the priority and match string.
+ by the priority, chain and match string.
+ </p>
+ </column>
+
+ <column name="chain">
+ <p>
+ The routing policy rule's chain name. Only rules with empty chain name
+ are traversed by default. Other chains are traversed as response to
+ jump action.
</p>
</column>
@@ -3995,9 +4003,21 @@ or
<code>reroute</code>: Reroute packet to <ref column="nexthop"/> or
<ref column="nexthops"/>.
</li>
+
+ <li>
+ <code>jump</code>: Start examining rules that have the same
+ <ref column="chain"/> value as specified in
+ <ref column="jump_chain"/>.
+ </li>
</ul>
</column>
+ <column name="jump_chain">
+ <p>
+ The routing policy rule's chain name selected to be examined next.
+ </p>
+ </column>
+
<column name="nexthop">
<p>
Note: This column is deprecated in favor of <ref column="nexthops"/>.
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index d4f61cb77..74024946e 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -2318,7 +2318,7 @@ dnl
---------------------------------------------------------------------
OVN_NBCTL_TEST([ovn_nbctl_policies], [policies], [
AT_CHECK([ovn-nbctl lr-add lr0])
-dnl Add policies with allow and drop actions
+dnl Add policies with allow, drop and jump actions
AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop])
AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.2.0/24" allow
pkt_mark=100,foo=bar])
AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.1.0/24" allow])
@@ -2330,6 +2330,8 @@ AT_CHECK([ovn-nbctl --bfd lr-policy-add lr0 103 "ip4.src ==
1.2.3.0/24" reroute
AT_CHECK([ovn-nbctl --bfd lr-policy-add lr0 103 "ip4.src == 1.2.3.0/24"
drop], [1], [],
[ovn-nbctl: BFD is valid only with reroute action.
])
+AT_CHECK([ovn-nbctl --chain=inbound lr-policy-add lr0 100 "ip4.src ==
1.1.1.0/24" drop])
+AT_CHECK([ovn-nbctl --chain=outbound lr-policy-add lr0 100 "ip4.src ==
2.1.1.0/24" jump inbound])
dnl Incomplete option set.
AT_CHECK([ovn-nbctl lr-policy-add lr0 200 "ip4.src == 1.1.4.0/24" reroute
192.168.0.10 foo], [1], [],
@@ -2340,6 +2342,14 @@ AT_CHECK([ovn-nbctl lr-policy-add lr0 200 "ip4.src ==
1.1.4.0/24" allow bar=], [
[ovn-nbctl: No value specified for the option : bar
])
+AT_CHECK([ovn-nbctl --chain lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop], [1], [],
+ [ovn-nbctl: unknown command 'lr0'; use --help for help
+])
+
+AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 2.1.1.0/24" jump], [1],
[],
+ [ovn-nbctl: Chain name is required when action is jump.
+])
+
dnl Add duplicated policy
AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop], [1],
[],
[ovn-nbctl: Same routing policy already existed on the logical router lr0.
@@ -2349,35 +2359,69 @@ AT_CHECK([ovn-nbctl --may-exist lr-policy-add lr0 100
"ip4.src == 1.1.1.0/24" dr
dnl Add duplicated policy
AT_CHECK([ovn-nbctl lr-policy-add lr0 103 "ip4.src == 1.1.1.0/24" deny], [1],
[],
- [ovn-nbctl: deny: action must be one of "allow", "drop", and "reroute"
+ [ovn-nbctl: deny: action must be one of "allow", "drop", "reroute", "jump"
])
dnl Delete by priority and match string
AT_CHECK([ovn-nbctl lr-policy-del lr0 100 "ip4.src == 1.1.1.0/24"])
+
+AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl
+Routing Policies
+Chain <Default>:
+ 101 ip4.src == 2.1.1.0/24 allow
+ 101 ip4.src == 2.1.2.0/24 drop
+ 101 ip6.src == 2002::/64 drop
+ 100 ip4.src == 1.1.2.0/24 allow
pkt_mark=100,foo=bar
+
+Chain inbound:
+ 100 ip4.src == 1.1.1.0/24 drop
+
+Chain outbound:
+ 100 ip4.src == 2.1.1.0/24 jump
-> inbound
+])
+
+AT_CHECK([ovn-nbctl --chain inbound lr-policy-del lr0 100 "ip4.src ==
1.1.1.0/24"])
+
AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl
Routing Policies
+Chain <Default>:
101 ip4.src == 2.1.1.0/24 allow
101 ip4.src == 2.1.2.0/24 drop
101 ip6.src == 2002::/64 drop
100 ip4.src == 1.1.2.0/24 allow
pkt_mark=100,foo=bar
+
+Chain outbound:
+ 100 ip4.src == 2.1.1.0/24 jump
-> inbound
])
dnl Delete all policies for given priority
AT_CHECK([ovn-nbctl lr-policy-del lr0 101])
AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl
Routing Policies
+Chain <Default>:
100 ip4.src == 1.1.2.0/24 allow
pkt_mark=100,foo=bar
-])
+Chain outbound:
+ 100 ip4.src == 2.1.1.0/24 jump
-> inbound
+])
dnl Delete policy by specified uuid
-uuid=$(ovn-nbctl --bare --column _uuid list logical_router_policy)
+uuid=$(ovn-nbctl --bare --column _uuid find logical_router_policy
'chain{<=}""')
AT_CHECK([ovn-nbctl lr-policy-del lr0 $uuid])
-AT_CHECK([ovn-nbctl list logical-router-policy], [0], [dnl
+AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl
+Routing Policies
+
+Chain outbound:
+ 100 ip4.src == 2.1.1.0/24 jump
-> inbound
])
AT_CHECK([ovn-nbctl --if-exists lr-policy-del lr0 $uuid])
+AT_CHECK([ovn-nbctl --chain outbound lr-policy-del lr0])
+
+AT_CHECK([ovn-nbctl lr-policy-list lr0], [0], [dnl
+])
+
dnl Add policy with reroute action
AT_CHECK([ovn-nbctl lr-policy-add lr0 102 "ip4.src == 3.1.2.0/24" reroute
3.3.3.3])
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7abd4d3f3..0c7a1feb1 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3533,6 +3533,33 @@ sed 's/reg8\[[0..15\]] == [[0-9]]*/reg8\[[0..15\]] ==
<cleared>/' | ovn_strip_lf
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Router policies - chains])
+ovn_start
+
+check ovn-nbctl --wait=sb lr-add lr0
+check ovn-nbctl --wait=sb lr-policy-add lr0 112 "ip4.src == 10.0.0.0/24" jump
nochain
+check ovn-nbctl --wait=sb lr-policy-add lr0 102 "ip4.src == 10.0.0.0/24" jump
inbound
+check ovn-nbctl --wait=sb lr-policy-add lr0 101 "ip4.src == 10.0.16.0/24" jump
inbound
+check ovn-nbctl --wait=sb lr-policy-add lr0 100 "ip4.src == 192.168.0.1/32"
allow
+check ovn-nbctl --wait=sb --chain=inbound lr-policy-add lr0 201 "1" allow
+
+AT_CHECK([grep -qE "Logical router: lr0, policy action 'jump' follows to
non-existent chain nochain" northd/ovn-northd.log], [0])
+
+ovn-sbctl dump-flows lr0 > lr0flows2
+AT_CAPTURE_FILE([lr0flows2])
+
+AT_CHECK([grep "lr_in_policy[[^_]]" lr0flows2 | ovn_strip_lflows | sort], [0],
[dnl
+ table=??(lr_in_policy ), priority=0 , match=(1),
action=(reg8[[0..15]] = 0; next;)
+ table=??(lr_in_policy ), priority=100 , match=(reg9[[16..31]] == 0 &&
(ip4.src == 192.168.0.1/32)), action=(reg8[[0..15]] = 0; next;)
+ table=??(lr_in_policy ), priority=101 , match=(reg9[[16..31]] == 0 &&
(ip4.src == 10.0.16.0/24)), action=(reg9[[16..31]]=1; next(pipeline=ingress, table=??);)
+ table=??(lr_in_policy ), priority=102 , match=(reg9[[16..31]] == 0 &&
(ip4.src == 10.0.0.0/24)), action=(reg9[[16..31]]=1; next(pipeline=ingress, table=??);)
+ table=??(lr_in_policy ), priority=201 , match=(reg9[[16..31]] == 1 &&
(1)), action=(reg8[[0..15]] = 0; next;)
+])
+
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([ACL allow-stateless omit conntrack - Logical_Switch])
ovn_start
diff --git a/tests/ovn.at b/tests/ovn.at
index da0bb7e16..4feb2a2db 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -8205,6 +8205,99 @@ echo $expected | ovstest test-ovn expr-to-packets >
3.expected
OVN_CHECK_PACKETS([pbr-hv/vif3-tx.pcap], [3.expected])
+#
+# Test routing policy chains
+#
+
+# Create 3 chains: any drop, any allow, any reroute
+check ovn-nbctl --wait=hv lr-policy-del R1
+check ovn-nbctl --wait=hv --chain anydrop lr-policy-add R1 111 "1" drop
+check ovn-nbctl --wait=hv --chain anyallow lr-policy-add R1 112 "1" allow
+check ovn-nbctl --wait=hv --chain anyreroute lr-policy-add R1 113 "1" reroute
20.20.1.2
+
+reset_pcap_file vif2 pbr-hv/vif2
+reset_pcap_file vif3 pbr-hv/vif3
+
+# Create a policy jumping to drop chain
+check ovn-nbctl --wait=hv lr-policy-add R1 201 "ip4.src==192.168.1.0/24 &&
ip4.dst==172.16.1.0/24" jump anydrop
+
+# Check logical flow
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "192.168.1.0" | wc
-l], [0], [dnl
+1
+])
+
+# Send packet.
+packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
+ ip4 && ip.ttl==64 && ip4.src==$ls1_p1_ip && ip4.dst==$ls2_p1_ip &&
+ udp && udp.src==53 && udp.dst==4369"
+
+OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
+
+# Check if packet hit the drop policy
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+ grep "actions=drop" | \
+ grep "priority=111" | grep "n_packets=1" -c)"])
+
+# Expected to drop the packet.
+$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" pbr-hv/vif2-tx.pcap > vif2.packets
+rcvd_packet=`cat vif2.packets`
+
+AT_FAIL_IF([test "$rcvd_packet" != ""])
+
+# Create a priority policy jumping to allow chain
+check ovn-nbctl --wait=hv lr-policy-add R1 301 "ip4.src==192.168.1.0/24 &&
ip4.dst==172.16.1.0/24" jump anyallow
+
+# Check logical flow
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | grep "192.168.1.0" -c],
[0], [dnl
+2
+])
+
+# Send packet.
+packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
+ ip4 && ip.ttl==64 && ip4.src==$ls1_p1_ip && ip4.dst==$ls2_p1_ip &&
+ udp && udp.src==53 && udp.dst==4369"
+OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
+
+# Check if packet hit the allow policy
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+ grep "priority=112" | grep "n_packets=1" -c)"])
+
+# Expected packet has TTL decreased by 1
+expected="eth.src==$ls2_ro_mac && eth.dst==$ls2_p1_mac &&
+ ip4 && ip.ttl==63 && ip4.src==$ls1_p1_ip && ip4.dst==$ls2_p1_ip &&
+ udp && udp.src==53 && udp.dst==4369"
+echo $expected | ovstest test-ovn expr-to-packets > expected
+
+OVN_CHECK_PACKETS([pbr-hv/vif2-tx.pcap], [expected])
+
+# Create a priority policy jumping to reroute chain
+check ovn-nbctl --wait=hv lr-policy-add R1 401 "ip4.src==192.168.1.0/24 &&
ip4.dst==172.16.1.0/24" jump anyreroute
+
+# Check logical flow
+AT_CHECK([ovn-sbctl dump-flows | grep lr_in_policy | \
+ grep "192.168.1.0" | \
+ grep "priority=30" -c], [0], [dnl
+1
+])
+
+# Send packet.
+packet="inport==\"ls1-lp1\" && eth.src==$ls1_p1_mac && eth.dst==$ls1_ro_mac &&
+ ip4 && ip.ttl==64 && ip4.src==$ls1_p1_ip && ip4.dst==$ls2_p1_ip &&
+ udp && udp.src==53 && udp.dst==4369"
+OVS_WAIT_UNTIL([as pbr-hv ovs-appctl -t ovn-controller inject-pkt "$packet"])
+
+# Check if packet hit the allow policy
+OVS_WAIT_UNTIL([test "1" = "$(ovs-ofctl dump-flows br-int | \
+ grep "priority=113" | grep "n_packets=1" -c)"])
+
+# Expected packet has TTL decreased by 1
+expected="eth.src==$ls3_ro_mac && eth.dst==$ls3_p1_mac &&
+ ip4 && ip.ttl==63 && ip4.src==$ls1_p1_ip && ip4.dst==$ls2_p1_ip &&
+ udp && udp.src==53 && udp.dst==4369"
+echo $expected | ovstest test-ovn expr-to-packets > 3.expected
+
+OVN_CHECK_PACKETS([pbr-hv/vif3-tx.pcap], [3.expected])
+
OVN_CLEANUP([pbr-hv])
AT_CLEANUP
])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index f5277af7c..223307c43 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -56,6 +56,14 @@ VLOG_DEFINE_THIS_MODULE(nbctl);
* change the database at all? */
static bool force_wait = false;
+static char *
+string_ptr(char *ptr)
+{
+ static char *s = "";
+
+ return (ptr) ? ptr : s;
+}
+
static void
nbctl_add_base_prerequisites(struct ovsdb_idl *idl,
enum nbctl_wait_type wait_type)
@@ -4091,6 +4099,7 @@ nbctl_pre_lr_policy_add(struct ctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_priority);
ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_match);
+ ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_chain);
ovsdb_idl_add_column(ctx->idl, &nbrec_bfd_col_dst_ip);
ovsdb_idl_add_column(ctx->idl, &nbrec_bfd_col_logical_port);
@@ -4114,13 +4123,16 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
const char *action = ctx->argv[4];
size_t n_nexthops = 0;
char **nexthops = NULL;
+ char *chain_s = shash_find_data(&ctx->options, "--chain");
bool reroute = false;
+ bool jump = false;
+
/* Validate action. */
if (strcmp(action, "allow") && strcmp(action, "drop")
- && strcmp(action, "reroute")) {
+ && strcmp(action, "jump") && strcmp(action, "reroute")) {
ctl_error(ctx, "%s: action must be one of \"allow\", \"drop\", "
- "and \"reroute\"", action);
+ "\"reroute\", \"jump\"", action);
return;
}
if (!strcmp(action, "reroute")) {
@@ -4131,14 +4143,23 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
reroute = true;
}
+ if (!strcmp(action, "jump")) {
+ if (ctx->argc < 6) {
+ ctl_error(ctx, "Chain name is required when action is jump.");
+ return;
+ }
+ jump = true;
+ }
+
/* Check if same routing policy already exists.
* A policy is uniquely identified by priority and match */
bool may_exist = !!shash_find(&ctx->options, "--may-exist");
size_t i;
for (i = 0; i < lr->n_policies; i++) {
const struct nbrec_logical_router_policy *policy = lr->policies[i];
- if (policy->priority == priority &&
- !strcmp(policy->match, ctx->argv[3])) {
+ if (policy->priority == priority
+ && !strcmp(policy->match, ctx->argv[3])
+ && !strcmp(string_ptr(policy->chain), string_ptr(chain_s))) {
if (!may_exist) {
ctl_error(ctx, "Same routing policy already existed on the "
"logical router %s.", ctx->argv[1]);
@@ -4200,6 +4221,15 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
nbrec_logical_router_policy_set_priority(policy, priority);
nbrec_logical_router_policy_set_match(policy, ctx->argv[3]);
nbrec_logical_router_policy_set_action(policy, action);
+
+ if (chain_s) {
+ nbrec_logical_router_policy_set_chain(policy, chain_s);
+ }
+
+ if (jump) {
+ nbrec_logical_router_policy_set_jump_chain(policy, ctx->argv[5]);
+ }
+
if (reroute) {
nbrec_logical_router_policy_set_nexthops(
policy, (const char **)nexthops, n_nexthops);
@@ -4207,7 +4237,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
/* Parse the options. */
struct smap options = SMAP_INITIALIZER(&options);
- for (i = reroute ? 6 : 5; i < ctx->argc; i++) {
+ for (i = (reroute || jump) ? 6 : 5; i < ctx->argc; i++) {
char *key, *value;
value = xstrdup(ctx->argv[i]);
key = strsep(&value, "=");
@@ -4319,84 +4349,77 @@ nbctl_pre_lr_policy_del(struct ctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_priority);
ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_match);
+ ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_chain);
}
static void
nbctl_lr_policy_del(struct ctl_context *ctx)
{
const struct nbrec_logical_router *lr;
- int64_t priority = 0;
char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], true, &lr);
+
if (error) {
ctx->error = error;
return;
}
- if (ctx->argc == 2) {
- /* If a priority is not specified, delete all policies. */
+ const char *chain = string_ptr(shash_find_data(&ctx->options, "--chain"));
+
+ /* An uuid or priority either chain are not specified.
+ Delete all policies for lr */
+ if (ctx->argc == 2 && !*chain) {
nbrec_logical_router_set_policies(lr, NULL, 0);
return;
}
- const struct uuid *lr_policy_uuid = NULL;
+ int64_t priority = 0;
struct uuid uuid_from_cmd;
- if (uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
- lr_policy_uuid = &uuid_from_cmd;
- } else {
- error = parse_priority(ctx->argv[2], &priority);
- if (error) {
- ctx->error = error;
- return;
- }
- }
- /* If uuid was specified, delete routing policy with the
- * specified uuid. */
- if (ctx->argc == 3) {
- size_t i;
- if (lr_policy_uuid) {
- for (i = 0; i < lr->n_policies; i++) {
- if (uuid_equals(lr_policy_uuid,
- &(lr->policies[i]->header_.uuid))) {
- nbrec_logical_router_update_policies_delvalue(
- lr, lr->policies[i]);
- break;
- }
- }
- if (i == lr->n_policies) {
- if (!shash_find(&ctx->options, "--if-exists")) {
- ctl_error(ctx, "Logical router policy uuid is not found.");
- }
+ /* An uuid is specified, delete the policy with uuid only */
+ if (ctx->argc == 3 && uuid_from_string(&uuid_from_cmd, ctx->argv[2])) {
+ for (int i = 0; i < lr->n_policies; i++) {
+ if (uuid_equals(&uuid_from_cmd,
+ &(lr->policies[i]->header_.uuid))) {
+ nbrec_logical_router_update_policies_delvalue(
+ lr, lr->policies[i]);
return;
}
+ }
- /* If match is not specified, delete all routing policies with the
- * specified priority. */
- } else {
- for (i = 0; i < lr->n_policies; i++) {
- if (priority == lr->policies[i]->priority) {
- nbrec_logical_router_update_policies_delvalue(
- lr, lr->policies[i]);
- }
- }
+ if (!shash_find(&ctx->options, "--if-exists")) {
+ ctl_error(ctx, "Logical router policy uuid is not found.");
}
+
return;
}
- /* Delete policy that has the same priority and match string */
- for (int i = 0; i < lr->n_policies; i++) {
- struct nbrec_logical_router_policy *routing_policy = lr->policies[i];
- if (priority == routing_policy->priority &&
- !strcmp(ctx->argv[3], routing_policy->match)) {
- nbrec_logical_router_update_policies_delvalue(lr, routing_policy);
+ if (ctx->argc >= 3) {
+ error = parse_priority(ctx->argv[2], &priority);
+ if (error) {
+ ctx->error = error;
return;
}
}
+
+ char *match = (ctx->argc == 4) ? ctx->argv[3] : NULL;
+
+ for (int i = 0; i < lr->n_policies; i++) {
+ struct nbrec_logical_router_policy *policy = lr->policies[i];
+
+ /* Delete policies selected by chain, priority (if set),
+ match (if set). Sure, at least one option is set here */
+ if ((!priority || priority == policy->priority)
+ && (!strcmp(chain, string_ptr(policy->chain)))
+ && (!match || !strcmp(match, policy->match))) {
+ nbrec_logical_router_update_policies_delvalue(lr, policy);
+ }
+ }
}
- struct routing_policy {
+struct routing_policy {
int priority;
char *match;
+ char *chain;
const struct nbrec_logical_router_policy *policy;
};
@@ -4405,6 +4428,12 @@ routing_policy_cmp(const void *policy1_, const void *policy2_)
{
const struct routing_policy *policy1p = policy1_;
const struct routing_policy *policy2p = policy2_;
+ int chain_match = strcmp(policy1p->chain, policy2p->chain);
+
+ if (chain_match) {
+ return chain_match;
+ }
+
if (policy1p->priority != policy2p->priority) {
return policy1p->priority > policy2p->priority ? -1 : 1;
} else {
@@ -4416,17 +4445,18 @@ static void
print_routing_policy(const struct nbrec_logical_router_policy *policy,
struct ds *s)
{
- if (policy->n_nexthops) {
- ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority,
- policy->match, policy->action);
- for (int i = 0; i < policy->n_nexthops; i++) {
- char *next_hop = normalize_prefix_str(policy->nexthops[i]);
- ds_put_format(s, i ? ", %s" : " %25s", next_hop ? next_hop : "");
- free(next_hop);
- }
- } else {
- ds_put_format(s, "%10"PRId64" %50s %15s", policy->priority,
- policy->match, policy->action);
+ ds_put_format(s, "%10"PRId64" %50s %15s",
+ policy->priority, policy->match, policy->action);
+
+ if (strcmp(policy->action, "jump") == 0
+ && policy->jump_chain && *policy->jump_chain) {
+ ds_put_format(s, " -> %s", policy->jump_chain);
+ }
+
+ for (int i = 0; i < policy->n_nexthops; i++) {
+ char *next_hop = normalize_prefix_str(policy->nexthops[i]);
+ ds_put_format(s, i ? ", %s" : " %25s", next_hop ? next_hop : "");
+ free(next_hop);
}
if (!smap_is_empty(&policy->options) || policy->n_bfd_sessions) {
@@ -4457,6 +4487,9 @@ nbctl_pre_lr_policy_list(struct ctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_options);
ovsdb_idl_add_column(ctx->idl,
&nbrec_logical_router_policy_col_bfd_sessions);
+ ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_policy_col_chain);
+ ovsdb_idl_add_column(ctx->idl,
+ &nbrec_logical_router_policy_col_jump_chain);
}
static void
@@ -4476,6 +4509,7 @@ nbctl_lr_policy_list(struct ctl_context *ctx)
= lr->policies[i];
policies[n_policies].priority = policy->priority;
policies[n_policies].match = policy->match;
+ policies[n_policies].chain = string_ptr(policy->chain);
policies[n_policies].policy = policy;
n_policies++;
}
@@ -4483,9 +4517,31 @@ nbctl_lr_policy_list(struct ctl_context *ctx)
if (n_policies) {
ds_put_cstr(&ctx->output, "Routing Policies\n");
}
+
+ /* Print chain headings only if there are non-default chains.
+ Otherwise print format will be fully backward compatible.
+ Note that at this point policies are sorted out by chain names too.
+ */
+ char *chain_current;
+ int print_chain_head = (n_policies
+ && policies[n_policies - 1].chain[0]);
+
for (int i = 0; i < n_policies; i++) {
+ /* Print chain heading */
+ if (print_chain_head
+ && (i == 0
+ || strcmp(chain_current, policies[i].chain))) {
+ chain_current = policies[i].chain;
+ if (*chain_current) {
+ ds_put_format(&ctx->output, "\nChain %s:\n", chain_current);
+ } else {
+ ds_put_format(&ctx->output, "Chain <Default>:\n");
+ }
+ }
+
print_routing_policy(policies[i].policy, &ctx->output);
}
+
free(policies);
}
@@ -8094,10 +8150,11 @@ static const struct ctl_command_syntax nbctl_commands[] = {
/* Policy commands */
{ "lr-policy-add", 4, INT_MAX,
"ROUTER PRIORITY MATCH ACTION [NEXTHOP] [OPTIONS - KEY=VALUE ...]",
- nbctl_pre_lr_policy_add, nbctl_lr_policy_add, NULL, "--may-exist,--bfd?",
- RW },
+ nbctl_pre_lr_policy_add, nbctl_lr_policy_add, NULL,
+ "--may-exist,--bfd?,--chain=", RW },
{ "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]",
- nbctl_pre_lr_policy_del, nbctl_lr_policy_del, NULL, "--if-exists", RW },
+ nbctl_pre_lr_policy_del, nbctl_lr_policy_del, NULL,
+ "--if-exists,--chain=", RW },
{ "lr-policy-list", 1, 1, "ROUTER", nbctl_pre_lr_policy_list,
nbctl_lr_policy_list, NULL, "", RO },
