Signed-off-by: Justin Pettit <[email protected]>
---
include/ovn/actions.h | 1 +
ovn/lib/actions.c | 56 +++++++++++++++++++++------
ovn/northd/ovn-northd.c | 4 ++
ovn/ovn-nb.ovsschema | 5 ++-
ovn/ovn-nb.xml | 9 +++++
ovn/utilities/ovn-nbctl.8.xml | 6 ++-
ovn/utilities/ovn-nbctl.c | 13 +++++--
tests/ovn.at | 73 +++++++++++++++++++++++++++++++++++
8 files changed, 149 insertions(+), 18 deletions(-)
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 6384651938f1..1c0c67ce6ffa 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -284,6 +284,7 @@ struct ovnact_log {
uint8_t verdict; /* One of LOG_VERDICT_*. */
uint8_t severity; /* One of LOG_SEVERITY_*. */
char *name;
+ char *meter;
};
/* OVNACT_SET_METER. */
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 26cdb4fdd482..6d4ed1e2c4ed 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -78,7 +78,7 @@ ovnact_init(struct ovnact *ovnact, enum ovnact_type type,
size_t len)
static size_t
encode_start_controller_op(enum action_opcode opcode, bool pause,
- struct ofpbuf *ofpacts)
+ uint32_t meter_id, struct ofpbuf *ofpacts)
{
size_t ofs = ofpacts->size;
@@ -86,6 +86,7 @@ encode_start_controller_op(enum action_opcode opcode, bool
pause,
oc->max_len = UINT16_MAX;
oc->reason = OFPR_ACTION;
oc->pause = pause;
+ oc->meter_id = meter_id;
struct action_header ah = { .opcode = htonl(opcode) };
ofpbuf_put(ofpacts, &ah, sizeof ah);
@@ -105,7 +106,8 @@ encode_finish_controller_op(size_t ofs, struct ofpbuf
*ofpacts)
static void
encode_controller_op(enum action_opcode opcode, struct ofpbuf *ofpacts)
{
- size_t ofs = encode_start_controller_op(opcode, false, ofpacts);
+ size_t ofs = encode_start_controller_op(opcode, false, NX_CTLR_NO_METER,
+ ofpacts);
encode_finish_controller_op(ofs, ofpacts);
}
@@ -1245,7 +1247,8 @@ encode_nested_actions(const struct ovnact_nest *on,
* converted to OpenFlow, as its userdata. ovn-controller will convert the
* packet to ARP or NA and then send the packet and actions back to the
* switch inside an OFPT_PACKET_OUT message. */
- size_t oc_offset = encode_start_controller_op(opcode, false, ofpacts);
+ size_t oc_offset = encode_start_controller_op(opcode, false,
+ NX_CTLR_NO_METER, ofpacts);
ofpacts_put_openflow_actions(inner_ofpacts.data, inner_ofpacts.size,
ofpacts, OFP13_VERSION);
encode_finish_controller_op(oc_offset, ofpacts);
@@ -1738,7 +1741,8 @@ encode_PUT_DHCPV4_OPTS(const struct ovnact_put_opts *pdo,
struct mf_subfield dst = expr_resolve_field(&pdo->dst);
size_t oc_offset = encode_start_controller_op(ACTION_OPCODE_PUT_DHCP_OPTS,
- true, ofpacts);
+ true, NX_CTLR_NO_METER,
+ ofpacts);
nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
ovs_be32 ofs = htonl(dst.ofs);
ofpbuf_put(ofpacts, &ofs, sizeof ofs);
@@ -1769,7 +1773,7 @@ encode_PUT_DHCPV6_OPTS(const struct ovnact_put_opts *pdo,
struct mf_subfield dst = expr_resolve_field(&pdo->dst);
size_t oc_offset = encode_start_controller_op(
- ACTION_OPCODE_PUT_DHCPV6_OPTS, true, ofpacts);
+ ACTION_OPCODE_PUT_DHCPV6_OPTS, true, NX_CTLR_NO_METER, ofpacts);
nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
ovs_be32 ofs = htonl(dst.ofs);
ofpbuf_put(ofpacts, &ofs, sizeof ofs);
@@ -1864,7 +1868,8 @@ encode_DNS_LOOKUP(const struct ovnact_dns_lookup *dl,
struct mf_subfield dst = expr_resolve_field(&dl->dst);
size_t oc_offset = encode_start_controller_op(ACTION_OPCODE_DNS_LOOKUP,
- true, ofpacts);
+ true, NX_CTLR_NO_METER,
+ ofpacts);
nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
ovs_be32 ofs = htonl(dst.ofs);
ofpbuf_put(ofpacts, &ofs, sizeof ofs);
@@ -2027,7 +2032,7 @@ encode_PUT_ND_RA_OPTS(const struct ovnact_put_opts *po,
struct mf_subfield dst = expr_resolve_field(&po->dst);
size_t oc_offset = encode_start_controller_op(
- ACTION_OPCODE_PUT_ND_RA_OPTS, true, ofpacts);
+ ACTION_OPCODE_PUT_ND_RA_OPTS, true, NX_CTLR_NO_METER, ofpacts);
nx_put_header(ofpacts, dst.field->id, OFP13_VERSION, false);
ovs_be32 ofs = htonl(dst.ofs);
ofpbuf_put(ofpacts, &ofs, sizeof ofs);
@@ -2101,6 +2106,19 @@ parse_log_arg(struct action_context *ctx, struct
ovnact_log *log)
}
}
lexer_syntax_error(ctx->lexer, "expecting severity");
+ } else if (lexer_match_id(ctx->lexer, "meter")) {
+ if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
+ return;
+ }
+ /* If multiple meters are given, use the most recent. */
+ if (ctx->lexer->token.type == LEX_T_STRING) {
+ free(log->meter);
+ log->meter = xstrdup(ctx->lexer->token.s);
+ } else {
+ lexer_syntax_error(ctx->lexer, "expecting string");
+ return;
+ }
+ lexer_get(ctx->lexer);
} else {
lexer_syntax_error(ctx->lexer, NULL);
}
@@ -2136,16 +2154,31 @@ format_LOG(const struct ovnact_log *log, struct ds *s)
}
ds_put_format(s, "verdict=%s, ", log_verdict_to_string(log->verdict));
- ds_put_format(s, "severity=%s);", log_severity_to_string(log->severity));
+ ds_put_format(s, "severity=%s", log_severity_to_string(log->severity));
+
+ if (log->meter) {
+ ds_put_format(s, ",meter=%s", log->meter);
+ }
+
+ ds_put_cstr(s, ");");
}
static void
encode_LOG(const struct ovnact_log *log,
- const struct ovnact_encode_params *ep OVS_UNUSED,
- struct ofpbuf *ofpacts)
+ const struct ovnact_encode_params *ep, struct ofpbuf *ofpacts)
{
+ uint32_t meter_id = NX_CTLR_NO_METER;
+
+ if (log->meter) {
+ meter_id = ovn_extend_table_assign_id(ep->meter_table, log->meter);
+ if (meter_id == EXT_TABLE_ID_INVALID) {
+ VLOG_WARN("log specified unknown meter: %"PRIu32, meter_id);
+ return;
+ }
+ }
+
size_t oc_offset = encode_start_controller_op(ACTION_OPCODE_LOG, false,
- ofpacts);
+ meter_id, ofpacts);
struct log_pin_header *lph = ofpbuf_put_uninit(ofpacts, sizeof *lph);
lph->verdict = log->verdict;
@@ -2163,6 +2196,7 @@ static void
ovnact_log_free(struct ovnact_log *log)
{
free(log->name);
+ free(log->meter);
}
static void
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 45557170edc8..ccf74f7c5299 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3140,6 +3140,10 @@ build_acl_log(struct ds *actions, const struct nbrec_acl
*acl)
ds_put_cstr(actions, "verdict=allow, ");
}
+ if (acl->meter) {
+ ds_put_format(actions, "meter=\"%s\", ", acl->meter);
+ }
+
ds_chomp(actions, ' ');
ds_chomp(actions, ',');
ds_put_cstr(actions, "); ");
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index 9a0d8ec70514..3e7164baafaa 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
{
"name": "OVN_Northbound",
- "version": "5.12.0",
- "cksum": "2812995200 20238",
+ "version": "5.13.0",
+ "cksum": "1278623084 20312",
"tables": {
"NB_Global": {
"columns": {
@@ -166,6 +166,7 @@
"notice", "info",
"debug"]]},
"min": 0, "max": 1}},
+ "meter": {"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 1feb2af52027..e124d9489e7c 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -1159,6 +1159,15 @@
<code>info</code>.
</p>
</column>
+
+ <column name="meter">
+ <p>
+ The name of a meter to rate-limit log messages for the ACL.
+ The string must match the <ref column="name" table="meter"/>
+ column of a row in the <ref table="Meter"/> table. By
+ default, log messages are not rate-limited.
+ </p>
+ </column>
</group>
<group title="Common Columns">
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index a8ea7d8cb1e1..269e0fb37e8b 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -85,7 +85,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>--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>--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>
@@ -105,7 +105,9 @@
logging). The severity must be one of <code>alert</code>,
<code>warning</code>, <code>notice</code>, <code>info</code>, or
<code>debug</code>. If a severity is not specified, the default is
- <code>info</code>.
+ <code>info</code>. The <code>--meter=<var>meter</var></code> option
+ is used to rate-limit packet logging. The <var>meter</var> argument
+ names a meter configured by <code>meter-add</code>.
</p>
</dd>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 9f0e6347c104..a82e9c96f446 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1822,7 +1822,10 @@ nbctl_acl_list(struct ctl_context *ctx)
ds_put_format(&ctx->output, "name=%s,", acl->name);
}
if (acl->severity) {
- ds_put_format(&ctx->output, "severity=%s", acl->severity);
+ ds_put_format(&ctx->output, "severity=%s,", acl->severity);
+ }
+ if (acl->meter) {
+ ds_put_format(&ctx->output, "meter=\"%s\",", acl->meter);
}
ds_chomp(&ctx->output, ',');
ds_put_cstr(&ctx->output, ")");
@@ -1927,7 +1930,8 @@ nbctl_acl_add(struct ctl_context *ctx)
bool log = shash_find(&ctx->options, "--log") != NULL;
const char *severity = shash_find_data(&ctx->options, "--severity");
const char *name = shash_find_data(&ctx->options, "--name");
- if (log || severity || name) {
+ const char *meter = shash_find_data(&ctx->options, "--meter");
+ if (log || severity || name || meter) {
nbrec_acl_set_log(acl, true);
}
if (severity) {
@@ -1940,6 +1944,9 @@ nbctl_acl_add(struct ctl_context *ctx)
if (name) {
nbrec_acl_set_name(acl, name);
}
+ if (meter) {
+ nbrec_acl_set_meter(acl, meter);
+ }
/* Check if same acl already exists for the ls/portgroup */
size_t n_acls = pg ? pg->n_acls : ls->n_acls;
@@ -4801,7 +4808,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=", RW },
+ "--log,--may-exist,--type=,--name=,--severity=,--meter=", 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.at b/tests/ovn.at
index d1a8967dd300..81edeafc0848 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -6153,6 +6153,79 @@ OVN_CLEANUP([hv])
AT_CLEANUP
+AT_SETUP([ovn -- ACL rate-limited logging])
+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
+
+
+# Add an ACL that rate-limits logs at 10 per second.
+ovn-nbctl meter-add http-rl1 pktps drop 10
+ovn-nbctl --log --severity=alert --meter=http-rl1 --name=http-acl1 acl-add
lsw0 to-lport 1000 'tcp.dst==80' drop
+
+# Add an ACL that rate-limits logs at 5 per second.
+ovn-nbctl meter-add http-rl2 pktps drop 5
+ovn-nbctl --log --severity=alert --meter=http-rl2 --name=http-acl2 acl-add
lsw0 to-lport 1000 'tcp.dst==81' allow
+
+# Add an ACL that doesn't rate-limit logs.
+ovn-nbctl --log --severity=alert --name=http-acl3 acl-add lsw0 to-lport 1000
'tcp.dst==82' drop
+
+
+# For each ACL, send 100 packets.
+for i in `seq 1 100`; do
+ ovs-appctl netdev-dummy/receive lp1
'in_port(1),eth(src=f0:00:00:00:00:01,dst=f0:00:00:00:00:02),eth_type(0x0800),ipv4(src=192.168.1.2,dst=192.168.1.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=7777,dst=80)'
+
+ ovs-appctl netdev-dummy/receive lp1
'in_port(1),eth(src=f0:00:00:00:00:01,dst=f0:00:00:00:00:02),eth_type(0x0800),ipv4(src=192.168.1.2,dst=192.168.1.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=7777,dst=81)'
+
+ ovs-appctl netdev-dummy/receive lp1
'in_port(1),eth(src=f0:00:00:00:00:01,dst=f0:00:00:00:00:02),eth_type(0x0800),ipv4(src=192.168.1.2,dst=192.168.1.3,proto=6,tos=0,ttl=64,frag=no),tcp(src=7777,dst=82)'
+done
+
+# Print some information that may help debugging.
+as hv ovs-appctl -t ovn-controller meter-table-list
+as hv ovs-ofctl -O OpenFlow13 meter-stats br-int
+
+# The userspace meter implementation doesn't precisely enforce counts,
+# so we just check that exactly 100 "http-acl3" actions were logged and
+# that there were more "http-acl1" actions than "http-acl2" ones.
+OVS_WAIT_UNTIL([ test 100 = $(grep -c 'http-acl3' hv/ovn-controller.log) ])
+
+n_acl1=$(grep -c 'http-acl1' hv/ovn-controller.log)
+n_acl2=$(grep -c 'http-acl2' hv/ovn-controller.log)
+n_acl3=$(grep -c 'http-acl3' hv/ovn-controller.log)
+
+AT_CHECK([ test $n_acl3 -gt $n_acl1 ], [0], [])
+AT_CHECK([ test $n_acl1 -gt $n_acl2 ], [0], [])
+
+
+OVN_CLEANUP([hv])
+AT_CLEANUP
+
+
AT_SETUP([ovn -- DSCP marking and meter check])
AT_KEYWORDS([ovn])
ovn_start
--
2.17.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
