Allow adding label to an ACL to identify which ACL allowed a particular flow in the connection tracking table.
The ACL label covers 32 bits at the end of ct_label. Since only allowed connections are committed, only "allow" and "allow-related" ACLs can have the label. If the ACL allowing the connection changes, the label associated with the new ACL gets updated in the ct_label field. This is achieved by committing every packet that hits the ACL with the label to the connection tracking table. In case the new ACL doesn't have a label, the ct_label field is not cleared. This is done to prevent any performance change with ACLs that don't have label set. For the packets which hits an ACL without label, the behaviour remains the same as before with respect to the conntrack commit. Performance: We used ftrace to measure the time taken by an extra conntrack commit for the packets hitting the ACL with label. We measured the time taken to execute the ovs_ct_execute call inside the sock_sendmsg call. ACL used :- from-lport 2000 (tcp && ip4.src == 10.0.0.11 && ip4.dst == 10.0.0.12) allow-related --label=0x1234 It was observed that the extra ovs_ct_execute call accounted for 1-2% of the round trip time (sock_sendmsg duration). The actual percentage is expected to be lesser since it doesn't take into account the tracing overhead which is substantial for smaller functions. Signed-off-by: Priyankar Jain <[email protected]> --- lib/logical-fields.c | 2 + northd/ovn-northd.c | 53 +++++++++++++++---- northd/ovn_northd.dl | 75 +++++++++++++++++++------- ovn-nb.ovsschema | 3 +- ovn-nb.xml | 12 +++++ tests/ovn-nbctl.at | 22 ++++++++ tests/ovn-northd.at | 107 ++++++++++++++++++++++++++++++++++++-- tests/ovn.at | 1 + utilities/ovn-nbctl.8.xml | 2 +- utilities/ovn-nbctl.c | 32 +++++++++++- 10 files changed, 273 insertions(+), 36 deletions(-) diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 72853013e..7b3d431e0 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -146,6 +146,8 @@ ovn_init_symtab(struct shash *symtab) "ct_label[32..79]", WR_CT_COMMIT); expr_symtab_add_subfield_scoped(symtab, "ct_label.ecmp_reply_port", NULL, "ct_label[80..95]", WR_CT_COMMIT); + expr_symtab_add_subfield_scoped(symtab, "ct_label.label", NULL, + "ct_label[96..127]", WR_CT_COMMIT); expr_symtab_add_field(symtab, "ct_state", MFF_CT_STATE, NULL, false); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 562dc62b2..92590a49d 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -238,6 +238,7 @@ enum ovn_stage { #define REGBIT_ACL_HINT_BLOCK "reg0[10]" #define REGBIT_LKUP_FDB "reg0[11]" #define REGBIT_HAIRPIN_REPLY "reg0[12]" +#define REGBIT_ACL_LABEL "reg0[13]" #define REG_ORIG_DIP_IPV4 "reg1" #define REG_ORIG_DIP_IPV6 "xxreg1" @@ -270,6 +271,9 @@ enum ovn_stage { #define REG_SRC_IPV4 "reg1" #define REG_SRC_IPV6 "xxreg1" +/* Register used for setting a label for ACLs in a Logical Switch. */ +#define REG_LABEL "reg3" + #define FLAGBIT_NOT_VXLAN "flags[1] == 0" /* @@ -278,14 +282,15 @@ enum ovn_stage { * Logical Switch pipeline: * +----+----------------------------------------------+---+------------------+ * | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | | | - * | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | X | | - * | | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | X | | + * | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | | | + * | | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | | | + * | | REGBIT_ACL_LABEL | X | | * +----+----------------------------------------------+ X | | * | R1 | ORIG_DIP_IPV4 (>= IN_STATEFUL) | R | | * +----+----------------------------------------------+ E | | * | R2 | ORIG_TP_DPORT (>= IN_STATEFUL) | G | | * +----+----------------------------------------------+ 0 | | - * | R3 | UNUSED | | | + * | R3 | ACL LABEL | | | * +----+----------------------------------------------+---+------------------+ * | R4 | UNUSED | | | * +----+----------------------------------------------+ X | ORIG_DIP_IPV6 | @@ -5732,7 +5737,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, ds_clear(actions); ds_put_format(match, REGBIT_ACL_HINT_ALLOW_NEW " == 1 && (%s)", acl->match); + ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; "); + if (acl->label) { + ds_put_format(actions, REGBIT_ACL_LABEL" = 1; " + REG_LABEL" = %s; ", acl->label); + } build_acl_log(actions, acl, meter_groups); ds_put_cstr(actions, "next;"); ovn_lflow_add_with_hint(lflows, od, stage, @@ -5743,15 +5753,21 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, /* Match on traffic in the request direction for an established * connection tracking entry that has not been marked for - * deletion. There is no need to commit here, so we can just - * proceed to the next table. We use this to ensure that this + * deletion. We use this to ensure that this * connection is still allowed by the currently defined - * policy. Match untracked packets too. */ + * policy. Match untracked packets too. + * Commit the connection only if the ACL has a label. This is done + * to update the connection tracking entry label in case the ACL + * allowing the connection changes. */ ds_clear(match); ds_clear(actions); ds_put_format(match, REGBIT_ACL_HINT_ALLOW " == 1 && (%s)", acl->match); - + if (acl->label) { + ds_put_cstr(actions, REGBIT_CONNTRACK_COMMIT" = 1; "); + ds_put_format(actions, REGBIT_ACL_LABEL" = 1; " + REG_LABEL" = %s; ", acl->label); + } build_acl_log(actions, acl, meter_groups); ds_put_cstr(actions, "next;"); ovn_lflow_add_with_hint(lflows, od, stage, @@ -6240,15 +6256,34 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows) ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 0, "1", "next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 0, "1", "next;"); + /* If REGBIT_CONNTRACK_COMMIT is set as 1 and + * REGBIT_CONNTRACK_SET_LABEL is set to 1, then the packets should be + * committed to conntrack. + * We always set ct_mark.blocked to 0 here as + * any packet that makes it this far is part of a connection we + * want to allow to continue. */ + ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100, + REGBIT_CONNTRACK_COMMIT" == 1 && " + REGBIT_ACL_LABEL" == 1", + "ct_commit { ct_label.blocked = 0; " + "ct_label.label = " REG_LABEL "; }; next;"); + ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100, + REGBIT_CONNTRACK_COMMIT" == 1 && " + REGBIT_ACL_LABEL" == 1", + "ct_commit { ct_label.blocked = 0; " + "ct_label.label = " REG_LABEL "; }; next;"); + /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be * committed to conntrack. We always set ct_label.blocked to 0 here as * any packet that makes it this far is part of a connection we * want to allow to continue. */ ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100, - REGBIT_CONNTRACK_COMMIT" == 1", + REGBIT_CONNTRACK_COMMIT" == 1 && " + REGBIT_ACL_LABEL" == 0", "ct_commit { ct_label.blocked = 0; }; next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100, - REGBIT_CONNTRACK_COMMIT" == 1", + REGBIT_CONNTRACK_COMMIT" == 1 && " + REGBIT_ACL_LABEL" == 0", "ct_commit { ct_label.blocked = 0; }; next;"); } diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 499130d7e..175642dad 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -1502,13 +1502,14 @@ function s_ROUTER_OUT_DELIVERY(): Stage { Stage{ Egress, 4, "lr_out_deliv * Logical Switch pipeline: * +----+----------------------------------------------+---+------------------+ * | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | | | - * | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | X | | + * | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | | | + * | | REGBIT_ACL_LABEL | X | | * +----+----------------------------------------------+ X | | * | R1 | ORIG_DIP_IPV4 (>= IN_STATEFUL) | R | | * +----+----------------------------------------------+ E | | * | R2 | ORIG_TP_DPORT (>= IN_STATEFUL) | G | | * +----+----------------------------------------------+ 0 | | - * | R3 | UNUSED | | | + * | R3 | ACL_LABEL | | | * +----+----------------------------------------------+---+------------------+ * | R4 | UNUSED | | | * +----+----------------------------------------------+ X | ORIG_DIP_IPV6 | @@ -1581,6 +1582,7 @@ function rEGBIT_ACL_HINT_DROP() : string = "reg0[9]" function rEGBIT_ACL_HINT_BLOCK() : string = "reg0[10]" function rEGBIT_LKUP_FDB() : string = "reg0[11]" function rEGBIT_HAIRPIN_REPLY() : string = "reg0[12]" +function rEGBIT_ACL_LABEL() : string = "reg0[13]" function rEG_ORIG_DIP_IPV4() : string = "reg1" function rEG_ORIG_DIP_IPV6() : string = "xxreg1" @@ -1607,6 +1609,9 @@ function rEG_INPORT_ETH_ADDR() : string = "xreg0[0..47]" function rEG_ECMP_GROUP_ID() : string = "reg8[0..15]" function rEG_ECMP_MEMBER_ID() : string = "reg8[16..31]" +/* Register used for setting a label for ACLs in a Logical Switch. */ +function rEG_LABEL() : string = "reg3" + function fLAGBIT_NOT_VXLAN() : string = "flags[1] == 0" function mFF_N_LOG_REGS() : bit<32> = 10 @@ -2639,26 +2644,40 @@ for (&SwitchACL(.sw = sw, .acl = acl, .has_fair_meter = fair_meter)) { * that case here and un-set ct_label.blocked (which will be done * by ct_commit in the "stateful" stage) to indicate that the * connection should be allowed to resume. + * If the ACL has a label, then load REG_LABEL with the label and + * set the REGBIT_ACL_LABEL field. */ - Flow(.logical_datapath = sw._uuid, - .stage = stage, - .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = "${rEGBIT_ACL_HINT_ALLOW_NEW()} == 1 && (${acl.__match})", - .actions = "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${acl_log}next;", - .external_ids = stage_hint); + var __action = match (acl.label) { + None -> "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${acl_log}next;", + Some{label} -> "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${rEGBIT_ACL_LABEL()} = 1;\ + \ ${rEG_LABEL()} = ${label}; ${acl_log}next;", + } in Flow(.logical_datapath = sw._uuid, + .stage = stage, + .priority = acl.priority + oVN_ACL_PRI_OFFSET(), + .__match = "${rEGBIT_ACL_HINT_ALLOW_NEW()} == 1 && (${acl.__match})", + .actions = __action, + .external_ids = stage_hint); /* Match on traffic in the request direction for an established * connection tracking entry that has not been marked for - * deletion. There is no need to commit here, so we can just - * proceed to the next table. We use this to ensure that this + * deletion. We use this to ensure that this * connection is still allowed by the currently defined - * policy. Match untracked packets too. */ - Flow(.logical_datapath = sw._uuid, - .stage = stage, - .priority = acl.priority + oVN_ACL_PRI_OFFSET(), - .__match = "${rEGBIT_ACL_HINT_ALLOW()} == 1 && (${acl.__match})", - .actions = "${acl_log}next;", - .external_ids = stage_hint) + * policy. Match untracked packets too. + * Commit the connection only if the ACL has a label. This is done to + * update the connection tracking entry label in case the ACL + * allowing the connection changes. + */ + var __action = match (acl.label) { + None -> "${acl_log}next;", + Some{label} -> "${rEGBIT_CONNTRACK_COMMIT()} = 1; ${rEGBIT_ACL_LABEL()} = 1;\ + \ ${rEG_LABEL()} = ${label}; ${acl_log}next;", + + } in Flow(.logical_datapath = sw._uuid, + .stage = stage, + .priority = acl.priority + oVN_ACL_PRI_OFFSET(), + .__match = "${rEGBIT_ACL_HINT_ALLOW()} == 1 && (${acl.__match})", + .actions = __action, + .external_ids = stage_hint) } } else if (acl.action == "allow-stateless") { Flow(.logical_datapath = sw._uuid, @@ -2870,6 +2889,24 @@ for (&Switch(._uuid = ls_uuid)) { .actions = "next;", .external_ids = map_empty()); + /* If REGBIT_CONNTRACK_COMMIT is set as 1 and REGBIT_CONNTRACK_SET_LABEL + * is set to 1, then the packets should be + * committed to conntrack. We always set ct_label.blocked to 0 here as + * any packet that makes it this far is part of a connection we + * want to allow to continue. */ + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_STATEFUL(), + .priority = 100, + .__match = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 1", + .actions = "ct_commit { ct_label.blocked = 0; ct_label.label = ${rEG_LABEL()}; }; next;", + .external_ids = map_empty()); + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_OUT_STATEFUL(), + .priority = 100, + .__match = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 1", + .actions = "ct_commit { ct_label.blocked = 0; ct_label.label = ${rEG_LABEL()}; }; next;", + .external_ids = map_empty()); + /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be * committed to conntrack. We always set ct_label.blocked to 0 here as * any packet that makes it this far is part of a connection we @@ -2877,13 +2914,13 @@ for (&Switch(._uuid = ls_uuid)) { Flow(.logical_datapath = ls_uuid, .stage = s_SWITCH_IN_STATEFUL(), .priority = 100, - .__match = "${rEGBIT_CONNTRACK_COMMIT()} == 1", + .__match = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 0", .actions = "ct_commit { ct_label.blocked = 0; }; next;", .external_ids = map_empty()); Flow(.logical_datapath = ls_uuid, .stage = s_SWITCH_OUT_STATEFUL(), .priority = 100, - .__match = "${rEGBIT_CONNTRACK_COMMIT()} == 1", + .__match = "${rEGBIT_CONNTRACK_COMMIT()} == 1 && ${rEGBIT_ACL_LABEL()} == 0", .actions = "ct_commit { ct_label.blocked = 0; }; next;", .external_ids = map_empty()) } diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index faf619a1c..ddd4637cd 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", "version": "5.32.0", - "cksum": "204590300 28863", + "cksum": "4049685538 28937", "tables": { "NB_Global": { "columns": { @@ -233,6 +233,7 @@ "debug"]]}, "min": 0, "max": 1}}, "meter": {"type": {"key": "string", "min": 0, "max": 1}}, + "label": {"type": {"key": "string", "min": 0, "max": 1}}, "external_ids": { "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}}}, diff --git a/ovn-nb.xml b/ovn-nb.xml index b6a0d1f43..cc79b30ac 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -1779,6 +1779,18 @@ and <code>deny</code> as <ref column="action"/>.) </p> + <column name="label"> + <p> + Associates an identifier with the ACL. + The same value will be written to corresponding connection + tracker entry. The value should be in hex, for example: 0x1234. + This value can help in debugging from connection tracker side. + For example, through this "label" we can backtrack to the ACL rule + which is causing a "leaked" connection. Connection tracker entries are + created only for allowed connections so the label is valid only + for allow and allow-related actions. + </p> + </column> <column name="priority"> <p> The ACL rule's priority. Rules with numerically higher priority diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 828777b82..53502196d 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -211,18 +211,39 @@ ovn_nbctl_test_acl() { AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 300 tcp drop]) AT_CHECK([ovn-nbctl $2 acl-add $1 from-lport 200 ip drop]) AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 100 ip drop]) + AT_CHECK([ovn-nbctl $2 --label=0x1234 acl-add $1 from-lport 70 icmp allow-related]) + AT_CHECK([ovn-nbctl $2 --label=0x1235 acl-add $1 to-lport 70 icmp allow-related]) + dnl Add duplicated ACL AT_CHECK([ovn-nbctl $2 acl-add $1 to-lport 100 ip drop], [1], [], [stderr]) AT_CHECK([grep 'already existed' stderr], [0], [ignore]) AT_CHECK([ovn-nbctl $2 --may-exist acl-add $1 to-lport 100 ip drop]) + dnl Add invalid ACL label + AT_CHECK([ovn-nbctl $2 --label=0x1234 acl-add $1 to-lport 50 ip drop], [1], [], [stderr]) + AT_CHECK([grep 'can only be set with actions' stderr], [0], [ignore]) + + AT_CHECK([ovn-nbctl $2 --label=1234 acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr]) + AT_CHECK([grep 'should start with "0x"' stderr], [0], [ignore]) + + AT_CHECK([ovn-nbctl $2 --label=0xagh acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr]) + AT_CHECK([grep 'should be in hex format' stderr], [0], [ignore]) + + AT_CHECK([ovn-nbctl $2 --label=0x acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr]) + AT_CHECK([grep 'should be between 0x0 and 0xffffffff' stderr], [0], [ignore]) + + AT_CHECK([ovn-nbctl $2 --label=0xfffffffff acl-add $1 to-lport 50 ip allow-related], [1], [], [stderr]) + AT_CHECK([grep 'label should be between 0x0 and 0xffffffff' stderr], [0], [ignore]) + AT_CHECK([ovn-nbctl $2 acl-list $1], [0], [dnl from-lport 600 (udp) drop log() from-lport 400 (tcp) drop from-lport 200 (ip) drop +from-lport 70 (icmp) allow-related label=0x1234 to-lport 500 (udp) drop log(name=test,severity=info) to-lport 300 (tcp) drop to-lport 100 (ip) drop + to-lport 70 (icmp) allow-related label=0x1235 ]) dnl Delete in one direction. @@ -231,6 +252,7 @@ from-lport 200 (ip) drop from-lport 600 (udp) drop log() from-lport 400 (tcp) drop from-lport 200 (ip) drop +from-lport 70 (icmp) allow-related label=0x1234 ]) dnl Delete all ACLs. diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 11461d3f4..123d7362a 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -3515,7 +3515,8 @@ check_stateful_flows() { AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl table=12(ls_in_stateful ), priority=0 , match=(1), action=(next;) - table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) table=12(ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80), action=(reg1 = 10.0.0.10; reg2[[0..15]] = 80; ct_lb(backends=10.0.0.4:8080);) ]) @@ -3537,7 +3538,8 @@ check_stateful_flows() { AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl table=7 (ls_out_stateful ), priority=0 , match=(1), action=(next;) - table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) ]) } @@ -3576,7 +3578,8 @@ AT_CHECK([grep "ls_in_pre_stateful" sw0flows | sort], [0], [dnl AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl table=12(ls_in_stateful ), priority=0 , match=(1), action=(next;) - table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) ]) AT_CHECK([grep "ls_out_pre_lb" sw0flows | sort], [0], [dnl @@ -3594,14 +3597,108 @@ AT_CHECK([grep "ls_out_pre_stateful" sw0flows | sort], [0], [dnl AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl table=7 (ls_out_stateful ), priority=0 , match=(1), action=(next;) - table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) ]) AT_CLEANUP ]) OVN_FOR_EACH_NORTHD([ -AT_SETUP([ct.inv usage]) +AT_SETUP([ovn -- ACL label usage]) +ovn_start + +check ovn-nbctl ls-add sw0 +check ovn-nbctl lsp-add sw0 sw0p1 + +check ovn-nbctl --wait=sb --label=0x1234 acl-add sw0 to-lport 1002 tcp allow-related +check ovn-nbctl --wait=sb --label=0x1234 acl-add sw0 from-lport 1002 tcp allow-related + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CAPTURE_FILE([sw0flows]) + +AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 2002 | sort], [0], [dnl + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) +]) +AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl + table=12(ls_in_stateful ), priority=0 , match=(1), action=(next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) +]) + +AT_CHECK([grep -w "ls_out_acl" sw0flows | grep 2002 | sort], [0], [dnl + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) +]) +AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl + table=7 (ls_out_stateful ), priority=0 , match=(1), action=(next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) +]) + +# Add new ACL without label +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1002 udp allow-related +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1002 udp allow-related + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CAPTURE_FILE([sw0flows]) + +AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 2002 | sort], [0], [dnl + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;) + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;) +]) +AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl + table=12(ls_in_stateful ), priority=0 , match=(1), action=(next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) +]) + +AT_CHECK([grep -w "ls_out_acl" sw0flows | grep 2002 | sort], [0], [dnl + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[7]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;) + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[8]] == 1 && (tcp)), action=(reg0[[1]] = 1; reg0[[13]] = 1; reg3 = 0x1234; next;) + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;) +]) +AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl + table=7 (ls_out_stateful ), priority=0 , match=(1), action=(next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) +]) + +# Delete new ACL with label +check ovn-nbctl --wait=sb acl-del sw0 to-lport 1002 tcp +check ovn-nbctl --wait=sb acl-del sw0 from-lport 1002 tcp + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CAPTURE_FILE([sw0flows]) + +AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 2002 | sort], [0], [dnl + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;) + table=9 (ls_in_acl ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;) +]) +AT_CHECK([grep "ls_in_stateful" sw0flows | sort], [0], [dnl + table=12(ls_in_stateful ), priority=0 , match=(1), action=(next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=12(ls_in_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) +]) + +AT_CHECK([grep -w "ls_out_acl" sw0flows | grep 2002 | sort], [0], [dnl + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[7]] == 1 && (udp)), action=(reg0[[1]] = 1; next;) + table=4 (ls_out_acl ), priority=2002 , match=(reg0[[8]] == 1 && (udp)), action=(next;) +]) +AT_CHECK([grep "ls_out_stateful" sw0flows | sort], [0], [dnl + table=7 (ls_out_stateful ), priority=0 , match=(1), action=(next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 0), action=(ct_commit { ct_label.blocked = 0; }; next;) + table=7 (ls_out_stateful ), priority=100 , match=(reg0[[1]] == 1 && reg0[[13]] == 1), action=(ct_commit { ct_label.blocked = 0; ct_label.label = reg3; }; next;) +]) +AT_CLEANUP +]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn -- ct.inv usage]) ovn_start check ovn-nbctl ls-add sw0 diff --git a/tests/ovn.at b/tests/ovn.at index e5d8869a8..29d0f7f1b 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -197,6 +197,7 @@ ct_label = NXM_NX_CT_LABEL ct_label.blocked = ct_label[0] ct_label.ecmp_reply_eth = ct_label[32..79] ct_label.ecmp_reply_port = ct_label[80..95] +ct_label.label = ct_label[96..127] ct_label.natted = ct_label[1] ct_mark = NXM_NX_CT_MARK ct_state = NXM_NX_CT_STATE diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index c3ff87e74..d1d8c24d6 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -399,7 +399,7 @@ must be either <code>switch</code> or <code>port-group</code>. </p> <dl> - <dt>[<code>--type=</code>{<code>switch</code> | <code>port-group</code>}] [<code>--log</code>] [<code>--meter=</code><var>meter</var>] [<code>--severity=</code><var>severity</var>] [<code>--name=</code><var>name</var>] [<code>--may-exist</code>] <code>acl-add</code> <var>entity</var> <var>direction</var> <var>priority</var> <var>match</var> <var>verdict</var></dt> + <dt>[<code>--type=</code>{<code>switch</code> | <code>port-group</code>}] [<code>--log</code>] [<code>--meter=</code><var>meter</var>] [<code>--severity=</code><var>severity</var>] [<code>--name=</code><var>name</var>] [<code>--label=</code><var>label</var>] [<code>--may-exist</code>] <code>acl-add</code> <var>entity</var> <var>direction</var> <var>priority</var> <var>match</var> <var>verdict</var></dt> <dd> <p> Adds the specified ACL to <var>entity</var>. <var>direction</var> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 3144fba49..a51a6ea8e 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -1988,6 +1988,9 @@ nbctl_acl_list(struct ctl_context *ctx) ds_chomp(&ctx->output, ','); ds_put_cstr(&ctx->output, ")"); } + if (acl->label) { + ds_put_format(&ctx->output, " label=%s", acl->label); + } ds_put_cstr(&ctx->output, "\n"); } @@ -2070,6 +2073,7 @@ nbctl_pre_acl_list(struct ctl_context *ctx) ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_name); ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_severity); ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_meter); + ovsdb_idl_add_column(ctx->idl, &nbrec_acl_col_label); } static void @@ -2137,6 +2141,32 @@ nbctl_acl_add(struct ctl_context *ctx) nbrec_acl_set_meter(acl, meter); } + /* Set the ACL label */ + const char *label = shash_find_data(&ctx->options, "--label"); + if (label) { + /* Ensure that the action is either allow or allow-related */ + if (strcmp(action, "allow") && strcmp(action, "allow-related")) { + ctl_error(ctx, "label can only be set with actions \"allow\" or " + "\"allow-related\""); + return; + } + /* Validate that label is in the hex format (for example: 0x1234) */ + size_t label_len = strlen(label); + if (strncmp(label, "0x", 2)) { + ctl_error(ctx, "label: %s, should start with \"0x\"", label); + return; + } + if (label_len < 3 || label_len > 10) { + ctl_error(ctx, "label should be between 0x0 and 0xffffffff"); + return; + } + if (strspn(label + 2, "0123456789abcdefABCDEF") + 2 != label_len) { + ctl_error(ctx, "label: %s, should be in hex format", label); + return; + } + nbrec_acl_set_label(acl, label); + } + /* Check if same acl already exists for the ls/portgroup */ size_t n_acls = pg ? pg->n_acls : ls->n_acls; struct nbrec_acl **acls = pg ? pg->acls : ls->acls; @@ -6593,7 +6623,7 @@ static const struct ctl_command_syntax nbctl_commands[] = { /* acl commands. */ { "acl-add", 5, 6, "{SWITCH | PORTGROUP} DIRECTION PRIORITY MATCH ACTION", nbctl_pre_acl, nbctl_acl_add, NULL, - "--log,--may-exist,--type=,--name=,--severity=,--meter=", RW }, + "--log,--may-exist,--type=,--name=,--severity=,--meter=,--label=", RW }, { "acl-del", 1, 4, "{SWITCH | PORTGROUP} [DIRECTION [PRIORITY MATCH]]", nbctl_pre_acl, nbctl_acl_del, NULL, "--type=", RW }, { "acl-list", 1, 1, "{SWITCH | PORTGROUP}", -- 2.22.3 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
