[ovs-dev] [PATCH 1/2] ovn: Support port groups in ACLs
Han Zhou
zhouhan at gmail.com
Fri Mar 2 00:03:42 UTC 2018
Hi Mark, thanks for the testing! It looks great! And thanks for pointing
out that the feature enables the conjunctive match. I am glad this helps.
On Thu, Mar 1, 2018 at 3:49 PM, Mark Michelson <mmichels at redhat.com> wrote:
> Hi Han,
>
> Current criticisms notwithstanding, I decided to test your patch with a
> scenario that I thought it would help with.
>
> The base scenario is that I create an address set with 1000 addresses in
> it. I then create logical switch ports and create a new ACL for each port:
>
> ovn-nbctl acl-add ls0 to-lport 1000 "outport == \"lsp${COUNT}\" && ip4.dst
> == \$set1" allow
>
> The idea is that I'm creating a bunch of ACLs with identical L3
> restrictions but with different ports. I ran this using the 'time' utility,
> and here's the results:
>
> For 100 ports: 2m17.979s
> For 200 ports: 9m29.771s
>
> The big reason for this is that the ACLs generate a lot of flows. For 100
> ports, it generates 102501 flows in table 44 (ACL egress table). For 200
> ports, it generates 205001 flows in that table. Dobuling the number of
> ports doubles the number of flows. And doubling the number of flows
> quadruples the amount of time it takes to run.
>
> I applied your patch and modified the scenario a bit. Instead of adding an
> ACL for each new port, I instead created one ACL. It looks like:
>
> ovn-nbctl acl-add ls0 to-lport 1000 "outport == @pg1 && ip4.dst == \$set1"
> allow
>
> For each added port, I add the port to group pg1. Re-running the scenario,
> I got pretty much the same results. Honestly, I expected this.
>
> Now here's the exciting part. I then applied Numan Siddique's conjunctive
> match patch on top of yours. I then re-ran the test and got these results:
>
> For 100 ports: 6.453s
> For 200 ports: 16.008s
>
> Yeah, that's much better! I actually thought I had messed something up at
> first, but after double-checking the generated flows, all was correct. With
> 100 ports, table 44 has 1127 flows. Rather than requiring 100 ports * 1000
> addresses, it creates a conjunctive match of 100 ports + 1000 addresses.
> For 200 ports, table 44 has 1227 flows. It added exactly 100 flows over the
> 100 port scenario.
>
> From a performance perspective, this is great! I'll give the actual code a
> review in the upcoming days, hopefully before the weekend.
>
>
> On 02/28/2018 09:37 PM, Han Zhou wrote:
>
>> This patch enables using port group names in ACL match conditions.
>> Users can create a port group in northbound DB Port_Group table,
>> and then use the name of the port group in ACL match conditions
>> for "inport" or "outport". It can help reduce the number of ACLs
>> for CMS clients such as OpenStack Neutron, for the use cases
>> where a group of logical ports share same ACL rules except the
>> "inport"/"outport" part. Without this patch, the clients have to
>> create N (N = number of lports) ACLs, and this patch helps achieve
>> the same goal with only one ACL. E.g.:
>>
>> to-lport 1000 "outport == @port_group1 && ip4.src == {IP1, IP2, ...}"
>> allow-related
>>
>> There was a similar attempt by Zong Kai Li in 2016 [1]. This patch
>> takes a slightly different approach by using weak refs instead of
>> strings, which requires a new table instead of reusing the address
>> set table. This way it will also benefit for a follow up patch that
>> enables generating address sets automatically from port groups to
>> avoid a lot a trouble from client perspective [2].
>>
>> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2016-August/0
>> 77118.html
>> [2] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-Febr
>> uary/046260.html
>>
>> Reported-by: Daniel Alvarez Sanchez <dalvarez at redhat.com>
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-Febr
>> uary/046166.html
>> Signed-off-by: Han Zhou <hzhou8 at ebay.com>
>> ---
>> NEWS | 2 +
>> include/ovn/expr.h | 24 +++++---
>> include/ovn/lex.h | 1 +
>> ovn/controller/lflow.c | 16 +++--
>> ovn/controller/lflow.h | 1 +
>> ovn/controller/ofctrl.c | 6 +-
>> ovn/controller/ofctrl.h | 3 +-
>> ovn/controller/ovn-controller.c | 31 +++++++---
>> ovn/lib/actions.c | 3 +-
>> ovn/lib/expr.c | 127 ++++++++++++++++++++++++++++--
>> ----------
>> ovn/lib/lex.c | 20 +++++++
>> ovn/northd/ovn-northd.c | 47 +++++++++++++++
>> ovn/ovn-nb.ovsschema | 15 ++++-
>> ovn/ovn-nb.xml | 30 ++++++++++
>> ovn/ovn-sb.ovsschema | 10 +++-
>> ovn/ovn-sb.xml | 20 +++++++
>> ovn/utilities/ovn-trace.c | 27 +++++++--
>> tests/ovn.at | 34 +++++++++++
>> tests/test-ovn.c | 43 ++++++++++----
>> 19 files changed, 381 insertions(+), 79 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 4230189..da2c3ec 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -14,6 +14,8 @@ Post-v2.9.0
>> * OFPT_ROLE_STATUS is now available in OpenFlow 1.3.
>> - Linux kernel 4.14
>> * Add support for compiling OVS with the latest Linux 4.14 kernel
>> + - OVN:
>> + * Port_Group is supported in ACL match conditions.
>> v2.9.0 - 19 Feb 2018
>> --------------------
>> diff --git a/include/ovn/expr.h b/include/ovn/expr.h
>> index 711713e..3995e62 100644
>> --- a/include/ovn/expr.h
>> +++ b/include/ovn/expr.h
>> @@ -382,9 +382,11 @@ expr_from_node(const struct ovs_list *node)
>> void expr_format(const struct expr *, struct ds *);
>> void expr_print(const struct expr *);
>> struct expr *expr_parse(struct lexer *, const struct shash *symtab,
>> - const struct shash *addr_sets);
>> + const struct shash *addr_sets,
>> + const struct shash *port_groups);
>> struct expr *expr_parse_string(const char *, const struct shash *symtab,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> char **errorp);
>> struct expr *expr_clone(struct expr *);
>> @@ -404,6 +406,7 @@ bool expr_is_normalized(const struct expr *);
>> char *expr_parse_microflow(const char *, const struct shash *symtab,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> bool (*lookup_port)(const void *aux,
>> const char *port_name,
>> unsigned int *portp),
>> @@ -486,19 +489,22 @@ void expr_constant_set_format(const struct
>> expr_constant_set *, struct ds *);
>> void expr_constant_set_destroy(struct expr_constant_set *cs);
>>
>> -/* Address sets.
>> +/* Constant sets.
>> *
>> - * Instead of referring to a set of value as:
>> + * For example, instead of referring to a set of IP addresses as:
>> * {addr1, addr2, ..., addrN}
>> * You can register a set of values and refer to them as:
>> * $name
>> - * The address set entries should all have integer/masked-integer values.
>> - * The values that don't qualify are ignored.
>> + *
>> + * If convert_to_integer is true, the set must contain
>> + * integer/masked-integer values. The values that don't qualify
>> + * are ignored.
>> */
>> -void expr_addr_sets_add(struct shash *addr_sets, const char *name,
>> - const char * const *values, size_t n_values);
>> -void expr_addr_sets_remove(struct shash *addr_sets, const char *name);
>> -void expr_addr_sets_destroy(struct shash *addr_sets);
>> +void expr_const_sets_add(struct shash *const_sets, const char *name,
>> + const char * const *values, size_t n_values,
>> + bool convert_to_integer);
>> +void expr_const_sets_remove(struct shash *const_sets, const char *name);
>> +void expr_const_sets_destroy(struct shash *const_sets);
>> #endif /* ovn/expr.h */
>> diff --git a/include/ovn/lex.h b/include/ovn/lex.h
>> index afa4a23..8d55857 100644
>> --- a/include/ovn/lex.h
>> +++ b/include/ovn/lex.h
>> @@ -37,6 +37,7 @@ enum lex_type {
>> LEX_T_INTEGER, /* 12345 or 1.2.3.4 or ::1 or
>> 01:02:03:04:05 */
>> LEX_T_MASKED_INTEGER, /* 12345/10 or 1.2.0.0/16 or ::2/127
>> or... */
>> LEX_T_MACRO, /* $NAME */
>> + LEX_T_PORT_GROUP, /* @NAME */
>> LEX_T_ERROR, /* invalid input */
>> /* Bare tokens. */
>> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
>> index df125b1..cc2a020 100644
>> --- a/ovn/controller/lflow.c
>> +++ b/ovn/controller/lflow.c
>> @@ -70,6 +70,7 @@ static void consider_logical_flow(struct controller_ctx
>> *ctx,
>> struct hmap *nd_ra_opts,
>> uint32_t *conj_id_ofs,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> struct hmap *flow_table,
>> struct sset *active_tunnels,
>> struct sset *local_lport_ids);
>> @@ -149,6 +150,7 @@ add_logical_flows(struct controller_ctx *ctx,
>> struct ovn_extend_table *meter_table,
>> const struct sbrec_chassis *chassis,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> struct hmap *flow_table,
>> struct sset *active_tunnels,
>> struct sset *local_lport_ids)
>> @@ -179,8 +181,8 @@ add_logical_flows(struct controller_ctx *ctx,
>> lflow, local_datapaths,
>> group_table, meter_table, chassis,
>> &dhcp_opts, &dhcpv6_opts, &nd_ra_opts,
>> - &conj_id_ofs, addr_sets, flow_table,
>> - active_tunnels, local_lport_ids);
>> + &conj_id_ofs, addr_sets, port_groups,
>> + flow_table, active_tunnels,
>> local_lport_ids);
>> }
>> dhcp_opts_destroy(&dhcp_opts);
>> @@ -201,6 +203,7 @@ consider_logical_flow(struct controller_ctx *ctx,
>> struct hmap *nd_ra_opts,
>> uint32_t *conj_id_ofs,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> struct hmap *flow_table,
>> struct sset *active_tunnels,
>> struct sset *local_lport_ids)
>> @@ -258,7 +261,8 @@ consider_logical_flow(struct controller_ctx *ctx,
>> struct hmap matches;
>> struct expr *expr;
>> - expr = expr_parse_string(lflow->match, &symtab, addr_sets,
>> &error);
>> + expr = expr_parse_string(lflow->match, &symtab, addr_sets,
>> port_groups,
>> + &error);
>> if (!error) {
>> if (prereqs) {
>> expr = expr_combine(EXPR_T_AND, expr, prereqs);
>> @@ -450,13 +454,15 @@ lflow_run(struct controller_ctx *ctx,
>> struct ovn_extend_table *group_table,
>> struct ovn_extend_table *meter_table,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> struct hmap *flow_table,
>> struct sset *active_tunnels,
>> struct sset *local_lport_ids)
>> {
>> add_logical_flows(ctx, chassis_index, local_datapaths,
>> - group_table, meter_table, chassis, addr_sets,
>> flow_table,
>> - active_tunnels, local_lport_ids);
>> + group_table, meter_table, chassis, addr_sets,
>> + port_groups, flow_table, active_tunnels,
>> + local_lport_ids);
>> add_neighbor_flows(ctx, flow_table);
>> }
>> diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
>> index 22bf534..dcf2fe7 100644
>> --- a/ovn/controller/lflow.h
>> +++ b/ovn/controller/lflow.h
>> @@ -69,6 +69,7 @@ void lflow_run(struct controller_ctx *,
>> struct ovn_extend_table *group_table,
>> struct ovn_extend_table *meter_table,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> struct hmap *flow_table,
>> struct sset *active_tunnels,
>> struct sset *local_lport_ids);
>> diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
>> index 8d6d1b6..e3ee6b1 100644
>> --- a/ovn/controller/ofctrl.c
>> +++ b/ovn/controller/ofctrl.c
>> @@ -1145,7 +1145,8 @@ ofctrl_lookup_port(const void *br_int_, const char
>> *port_name,
>> * must free(). */
>> char *
>> ofctrl_inject_pkt(const struct ovsrec_bridge *br_int, const char
>> *flow_s,
>> - const struct shash *addr_sets)
>> + const struct shash *addr_sets,
>> + const struct shash *port_groups)
>> {
>> int version = rconn_get_version(swconn);
>> if (version < 0) {
>> @@ -1154,7 +1155,8 @@ ofctrl_inject_pkt(const struct ovsrec_bridge
>> *br_int, const char *flow_s,
>> struct flow uflow;
>> char *error = expr_parse_microflow(flow_s, &symtab, addr_sets,
>> - ofctrl_lookup_port, br_int,
>> &uflow);
>> + port_groups, ofctrl_lookup_port,
>> + br_int, &uflow);
>> if (error) {
>> return error;
>> }
>> diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
>> index d53bc68..a8b3afc 100644
>> --- a/ovn/controller/ofctrl.h
>> +++ b/ovn/controller/ofctrl.h
>> @@ -47,7 +47,8 @@ struct ovn_flow *ofctrl_dup_flow(struct ovn_flow
>> *source);
>> void ofctrl_ct_flush_zone(uint16_t zone_id);
>> char *ofctrl_inject_pkt(const struct ovsrec_bridge *br_int,
>> - const char *flow_s, const struct shash
>> *addr_sets);
>> + const char *flow_s, const struct shash
>> *addr_sets,
>> + const struct shash *port_groups);
>> /* Flow table interfaces to the rest of ovn-controller. */
>> void ofctrl_add_flow(struct hmap *desired_flows, uint8_t table_id,
>> diff --git a/ovn/controller/ovn-controller.c
>> b/ovn/controller/ovn-controller.c
>> index 7592bda..492e4fa 100644
>> --- a/ovn/controller/ovn-controller.c
>> +++ b/ovn/controller/ovn-controller.c
>> @@ -288,9 +288,22 @@ addr_sets_init(struct controller_ctx *ctx, struct
>> shash *addr_sets)
>> {
>> const struct sbrec_address_set *as;
>> SBREC_ADDRESS_SET_FOR_EACH (as, ctx->ovnsb_idl) {
>> - expr_addr_sets_add(addr_sets, as->name,
>> - (const char *const *) as->addresses,
>> - as->n_addresses);
>> + expr_const_sets_add(addr_sets, as->name,
>> + (const char *const *) as->addresses,
>> + as->n_addresses, true);
>> + }
>> +}
>> +
>> +/* Iterate port groups in the southbound database. Create and update the
>> + * corresponding symtab entries as necessary. */
>> +static void
>> +port_groups_init(struct controller_ctx *ctx, struct shash *port_groups)
>> +{
>> + const struct sbrec_port_group *pg;
>> + SBREC_PORT_GROUP_FOR_EACH (pg, ctx->ovnsb_idl) {
>> + expr_const_sets_add(port_groups, pg->name,
>> + (const char *const *) pg->ports,
>> + pg->n_ports, false);
>> }
>> }
>> @@ -696,6 +709,8 @@ main(int argc, char *argv[])
>> if (br_int && chassis) {
>> struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
>> addr_sets_init(&ctx, &addr_sets);
>> + struct shash port_groups = SHASH_INITIALIZER(&port_groups);
>> + port_groups_init(&ctx, &port_groups);
>> patch_run(&ctx, br_int, chassis);
>> @@ -713,8 +728,8 @@ main(int argc, char *argv[])
>> struct hmap flow_table =
>> HMAP_INITIALIZER(&flow_table);
>> lflow_run(&ctx, chassis,
>> &chassis_index, &local_datapaths,
>> &group_table,
>> - &meter_table, &addr_sets, &flow_table,
>> - &active_tunnels, &local_lport_ids);
>> + &meter_table, &addr_sets, &port_groups,
>> + &flow_table, &active_tunnels,
>> &local_lport_ids);
>> if (chassis_id) {
>> bfd_run(&ctx, br_int, chassis, &local_datapaths,
>> @@ -740,7 +755,7 @@ main(int argc, char *argv[])
>> if (pending_pkt.conn) {
>> char *error = ofctrl_inject_pkt(br_int,
>> pending_pkt.flow_s,
>> - &addr_sets);
>> + &port_groups,
>> &addr_sets);
>> if (error) {
>> unixctl_command_reply_error(pending_pkt.conn,
>> error);
>> free(error);
>> @@ -754,8 +769,10 @@ main(int argc, char *argv[])
>> update_sb_monitors(ctx.ovnsb_idl, chassis,
>> &local_lports, &local_datapaths);
>> - expr_addr_sets_destroy(&addr_sets);
>> + expr_const_sets_destroy(&addr_sets);
>> shash_destroy(&addr_sets);
>> + expr_const_sets_destroy(&port_groups);
>> + shash_destroy(&port_groups);
>> }
>> /* If we haven't handled the pending packet insertion
>> diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
>> index fde3bff..8845e77 100644
>> --- a/ovn/lib/actions.c
>> +++ b/ovn/lib/actions.c
>> @@ -234,7 +234,8 @@ add_prerequisite(struct action_context *ctx, const
>> char *prerequisite)
>> struct expr *expr;
>> char *error;
>> - expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL,
>> &error);
>> + expr = expr_parse_string(prerequisite, ctx->pp->symtab, NULL, NULL,
>> + &error);
>> ovs_assert(!error);
>> ctx->prereqs = expr_combine(EXPR_T_AND, ctx->prereqs, expr);
>> }
>> diff --git a/ovn/lib/expr.c b/ovn/lib/expr.c
>> index b0aa672..1a33269 100644
>> --- a/ovn/lib/expr.c
>> +++ b/ovn/lib/expr.c
>> @@ -464,6 +464,7 @@ struct expr_context {
>> struct lexer *lexer; /* Lexer for pulling more tokens. */
>> const struct shash *symtab; /* Symbol table. */
>> const struct shash *addr_sets; /* Address set table. */
>> + const struct shash *port_groups; /* Port groups table. */
>> bool not; /* True inside odd number of NOT
>> operators. */
>> };
>> @@ -752,6 +753,36 @@ parse_addr_sets(struct expr_context *ctx, struct
>> expr_constant_set *cs,
>> }
>> static bool
>> +parse_port_group(struct expr_context *ctx, struct expr_constant_set *cs,
>> + size_t *allocated_values)
>> +{
>> + struct expr_constant_set *port_group
>> + = (ctx->port_groups
>> + ? shash_find_data(ctx->port_groups, ctx->lexer->token.s)
>> + : NULL);
>> + if (!port_group) {
>> + lexer_syntax_error(ctx->lexer, "expecting port group name");
>> + return false;
>> + }
>> +
>> + if (!assign_constant_set_type(ctx, cs, EXPR_C_STRING)) {
>> + return false;
>> + }
>> +
>> + size_t n_values = cs->n_values + port_group->n_values;
>> + if (n_values >= *allocated_values) {
>> + cs->values = xrealloc(cs->values, n_values * sizeof *cs->values);
>> + *allocated_values = n_values;
>> + }
>> + for (size_t i = 0; i < port_group->n_values; i++) {
>> + cs->values[cs->n_values++].string =
>> + xstrdup(port_group->values[i].string);
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static bool
>> parse_constant(struct expr_context *ctx, struct expr_constant_set *cs,
>> size_t *allocated_values)
>> {
>> @@ -788,6 +819,12 @@ parse_constant(struct expr_context *ctx, struct
>> expr_constant_set *cs,
>> }
>> lexer_get(ctx->lexer);
>> return true;
>> + } else if (ctx->lexer->token.type == LEX_T_PORT_GROUP) {
>> + if (!parse_port_group(ctx, cs, allocated_values)) {
>> + return false;
>> + }
>> + lexer_get(ctx->lexer);
>> + return true;
>> } else {
>> lexer_syntax_error(ctx->lexer, "expecting constant");
>> return false;
>> @@ -939,65 +976,74 @@ expr_constant_set_destroy(struct expr_constant_set
>> *cs)
>> }
>> }
>> -/* Adds an address set named 'name' to 'addr_sets', replacing any
>> existing
>> - * address set entry with the given name. */
>> +/* Adds an constant set named 'name' to 'const_sets', replacing any
>> existing
>> + * constant set entry with the given name. */
>> void
>> -expr_addr_sets_add(struct shash *addr_sets, const char *name,
>> - const char *const *values, size_t n_values)
>> +expr_const_sets_add(struct shash *const_sets, const char *name,
>> + const char *const *values, size_t n_values,
>> + bool convert_to_integer)
>> {
>> /* Replace any existing entry for this name. */
>> - expr_addr_sets_remove(addr_sets, name);
>> + expr_const_sets_remove(const_sets, name);
>> struct expr_constant_set *cs = xzalloc(sizeof *cs);
>> - cs->type = EXPR_C_INTEGER;
>> cs->in_curlies = true;
>> cs->n_values = 0;
>> cs->values = xmalloc(n_values * sizeof *cs->values);
>> - for (size_t i = 0; i < n_values; i++) {
>> - /* Use the lexer to convert each address set into the proper
>> - * integer format. */
>> - struct lexer lex;
>> - lexer_init(&lex, values[i]);
>> - lexer_get(&lex);
>> - if (lex.token.type != LEX_T_INTEGER
>> - && lex.token.type != LEX_T_MASKED_INTEGER) {
>> - VLOG_WARN("Invalid address set entry: '%s', token type: %d",
>> - values[i], lex.token.type);
>> - } else {
>> - union expr_constant *c = &cs->values[cs->n_values++];
>> - c->value = lex.token.value;
>> - c->format = lex.token.format;
>> - c->masked = lex.token.type == LEX_T_MASKED_INTEGER;
>> - if (c->masked) {
>> - c->mask = lex.token.mask;
>> + if (convert_to_integer) {
>> + cs->type = EXPR_C_INTEGER;
>> + for (size_t i = 0; i < n_values; i++) {
>> + /* Use the lexer to convert each constant set into the proper
>> + * integer format. */
>> + struct lexer lex;
>> + lexer_init(&lex, values[i]);
>> + lexer_get(&lex);
>> + if (lex.token.type != LEX_T_INTEGER
>> + && lex.token.type != LEX_T_MASKED_INTEGER) {
>> + VLOG_WARN("Invalid constant set entry: '%s', token type:
>> %d",
>> + values[i], lex.token.type);
>> + } else {
>> + union expr_constant *c = &cs->values[cs->n_values++];
>> + c->value = lex.token.value;
>> + c->format = lex.token.format;
>> + c->masked = lex.token.type == LEX_T_MASKED_INTEGER;
>> + if (c->masked) {
>> + c->mask = lex.token.mask;
>> + }
>> }
>> + lexer_destroy(&lex);
>> + }
>> + } else {
>> + cs->type = EXPR_C_STRING;
>> + for (size_t i = 0; i < n_values; i++) {
>> + union expr_constant *c = &cs->values[cs->n_values++];
>> + c->string = xstrdup(values[i]);
>> }
>> - lexer_destroy(&lex);
>> }
>> - shash_add(addr_sets, name, cs);
>> + shash_add(const_sets, name, cs);
>> }
>> void
>> -expr_addr_sets_remove(struct shash *addr_sets, const char *name)
>> +expr_const_sets_remove(struct shash *const_sets, const char *name)
>> {
>> - struct expr_constant_set *cs = shash_find_and_delete(addr_sets,
>> name);
>> + struct expr_constant_set *cs = shash_find_and_delete(const_sets,
>> name);
>> if (cs) {
>> expr_constant_set_destroy(cs);
>> free(cs);
>> }
>> }
>> -/* Destroy all contents of 'addr_sets'. */
>> +/* Destroy all contents of 'const_sets'. */
>> void
>> -expr_addr_sets_destroy(struct shash *addr_sets)
>> +expr_const_sets_destroy(struct shash *const_sets)
>> {
>> struct shash_node *node, *next;
>> - SHASH_FOR_EACH_SAFE (node, next, addr_sets) {
>> + SHASH_FOR_EACH_SAFE (node, next, const_sets) {
>> struct expr_constant_set *cs = node->data;
>> - shash_delete(addr_sets, node);
>> + shash_delete(const_sets, node);
>> expr_constant_set_destroy(cs);
>> free(cs);
>> }
>> @@ -1214,11 +1260,13 @@ expr_parse__(struct expr_context *ctx)
>> * lexer->error is NULL. */
>> struct expr *
>> expr_parse(struct lexer *lexer, const struct shash *symtab,
>> - const struct shash *addr_sets)
>> + const struct shash *addr_sets,
>> + const struct shash *port_groups)
>> {
>> struct expr_context ctx = { .lexer = lexer,
>> .symtab = symtab,
>> - .addr_sets = addr_sets };
>> + .addr_sets = addr_sets,
>> + .port_groups = port_groups };
>> return lexer->error ? NULL : expr_parse__(&ctx);
>> }
>> @@ -1230,13 +1278,15 @@ expr_parse(struct lexer *lexer, const struct
>> shash *symtab,
>> * error (with free()). */
>> struct expr *
>> expr_parse_string(const char *s, const struct shash *symtab,
>> - const struct shash *addr_sets, char **errorp)
>> + const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> + char **errorp)
>> {
>> struct lexer lexer;
>> lexer_init(&lexer, s);
>> lexer_get(&lexer);
>> - struct expr *expr = expr_parse(&lexer, symtab, addr_sets);
>> + struct expr *expr = expr_parse(&lexer, symtab, addr_sets,
>> port_groups);
>> lexer_force_end(&lexer);
>> *errorp = lexer_steal_error(&lexer);
>> if (*errorp) {
>> @@ -1460,7 +1510,7 @@ expr_get_level(const struct expr *expr)
>> static enum expr_level
>> expr_parse_level(const char *s, const struct shash *symtab, char
>> **errorp)
>> {
>> - struct expr *expr = expr_parse_string(s, symtab, NULL, errorp);
>> + struct expr *expr = expr_parse_string(s, symtab, NULL, NULL, errorp);
>> enum expr_level level = expr ? expr_get_level(expr) :
>> EXPR_L_NOMINAL;
>> expr_destroy(expr);
>> return level;
>> @@ -1618,7 +1668,7 @@ parse_and_annotate(const char *s, const struct
>> shash *symtab,
>> char *error;
>> struct expr *expr;
>> - expr = expr_parse_string(s, symtab, NULL, &error);
>> + expr = expr_parse_string(s, symtab, NULL, NULL, &error);
>> if (expr) {
>> expr = expr_annotate__(expr, symtab, nesting, &error);
>> }
>> @@ -3277,6 +3327,7 @@ expr_parse_microflow__(struct lexer *lexer,
>> char * OVS_WARN_UNUSED_RESULT
>> expr_parse_microflow(const char *s, const struct shash *symtab,
>> const struct shash *addr_sets,
>> + const struct shash *port_groups,
>> bool (*lookup_port)(const void *aux,
>> const char *port_name,
>> unsigned int *portp),
>> @@ -3286,7 +3337,7 @@ expr_parse_microflow(const char *s, const struct
>> shash *symtab,
>> lexer_init(&lexer, s);
>> lexer_get(&lexer);
>> - struct expr *e = expr_parse(&lexer, symtab, addr_sets);
>> + struct expr *e = expr_parse(&lexer, symtab, addr_sets, port_groups);
>> lexer_force_end(&lexer);
>> if (e) {
>> diff --git a/ovn/lib/lex.c b/ovn/lib/lex.c
>> index 2f49af0..9463640 100644
>> --- a/ovn/lib/lex.c
>> +++ b/ovn/lib/lex.c
>> @@ -231,6 +231,10 @@ lex_token_format(const struct lex_token *token,
>> struct ds *s)
>> ds_put_format(s, "$%s", token->s);
>> break;
>> + case LEX_T_PORT_GROUP:
>> + ds_put_format(s, "@%s", token->s);
>> + break;
>> +
>> case LEX_T_LPAREN:
>> ds_put_cstr(s, "(");
>> break;
>> @@ -573,6 +577,18 @@ lex_parse_addr_set(const char *p, struct lex_token
>> *token)
>> return lex_parse_id(p, LEX_T_MACRO, token);
>> }
>> +static const char *
>> +lex_parse_port_group(const char *p, struct lex_token *token)
>> +{
>> + p++;
>> + if (!lex_is_id1(*p)) {
>> + lex_error(token, "`$' must be followed by a valid identifier.");
>> + return p;
>> + }
>> +
>> + return lex_parse_id(p, LEX_T_PORT_GROUP, token);
>> +}
>> +
>> /* Initializes 'token' and parses the first token from the beginning of
>> * null-terminated string 'p' into 'token'. Stores a pointer to the
>> start of
>> * the token (after skipping white space and comments, if any) into
>> '*startp'.
>> @@ -747,6 +763,10 @@ next:
>> p = lex_parse_addr_set(p, token);
>> break;
>> + case '@':
>> + p = lex_parse_port_group(p, token);
>> + break;
>> +
>> case ':':
>> if (p[1] != ':') {
>> token->type = LEX_T_COLON;
>> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
>> index d83681c..2924d8f 100644
>> --- a/ovn/northd/ovn-northd.c
>> +++ b/ovn/northd/ovn-northd.c
>> @@ -6135,6 +6135,49 @@ sync_address_sets(struct northd_context *ctx)
>> shash_destroy(&sb_address_sets);
>> }
>> +/* Each port group in Port_Group table in OVN_Northbound has a
>> corresponding
>> + * entry in Port_Group table in OVN_Southbound. In OVN_Northbound the
>> entries
>> + * contains lport uuids, while in OVN_Southbound we store the lport
>> names.
>> + */
>> +static void
>> +sync_port_groups(struct northd_context *ctx)
>> +{
>> + struct shash sb_port_groups = SHASH_INITIALIZER(&sb_port_groups);
>> +
>> + const struct sbrec_port_group *sb_port_group;
>> + SBREC_PORT_GROUP_FOR_EACH (sb_port_group, ctx->ovnsb_idl) {
>> + shash_add(&sb_port_groups, sb_port_group->name, sb_port_group);
>> + }
>> +
>> + const struct nbrec_port_group *nb_port_group;
>> + NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) {
>> + sb_port_group = shash_find_and_delete(&sb_port_groups,
>> + nb_port_group->name);
>> + if (!sb_port_group) {
>> + sb_port_group = sbrec_port_group_insert(ctx->ovnsb_txn);
>> + sbrec_port_group_set_name(sb_port_group,
>> nb_port_group->name);
>> + }
>> +
>> + const char **nb_port_names = xcalloc(nb_port_group->n_ports,
>> + sizeof *nb_port_names);
>> + int i;
>> + for (i = 0; i < nb_port_group->n_ports; i++) {
>> + nb_port_names[i] = nb_port_group->ports[i]->name;
>> + }
>> + sbrec_port_group_set_ports(sb_port_group,
>> + nb_port_names,
>> + nb_port_group->n_ports);
>> + free(nb_port_names);
>> + }
>> +
>> + struct shash_node *node, *next;
>> + SHASH_FOR_EACH_SAFE (node, next, &sb_port_groups) {
>> + sbrec_port_group_delete(node->data);
>> + shash_delete(&sb_port_groups, node);
>> + }
>> + shash_destroy(&sb_port_groups);
>> +}
>> +
>> /*
>> * struct 'dns_info' is used to sync the DNS records between OVN
>> Northbound db
>> * and Southbound db.
>> @@ -6251,6 +6294,7 @@ ovnnb_db_run(struct northd_context *ctx, struct
>> chassis_index *chassis_index,
>> build_lflows(ctx, &datapaths, &ports);
>> sync_address_sets(ctx);
>> + sync_port_groups(ctx);
>> sync_dns_entries(ctx, &datapaths);
>> struct ovn_datapath *dp, *next_dp;
>> @@ -6848,6 +6892,9 @@ main(int argc, char *argv[])
>> ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set);
>> add_column_noalert(ovnsb_idl_loop.idl,
>> &sbrec_address_set_col_name);
>> add_column_noalert(ovnsb_idl_loop.idl,
>> &sbrec_address_set_col_addresses);
>> + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_port_group);
>> + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_group_col_name);
>> + add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_group_col_ports);
>> ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_dns);
>> add_column_noalert(ovnsb_idl_loop.idl, &sbrec_dns_col_datapaths);
>> diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
>> index 32f9d5a..2d09282 100644
>> --- a/ovn/ovn-nb.ovsschema
>> +++ b/ovn/ovn-nb.ovsschema
>> @@ -1,7 +1,7 @@
>> {
>> "name": "OVN_Northbound",
>> "version": "5.10.0",
>> - "cksum": "626737541 17810",
>> + "cksum": "222987318 18430",
>> "tables": {
>> "NB_Global": {
>> "columns": {
>> @@ -114,6 +114,19 @@
>> "min": 0, "max": "unlimited"}}},
>> "indexes": [["name"]],
>> "isRoot": true},
>> + "Port_Group": {
>> + "columns": {
>> + "name": {"type": "string"},
>> + "ports": {"type": {"key": {"type": "uuid",
>> + "refTable":
>> "Logical_Switch_Port",
>> + "refType": "weak"},
>> + "min": 0,
>> + "max": "unlimited"}},
>> + "external_ids": {
>> + "type": {"key": "string", "value": "string",
>> + "min": 0, "max": "unlimited"}}},
>> + "indexes": [["name"]],
>> + "isRoot": true},
>> "Load_Balancer": {
>> "columns": {
>> "name": {"type": "string"},
>> diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
>> index b7a5b6b..83727c5 100644
>> --- a/ovn/ovn-nb.xml
>> +++ b/ovn/ovn-nb.xml
>> @@ -922,6 +922,36 @@
>> </group>
>> </table>
>> + <table name="Port_Group" title="Port Groups">
>> + <p>
>> + Each row in this table represents a named group of logical switch
>> ports.
>> + </p>
>> +
>> + <p>
>> + Port groups may be used in the <ref column="match" table="ACL"/>
>> column
>> + of the <ref table="ACL"/> table. For syntax information, see the
>> details
>> + of the expression language used for the <ref column="match"
>> + table="Logical_Flow" db="OVN_Southbound"/> column in the <ref
>> + table="Logical_Flow" db="OVN_Southbound"/> table of the <ref
>> + db="OVN_Southbound"/> database.
>> + </p>
>> +
>> + <column name="name">
>> + A name for the port group. Names are ASCII and must match
>> + <code>[a-zA-Z_.][a-zA-Z_.0-9]*</code>.
>> + </column>
>> +
>> + <column name="ports">
>> + The logical switch ports belonging to the group in uuids.
>> + </column>
>> +
>> + <group title="Common Columns">
>> + <column name="external_ids">
>> + See <em>External IDs</em> at the beginning of this document.
>> + </column>
>> + </group>
>> + </table>
>> +
>> <table name="Load_Balancer" title="load balancer">
>> <p>
>> Each row represents one load balancer.
>> diff --git a/ovn/ovn-sb.ovsschema b/ovn/ovn-sb.ovsschema
>> index 2abcc6b..9e271d4 100644
>> --- a/ovn/ovn-sb.ovsschema
>> +++ b/ovn/ovn-sb.ovsschema
>> @@ -1,7 +1,7 @@
>> {
>> "name": "OVN_Southbound",
>> "version": "1.15.0",
>> - "cksum": "70426956 13327",
>> + "cksum": "1839738004 13639",
>> "tables": {
>> "SB_Global": {
>> "columns": {
>> @@ -55,6 +55,14 @@
>> "max": "unlimited"}}},
>> "indexes": [["name"]],
>> "isRoot": true},
>> + "Port_Group": {
>> + "columns": {
>> + "name": {"type": "string"},
>> + "ports": {"type": {"key": "string",
>> + "min": 0,
>> + "max": "unlimited"}}},
>> + "indexes": [["name"]],
>> + "isRoot": true},
>> "Logical_Flow": {
>> "columns": {
>> "logical_datapath": {"type": {"key": {"type": "uuid",
>> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
>> index f000b16..2eac943 100644
>> --- a/ovn/ovn-sb.xml
>> +++ b/ovn/ovn-sb.xml
>> @@ -377,6 +377,18 @@
>> <column name="addresses"/>
>> </table>
>> + <table name="Port_Group" title="Port Groups">
>> + <p>
>> + This table contains names for the logical switch ports in the
>> + <ref db="OVN_Northbound"/> database that belongs to the same group
>> + that is defined in <ref table="Port_Group" db="OVN_Northbound"/>
>> + in the <ref db="OVN_Northbound"/> database.
>> + </p>
>> +
>> + <column name="name"/>
>> + <column name="ports"/>
>> + </table>
>> +
>> <table name="Logical_Flow" title="Logical Network Flows">
>> <p>
>> Each row in this table represents one logical flow.
>> @@ -770,6 +782,14 @@
>> <code>$set1</code>.
>> </p>
>> + <p>
>> + You may refer to a group of logical switch ports stored in the
>> + <ref table="Port_Group"/> table by its <ref column="name"
>> + table="Port_Group"/>. An <ref table="Port_Group"/> with a name
>> + of <code>port_group1</code> can be referred to as
>> + <code>@port_group1</code>.
>> + </p>
>> +
>> <p><em>Miscellaneous</em></p>
>> <p>
>> diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
>> index db39c49..5a9e73d 100644
>> --- a/ovn/utilities/ovn-trace.c
>> +++ b/ovn/utilities/ovn-trace.c
>> @@ -418,6 +418,9 @@ static struct shash symtab;
>> /* Address sets. */
>> static struct shash address_sets;
>> +/* Port groups. */
>> +static struct shash port_groups;
>> +
>> /* DHCP options. */
>> static struct hmap dhcp_opts; /* Contains "struct gen_opts_map"s. */
>> static struct hmap dhcpv6_opts; /* Contains "struct gen_opts_map"s. */
>> @@ -697,9 +700,22 @@ read_address_sets(void)
>> const struct sbrec_address_set *sbas;
>> SBREC_ADDRESS_SET_FOR_EACH (sbas, ovnsb_idl) {
>> - expr_addr_sets_add(&address_sets, sbas->name,
>> + expr_const_sets_add(&address_sets, sbas->name,
>> (const char *const *) sbas->addresses,
>> - sbas->n_addresses);
>> + sbas->n_addresses, true);
>> + }
>> +}
>> +
>> +static void
>> +read_port_groups(void)
>> +{
>> + shash_init(&port_groups);
>> +
>> + const struct sbrec_port_group *sbpg;
>> + SBREC_PORT_GROUP_FOR_EACH (sbpg, ovnsb_idl) {
>> + expr_const_sets_add(&port_groups, sbpg->name,
>> + (const char *const *) sbpg->ports,
>> + sbpg->n_ports, false);
>> }
>> }
>> @@ -796,7 +812,8 @@ read_flows(void)
>> char *error;
>> struct expr *match;
>> - match = expr_parse_string(sblf->match, &symtab, &address_sets,
>> &error);
>> + match = expr_parse_string(sblf->match, &symtab, &address_sets,
>> + &port_groups, &error);
>> if (error) {
>> VLOG_WARN("%s: parsing expression failed (%s)",
>> sblf->match, error);
>> @@ -937,6 +954,7 @@ read_db(void)
>> read_ports();
>> read_mcgroups();
>> read_address_sets();
>> + read_port_groups();
>> read_gen_opts();
>> read_flows();
>> read_mac_bindings();
>> @@ -2009,7 +2027,8 @@ trace(const char *dp_s, const char *flow_s)
>> struct flow uflow;
>> char *error = expr_parse_microflow(flow_s, &symtab, &address_sets,
>> - ovntrace_lookup_port, dp, &uflow);
>> + &port_groups,
>> ovntrace_lookup_port,
>> + dp, &uflow);
>> if (error) {
>> char *s = xasprintf("error parsing flow: %s\n", error);
>> free(error);
>> diff --git a/tests/ovn.at b/tests/ovn.at
>> index 8ee3bf0..c8ab7b7 100644
>> --- a/tests/ovn.at
>> +++ b/tests/ovn.at
>> @@ -304,6 +304,7 @@ inport = "eth0" => Syntax error at `=' expecting
>> relational operator.
>> 123 == 123 => Syntax error at `123' expecting field name.
>> $name => Syntax error at `$name' expecting address set name.
>> + at name => Syntax error at `@name' expecting port group name.
>> 123 == xyzzy => Syntax error at `xyzzy' expecting field name.
>> xyzzy == 1 => Syntax error at `xyzzy' expecting field name.
>> @@ -657,6 +658,24 @@ ip,nw_src=8.0.0.0/8.0.0.0
>> ])
>> AT_CLEANUP
>> +AT_SETUP([ovn -- converting expressions to flows -- port groups])
>> +AT_KEYWORDS([expression])
>> +expr_to_flow () {
>> + echo "$1" | ovstest test-ovn expr-to-flows | sort
>> +}
>> +AT_CHECK([expr_to_flow 'outport == @pg1'], [0], [dnl
>> +reg15=0x11
>> +reg15=0x12
>> +reg15=0x13
>> +])
>> +AT_CHECK([expr_to_flow 'outport == {@pg_empty}'], [0], [dnl
>> +(no flows)
>> +])
>> +AT_CHECK([expr_to_flow 'outport == {"lsp1", @pg_empty}'], [0], [dnl
>> +reg15=0x11
>> +])
>> +AT_CLEANUP
>> +
>> AT_SETUP([ovn -- action parsing])
>> dnl Unindented text is input (a set of OVN logical actions).
>> dnl Indented text is expected output.
>> @@ -1194,6 +1213,13 @@ ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type ==
>> 0x1236 && outport == "lp33"' d
>> ovn-nbctl create Address_Set name=set1 addresses=\"f0:00:00:00:00:11\
>> ",\"f0:00:00:00:00:21\",\"f0:00:00:00:00:31\"
>> ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1237 && eth.src ==
>> $set1 && outport == "lp33"' drop
>> +get_lsp_uuid () {
>> + ovn-nbctl lsp-list lsw0 | grep $1 | awk '{ print $1 }'
>> +}
>> +
>> +ovn-nbctl create Port_Group name=pg1 ports=`get_lsp_uuid
>> lp22`,`get_lsp_uuid lp33`
>> +ovn-nbctl acl-add lsw0 to-lport 1000 'eth.type == 0x1238 && outport ==
>> @pg1' drop
>> +
>> # Pre-populate the hypervisors' ARP tables so that we don't lose any
>> # packets for ARP resolution (native tunneling doesn't queue packets
>> # for ARP resolution).
>> @@ -1334,10 +1360,18 @@ for is in 1 2 3; do
>> else
>> acl4=$d
>> fi
>> + if test $d = $s || test $d = 22 || test $d = 33; then
>> + # dest of 22 and 33 should be dropped
>> + # due to the 5th ACL that uses port_group(pg1).
>> + acl5=
>> + else
>> + acl5=$d
>> + fi
>> test_packet $s f000000000$d f000000000$s 1234
>> #7, acl1
>> test_packet $s f000000000$d f000000000$s 1235 $acl2
>> #7, acl2
>> test_packet $s f000000000$d f000000000$s 1236 $acl3
>> #7, acl3
>> test_packet $s f000000000$d f000000000$s 1237 $acl4
>> #7, acl4
>> + test_packet $s f000000000$d f000000000$s 1238 $acl5 #7,
>> acl5
>> test_packet $s f000000000$d f00000000055
>> 810000091234 #4
>> test_packet $s f000000000$d 0100000000$s $s$d
>> #5
>> diff --git a/tests/test-ovn.c b/tests/test-ovn.c
>> index 7452753..d4a5d59 100644
>> --- a/tests/test-ovn.c
>> +++ b/tests/test-ovn.c
>> @@ -211,10 +211,24 @@ create_addr_sets(struct shash *addr_sets)
>> };
>> static const char *const addrs4[] = { NULL };
>> - expr_addr_sets_add(addr_sets, "set1", addrs1, 3);
>> - expr_addr_sets_add(addr_sets, "set2", addrs2, 3);
>> - expr_addr_sets_add(addr_sets, "set3", addrs3, 3);
>> - expr_addr_sets_add(addr_sets, "set4", addrs4, 0);
>> + expr_const_sets_add(addr_sets, "set1", addrs1, 3, true);
>> + expr_const_sets_add(addr_sets, "set2", addrs2, 3, true);
>> + expr_const_sets_add(addr_sets, "set3", addrs3, 3, true);
>> + expr_const_sets_add(addr_sets, "set4", addrs4, 0, true);
>> +}
>> +
>> +static void
>> +create_port_groups(struct shash *port_groups)
>> +{
>> + shash_init(port_groups);
>> +
>> + static const char *const pg1[] = {
>> + "lsp1", "lsp2", "lsp3",
>> + };
>> + static const char *const pg2[] = { NULL };
>> +
>> + expr_const_sets_add(port_groups, "pg1", pg1, 3, false);
>> + expr_const_sets_add(port_groups, "pg_empty", pg2, 0, false);
>> }
>> static bool
>> @@ -245,23 +259,29 @@ test_parse_expr__(int steps)
>> {
>> struct shash symtab;
>> struct shash addr_sets;
>> + struct shash port_groups;
>> struct simap ports;
>> struct ds input;
>> create_symtab(&symtab);
>> create_addr_sets(&addr_sets);
>> + create_port_groups(&port_groups);
>> simap_init(&ports);
>> simap_put(&ports, "eth0", 5);
>> simap_put(&ports, "eth1", 6);
>> simap_put(&ports, "LOCAL", ofp_to_u16(OFPP_LOCAL));
>> + simap_put(&ports, "lsp1", 0x11);
>> + simap_put(&ports, "lsp2", 0x12);
>> + simap_put(&ports, "lsp3", 0x13);
>> ds_init(&input);
>> while (!ds_get_test_line(&input, stdin)) {
>> struct expr *expr;
>> char *error;
>> - expr = expr_parse_string(ds_cstr(&input), &symtab,
>> &addr_sets, &error);
>> + expr = expr_parse_string(ds_cstr(&input), &symtab, &addr_sets,
>> + &port_groups, &error);
>> if (!error && steps > 0) {
>> expr = expr_annotate(expr, &symtab, &error);
>> }
>> @@ -298,8 +318,10 @@ test_parse_expr__(int steps)
>> simap_destroy(&ports);
>> expr_symtab_destroy(&symtab);
>> shash_destroy(&symtab);
>> - expr_addr_sets_destroy(&addr_sets);
>> + expr_const_sets_destroy(&addr_sets);
>> shash_destroy(&addr_sets);
>> + expr_const_sets_destroy(&port_groups);
>> + shash_destroy(&port_groups);
>> }
>> static void
>> @@ -373,7 +395,7 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx)
>> ovn_init_symtab(&symtab);
>> struct flow uflow;
>> - char *error = expr_parse_microflow(ctx->argv[1], &symtab, NULL,
>> + char *error = expr_parse_microflow(ctx->argv[1], &symtab, NULL,
>> NULL,
>> lookup_atoi_cb, NULL, &uflow);
>> if (error) {
>> ovs_fatal(0, "%s", error);
>> @@ -383,7 +405,7 @@ test_evaluate_expr(struct ovs_cmdl_context *ctx)
>> while (!ds_get_test_line(&input, stdin)) {
>> struct expr *expr;
>> - expr = expr_parse_string(ds_cstr(&input), &symtab, NULL,
>> &error);
>> + expr = expr_parse_string(ds_cstr(&input), &symtab, NULL, NULL,
>> &error);
>> if (!error) {
>> expr = expr_annotate(expr, &symtab, &error);
>> }
>> @@ -857,7 +879,8 @@ test_tree_shape_exhaustively(struct expr *expr,
>> struct shash *symtab,
>> expr_format(expr, &s);
>> char *error;
>> - modified = expr_parse_string(ds_cstr(&s), symtab, NULL,
>> &error);
>> + modified = expr_parse_string(ds_cstr(&s), symtab, NULL,
>> + NULL, &error);
>> if (error) {
>> fprintf(stderr, "%s fails to parse (%s)\n",
>> ds_cstr(&s), error);
>> @@ -1163,7 +1186,7 @@ test_expr_to_packets(struct ovs_cmdl_context *ctx
>> OVS_UNUSED)
>> while (!ds_get_test_line(&input, stdin)) {
>> struct flow uflow;
>> char *error = expr_parse_microflow(ds_cstr(&input), &symtab,
>> NULL,
>> - lookup_atoi_cb, NULL, &uflow);
>> + NULL, lookup_atoi_cb, NULL,
>> &uflow);
>> if (error) {
>> puts(error);
>> free(error);
>>
>>
>
More information about the dev
mailing list