This patch allows user to associate a value with acl, which will be assigned to ct.label of the corresponding connection tracking entry.
This value can be used to map a ct entry with corresponding OVN ACL or higher level constructs like security group. Signed-off-by: Ankur Sharma <ankur.sha...@nutanix.com> --- ovn/northd/ovn-northd.c | 37 ++++++++++++++++++++++++------ ovn/ovn-nb.ovsschema | 5 +++-- ovn/ovn-nb.xml | 12 ++++++++++ ovn/utilities/ovn-nbctl.c | 24 +++++++++++++++++++- tests/ovn-nbctl.at | 12 ++++++++-- tests/ovn.at | 57 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 12 deletions(-) diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 3254a61..3c73f36 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -168,12 +168,13 @@ enum ovn_stage { #define OVN_ACL_PRI_OFFSET 1000 /* Register definitions specific to switches. */ -#define REGBIT_CONNTRACK_DEFRAG "reg0[0]" -#define REGBIT_CONNTRACK_COMMIT "reg0[1]" -#define REGBIT_CONNTRACK_NAT "reg0[2]" -#define REGBIT_DHCP_OPTS_RESULT "reg0[3]" -#define REGBIT_DNS_LOOKUP_RESULT "reg0[4]" -#define REGBIT_ND_RA_OPTS_RESULT "reg0[5]" +#define REGBIT_CONNTRACK_DEFRAG "reg0[0]" +#define REGBIT_CONNTRACK_COMMIT "reg0[1]" +#define REGBIT_CONNTRACK_NAT "reg0[2]" +#define REGBIT_CONNTRACK_SET_LABEL "reg0[3]" +#define REGBIT_DHCP_OPTS_RESULT "reg0[4]" +#define REGBIT_DNS_LOOKUP_RESULT "reg0[5]" +#define REGBIT_ND_RA_OPTS_RESULT "reg0[6]" /* Register definitions for switches and routers. */ #define REGBIT_NAT_REDIRECT "reg9[0]" @@ -3715,7 +3716,14 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, " || (!ct.new && ct.est && !ct.rpl " "&& ct.blocked == 1)) " "&& (%s)", acl->match); - ds_put_cstr(&actions, REGBIT_CONNTRACK_COMMIT" = 1; "); + if (acl->label) { + ds_put_format(&actions, REGBIT_CONNTRACK_COMMIT" = 1; " + ""REGBIT_CONNTRACK_SET_LABEL" = 1; " + "xxreg1 = %s; ", acl->label); + } else { + ds_put_cstr(&actions, REGBIT_CONNTRACK_COMMIT" = 1; "); + } + build_acl_log(&actions, acl); ds_put_cstr(&actions, "next;"); ovn_lflow_add_with_hint(lflows, od, stage, @@ -4153,6 +4161,21 @@ 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, 150, + REGBIT_CONNTRACK_COMMIT" == 1 && " + ""REGBIT_CONNTRACK_SET_LABEL" == 1", + "ct_commit(ct_mark=0/1, ct_label=xxreg1); next;"); + ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 150, + REGBIT_CONNTRACK_COMMIT" == 1 && " + ""REGBIT_CONNTRACK_SET_LABEL" == 1", + "ct_commit(ct_mark=0/1, ct_label=xxreg1); next;"); + /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be * committed to conntrack. We always set ct.blocked to 0 here as * any packet that makes it this far is part of a connection we diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema index 2c87cbb..75f34c8 100644 --- a/ovn/ovn-nb.ovsschema +++ b/ovn/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "5.16.0", - "cksum": "923459061 23095", + "version": "5.17.0", + "cksum": "1043090930 23169", "tables": { "NB_Global": { "columns": { @@ -177,6 +177,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/ovn-nb.xml b/ovn/ovn-nb.xml index cbaa949..7c10278 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -1299,6 +1299,18 @@ default, log messages are not rate-limited. </p> </column> + + <column name="label"> + <p> + Associates an identifier with the ACL. + Same value will be written to corresponding connection + tracker entry. 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. + </p> + </column> + </group> <group title="Common Columns"> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index e86ab7f..f52377d 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -2065,6 +2065,11 @@ 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"); } @@ -2186,6 +2191,23 @@ nbctl_acl_add(struct ctl_context *ctx) nbrec_acl_set_meter(acl, meter); } + /* Label */ + const char *label = shash_find_data(&ctx->options, "--label"); + if (label) { + /* Validate that label is in the hex format (for example: 0x1234) */ + if (strncmp(label, "0x", 2)) { + ctl_error(ctx, "Label: %s, should start with \"0x\"", label); + return; + } + + if (label[strspn(label + 2, "0123456789abcdefABCDEF") + 2]) { + 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; @@ -5512,7 +5534,7 @@ static const struct ctl_command_syntax nbctl_commands[] = { /* acl commands. */ { "acl-add", 5, 6, "{SWITCH | PORTGROUP} DIRECTION PRIORITY MATCH ACTION", NULL, 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]]", NULL, nbctl_acl_del, NULL, "--type=", RW }, { "acl-list", 1, 1, "{SWITCH | PORTGROUP}", diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 18c5c1d..0c5b681 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -210,19 +210,27 @@ ovn_nbctl_test_acl() { AT_CHECK([ovn-nbctl $2 acl-add $1 from-lport 400 tcp drop]) 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 to-lport 100 ip drop]) + 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=1234 acl-add $1 to-lport 50 ip drop], [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 drop], [1], [], [stderr]) + AT_CHECK([grep 'should be in hex format' 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 to-lport 500 (udp) drop log(name=test,severity=info) to-lport 300 (tcp) drop - to-lport 100 (ip) drop + to-lport 100 (ip) drop label=0x1234 ]) dnl Delete in one direction. diff --git a/tests/ovn.at b/tests/ovn.at index f4e3650..547da49 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -12177,6 +12177,63 @@ AT_CHECK([cat 2.packets], [0], []) AT_CLEANUP +AT_SETUP([ovn -- ACL label]) +AT_KEYWORDS([ovn]) +ovn_start + +net_add n1 + +sim_add hv +as hv +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 +for i in lp1 lp2; do + ovs-vsctl -- add-port br-int $i -- \ + set interface $i external-ids:iface-id=$i \ + options:tx_pcap=hv/$i-tx.pcap \ + options:rxq_pcap=hv/$i-rx.pcap +done + +lp1_mac="f0:00:00:00:00:01" +lp1_ip="192.168.1.2" + +lp2_mac="f0:00:00:00:00:02" +lp2_ip="192.168.1.3" + +ovn-nbctl ls-add lsw0 +ovn-nbctl --wait=sb lsp-add lsw0 lp1 +ovn-nbctl --wait=sb lsp-add lsw0 lp2 +ovn-nbctl lsp-set-addresses lp1 $lp1_mac +ovn-nbctl lsp-set-addresses lp2 $lp2_mac +ovn-nbctl --wait=sb sync + +ovn-nbctl --label=0x1234 acl-add lsw0 to-lport 1000 'tcp.dst==82' allow-related + +ovn-sbctl dump-flows + +# Check logical flow +AT_CHECK([ovn-sbctl dump-flows | grep ls_out_acl | grep "xxreg1 = 0x1234;" | wc -l], [0], [dnl +1 +]) + +AT_CHECK([ovn-sbctl dump-flows | grep ls_out_stateful | grep "ct_label=xxreg1" | wc -l], [0], [dnl +1 +]) + +# Send packet. +packet="inport==\"lp1\" && eth.src==$lp1_mac && eth.dst==$lp2_mac && + ip4 && ip.ttl==64 && ip4.src==$lp1_ip && ip4.dst==$lp2_ip && + tcp && tcp.flags==2 && tcp.src==4362 && tcp.dst==82" +as hv ovs-appctl -t ovn-controller inject-pkt "$packet" + +# Check connection tracker state +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep labels=0x1234 | wc -l], [0], [dnl +1 +]) + +OVN_CLEANUP([hv]) +AT_CLEANUP + AT_SETUP([ovn -- TTL exceeded]) AT_KEYWORDS([ttl-exceeded]) AT_SKIP_IF([test $HAVE_PYTHON = no]) -- 1.8.3.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev