On Fri, Mar 2, 2018 at 12:49 AM, Mark Michelson <mmich...@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.
>

This sounds great Mark! How are you doing those tests? Just invoking
ovn-nbctl 2 times per port (1 for creating the port + 1 for inserting the
ACL)?  so, every time you create a port, you'll be opening 2 new
connections to the OVSDB and getting a full dump of the database contents.
That said, maybe with 200 ports it doesn't make a huge difference but the
most realistic way IMHO would be to open a single IDL connection and then
try to create the ports on that one. Of course, after Han's patch, the NB
database size is much smaller so doing everything on a single IDL
connection won't be that noticeable but I think it would be in your
previous scenario (inserting ACL's for each new port). I wonder how the
times would look like as I expect them to be far less than those 9.5
minutes you get for 200 ports.
In my tests using Python IDL it made a huge difference!


>
> 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 <dalva...@redhat.com>
>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-Febr
>> uary/046166.html
>> Signed-off-by: Han Zhou <hzh...@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.
>> +@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);
>>
>>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to