Re: [ovs-dev] [PATCH v9] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread 0-day Robot
Bleep bloop.  Greetings Wan Junjie, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#195 FILE: lib/ofp-meter.c:871:
error = parse_ofp_meter_mod_str(&(*mms)[*n_mms], ds_cstr(), command,

Lines checked: 702, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v9] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread Wan Junjie via dev
put dump-meters' result in one line so add-meters can handle.
save and restore meters when restart ovs.
bundle functions are not implemented in this patch.

Signed-off-by: Wan Junjie 

---
v9:
fix verbosity mask bits for testcase
apologies for mess

v8:
fix missing code for testcase

v7:
typo in code

v6:
code style

v5:
merge oneline to verbosity higher bits
remove duplicate dump_meters code

v4:
code refactor according to comments

v3:
add '--oneline' option for dump-meter(s) command

v2:
fix failed testcases, as dump-meters format changes
---
 include/openvswitch/ofp-meter.h |   9 ++-
 include/openvswitch/ofp-print.h |  10 +++
 lib/ofp-meter.c | 100 ++-
 lib/ofp-print.c |  19 +++--
 tests/dpif-netdev.at|  44 --
 utilities/ovs-ofctl.8.in|  25 +-
 utilities/ovs-ofctl.c   | 137 
 utilities/ovs-save  |   8 ++
 8 files changed, 301 insertions(+), 51 deletions(-)

diff --git a/include/openvswitch/ofp-meter.h b/include/openvswitch/ofp-meter.h
index 6776eae87..a8ee2d61d 100644
--- a/include/openvswitch/ofp-meter.h
+++ b/include/openvswitch/ofp-meter.h
@@ -17,6 +17,7 @@
 #ifndef OPENVSWITCH_OFP_METER_H
 #define OPENVSWITCH_OFP_METER_H 1
 
+#include 
 #include "openflow/openflow.h"
 #include "openvswitch/ofp-protocol.h"
 
@@ -61,7 +62,8 @@ int ofputil_decode_meter_config(struct ofpbuf *,
 struct ofputil_meter_config *,
 struct ofpbuf *bands);
 void ofputil_format_meter_config(struct ds *,
- const struct ofputil_meter_config *);
+ const struct ofputil_meter_config *,
+ bool oneline);
 
 struct ofputil_meter_mod {
 uint16_t command;
@@ -79,6 +81,11 @@ char *parse_ofp_meter_mod_str(struct ofputil_meter_mod *, 
const char *string,
 OVS_WARN_UNUSED_RESULT;
 void ofputil_format_meter_mod(struct ds *, const struct ofputil_meter_mod *);
 
+char *parse_ofp_meter_mod_file(const char *file_name, int command,
+   struct ofputil_meter_mod **mms, size_t *n_mms,
+   enum ofputil_protocol *usable_protocols)
+OVS_WARN_UNUSED_RESULT;
+
 struct ofputil_meter_stats {
 uint32_t meter_id;
 uint32_t flow_count;
diff --git a/include/openvswitch/ofp-print.h b/include/openvswitch/ofp-print.h
index d76f06872..cd7261c4b 100644
--- a/include/openvswitch/ofp-print.h
+++ b/include/openvswitch/ofp-print.h
@@ -38,6 +38,16 @@ struct dp_packet;
 extern "C" {
 #endif
 
+/* manipulate higher bits in verbosity for other usage */
+#define ONELINE_BIT 7
+#define ONELINE_MASK (1 << ONELINE_BIT)
+#define VERBOSITY_MASK (~ONELINE_MASK)
+
+#define VERBOSITY(verbosity) (verbosity & VERBOSITY_MASK)
+
+#define ONELINE_SET(verbosity) (verbosity | ONELINE_MASK)
+#define ONELINE_GET(verbosity) (verbosity & ONELINE_MASK)
+
 void ofp_print(FILE *, const void *, size_t, const struct ofputil_port_map *,
const struct ofputil_table_map *, int verbosity);
 void ofp_print_packet(FILE *stream, const void *data,
diff --git a/lib/ofp-meter.c b/lib/ofp-meter.c
index 9ea40a0bf..6d760620d 100644
--- a/lib/ofp-meter.c
+++ b/lib/ofp-meter.c
@@ -15,6 +15,7 @@
  */
 
 #include 
+#include 
 #include "openvswitch/ofp-meter.h"
 #include "byte-order.h"
 #include "nx-match.h"
@@ -57,7 +58,7 @@ void
 ofputil_format_meter_band(struct ds *s, enum ofp13_meter_flags flags,
   const struct ofputil_meter_band *mb)
 {
-ds_put_cstr(s, "\ntype=");
+ds_put_cstr(s, "type=");
 switch (mb->type) {
 case OFPMBT13_DROP:
 ds_put_cstr(s, "drop");
@@ -343,7 +344,8 @@ ofp_print_meter_flags(struct ds *s, enum ofp13_meter_flags 
flags)
 
 void
 ofputil_format_meter_config(struct ds *s,
-const struct ofputil_meter_config *mc)
+const struct ofputil_meter_config *mc,
+bool oneline)
 {
 uint16_t i;
 
@@ -354,6 +356,7 @@ ofputil_format_meter_config(struct ds *s,
 
 ds_put_cstr(s, "bands=");
 for (i = 0; i < mc->n_bands; ++i) {
+ds_put_cstr(s, oneline ? " ": "\n");
 ofputil_format_meter_band(s, mc->flags, >bands[i]);
 }
 ds_put_char(s, '\n');
@@ -578,6 +581,24 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, 
char *string,
 
 /* Meters require at least OF 1.3. */
 *usable_protocols = OFPUTIL_P_OF13_UP;
+if (command == -2) {
+size_t len;
+
+string += strspn(string, " \t\r\n");   /* Skip white space. */
+len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */
+
+if (!strncmp(string, "add", len)) {
+command = OFPMC13_ADD;
+} else if (!strncmp(string, "delete", len)) {
+command = OFPMC13_DELETE;
+} else if (!strncmp(string, "modify", len)) {
+command 

Re: [ovs-dev] [PATCH ovn v4 0/2] expr: Optimize OR expressions.

2023-04-10 Thread Han Zhou
On Wed, Apr 5, 2023 at 12:50 PM Ilya Maximets  wrote:
>
> On 4/5/23 21:48, Ilya Maximets wrote:
> > This patch set covers removal of expressions which are subsets of other
> > wider expressions.  This allows to avoid flow explosion in case of
> > negative matches.  More details are in commit messages.
> >
> > Version 2:
> >   * Became a patch set.
> >   * Added tests and missing bitmap.h include.
> >   * Code switched to work with bitwise maskable fields only (ORDINAL).
> >   * Added a new patch to combine smaller expressions into wider ones.
> >   * Added a patch to fix a crash uncovered with expression aggregation.
> >
> > Version 3:
> >   * Dropped patch 3 for performance reasons for now, because it doesn't
> > allow to make use of I-P in many cases.
> >   * Patch 1 re-worked to not cause performance issues for normal
> > address sets generated in OVN.
> >   * Performance of the patch 1 significantly improved by not perfroming
> > a full n^2 search and not comparing huge empty parts of subvalues.
> > The patch became a bit less straightforward, but I hope it's still
> > fairly readable.
> >
> > Version 4:
> >   * Added extra comments.
> >   * Added ACK from Han to patch 2.
> >   * Re-worked path shortening (next[]) to track the last non-NULL entry.
> >   * Restricted superset optmization to expressions that do not track
> > address sets.  To preserve ability to use I-P. [Han]

Thanks Ilya. Ideally the restriction can be more relaxed to allow merging
for expr resulted from negation on address sets, because address set I-P
doesn't support negation anyway and it would be expensive without merging
for supersets. But I think the current approach should be fine because from
what I can tell it is less common to apply negation on address sets. This
can be improved further if needed.

So, I went ahead and applied the series to main.

Regards,
Han

> Forgot to add:
>
> * Fixed the memset value: s/0xf/0xff/.  [Han]
>
> >
> > Ilya Maximets (2):
> >   expr: Remove supersets from OR expressions.
> >   expr: Avoid crash if all sub-expressions crushed down to 'true'.
> >
> >  include/ovn/expr.h |   1 +
> >  lib/expr.c | 255 +++--
> >  tests/ovn.at   |  12 +++
> >  3 files changed, 212 insertions(+), 56 deletions(-)
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn v2 3/4] ovn-nbctl: Add tier ACL options.

2023-04-10 Thread Mark Michelson
This modifies the acl-add and acl-del commands so that an ACL
tier can be specified when adding or deleting ACLs.

For acl-add, if the tier is specified, then the ACL created by the
command will have that tier set.

For acl-del, if the tier is specified, then the tier will be one of the
criteria used when deciding which ACLs to delete. Because the tier is
not any more or less specific than the other criteria used for deleting
ACLs, a bitmap approach is used to determine the final set of ACLs that
should be deleted.

Signed-off-by: Mark Michelson 
---
 tests/ovn-nbctl.at|  81 ++
 utilities/ovn-nbctl.c | 131 +-
 2 files changed, 172 insertions(+), 40 deletions(-)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index aa5baade4..3352064d1 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -2616,6 +2616,87 @@ ovn-nbctl: no row "foo1" in table Logical_Switch
 
 dnl -
 
+OVN_NBCTL_TEST([acl_tiers], [ACL tier operations], [
+check ovn-nbctl ls-add ls
+#check ovn-nbctl acl-add ls from-lport 1000 "ip" drop
+#check_column 0 nb:ACL tier priority=1000
+#
+#check ovn-nbctl acl-del ls
+check ovn-nbctl --tier=3 acl-add ls from-lport 1000 "ip" drop
+check_column 3 nb:ACL tier priority=1000
+
+check ovn-nbctl --tier=3 acl-add ls from-lport 1001 "ip" drop
+check_column 3 nb:ACL tier priority=1001
+
+check ovn-nbctl --tier=2 acl-add ls from-lport 1002 "ip" drop
+check_column 2 nb:ACL tier priority=1002
+
+# Removing the tier 3 acls from ls should result in 1 ACL
+# remaining.
+check ovn-nbctl --tier=3 acl-del ls
+check_row_count nb:ACL 1
+check_column 2 nb:ACL tier priority=1002
+
+# Add two egress ACLs at tier 2.
+check ovn-nbctl --tier=2 acl-add ls to-lport 1000 "ip" drop
+check ovn-nbctl --tier=2 acl-add ls to-lport 1001 "ip" drop
+
+check_row_count nb:ACL 3 tier=2
+
+# This should remove the egress tier 2 ACLs and leave the
+# ingress tier 2 ACL
+check ovn-nbctl --tier=2 acl-del ls to-lport
+check_row_count nb:ACL 1
+check_column 2 nb:ACL tier priority=1002
+check_column from-lport nb:ACL direction priority=1002
+
+# Re-add two ingress ACLs at tier 2.
+check ovn-nbctl --tier=2 acl-add ls from-lport 1000 "ip" drop
+check ovn-nbctl --tier=2 acl-add ls from-lport 1001 "ip" drop
+
+check_row_count nb:ACL 3
+
+# Attempt to remove all tier 3 ACLs. All three ACLs are tier 2
+# so this shouldn't have any effect.
+check ovn-nbctl --tier=3 acl-del ls
+check_row_count nb:ACL 3
+
+# Attempt to remove all ingress tier 3 ACLs. All three ACLs are tier
+# 2, so this shouldn't have any effect.
+check ovn-nbctl --tier=3 acl-del ls from-lport
+check_row_count nb:ACL 3
+
+# Attempt to remove the 1000 priority ACL but specify tier 3. Since
+# all ACLs are tier 2, this should have no effect.
+check ovn-nbctl --tier=3 acl-del ls from-lport 1000 "ip"
+check_row_count nb:ACL 3
+
+# Specifying the proper tier should result in all ACLs being deleted.
+check ovn-nbctl --tier=2 acl-del ls
+check_row_count nb:ACL 0
+
+# Now let's experiment with identical ACLs at different tiers.
+check ovn-nbctl --tier=1 acl-add ls from-lport 1000 "ip" drop
+check ovn-nbctl --tier=2 acl-add ls from-lport 1000 "ip" drop
+check ovn-nbctl --tier=3 acl-add ls from-lport 1000 "ip" drop
+check_row_count nb:ACL 3
+check_row_count nb:ACL 1 tier=1
+check_row_count nb:ACL 1 tier=2
+check_row_count nb:ACL 1 tier=3
+
+# Specifying tier 1 should result in only one ACL being deleted.
+check ovn-nbctl --tier=1 acl-del ls from-lport 1000 "ip"
+check_row_count nb:ACL 2
+check_row_count nb:ACL 1 tier=2
+check_row_count nb:ACL 1 tier=3
+
+# Not specifying a tier should result in all ACLs being deleted.
+check ovn-nbctl acl-del ls from-lport 1000 "ip"
+check_row_count nb:ACL 0
+])
+
+dnl -
+
 AT_SETUP([ovn-nbctl - daemon retry connection])
 OVN_NBCTL_TEST_START daemon
 AT_CHECK([kill `cat ovsdb-server.pid`])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 22313ecd5..7910b9d3f 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -48,6 +48,7 @@
 #include "unixctl.h"
 #include "util.h"
 #include "openvswitch/vlog.h"
+#include "bitmap.h"
 
 VLOG_DEFINE_THIS_MODULE(nbctl);
 
@@ -2100,6 +2101,8 @@ acl_cmp(const void *acl1_, const void *acl2_)
 return after_lb2 ? -1 : 1;
 } else if (acl1->priority != acl2->priority) {
 return acl1->priority > acl2->priority ? -1 : 1;
+} else if (acl1->tier != acl2->tier) {
+return acl1->tier > acl2->tier ? -1 : 1;
 } else {
 return strcmp(acl1->match, acl2->match);
 }
@@ -2283,6 +2286,7 @@ nbctl_pre_acl(struct ctl_context *ctx)
 ovsdb_idl_add_column(ctx->idl, _acl_col_priority);
 ovsdb_idl_add_column(ctx->idl, _acl_col_match);
 ovsdb_idl_add_column(ctx->idl, _acl_col_options);
+ovsdb_idl_add_column(ctx->idl, _acl_col_tier);
 }
 
 static void
@@ -2390,6 

[ovs-dev] [PATCH ovn v2 4/4] acls: Add "pass" ACL action.

2023-04-10 Thread Mark Michelson
This allows for evaluating ACLs at the current tier to stop, and to
start evaluating ACLs at the next tier. If not using tiers, or if we
match on the final ACL tier, then a "pass" verdict results in the
default ACL action being applied.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2134138

Signed-off-by: Mark Michelson 
---
 northd/northd.c   |  9 -
 ovn-nb.ovsschema  |  4 ++--
 ovn-nb.xml| 10 ++
 tests/ovn-northd.at   | 46 +++
 tests/system-ovn.at   | 41 ++
 utilities/ovn-nbctl.c |  2 +-
 6 files changed, 104 insertions(+), 8 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 36ca94ebb..da93bfe62 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -6411,6 +6411,8 @@ build_acl_log(struct ds *actions, const struct nbrec_acl 
*acl,
 ds_put_cstr(actions, "verdict=drop, ");
 } else if (!strcmp(acl->action, "reject")) {
 ds_put_cstr(actions, "verdict=reject, ");
+} else if (!strcmp(acl->action, "pass")) {
+ds_put_cstr(actions, "verdict=pass, ");
 } else if (!strcmp(acl->action, "allow")
 || !strcmp(acl->action, "allow-related")
 || !strcmp(acl->action, "allow-stateless")) {
@@ -6447,12 +6449,15 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
*od,
 static const char *allow = REGBIT_ACL_VERDICT_ALLOW " = 1; ";
 static const char *drop = REGBIT_ACL_VERDICT_DROP " = 1; ";
 static const char *reject = REGBIT_ACL_VERDICT_REJECT " = 1; ";
+static const char *pass = "";
 
 const char *verdict;
 if (!strcmp(acl->action, "drop")) {
 verdict = drop;
 } else if (!strcmp(acl->action, "reject")) {
 verdict = reject;
+} else if (!strcmp(acl->action, "pass")) {
+verdict = pass;
 } else {
 verdict = allow;
 }
@@ -6472,7 +6477,9 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
 match_tier_len = match->length;
 }
 
-if (!has_stateful || !strcmp(acl->action, "allow-stateless")) {
+if (!has_stateful
+|| !strcmp(acl->action, "pass")
+|| !strcmp(acl->action, "allow-stateless")) {
 ds_put_cstr(actions, "next;");
 ds_put_format(match, "(%s)", acl->match);
 ovn_lflow_add_with_hint(lflows, od, stage, priority,
diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
index f12d39542..e713cce46 100644
--- a/ovn-nb.ovsschema
+++ b/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
 "name": "OVN_Northbound",
 "version": "7.0.0",
-"cksum": "3195094080 33650",
+"cksum": "2504399077 33658",
 "tables": {
 "NB_Global": {
 "columns": {
@@ -260,7 +260,7 @@
 "enum": ["set",
["allow", "allow-related",
 "allow-stateless", "drop",
-"reject"]]}}},
+"reject", "pass"]]}}},
 "log": {"type": "boolean"},
 "severity": {"type": {"key": {"type": "string",
   "enum": ["set",
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 19cbd191d..3670cad03 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -2254,6 +2254,16 @@ or
   ICMPv4/ICMPv6 unreachable message for other IPv4/IPv6-based
   protocols.
 
+
+
+  pass: Pass to the next ACL tier. If using multiple ACL
+  tiers, a match on this ACL will stop evaluating ACLs at the current
+  tier and move to the next one. If not using ACL tiers or if a
+  pass ACL is matched at the final tier, then the
+  
+  option from the  table is used to
+  determine how to proceed.
+
   
 
 
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 507db7e7a..19385fe06 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -9126,3 +9126,49 @@ acl_test to-lport "" pg ls_out_acl_eval 
ls_out_acl_action 4
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([ACL "pass" logical flows])
+AT_KEYWORDS([acl])
+
+ovn_start
+check ovn-nbctl ls-add ls
+check ovn-nbctl lsp-add ls lsp
+check ovn-nbctl pg-add pg lsp
+
+m4_define([ACL_FLOWS], [grep -w $1 lflows | grep "$2" | sed 
's/table=../table=??/' | sed "s/\($1[[^)]]*\)/$1/" | sort])
+
+acl_test() {
+direction=$1
+options=$2
+thing=$3
+eval_stage=$4
+
+# Baseline. Ensure no ACL eval flows are present.
+ovn-sbctl lflow-list ls > lflows
+AT_CHECK([ACL_FLOWS([$eval_stage], [priority=2000])], [0], [])
+
+# Add an ACL with the "pass" verdict. Ensure that it is in the logical flow
+# table and that it simply moves to the next table without setting a 
specific
+# verdict bit.
+check ovn-nbctl --wait=sb $options acl-add $thing $direction 1000 
"ip4.addr == 80.111.111.112" pass
+

[ovs-dev] [PATCH ovn v2 2/4] northd: Add tiered ACL support.

2023-04-10 Thread Mark Michelson
With this commit, ACLs can now be arranged in hierarchical tiers. A tier
number can be assigned to an ACL. When evaluating ACLs, we first will
consider ACLs at tier 0. If no matching ACL is found, then we move to
tier 1. This continues until a matching ACL is found, or we reach the
maximum tier. If no match is found, then the default acl action is
applied.

Signed-off-by: Mark Michelson 
---
 northd/northd.c |  96 +++---
 northd/northd.h |   1 +
 ovn-nb.ovsschema|   5 +-
 ovn-nb.xml  |  20 ++
 tests/ovn-northd.at | 162 +++-
 tests/system-ovn.at | 107 +
 6 files changed, 347 insertions(+), 44 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 39de54e5b..36ca94ebb 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -242,6 +242,7 @@ enum ovn_stage {
 #define REGBIT_ACL_VERDICT_ALLOW "reg8[16]"
 #define REGBIT_ACL_VERDICT_DROP "reg8[17]"
 #define REGBIT_ACL_VERDICT_REJECT "reg8[18]"
+#define REG_ACL_TIER "reg8[30..31]"
 
 /* Indicate that this packet has been recirculated using egress
  * loopback.  This allows certain checks to be bypassed, such as a
@@ -5704,36 +5705,51 @@ ovn_ls_port_group_destroy(struct hmap *nb_pgs)
 hmap_destroy(nb_pgs);
 }
 
+static bool
+od_set_acl_flags(struct ovn_datapath *od, struct nbrec_acl **acls,
+ size_t n_acls)
+{
+/* A true return indicates that there are no possible ACL flags
+ * left to set on od. A false return indicates that further ACLs
+ * should be explored in case more flags need to be set on od
+ */
+if (!n_acls) {
+return false;
+}
+
+od->has_acls = true;
+for (size_t i = 0; i < n_acls; i++) {
+const struct nbrec_acl *acl = acls[i];
+if (acl->tier > od->max_acl_tier) {
+od->max_acl_tier = acl->tier;
+}
+if (!od->has_stateful_acl && !strcmp(acl->action, "allow-related")) {
+od->has_stateful_acl = true;
+}
+if (od->has_stateful_acl &&
+od->max_acl_tier == nbrec_acl_col_tier.type.value.integer.max) {
+return true;
+}
+}
+
+return false;
+}
+
 static void
 ls_get_acl_flags(struct ovn_datapath *od)
 {
 od->has_acls = false;
 od->has_stateful_acl = false;
+od->max_acl_tier = 0;
 
-if (od->nbs->n_acls) {
-od->has_acls = true;
-
-for (size_t i = 0; i < od->nbs->n_acls; i++) {
-struct nbrec_acl *acl = od->nbs->acls[i];
-if (!strcmp(acl->action, "allow-related")) {
-od->has_stateful_acl = true;
-return;
-}
-}
+if (od_set_acl_flags(od, od->nbs->acls, od->nbs->n_acls)) {
+return;
 }
 
 struct ovn_ls_port_group *ls_pg;
 HMAP_FOR_EACH (ls_pg, key_node, >nb_pgs) {
-if (ls_pg->nb_pg->n_acls) {
-od->has_acls = true;
-
-for (size_t i = 0; i < ls_pg->nb_pg->n_acls; i++) {
-struct nbrec_acl *acl = ls_pg->nb_pg->acls[i];
-if (!strcmp(acl->action, "allow-related")) {
-od->has_stateful_acl = true;
-return;
-}
-}
+if (od_set_acl_flags(od, ls_pg->nb_pg->acls, ls_pg->nb_pg->n_acls)) {
+return;
 }
 }
 }
@@ -6448,10 +6464,19 @@ consider_acl(struct hmap *lflows, struct ovn_datapath 
*od,
 size_t log_verdict_len = actions->length;
 uint16_t priority = acl->priority + OVN_ACL_PRI_OFFSET;
 
+/* All ACLS will start by matching on their respective tier. */
+size_t match_tier_len = 0;
+ds_clear(match);
+if (od->max_acl_tier) {
+ds_put_format(match, REG_ACL_TIER " == %"PRId64" && ", acl->tier);
+match_tier_len = match->length;
+}
+
 if (!has_stateful || !strcmp(acl->action, "allow-stateless")) {
 ds_put_cstr(actions, "next;");
+ds_put_format(match, "(%s)", acl->match);
 ovn_lflow_add_with_hint(lflows, od, stage, priority,
-acl->match, ds_cstr(actions),
+ds_cstr(match), ds_cstr(actions),
 >header_);
 return;
 }
@@ -6476,7 +6501,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
  * by ct_commit in the "stateful" stage) to indicate that the
  * connection should be allowed to resume.
  */
-ds_clear(match);
+ds_truncate(match, match_tier_len);
 ds_put_format(match, REGBIT_ACL_HINT_ALLOW_NEW " == 1 && (%s)",
   acl->match);
 
@@ -6499,7 +6524,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od,
  * 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_truncate(match, 

Re: [ovs-dev] [PATCH v8] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread 0-day Robot
Bleep bloop.  Greetings Wan Junjie, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#195 FILE: lib/ofp-meter.c:871:
error = parse_ofp_meter_mod_str(&(*mms)[*n_mms], ds_cstr(), command,

Lines checked: 702, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v8] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread Wan Junjie via dev
put dump-meters' result in one line so add-meters can handle.
save and restore meters when restart ovs.
bundle functions are not implemented in this patch.

Signed-off-by: Wan Junjie 

---
v8:
fix missing code for testcase

v7:
typo in code

v6:
code style

v5:
merge oneline to verbosity higher bits
remove duplicate dump_meters code

v4:
code refactor according to comments

v3:
add '--oneline' option for dump-meter(s) command

v2:
fix failed testcases, as dump-meters format changes
---
 include/openvswitch/ofp-meter.h |   9 ++-
 include/openvswitch/ofp-print.h |  10 +++
 lib/ofp-meter.c | 100 ++-
 lib/ofp-print.c |  19 +++--
 tests/dpif-netdev.at|  44 --
 utilities/ovs-ofctl.8.in|  25 +-
 utilities/ovs-ofctl.c   | 137 
 utilities/ovs-save  |   8 ++
 8 files changed, 301 insertions(+), 51 deletions(-)

diff --git a/include/openvswitch/ofp-meter.h b/include/openvswitch/ofp-meter.h
index 6776eae87..a8ee2d61d 100644
--- a/include/openvswitch/ofp-meter.h
+++ b/include/openvswitch/ofp-meter.h
@@ -17,6 +17,7 @@
 #ifndef OPENVSWITCH_OFP_METER_H
 #define OPENVSWITCH_OFP_METER_H 1
 
+#include 
 #include "openflow/openflow.h"
 #include "openvswitch/ofp-protocol.h"
 
@@ -61,7 +62,8 @@ int ofputil_decode_meter_config(struct ofpbuf *,
 struct ofputil_meter_config *,
 struct ofpbuf *bands);
 void ofputil_format_meter_config(struct ds *,
- const struct ofputil_meter_config *);
+ const struct ofputil_meter_config *,
+ bool oneline);
 
 struct ofputil_meter_mod {
 uint16_t command;
@@ -79,6 +81,11 @@ char *parse_ofp_meter_mod_str(struct ofputil_meter_mod *, 
const char *string,
 OVS_WARN_UNUSED_RESULT;
 void ofputil_format_meter_mod(struct ds *, const struct ofputil_meter_mod *);
 
+char *parse_ofp_meter_mod_file(const char *file_name, int command,
+   struct ofputil_meter_mod **mms, size_t *n_mms,
+   enum ofputil_protocol *usable_protocols)
+OVS_WARN_UNUSED_RESULT;
+
 struct ofputil_meter_stats {
 uint32_t meter_id;
 uint32_t flow_count;
diff --git a/include/openvswitch/ofp-print.h b/include/openvswitch/ofp-print.h
index d76f06872..b7f8d15e9 100644
--- a/include/openvswitch/ofp-print.h
+++ b/include/openvswitch/ofp-print.h
@@ -38,6 +38,16 @@ struct dp_packet;
 extern "C" {
 #endif
 
+/* manipulate higher bits in verbosity for other usage */
+#define ONELINE_BIT 7
+#define ONELINE_MASK (1 << ONELINE_BIT)
+#define VERBOSITY_MASK (~ONELINE_MASK)
+
+#define VERBOSITY(verbosity) (verbosity & ~VERBOSITY_MASK)
+
+#define ONELINE_SET(verbosity) (verbosity | ONELINE_MASK)
+#define ONELINE_GET(verbosity) (verbosity & ONELINE_MASK)
+
 void ofp_print(FILE *, const void *, size_t, const struct ofputil_port_map *,
const struct ofputil_table_map *, int verbosity);
 void ofp_print_packet(FILE *stream, const void *data,
diff --git a/lib/ofp-meter.c b/lib/ofp-meter.c
index 9ea40a0bf..6d760620d 100644
--- a/lib/ofp-meter.c
+++ b/lib/ofp-meter.c
@@ -15,6 +15,7 @@
  */
 
 #include 
+#include 
 #include "openvswitch/ofp-meter.h"
 #include "byte-order.h"
 #include "nx-match.h"
@@ -57,7 +58,7 @@ void
 ofputil_format_meter_band(struct ds *s, enum ofp13_meter_flags flags,
   const struct ofputil_meter_band *mb)
 {
-ds_put_cstr(s, "\ntype=");
+ds_put_cstr(s, "type=");
 switch (mb->type) {
 case OFPMBT13_DROP:
 ds_put_cstr(s, "drop");
@@ -343,7 +344,8 @@ ofp_print_meter_flags(struct ds *s, enum ofp13_meter_flags 
flags)
 
 void
 ofputil_format_meter_config(struct ds *s,
-const struct ofputil_meter_config *mc)
+const struct ofputil_meter_config *mc,
+bool oneline)
 {
 uint16_t i;
 
@@ -354,6 +356,7 @@ ofputil_format_meter_config(struct ds *s,
 
 ds_put_cstr(s, "bands=");
 for (i = 0; i < mc->n_bands; ++i) {
+ds_put_cstr(s, oneline ? " ": "\n");
 ofputil_format_meter_band(s, mc->flags, >bands[i]);
 }
 ds_put_char(s, '\n');
@@ -578,6 +581,24 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, 
char *string,
 
 /* Meters require at least OF 1.3. */
 *usable_protocols = OFPUTIL_P_OF13_UP;
+if (command == -2) {
+size_t len;
+
+string += strspn(string, " \t\r\n");   /* Skip white space. */
+len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */
+
+if (!strncmp(string, "add", len)) {
+command = OFPMC13_ADD;
+} else if (!strncmp(string, "delete", len)) {
+command = OFPMC13_DELETE;
+} else if (!strncmp(string, "modify", len)) {
+command = OFPMC13_MODIFY;
+} else {
+len = 0;
+ 

Re: [ovs-dev] [PATCH v7] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread 0-day Robot
Bleep bloop.  Greetings Wan Junjie, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#195 FILE: lib/ofp-meter.c:871:
error = parse_ofp_meter_mod_str(&(*mms)[*n_mms], ds_cstr(), command,

Lines checked: 702, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v7] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread Wan Junjie via dev
put dump-meters' result in one line so add-meters can handle.
save and restore meters when restart ovs.
bundle functions are not implemented in this patch.

Signed-off-by: Wan Junjie 

---
v7:
typo in code

v6:
code style

v5:
merge oneline to verbosity higher bits
remove duplicate dump_meters code

v4:
code refactor according to comments

v3:
add '--oneline' option for dump-meter(s) command

v2:
fix failed testcases, as dump-meters format changes
---
 include/openvswitch/ofp-meter.h |   9 ++-
 include/openvswitch/ofp-print.h |  10 +++
 lib/ofp-meter.c | 100 ++-
 lib/ofp-print.c |  19 +++--
 tests/dpif-netdev.at|  44 --
 utilities/ovs-ofctl.8.in|  25 +-
 utilities/ovs-ofctl.c   | 137 
 utilities/ovs-save  |   8 ++
 8 files changed, 301 insertions(+), 51 deletions(-)

diff --git a/include/openvswitch/ofp-meter.h b/include/openvswitch/ofp-meter.h
index 6776eae87..a8ee2d61d 100644
--- a/include/openvswitch/ofp-meter.h
+++ b/include/openvswitch/ofp-meter.h
@@ -17,6 +17,7 @@
 #ifndef OPENVSWITCH_OFP_METER_H
 #define OPENVSWITCH_OFP_METER_H 1
 
+#include 
 #include "openflow/openflow.h"
 #include "openvswitch/ofp-protocol.h"
 
@@ -61,7 +62,8 @@ int ofputil_decode_meter_config(struct ofpbuf *,
 struct ofputil_meter_config *,
 struct ofpbuf *bands);
 void ofputil_format_meter_config(struct ds *,
- const struct ofputil_meter_config *);
+ const struct ofputil_meter_config *,
+ bool oneline);
 
 struct ofputil_meter_mod {
 uint16_t command;
@@ -79,6 +81,11 @@ char *parse_ofp_meter_mod_str(struct ofputil_meter_mod *, 
const char *string,
 OVS_WARN_UNUSED_RESULT;
 void ofputil_format_meter_mod(struct ds *, const struct ofputil_meter_mod *);
 
+char *parse_ofp_meter_mod_file(const char *file_name, int command,
+   struct ofputil_meter_mod **mms, size_t *n_mms,
+   enum ofputil_protocol *usable_protocols)
+OVS_WARN_UNUSED_RESULT;
+
 struct ofputil_meter_stats {
 uint32_t meter_id;
 uint32_t flow_count;
diff --git a/include/openvswitch/ofp-print.h b/include/openvswitch/ofp-print.h
index d76f06872..b7f8d15e9 100644
--- a/include/openvswitch/ofp-print.h
+++ b/include/openvswitch/ofp-print.h
@@ -38,6 +38,16 @@ struct dp_packet;
 extern "C" {
 #endif
 
+/* manipulate higher bits in verbosity for other usage */
+#define ONELINE_BIT 7
+#define ONELINE_MASK (1 << ONELINE_BIT)
+#define VERBOSITY_MASK (~ONELINE_MASK)
+
+#define VERBOSITY(verbosity) (verbosity & ~VERBOSITY_MASK)
+
+#define ONELINE_SET(verbosity) (verbosity | ONELINE_MASK)
+#define ONELINE_GET(verbosity) (verbosity & ONELINE_MASK)
+
 void ofp_print(FILE *, const void *, size_t, const struct ofputil_port_map *,
const struct ofputil_table_map *, int verbosity);
 void ofp_print_packet(FILE *stream, const void *data,
diff --git a/lib/ofp-meter.c b/lib/ofp-meter.c
index 9ea40a0bf..6d760620d 100644
--- a/lib/ofp-meter.c
+++ b/lib/ofp-meter.c
@@ -15,6 +15,7 @@
  */
 
 #include 
+#include 
 #include "openvswitch/ofp-meter.h"
 #include "byte-order.h"
 #include "nx-match.h"
@@ -57,7 +58,7 @@ void
 ofputil_format_meter_band(struct ds *s, enum ofp13_meter_flags flags,
   const struct ofputil_meter_band *mb)
 {
-ds_put_cstr(s, "\ntype=");
+ds_put_cstr(s, "type=");
 switch (mb->type) {
 case OFPMBT13_DROP:
 ds_put_cstr(s, "drop");
@@ -343,7 +344,8 @@ ofp_print_meter_flags(struct ds *s, enum ofp13_meter_flags 
flags)
 
 void
 ofputil_format_meter_config(struct ds *s,
-const struct ofputil_meter_config *mc)
+const struct ofputil_meter_config *mc,
+bool oneline)
 {
 uint16_t i;
 
@@ -354,6 +356,7 @@ ofputil_format_meter_config(struct ds *s,
 
 ds_put_cstr(s, "bands=");
 for (i = 0; i < mc->n_bands; ++i) {
+ds_put_cstr(s, oneline ? " ": "\n");
 ofputil_format_meter_band(s, mc->flags, >bands[i]);
 }
 ds_put_char(s, '\n');
@@ -578,6 +581,24 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, 
char *string,
 
 /* Meters require at least OF 1.3. */
 *usable_protocols = OFPUTIL_P_OF13_UP;
+if (command == -2) {
+size_t len;
+
+string += strspn(string, " \t\r\n");   /* Skip white space. */
+len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */
+
+if (!strncmp(string, "add", len)) {
+command = OFPMC13_ADD;
+} else if (!strncmp(string, "delete", len)) {
+command = OFPMC13_DELETE;
+} else if (!strncmp(string, "modify", len)) {
+command = OFPMC13_MODIFY;
+} else {
+len = 0;
+command = OFPMC13_ADD;

Re: [ovs-dev] [PATCH v6] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread Simon Horman
On Mon, Apr 10, 2023 at 09:23:46PM +0800, Wan Junjie via dev wrote:
> put dump-meters' result in one line so add-meters can handle.
> save and restore meters when restart ovs.
> bundle functions are not implemented in this patch.
> 
> Signed-off-by: Wan Junjie 

Hi,

unfortunately this one doesn't seem to compile.

Link: 
https://patchwork.ozlabs.org/project/openvswitch/patch/20230410132346.53249-1-wanjun...@bytedance.com/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn v8 1/2] northd: Update the is_stateless helper for rotuer nat

2023-04-10 Thread Simon Horman
On Thu, Apr 06, 2023 at 07:03:03PM +0200, Ales Musil wrote:
> The stateless can be applied only to "snat_and_dnat" nat
> type. Update the helper to reflect that.
> 
> Fixes: cc87c4827f47 ("OVN: Use ip4.src and ip4.dst actions for NAT rules")
> Signed-off-by: Ales Musil 
> ---
> v8: No change since v7.

Reviewed-by: Simon Horman 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH v6] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread 0-day Robot
Bleep bloop.  Greetings Wan Junjie, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line lacks whitespace around operator
#195 FILE: lib/ofp-meter.c:871:
error = parse_ofp_meter_mod_str(&(*mms)[*n_mms], ds_cstr(), command,

Lines checked: 702, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH v6] utilities/ofctl: add-meters for save and restore

2023-04-10 Thread Wan Junjie via dev
put dump-meters' result in one line so add-meters can handle.
save and restore meters when restart ovs.
bundle functions are not implemented in this patch.

Signed-off-by: Wan Junjie 

---
v6:
code style

v5:
merge oneline to verbosity higher bits
remove duplicate dump_meters code

v4:
code refactor according to comments

v3:
add '--oneline' option for dump-meter(s) command

v2:
fix failed testcases, as dump-meters format changes
---
 include/openvswitch/ofp-meter.h |   9 ++-
 include/openvswitch/ofp-print.h |  10 +++
 lib/ofp-meter.c | 100 ++-
 lib/ofp-print.c |  19 +++--
 tests/dpif-netdev.at|  44 --
 utilities/ovs-ofctl.8.in|  25 +-
 utilities/ovs-ofctl.c   | 137 
 utilities/ovs-save  |   8 ++
 8 files changed, 301 insertions(+), 51 deletions(-)

diff --git a/include/openvswitch/ofp-meter.h b/include/openvswitch/ofp-meter.h
index 6776eae87..a8ee2d61d 100644
--- a/include/openvswitch/ofp-meter.h
+++ b/include/openvswitch/ofp-meter.h
@@ -17,6 +17,7 @@
 #ifndef OPENVSWITCH_OFP_METER_H
 #define OPENVSWITCH_OFP_METER_H 1
 
+#include 
 #include "openflow/openflow.h"
 #include "openvswitch/ofp-protocol.h"
 
@@ -61,7 +62,8 @@ int ofputil_decode_meter_config(struct ofpbuf *,
 struct ofputil_meter_config *,
 struct ofpbuf *bands);
 void ofputil_format_meter_config(struct ds *,
- const struct ofputil_meter_config *);
+ const struct ofputil_meter_config *,
+ bool oneline);
 
 struct ofputil_meter_mod {
 uint16_t command;
@@ -79,6 +81,11 @@ char *parse_ofp_meter_mod_str(struct ofputil_meter_mod *, 
const char *string,
 OVS_WARN_UNUSED_RESULT;
 void ofputil_format_meter_mod(struct ds *, const struct ofputil_meter_mod *);
 
+char *parse_ofp_meter_mod_file(const char *file_name, int command,
+   struct ofputil_meter_mod **mms, size_t *n_mms,
+   enum ofputil_protocol *usable_protocols)
+OVS_WARN_UNUSED_RESULT;
+
 struct ofputil_meter_stats {
 uint32_t meter_id;
 uint32_t flow_count;
diff --git a/include/openvswitch/ofp-print.h b/include/openvswitch/ofp-print.h
index d76f06872..d0e54e6b0 100644
--- a/include/openvswitch/ofp-print.h
+++ b/include/openvswitch/ofp-print.h
@@ -38,6 +38,16 @@ struct dp_packet;
 extern "C" {
 #endif
 
+/* manipulate higher bits in verbosity for other usage */
+#define ONELINE_BIT 7
+#define ONELINE_MASK (1 << ONELINE_BIT)
+#define VERBOSITY_MASK (~ONLINE_MASK)
+
+#define VERBOSITY(verbosity) (verbosity & ~VERBOSITY_MASK)
+
+#define ONELINE_SET(verbosity) (verbosity | ONELINE_MASK)
+#define ONELINE_GET(verbosity) (verbosity & ONELINE_MASK)
+
 void ofp_print(FILE *, const void *, size_t, const struct ofputil_port_map *,
const struct ofputil_table_map *, int verbosity);
 void ofp_print_packet(FILE *stream, const void *data,
diff --git a/lib/ofp-meter.c b/lib/ofp-meter.c
index 9ea40a0bf..6d760620d 100644
--- a/lib/ofp-meter.c
+++ b/lib/ofp-meter.c
@@ -15,6 +15,7 @@
  */
 
 #include 
+#include 
 #include "openvswitch/ofp-meter.h"
 #include "byte-order.h"
 #include "nx-match.h"
@@ -57,7 +58,7 @@ void
 ofputil_format_meter_band(struct ds *s, enum ofp13_meter_flags flags,
   const struct ofputil_meter_band *mb)
 {
-ds_put_cstr(s, "\ntype=");
+ds_put_cstr(s, "type=");
 switch (mb->type) {
 case OFPMBT13_DROP:
 ds_put_cstr(s, "drop");
@@ -343,7 +344,8 @@ ofp_print_meter_flags(struct ds *s, enum ofp13_meter_flags 
flags)
 
 void
 ofputil_format_meter_config(struct ds *s,
-const struct ofputil_meter_config *mc)
+const struct ofputil_meter_config *mc,
+bool oneline)
 {
 uint16_t i;
 
@@ -354,6 +356,7 @@ ofputil_format_meter_config(struct ds *s,
 
 ds_put_cstr(s, "bands=");
 for (i = 0; i < mc->n_bands; ++i) {
+ds_put_cstr(s, oneline ? " ": "\n");
 ofputil_format_meter_band(s, mc->flags, >bands[i]);
 }
 ds_put_char(s, '\n');
@@ -578,6 +581,24 @@ parse_ofp_meter_mod_str__(struct ofputil_meter_mod *mm, 
char *string,
 
 /* Meters require at least OF 1.3. */
 *usable_protocols = OFPUTIL_P_OF13_UP;
+if (command == -2) {
+size_t len;
+
+string += strspn(string, " \t\r\n");   /* Skip white space. */
+len = strcspn(string, ", \t\r\n"); /* Get length of the first token. */
+
+if (!strncmp(string, "add", len)) {
+command = OFPMC13_ADD;
+} else if (!strncmp(string, "delete", len)) {
+command = OFPMC13_DELETE;
+} else if (!strncmp(string, "modify", len)) {
+command = OFPMC13_MODIFY;
+} else {
+len = 0;
+command = OFPMC13_ADD;
+}
+