This is RFC proposal, the code below is a working prototype, unit test is not
ready to look into.
1. Introduction
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,
accoring to their priorities.
If match occurs on rule having the 'jump' action, a traversal restarts for rules
that belongs to that chain, again, accoring to their priorities.
Any other actions will stop router policy flow.
(i.e. 'jump' have meaning of 'goto' but not 'call/return').
3. Change of utilities
ovn-nbctl is changed to accept and report new options.
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 chain name in first column
to print jump actions
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 one 16 bit register to hold chain ID numeric value
designating current chain to traverse (here for example Rx[0..15])
2. Reserve new stage at LR, ingress, to make initial assignment to
Rx[0..15] := 0
3. Generate router policy flow in the following manner:
3.1 lr_in_ip_routing_ecmp (not changed)
3.2 lr_in_policy_pre
match: 1
actoin: Rx[0..15] = 0; next;
3.3 lr_in_policy
match: (Rx[0..15] == <chain_id>) && (<nb:match>)
action: <nb:action_not_jump>
| 'Rx[0..15] = <jump_chain_id>; next(ingress,
lr_in_policy);'
Signed-off-by: Aleksandr Smirnov <[email protected]>
---
RFC: First post.
---
northd/northd.c | 68 ++++++++++++++++++++++++++++++++++++++++---
northd/northd.h | 19 ++++++------
ovn-nb.ovsschema | 8 +++--
ovn-nb.xml | 21 ++++++++++++-
tests/ovn-nbctl.at | 17 +++++++++--
tests/ovn-northd.at | 14 ++++++++-
utilities/ovn-nbctl.c | 56 ++++++++++++++++++++++++++++-------
7 files changed, 172 insertions(+), 31 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 0fe15ac59..0639b4881 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -189,6 +189,7 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2));
#define REG_SRC_IPV4 "reg1"
#define REG_SRC_IPV6 "xxreg1"
#define REG_DHCP_RELAY_DIP_IPV4 "reg2"
+#define REG_POLICY_CHAIN_ID "reg6[0..15]"
#define REG_ROUTE_TABLE_ID "reg7"
/* Register used to store backend ipv6 address
@@ -10756,11 +10757,50 @@ static bool check_bfd_state(const struct
nbrec_logical_router_policy *rule,
return true;
}
+
+
+static uint32_t
+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 0;
+ }
+
+ 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");
+ }
+
+ return id;
+}
+
+static uint32_t
+policy_chain_id(struct simap *chain_ids,
+ const char *chain_name)
+{
+ if (!chain_name || !chain_name[0]) {
+ return 0;
+ }
+
+ uint32_t id = simap_get(chain_ids, chain_name);
+ if (!id) {
+ id = policy_chain_add(chain_ids, chain_name);
+ }
+
+ return id;
+}
+
static void
build_routing_policy_flow(struct lflow_table *lflows, struct ovn_datapath *od,
const struct hmap *lr_ports, struct route_policy *rp,
const struct ovsdb_idl_row *stage_hint,
- struct lflow_ref *lflow_ref)
+ struct lflow_ref *lflow_ref,
+ struct simap *chain_ids)
{
const struct nbrec_logical_router_policy *rule = rp->rule;
struct ds match = DS_EMPTY_INITIALIZER;
@@ -10808,7 +10848,12 @@ build_routing_policy_flow(struct lflow_table *lflows,
struct ovn_datapath *od,
lrp_addr_s,
out_port->lrp_networks.ea_s,
out_port->json_key);
-
+ } else if (!strcmp(rule->action, "jump")) {
+ ds_put_format(&actions,
+ "%s=%d; next(pipeline=ingress,table=%d);",
+ REG_POLICY_CHAIN_ID,
+ policy_chain_id(chain_ids, rule->jump_chain),
+ 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")) {
@@ -10818,7 +10863,9 @@ 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);
+
+ ds_put_format(&match, "%s == %d && (%s)", REG_POLICY_CHAIN_ID,
+ policy_chain_id(chain_ids, rule->chain), rule->match);
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_POLICY, rule->priority,
ds_cstr(&match), ds_cstr(&actions), stage_hint,
@@ -13804,9 +13851,15 @@ build_ingress_policy_flows_for_lrouter(
ovs_assert(od->nbr);
/* This is a catch-all rule. It has the lowest priority (0)
* does a match-all("1") and pass-through (next) */
+
ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY, 0, "1",
REG_ECMP_GROUP_ID" = 0; next;",
lflow_ref);
+
+ ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY_PRE, 0, "1",
+ REG_POLICY_CHAIN_ID" = 0; next;",
+ lflow_ref);
+
ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY_ECMP, 150,
REG_ECMP_GROUP_ID" == 0", "next;",
lflow_ref);
@@ -13816,6 +13869,10 @@ build_ingress_policy_flows_for_lrouter(
/* Convert routing policies to flows. */
uint16_t ecmp_group_id = 1;
struct route_policy *rp;
+ struct simap chain_ids;
+
+ simap_init(&chain_ids);
+
HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key),
route_policies) {
const struct nbrec_logical_router_policy *rule = rp->rule;
@@ -13828,9 +13885,12 @@ build_ingress_policy_flows_for_lrouter(
ecmp_group_id++;
} else {
build_routing_policy_flow(lflows, od, lr_ports, rp,
- &rule->header_, lflow_ref);
+ &rule->header_, lflow_ref,
+ &chain_ids);
}
}
+
+ simap_destroy(&chain_ids);
}
/* Local router ingress table ARP_RESOLVE: ARP Resolution. */
diff --git a/northd/northd.h b/northd/northd.h
index 8f76d642d..c711e2587 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -491,17 +491,18 @@ enum ovn_stage {
PIPELINE_STAGE(ROUTER, IN, IP_ROUTING_PRE, 13, "lr_in_ip_routing_pre") \
PIPELINE_STAGE(ROUTER, IN, IP_ROUTING, 14, "lr_in_ip_routing") \
PIPELINE_STAGE(ROUTER, IN, IP_ROUTING_ECMP, 15, "lr_in_ip_routing_ecmp") \
- PIPELINE_STAGE(ROUTER, IN, POLICY, 16, "lr_in_policy") \
- PIPELINE_STAGE(ROUTER, IN, POLICY_ECMP, 17, "lr_in_policy_ecmp") \
- PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP_CHK, 18, \
+ PIPELINE_STAGE(ROUTER, IN, POLICY_PRE, 16, "lr_in_policy_pre") \
+ PIPELINE_STAGE(ROUTER, IN, POLICY, 17, "lr_in_policy") \
+ PIPELINE_STAGE(ROUTER, IN, POLICY_ECMP, 18, "lr_in_policy_ecmp") \
+ PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP_CHK, 19, \
"lr_in_dhcp_relay_resp_chk") \
- PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP, 19, \
+ PIPELINE_STAGE(ROUTER, IN, DHCP_RELAY_RESP, 20, \
"lr_in_dhcp_relay_resp") \
- PIPELINE_STAGE(ROUTER, IN, ARP_RESOLVE, 20, "lr_in_arp_resolve") \
- PIPELINE_STAGE(ROUTER, IN, CHK_PKT_LEN, 21, "lr_in_chk_pkt_len") \
- PIPELINE_STAGE(ROUTER, IN, LARGER_PKTS, 22, "lr_in_larger_pkts") \
- PIPELINE_STAGE(ROUTER, IN, GW_REDIRECT, 23, "lr_in_gw_redirect") \
- PIPELINE_STAGE(ROUTER, IN, ARP_REQUEST, 24, "lr_in_arp_request") \
+ PIPELINE_STAGE(ROUTER, IN, ARP_RESOLVE, 21, "lr_in_arp_resolve") \
+ PIPELINE_STAGE(ROUTER, IN, CHK_PKT_LEN, 22, "lr_in_chk_pkt_len") \
+ PIPELINE_STAGE(ROUTER, IN, LARGER_PKTS, 23, "lr_in_larger_pkts") \
+ PIPELINE_STAGE(ROUTER, IN, GW_REDIRECT, 24, "lr_in_gw_redirect") \
+ PIPELINE_STAGE(ROUTER, IN, ARP_REQUEST, 25, "lr_in_arp_request") \
\
/* Logical router egress stages. */ \
PIPELINE_STAGE(ROUTER, OUT, CHECK_DNAT_LOCAL, 0, \
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index b4a395c56..b81647fa1 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
- "version": "7.6.0",
- "cksum": "2171465655 38284",
+ "version": "7.6.1",
+ "cksum": "2213857026 38387",
"tables": {
"NB_Global": {
"columns": {
@@ -518,10 +518,12 @@
"priority": {"type": {"key": {"type": "integer",
"minInteger": 0,
"maxInteger": 32767}}},
+ "chain": {"type": "string"},
"match": {"type": "string"},
"action": {"type": {
"key": {"type": "string",
- "enum": ["set", ["allow", "drop", "reroute"]]}}},
+ "enum": ["set", ["allow", "drop", "reroute",
"jump"]]}}},
+ "jump_chain": {"type": "string"},
"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 2836f58f5..4476c7e7e 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -3906,7 +3906,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. Others chains are traversed as responce to
+ jump action.
</p>
</column>
@@ -3941,9 +3949,20 @@ or
<code>reroute</code>: Reroute packet to <ref column="nexthop"/> or
<ref column="nexthops"/>.
</li>
+
+ <li>
+ <code>jump</code>: Restart rules traversal for those having chain
+ name equal to <ref column="jump_chain"/>.
+ </li>
</ul>
</column>
+ <column name="jump_chain">
+ <p>
+ The routing policy rule's chain name selected to traverse.
+ </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 2efa13b93..7795d5ed4 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -2288,11 +2288,22 @@ OVN_NBCTL_TEST([ovn_nbctl_policies], [policies], [
AT_CHECK([ovn-nbctl lr-add lr0])
dnl Add policies with allow and drop 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 --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" allow])
+AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.2.0/24" jump
"inbound"])
+AT_CHECK([ovn-nbctl --chain "inbound" lr-policy-add lr0 101 "ip4.src ==
1.1.2.0/24" allow pkt_mark=100,foo=bar])
+AT_CHECK([ovn-nbctl lr-policy-add lr0 11 "ip6.src == 2002::/64" jump
"outbound"])
+
+ovn-nbctl list Logical_Router_Policy
+ovn-nbctl lr-policy-list lr0
+
AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.1.0/24" allow])
AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.2.0/24" drop])
-AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip6.src == 2002::/64" drop])
+
+
+
+
+
AT_CHECK([ovn-nbctl --bfd lr-policy-add lr0 103 "ip4.src == 1.2.3.0/24"
reroute 192.168.1.1], [1], [],
[ovn-nbctl: out lrp not found for 192.168.1.1 nexthop
])
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d6a8c4640..f423242b9 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3419,7 +3419,19 @@ check ovn-nbctl lsp-set-type public-lr0 router
check ovn-nbctl lsp-set-addresses public-lr0 router
check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
-check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3" reroute
172.168.0.101,172.168.0.102
+###check ovn-nbctl --wait=sb lr-policy-add lr0 10 "ip4.src == 10.0.0.3"
reroute 172.168.0.101,172.168.0.102
+
+AT_CHECK([ovn-nbctl --chain "inbound" lr-policy-add lr0 100 "ip4.src ==
1.1.1.0/24" drop])
+AT_CHECK([ovn-nbctl --chain "inbound" lr-policy-add lr0 100 "ip4.src ==
1.1.2.0/24" allow pkt_mark=100,foo=bar])
+AT_CHECK([ovn-nbctl --chain "outbound" lr-policy-add lr0 101 "ip4.src ==
2.1.1.0/24" allow])
+AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip4.src == 2.1.2.0/24" jump
"inbound"])
+AT_CHECK([ovn-nbctl lr-policy-add lr0 101 "ip6.src == 2002::/64" jump
"outbound"])
+
+ovn-nbctl list Logical_Router_Policy
+
+sleep 1
+
+ovn-sbctl lflow-list lr0
ovn-nbctl lr-policy-list lr0 > policy-list
AT_CAPTURE_FILE([policy-list])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index d45be75c7..13277db66 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4116,11 +4116,13 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
char **nexthops = NULL;
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,6 +4133,14 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
reroute = true;
}
+ if (!strcmp(action, "jump")) {
+ if (ctx->argc < 6) {
+ ctl_error(ctx, "Chain number 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");
@@ -4195,11 +4205,22 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
free(nexthops_arg);
}
+ const char *chain_s = shash_find_data(&ctx->options, "--chain");
+
struct nbrec_logical_router_policy *policy;
policy = nbrec_logical_router_policy_insert(ctx->txn);
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 +4228,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, "=");
@@ -4394,9 +4415,10 @@ nbctl_lr_policy_del(struct ctl_context *ctx)
}
}
- struct routing_policy {
+struct routing_policy {
int priority;
char *match;
+ char *chain;
const struct nbrec_logical_router_policy *policy;
};
@@ -4405,6 +4427,13 @@ 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,21 @@ static void
print_routing_policy(const struct nbrec_logical_router_policy *policy,
struct ds *s)
{
+ ds_put_format(s, "%25s %10"PRId64" %50s %15s",
+ (*policy->chain) ? policy->chain : "",
+ policy->priority, policy->match, policy->action);
+
+ if (strcmp(policy->action, "jump") == 0
+ && *policy->jump_chain) {
+ ds_put_format(s, " %s", policy->jump_chain);
+ }
+
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);
}
if (!smap_is_empty(&policy->options) || policy->n_bfd_sessions) {
@@ -4457,6 +4490,8 @@ 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 +4511,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 = policy->chain;
policies[n_policies].policy = policy;
n_policies++;
}
@@ -8100,7 +8136,7 @@ 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?",
+ 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 },
--
2.47.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
