On Thu, Sep 19, 2024 at 12:28 PM Numan Siddique <[email protected]> wrote:
>
> On Tue, Aug 20, 2024 at 3:05 AM <[email protected]> wrote:
> >
> > From: Karthik Chandrashekar <[email protected]>
> >
> > - This patch adds the ability to specify a custom set of packet headers
> >   for hash computation for ECMP routes similar to the support that was
> >   added for LB in 5af304e7478adcf5ac50ed41e96a55bebebff3e8
> > - ECMP routes by default use dp_hash as a selection_method for OVS flows.
> >   When ecmp_selection_fields is specified, the selection_method will be
> >   hash with the specified list of fields used for computing the hash.
> > - For simplicity, list of fields that are used in the select action
> >   is a union of all the fields specified in each Logical_Route_Static_Route
> >   that is part of a given ECMP route.
> > - In order to allow match based on L4 port numbers, the lr_in_ip_routing
> >   rules have been split into separate lflows with protocol specific
> >   fields when src_port or dst_port is specified in the 
> > ecmp_selectioin_fields.
> >   (This is based on the requirement that pre-requisites of fields
> >   must be provided by any flows that output to the group)
> >
> > Signed-off-by: Karthik Chandrashekar <[email protected]>
>
> Thanks for the patch.
>
> 1.  ovn-controller is crashing with this patch when I run the commands below.
>
> ovn-nbctl --ecmp
> --ecmp-selection-fields="ip_proto,eth_src,ip_src,ip_dst,dst_port,foo"
> lr-route-add lr0 80.0.0.0/24 172.168.0.132
> ovn-nbctl --ecmp
> --ecmp-selection-fields="ip_proto,eth_src,ip_src,ip_dst,dst_port,foo"
> lr-route-add lr0 80.0.0.0/24 172.168.0.133
>
> Notice the "foo" in the selection fields.  A wrong configuration
> should not result in crashing ovn-controller.
>
> 2.  Instead of specifying the selection fields in the options column,
> why didn't you add a new column "selection_fields" in the
> logical_router_static_route table ?
>      just like how it's done for load balancers ?
>     Like
>
>    "selection_fields": {
>                     "type": {"key": {"type": "string",
>                              "enum": ["set",
>                                 ["eth_src", "eth_dst", "ip_src", "ip_dst",
>                                  "tp_src", "tp_dst"]]},
>                              "min": 0, "max": "unlimited"}},
>
> If the user has set tp_src or tp_dst, you can insert ip_proto in the
> logical flow.
>
> 3.  The syntax of "select" ovn action seems odd to me.
>      Instead of - select(values=1=50, 2=100;
> hash_fields="eth_src,ip_dst,ip_proto,ip_src")
>      I'd suggest - select(values=(1=50, 2=100);
> hash_fields="eth_src,ip_dst,ip_proto,ip_src")
>
>

Also there seems to be memory leaks in ovn-northd.  Please take a look
at the CI run of this patch -
https://github.com/ovsrobot/ovn/actions/runs/10467084213/job/28985299712

Thanks
Numan

> Thanks
> Numan
>
> >
> > ---
> > v2:
> >   - Install separate logical flows for TCP and UDP in lr_in_ip_routing.
> >   - Add more test coverage.
> > ---
> > v3:
> >   - Address code review comments.
> >   - Install separate logical flows for TCP, UDP, SCTP in lr_in_ip_routing
> >     only when match on src_port or dst_port is specified.
> >   - Update NB and SB documentation.
> > ---
> >  include/ovn/actions.h |   1 +
> >  lib/actions.c         |  48 +++++++-
> >  northd/northd.c       | 123 +++++++++++++++++---
> >  northd/northd.h       |   1 +
> >  ovn-nb.xml            |  27 +++++
> >  ovn-sb.xml            |  10 ++
> >  tests/ovn-northd.at   |  69 ++++++++----
> >  tests/ovn.at          | 253 +++++++++++++++++++++++++++++++++++++++++-
> >  utilities/ovn-nbctl.c |  17 ++-
> >  9 files changed, 497 insertions(+), 52 deletions(-)
> >
> > diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> > index c8dd66ed8..a13444c5c 100644
> > --- a/include/ovn/actions.h
> > +++ b/include/ovn/actions.h
> > @@ -338,6 +338,7 @@ struct ovnact_select {
> >      struct ovnact_select_dst *dsts;
> >      size_t n_dsts;
> >      uint8_t ltable;             /* Logical table ID of next table. */
> > +    char *hash_fields;
> >      struct expr_field res_field;
> >  };
> >
> > diff --git a/lib/actions.c b/lib/actions.c
> > index c12d087e7..359da270d 100644
> > --- a/lib/actions.c
> > +++ b/lib/actions.c
> > @@ -1516,11 +1516,20 @@ parse_select_action(struct action_context *ctx, 
> > struct expr_field *res_field)
> >      struct ovnact_select_dst *dsts = NULL;
> >      size_t allocated_dsts = 0;
> >      size_t n_dsts = 0;
> > +    bool requires_hash_fields = false;
> > +    char *hash_fields = NULL;
> >
> >      lexer_get(ctx->lexer); /* Skip "select". */
> >      lexer_get(ctx->lexer); /* Skip '('. */
> >
> > -    while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
> > +    if (lexer_match_id(ctx->lexer, "values")) {
> > +        lexer_force_match(ctx->lexer, LEX_T_EQUALS);
> > +        requires_hash_fields = true;
> > +    }
> > +
> > +    enum lex_type value_end_token = (
> > +        requires_hash_fields ? LEX_T_SEMICOLON : LEX_T_RPAREN);
> > +    while (!lexer_match(ctx->lexer, value_end_token)) {
> >          struct ovnact_select_dst dst;
> >          if (!action_parse_uint16(ctx, &dst.id, "id")) {
> >              free(dsts);
> > @@ -1556,11 +1565,31 @@ parse_select_action(struct action_context *ctx, 
> > struct expr_field *res_field)
> >          return;
> >      }
> >
> > +    if (requires_hash_fields) {
> > +        if (!lexer_match_id(ctx->lexer, "hash_fields")) {
> > +            lexer_syntax_error(ctx->lexer, "expecting hash_fields");
> > +            free(dsts);
> > +            return;
> > +        }
> > +        if (!lexer_match(ctx->lexer, LEX_T_EQUALS) ||
> > +            ctx->lexer->token.type != LEX_T_STRING ||
> > +            lexer_lookahead(ctx->lexer) != LEX_T_RPAREN) {
> > +            lexer_syntax_error(ctx->lexer, "invalid hash_fields");
> > +            free(dsts);
> > +            return;
> > +        }
> > +
> > +        hash_fields = xstrdup(ctx->lexer->token.s);
> > +        lexer_get(ctx->lexer);
> > +        lexer_force_match(ctx->lexer, LEX_T_RPAREN);
> > +    }
> > +
> >      struct ovnact_select *select = ovnact_put_SELECT(ctx->ovnacts);
> >      select->ltable = ctx->pp->cur_ltable + 1;
> >      select->dsts = dsts;
> >      select->n_dsts = n_dsts;
> >      select->res_field = *res_field;
> > +    select->hash_fields = hash_fields;
> >  }
> >
> >  static void
> > @@ -1570,6 +1599,9 @@ format_SELECT(const struct ovnact_select *select, 
> > struct ds *s)
> >      ds_put_cstr(s, " = ");
> >      ds_put_cstr(s, "select");
> >      ds_put_char(s, '(');
> > +    if (select->hash_fields) {
> > +        ds_put_format(s, "values=");
> > +    }
> >      for (size_t i = 0; i < select->n_dsts; i++) {
> >          if (i) {
> >              ds_put_cstr(s, ", ");
> > @@ -1580,6 +1612,10 @@ format_SELECT(const struct ovnact_select *select, 
> > struct ds *s)
> >          ds_put_format(s, "=%"PRIu16, dst->weight);
> >      }
> >      ds_put_char(s, ')');
> > +    if (select->hash_fields) {
> > +      ds_chomp(s, ')');
> > +      ds_put_format(s, "; hash_fields=\"%s\")", select->hash_fields);
> > +    }
> >      ds_put_char(s, ';');
> >  }
> >
> > @@ -1594,9 +1630,14 @@ encode_SELECT(const struct ovnact_select *select,
> >      struct ofpact_group *og;
> >
> >      struct ds ds = DS_EMPTY_INITIALIZER;
> > -    ds_put_format(&ds, "type=select,selection_method=dp_hash");
> > +    ds_put_format(&ds, "type=select,selection_method=%s",
> > +                  select->hash_fields ? "hash": "dp_hash");
> > +    if (select->hash_fields) {
> > +      ds_put_format(&ds, ",fields(%s)", select->hash_fields);
> > +    }
> >
> > -    if (ovs_feature_is_supported(OVS_DP_HASH_L4_SYM_SUPPORT)) {
> > +    if (ovs_feature_is_supported(OVS_DP_HASH_L4_SYM_SUPPORT) &&
> > +        !select->hash_fields) {
> >          /* Select dp-hash l4_symmetric by setting the upper 32bits of
> >           * selection_method_param to value 1 (1 << 32): */
> >          ds_put_cstr(&ds, ",selection_method_param=0x100000000");
> > @@ -1629,6 +1670,7 @@ static void
> >  ovnact_select_free(struct ovnact_select *select)
> >  {
> >      free(select->dsts);
> > +    free(select->hash_fields);
> >  }
> >
> >  static void
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 7ceed63dd..6e0a7e12a 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -302,9 +302,9 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2));
> >   * same ip_prefix values:
> >   *  -  connected route overrides static one;
> >   *  -  static route overrides src-ip route. */
> > -#define ROUTE_PRIO_OFFSET_MULTIPLIER 3
> > -#define ROUTE_PRIO_OFFSET_STATIC 1
> > -#define ROUTE_PRIO_OFFSET_CONNECTED 2
> > +#define ROUTE_PRIO_OFFSET_MULTIPLIER 5
> > +#define ROUTE_PRIO_OFFSET_STATIC 2
> > +#define ROUTE_PRIO_OFFSET_CONNECTED 4
> >
> >  /* Returns the type of the datapath to which a flow with the given 'stage' 
> > may
> >   * be added. */
> > @@ -11219,12 +11219,32 @@ parsed_routes_add(struct ovn_datapath *od, const 
> > struct hmap *lr_ports,
> >                                                   false);
> >      new_pr->is_discard_route = is_discard_route;
> >
> > +    sset_init(&new_pr->ecmp_selection_fields);
> > +    const char *ecmp_selection_fields = smap_get(&route->options,
> > +                                                 "ecmp_selection_fields");
> > +    if (ecmp_selection_fields) {
> > +        struct sset field_set = SSET_INITIALIZER(&field_set);
> > +        sset_from_delimited_string(&field_set, ecmp_selection_fields, ",");
> > +        if ((sset_contains(&field_set, "src_port") ||
> > +             sset_contains(&field_set, "dst_port")) &&
> > +            !sset_contains(&field_set, "ip_proto")) {
> > +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> > +            VLOG_WARN_RL(&rl, "ip_proto must be specified in "
> > +                         "ecmp_selection_fields when matching on src_port 
> > or "
> > +                         "dst_port");
> > +        } else {
> > +            sset_clone(&new_pr->ecmp_selection_fields, &field_set);
> > +        }
> > +        sset_destroy(&field_set);
> > +    }
> > +
> >      size_t hash = uuid_hash(&od->key);
> >      struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr);
> >      if (!pr) {
> >          hmap_insert(routes, &new_pr->key_node, hash);
> >      } else {
> >          pr->stale = false;
> > +        sset_destroy(&new_pr->ecmp_selection_fields);
> >          free(new_pr);
> >      }
> >  }
> > @@ -11274,6 +11294,7 @@ struct ecmp_groups_node {
> >      uint32_t route_table_id;
> >      uint16_t route_count;
> >      struct ovs_list route_list; /* Contains ecmp_route_list_node */
> > +    struct sset selection_fields;
> >  };
> >
> >  static void
> > @@ -11289,6 +11310,14 @@ ecmp_groups_add_route(struct ecmp_groups_node 
> > *group,
> >      struct ecmp_route_list_node *er = xmalloc(sizeof *er);
> >      er->route = route;
> >      er->id = ++group->route_count;
> > +
> > +    if (group->route_count == 1) {
> > +        sset_clone(&group->selection_fields, 
> > &route->ecmp_selection_fields);
> > +    } else {
> > +        sset_intersect(&group->selection_fields,
> > +                       &route->ecmp_selection_fields);
> > +    }
> > +
> >      ovs_list_insert(&group->route_list, &er->list_node);
> >  }
> >
> > @@ -11311,6 +11340,7 @@ ecmp_groups_add(struct hmap *ecmp_groups,
> >      eg->is_src_route = route->is_src_route;
> >      eg->origin = smap_get_def(&route->route->options, "origin", "");
> >      eg->route_table_id = route->route_table_id;
> > +    sset_init(&eg->selection_fields);
> >      ovs_list_init(&eg->route_list);
> >      ecmp_groups_add_route(eg, route);
> >
> > @@ -11343,6 +11373,7 @@ ecmp_groups_destroy(struct hmap *ecmp_groups)
> >              free(er);
> >          }
> >          hmap_remove(ecmp_groups, &eg->hmap_node);
> > +        sset_destroy(&eg->selection_fields);
> >          free(eg);
> >      }
> >      hmap_destroy(ecmp_groups);
> > @@ -11413,7 +11444,8 @@ build_route_prefix_s(const struct in6_addr *prefix, 
> > unsigned int plen)
> >  static void
> >  build_route_match(const struct ovn_port *op_inport, uint32_t rtb_id,
> >                    const char *network_s, int plen, bool is_src_route,
> > -                  bool is_ipv4, struct ds *match, uint16_t *priority, int 
> > ofs)
> > +                  bool is_ipv4, struct ds *match, uint16_t *priority, int 
> > ofs,
> > +                  bool has_protocol_match)
> >  {
> >      const char *dir;
> >      /* The priority here is calculated to implement longest-prefix-match
> > @@ -11425,14 +11457,19 @@ build_route_match(const struct ovn_port 
> > *op_inport, uint32_t rtb_id,
> >          dir = "dst";
> >      }
> >
> > -    *priority = (plen * ROUTE_PRIO_OFFSET_MULTIPLIER) + ofs;
> > -
> >      if (op_inport) {
> >          ds_put_format(match, "inport == %s && ", op_inport->json_key);
> >      }
> >      if (rtb_id || ofs == ROUTE_PRIO_OFFSET_STATIC) {
> >          ds_put_format(match, "%s == %d && ", REG_ROUTE_TABLE_ID, rtb_id);
> >      }
> > +
> > +    if (has_protocol_match) {
> > +        ofs += 1;
> > +    }
> > +
> > +    *priority = (plen * ROUTE_PRIO_OFFSET_MULTIPLIER) + ofs;
> > +
> >      ds_put_format(match, "ip%s.%s == %s/%d", is_ipv4 ? "4" : "6", dir,
> >                    network_s, plen);
> >  }
> > @@ -11619,8 +11656,7 @@ static void
> >  build_ecmp_route_flow(struct lflow_table *lflows, struct ovn_datapath *od,
> >                        const struct hmap *lr_ports, struct ecmp_groups_node 
> > *eg,
> >                        struct lflow_ref *lflow_ref,
> > -                      struct simap *nexthops_table)
> > -
> > +                      struct simap *nexthops_table, const char *protocol)
> >  {
> >      bool is_ipv4 = IN6_IS_ADDR_V4MAPPED(&eg->prefix);
> >      uint16_t priority;
> > @@ -11631,22 +11667,66 @@ build_ecmp_route_flow(struct lflow_table *lflows, 
> > struct ovn_datapath *od,
> >      int ofs = !strcmp(eg->origin, ROUTE_ORIGIN_CONNECTED) ?
> >          ROUTE_PRIO_OFFSET_CONNECTED: ROUTE_PRIO_OFFSET_STATIC;
> >      build_route_match(NULL, eg->route_table_id, prefix_s, eg->plen,
> > -                      eg->is_src_route, is_ipv4, &route_match, &priority, 
> > ofs);
> > +                      eg->is_src_route, is_ipv4, &route_match, &priority, 
> > ofs,
> > +                      protocol != NULL);
> >      free(prefix_s);
> >
> > -    struct ds actions = DS_EMPTY_INITIALIZER;
> > -    ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; %s = %"PRIu16
> > -                  "; %s = select(", REG_ECMP_GROUP_ID, eg->id,
> > -                  REG_ECMP_MEMBER_ID);
> > +    char *src_port_match = NULL, *dst_port_match = NULL;
> > +    if (!sset_is_empty(&eg->selection_fields) && protocol) {
> > +        if (!strcmp(protocol, "tcp")) {
> > +            ds_put_format(&route_match, " && tcp");
> > +            src_port_match = "tcp_src";
> > +            dst_port_match = "tcp_dst";
> > +        } else if (!strcmp(protocol, "udp")) {
> > +            ds_put_format(&route_match, " && udp");
> > +            src_port_match = "udp_src";
> > +            dst_port_match = "udp_dst";
> > +        } else if (!strcmp(protocol, "sctp")) {
> > +            ds_put_format(&route_match, " && sctp");
> > +            src_port_match = "sctp_src";
> > +            dst_port_match = "sctp_dst";
> > +        }
> > +    }
> >
> > +    struct ds values = DS_EMPTY_INITIALIZER;
> >      bool is_first = true;
> >      LIST_FOR_EACH (er, list_node, &eg->route_list) {
> >          if (is_first) {
> >              is_first = false;
> >          } else {
> > -            ds_put_cstr(&actions, ", ");
> > +            ds_put_cstr(&values, ", ");
> >          }
> > -        ds_put_format(&actions, "%"PRIu16, er->id);
> > +        ds_put_format(&values, "%"PRIu16, er->id);
> > +    }
> > +
> > +    struct ds actions = DS_EMPTY_INITIALIZER;
> > +    if (!sset_is_empty(&eg->selection_fields)) {
> > +        struct sset field_set = SSET_INITIALIZER(&field_set);
> > +        sset_clone(&field_set, &eg->selection_fields);
> > +        if (protocol) {
> > +            if (sset_contains(&eg->selection_fields, "src_port") &&
> > +                src_port_match) {
> > +                sset_add(&field_set, src_port_match);
> > +            }
> > +            if (sset_contains(&eg->selection_fields, "dst_port") &&
> > +                dst_port_match) {
> > +                sset_add(&field_set, dst_port_match);
> > +            }
> > +        }
> > +        sset_find_and_delete(&field_set, "src_port");
> > +        sset_find_and_delete(&field_set, "dst_port");
> > +
> > +        ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; %s = 
> > %"PRIu16
> > +                      "; %s = select(values=%s", REG_ECMP_GROUP_ID, eg->id,
> > +                      REG_ECMP_MEMBER_ID, ds_cstr(&values));
> > +        ds_put_format(&actions, "; hash_fields=\"%s\"",
> > +                      sset_join(&field_set, ",", ""));
> > +
> > +        sset_destroy(&field_set);
> > +    } else {
> > +        ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; %s = 
> > %"PRIu16
> > +                      "; %s = select(%s", REG_ECMP_GROUP_ID, eg->id,
> > +                      REG_ECMP_MEMBER_ID, ds_cstr(&values));
> >      }
> >
> >      ds_put_cstr(&actions, ");");
> > @@ -11727,7 +11807,7 @@ add_route(struct lflow_table *lflows, struct 
> > ovn_datapath *od,
> >          }
> >      }
> >      build_route_match(op_inport, rtb_id, network_s, plen, is_src_route,
> > -                      is_ipv4, &match, &priority, ofs);
> > +                      is_ipv4, &match, &priority, ofs, false);
> >
> >      struct ds common_actions = DS_EMPTY_INITIALIZER;
> >      struct ds actions = DS_EMPTY_INITIALIZER;
> > @@ -13621,7 +13701,16 @@ build_static_route_flows_for_lrouter(
> >          /* add a flow in IP_ROUTING, and one flow for each member in
> >           * IP_ROUTING_ECMP. */
> >          build_ecmp_route_flow(lflows, od, lr_ports, group, lflow_ref,
> > -                              nexthops_table);
> > +                              nexthops_table, NULL);
> > +        if (sset_contains(&group->selection_fields, "src_port") ||
> > +            sset_contains(&group->selection_fields, "dst_port")) {
> > +            build_ecmp_route_flow(lflows, od, lr_ports, group, lflow_ref,
> > +                                  nexthops_table, "tcp");
> > +            build_ecmp_route_flow(lflows, od, lr_ports, group, lflow_ref,
> > +                                  nexthops_table, "udp");
> > +            build_ecmp_route_flow(lflows, od, lr_ports, group, lflow_ref,
> > +                                  nexthops_table, "sctp");
> > +        }
> >      }
> >      const struct unique_routes_node *ur;
> >      HMAP_FOR_EACH (ur, hmap_node, &unique_routes) {
> > diff --git a/northd/northd.h b/northd/northd.h
> > index c7f0b829b..756d2fabb 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -708,6 +708,7 @@ struct parsed_route {
> >      bool is_discard_route;
> >      const struct nbrec_logical_router *nbr;
> >      bool stale;
> > +    struct sset ecmp_selection_fields;
> >  };
> >
> >  void ovnnb_db_run(struct northd_input *input_data,
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 2836f58f5..a5935f986 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -3874,6 +3874,33 @@ or
> >          </p>
> >        </column>
> >
> > +      <column name="options" key="ecmp_selection_fields">
> > +        <p>
> > +          ECMP routes use OpenFlow groups of type <code>select</code> to
> > +          pick a nexthop among the list of available nexthops.
> > +          OVS supports two selection methods: <code>dp_hash</code> and
> > +          <code>hash</code> for hash computation and selecting
> > +          the buckets of a group. OVN by default uses <code>dp_hash</code>.
> > +          In order to use the <code>hash</code> selection method,
> > +          specify comma-separated list of selection fields. Please see the
> > +          OVS documentation (man ovs-ofctl) for more details on the
> > +          selection methods and fields.
> > +        </p>
> > +
> > +        <p>
> > +          To match on Layer 4 ports use <code>src_port</code> and
> > +          <code>dst_port</code>. This match is applicable only for TCP,
> > +          UDP, SCTP and will be ignored for all other IP packets. When
> > +          matching on Layer 4 ports, need to match on ip_proto.
> > +        </p>
> > +
> > +        <p>
> > +          Example: <code>{ip_proto,ip_src,ip_dst}</code> for a 3-tuple 
> > match.
> > +          Example: <code>{ip_proto,ip_src,ip_dst,src_port,dst_port}</code>
> > +          for a 5-tuple match.
> > +        </p>
> > +      </column>
> > +
> >        <column name="options" key="origin">
> >          In case ovn-interconnection has been learned this route, it will 
> > have
> >          its origin set: either "connected" or "static".  This key is 
> > supposed
> > diff --git a/ovn-sb.xml b/ovn-sb.xml
> > index c11296d7c..5ce66b762 100644
> > --- a/ovn-sb.xml
> > +++ b/ovn-sb.xml
> > @@ -2543,6 +2543,7 @@ tcp.flags = RST;
> >          </dd>
> >
> >          <dt><code><var>R</var> = select(<var>N1</var>[=<var>W1</var>], 
> > <var>N2</var>[=<var>W2</var>], ...);</code></dt>
> > +        <dt><code><var>R</var> = 
> > select(values=<var>N1</var>[=<var>W1</var>], <var>N2</var>[=<var>W2</var>], 
> > ...; hash_felds="<var>field1</var>,<var>field2</var>,...");</code></dt>
> >          <dd>
> >            <p>
> >              <b>Parameters</b>: Integer <var>N1</var>, <var>N2</var>..., 
> > with
> > @@ -2562,6 +2563,14 @@ tcp.flags = RST;
> >              selection method is based on the 5-tuple hash of packet header.
> >            </p>
> >
> > +          <p>
> > +            By default, <code>dp_hash</code> is used as the OpenFlow group
> > +            selection method, but if <code>values</code> and
> > +            <code>hash_fields</code> are specified, <code>hash</code> is 
> > used
> > +            as the selection method, and the fields listed are used as the
> > +            hash fields.
> > +          </p>
> > +
> >            <p>
> >              Processing automatically moves on to the next table, as if
> >              <code>next;</code> were specified.  The <code>select</code> 
> > action
> > @@ -2572,6 +2581,7 @@ tcp.flags = RST;
> >
> >            <p>
> >              <b>Example:</b> <code>reg8[16..31] = select(1=20, 2=30, 
> > 3=50);</code>
> > +            <b>Example:</b> <code>reg8[16..31] = select(values=1=20, 2=30, 
> > 3=50; hash_fields="ip_proto,src_ip,dst_ip");</code>
> >            </p>
> >          </dd>
> >
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 93ccbce6b..2f9858575 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -6807,9 +6807,9 @@ ovn-sbctl dump-flows lr0 > lr0flows
> >  AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], 
> > [dnl
> >    table=??(lr_in_ip_routing   ), priority=0    , match=(1), action=(drop;)
> >    table=??(lr_in_ip_routing   ), priority=10550, match=(nd_rs || nd_ra), 
> > action=(drop;)
> > -  table=??(lr_in_ip_routing   ), priority=194  , match=(inport == 
> > "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src = 
> > 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=74   , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
> > flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=97   , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = 
> > "lr0-public"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=124  , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
> > flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=162  , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = 
> > "lr0-public"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=324  , match=(inport == 
> > "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src = 
> > 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
> >  ])
> >
> >  AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | ovn_strip_lflows], 
> > [0], [dnl
> > @@ -6825,9 +6825,9 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> >    table=??(lr_in_ip_routing   ), priority=0    , match=(1), action=(drop;)
> >    table=??(lr_in_ip_routing   ), priority=10300, 
> > match=(ct_mark.ecmp_reply_port == 1 && reg7 == 0 && ip4.dst == 1.0.0.1/32), 
> > action=(ip.ttl--; flags.loopback = 1; eth.src = 00:00:20:20:12:13; reg1 = 
> > 192.168.0.1; outport = "lr0-public"; next;)
> >    table=??(lr_in_ip_routing   ), priority=10550, match=(nd_rs || nd_ra), 
> > action=(drop;)
> > -  table=??(lr_in_ip_routing   ), priority=194  , match=(inport == 
> > "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src = 
> > 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=74   , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
> > flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=97   , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] 
> > = 1; reg8[[16..31]] = select(1, 2);)
> > +  table=??(lr_in_ip_routing   ), priority=124  , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
> > flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=162  , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] 
> > = 1; reg8[[16..31]] = select(1, 2);)
> > +  table=??(lr_in_ip_routing   ), priority=324  , match=(inport == 
> > "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src = 
> > 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
> >  ])
> >  AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 
> > 's/192\.168\.0\..0/192.168.0.??/' | ovn_strip_lflows], [0], [dnl
> >    table=??(lr_in_ip_routing_ecmp), priority=0    , match=(1), 
> > action=(drop;)
> > @@ -6855,9 +6855,9 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> >    table=??(lr_in_ip_routing   ), priority=0    , match=(1), action=(drop;)
> >    table=??(lr_in_ip_routing   ), priority=10300, 
> > match=(ct_mark.ecmp_reply_port == 1 && reg7 == 0 && ip4.dst == 1.0.0.1/32), 
> > action=(ip.ttl--; flags.loopback = 1; eth.src = 00:00:20:20:12:13; reg1 = 
> > 192.168.0.1; outport = "lr0-public"; next;)
> >    table=??(lr_in_ip_routing   ), priority=10550, match=(nd_rs || nd_ra), 
> > action=(drop;)
> > -  table=??(lr_in_ip_routing   ), priority=194  , match=(inport == 
> > "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src = 
> > 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=74   , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
> > flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=97   , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] 
> > = 1; reg8[[16..31]] = select(1, 2);)
> > +  table=??(lr_in_ip_routing   ), priority=124  , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
> > flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=162  , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] 
> > = 1; reg8[[16..31]] = select(1, 2);)
> > +  table=??(lr_in_ip_routing   ), priority=324  , match=(inport == 
> > "lr0-public" && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; 
> > xxreg0 = ip6.dst; xxreg1 = fe80::200:20ff:fe20:1213; eth.src = 
> > 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
> >  ])
> >  AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 
> > 's/192\.168\.0\..0/192.168.0.??/' | ovn_strip_lflows], [0], [dnl
> >    table=??(lr_in_ip_routing_ecmp), priority=0    , match=(1), 
> > action=(drop;)
> > @@ -6874,14 +6874,41 @@ check ovn-nbctl --wait=sb lr-route-add lr0 
> > 1.0.0.0/24 192.168.0.10
> >  ovn-sbctl dump-flows lr0 > lr0flows
> >
> >  AT_CHECK([grep -e "lr_in_ip_routing.*192.168.0.10" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> > -  table=??(lr_in_ip_routing   ), priority=73   , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = 
> > "lr0-public"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=122  , match=(reg7 == 0 && 
> > ip4.dst == 1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = 
> > "lr0-public"; flags.loopback = 1; next;)
> >  ])
> >
> >  check ovn-nbctl --wait=sb lr-route-add lr0 2.0.0.0/24 lr0-public
> >
> >  ovn-sbctl dump-flows lr0 > lr0flows
> >  AT_CHECK([grep -e "lr_in_ip_routing.*2.0.0.0" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> > -  table=??(lr_in_ip_routing   ), priority=73   , match=(reg7 == 0 && 
> > ip4.dst == 2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > ip4.dst; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = 
> > "lr0-public"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=122  , match=(reg7 == 0 && 
> > ip4.dst == 2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > ip4.dst; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = 
> > "lr0-public"; flags.loopback = 1; next;)
> > +])
> > +
> > +check ovn-nbctl --ecmp-selection-fields="ip_proto,ip_src,ip_dst" 
> > lr-route-add lr0 3.3.0.0/16 192.168.0.11
> > +check ovn-nbctl --ecmp --ecmp-selection-fields="ip_proto,ip_src,ip_dst" 
> > lr-route-add lr0 3.3.0.0/16 192.168.0.12
> > +check ovn-nbctl --wait=sb sync
> > +
> > +ovn-sbctl dump-flows lr0 > lr0flows
> > +
> > +AT_CHECK([grep -e "(lr_in_ip_routing   ).*3.3.0.0" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> > +  table=??(lr_in_ip_routing   ), priority=82   , match=(reg7 == 0 && 
> > ip4.dst == 3.3.0.0/16), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] 
> > = 1; reg8[[16..31]] = select(values=1, 2; 
> > hash_fields="ip_dst,ip_proto,ip_src");)
> > +])
> > +
> > +check ovn-nbctl lr-route-del lr0 3.3.0.0/16
> > +check ovn-nbctl --wait=sb sync
> > +check_row_count ECMP_Nexthop 0
> > +
> > +check ovn-nbctl 
> > --ecmp-selection-fields="ip_proto,ip_src,ip_dst,src_port,dst_port" 
> > lr-route-add lr0 3.3.0.0/16 192.168.0.11
> > +check ovn-nbctl --ecmp 
> > --ecmp-selection-fields="ip_proto,ip_src,ip_dst,src_port,dst_port" 
> > lr-route-add lr0 3.3.0.0/16 192.168.0.12
> > +check ovn-nbctl --wait=sb sync
> > +
> > +ovn-sbctl dump-flows lr0 > lr0flows
> > +
> > +AT_CHECK([grep -e "(lr_in_ip_routing   ).*3.3.0.0" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> > +  table=??(lr_in_ip_routing   ), priority=82   , match=(reg7 == 0 && 
> > ip4.dst == 3.3.0.0/16), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] 
> > = 1; reg8[[16..31]] = select(values=1, 2; 
> > hash_fields="ip_dst,ip_proto,ip_src");)
> > +  table=??(lr_in_ip_routing   ), priority=83   , match=(reg7 == 0 && 
> > ip4.dst == 3.3.0.0/16 && sctp), action=(ip.ttl--; flags.loopback = 1; 
> > reg8[[0..15]] = 1; reg8[[16..31]] = select(values=1, 2; 
> > hash_fields="ip_dst,ip_proto,ip_src,sctp_dst,sctp_src");)
> > +  table=??(lr_in_ip_routing   ), priority=83   , match=(reg7 == 0 && 
> > ip4.dst == 3.3.0.0/16 && tcp), action=(ip.ttl--; flags.loopback = 1; 
> > reg8[[0..15]] = 1; reg8[[16..31]] = select(values=1, 2; 
> > hash_fields="ip_dst,ip_proto,ip_src,tcp_dst,tcp_src");)
> > +  table=??(lr_in_ip_routing   ), priority=83   , match=(reg7 == 0 && 
> > ip4.dst == 3.3.0.0/16 && udp), action=(ip.ttl--; flags.loopback = 1; 
> > reg8[[0..15]] = 1; reg8[[16..31]] = select(values=1, 2; 
> > hash_fields="ip_dst,ip_proto,ip_src,udp_dst,udp_src");)
> >  ])
> >
> >  AT_CLEANUP
> > @@ -7305,16 +7332,16 @@ AT_CHECK([grep "lr_in_ip_routing_pre" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> >  grep -e "(lr_in_ip_routing   ).*outport" lr0flows
> >
> >  AT_CHECK([grep -e "(lr_in_ip_routing   ).*outport" lr0flows | 
> > ovn_strip_lflows], [0], [dnl
> > -  table=??(lr_in_ip_routing   ), priority=1    , match=(reg7 == 0 && 
> > ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = 
> > "lrp0"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=1    , match=(reg7 == 2 && 
> > ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = 
> > "lrp0"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=194  , match=(inport == "lrp0" 
> > && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = 
> > ip6.dst; xxreg1 = fe80::200:ff:fe00:1; eth.src = 00:00:00:00:00:01; outport 
> > = "lrp0"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=194  , match=(inport == "lrp1" 
> > && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = 
> > ip6.dst; xxreg1 = fe80::200:ff:fe00:101; eth.src = 00:00:00:00:01:01; 
> > outport = "lrp1"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=194  , match=(inport == "lrp2" 
> > && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = 
> > ip6.dst; xxreg1 = fe80::200:ff:fe00:201; eth.src = 00:00:00:00:02:01; 
> > outport = "lrp2"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=73   , match=(reg7 == 1 && 
> > ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.1.10; reg1 = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport = 
> > "lrp1"; flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=74   , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = "lrp0"; 
> > flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=74   , match=(ip4.dst == 
> > 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport = "lrp1"; 
> > flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=74   , match=(ip4.dst == 
> > 192.168.2.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.2.1; eth.src = 00:00:00:00:02:01; outport = "lrp2"; 
> > flags.loopback = 1; next;)
> > -  table=??(lr_in_ip_routing   ), priority=97   , match=(reg7 == 2 && 
> > ip4.dst == 1.1.1.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.20; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = 
> > "lrp0"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=122  , match=(reg7 == 1 && 
> > ip4.dst == 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.1.10; reg1 = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport = 
> > "lrp1"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=124  , match=(ip4.dst == 
> > 192.168.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = "lrp0"; 
> > flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=124  , match=(ip4.dst == 
> > 192.168.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.1.1; eth.src = 00:00:00:00:01:01; outport = "lrp1"; 
> > flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=124  , match=(ip4.dst == 
> > 192.168.2.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 
> > = 192.168.2.1; eth.src = 00:00:00:00:02:01; outport = "lrp2"; 
> > flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=162  , match=(reg7 == 2 && 
> > ip4.dst == 1.1.1.1/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.20; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = 
> > "lrp0"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=2    , match=(reg7 == 0 && 
> > ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = 
> > "lrp0"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=2    , match=(reg7 == 2 && 
> > ip4.dst == 0.0.0.0/0), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 
> > 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:00:00:00:01; outport = 
> > "lrp0"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=324  , match=(inport == "lrp0" 
> > && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = 
> > ip6.dst; xxreg1 = fe80::200:ff:fe00:1; eth.src = 00:00:00:00:00:01; outport 
> > = "lrp0"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=324  , match=(inport == "lrp1" 
> > && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = 
> > ip6.dst; xxreg1 = fe80::200:ff:fe00:101; eth.src = 00:00:00:00:01:01; 
> > outport = "lrp1"; flags.loopback = 1; next;)
> > +  table=??(lr_in_ip_routing   ), priority=324  , match=(inport == "lrp2" 
> > && ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = 
> > ip6.dst; xxreg1 = fe80::200:ff:fe00:201; eth.src = 00:00:00:00:02:01; 
> > outport = "lrp2"; flags.loopback = 1; next;)
> >  ])
> >
> >  AT_CLEANUP
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 50c9f04da..3b88ceafc 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -2117,6 +2117,16 @@ reg0 = select(1, 2);
> >      encodes as group:20
> >      uses group: id(20), 
> > name(type=select,selection_method=dp_hash,bucket=bucket_id=0,weight:100,actions=load:1->xxreg0[[96..127]],resubmit(,oflow_in_table),bucket=bucket_id=1,weight:100,actions=load:2->xxreg0[[96..127]],resubmit(,oflow_in_table))
> >
> > +reg9[[16..31]] = select(values=1=50, 2=100, 3; 
> > hash_fields="ip_src,ip_dst");
> > +    formats as reg9[[16..31]] = select(values=1=50, 2=100, 3=100; 
> > hash_fields="ip_src,ip_dst");
> > +    encodes as group:21
> > +    uses group: id(21), 
> > name(type=select,selection_method=hash,fields(ip_src,ip_dst),bucket=bucket_id=0,weight:50,actions=load:1->xreg4[[16..31]],resubmit(,oflow_in_table),bucket=bucket_id=1,weight:100,actions=load:2->xreg4[[16..31]],resubmit(,oflow_in_table),bucket=bucket_id=2,weight:100,actions=load:3->xreg4[[16..31]],resubmit(,oflow_in_table))
> > +
> > +reg0 = select(values=1, 2; hash_fields="ip_dst,ip_src");
> > +    formats as reg0 = select(values=1=100, 2=100; 
> > hash_fields="ip_dst,ip_src");
> > +    encodes as group:22
> > +    uses group: id(22), 
> > name(type=select,selection_method=hash,fields(ip_dst,ip_src),bucket=bucket_id=0,weight:100,actions=load:1->xxreg0[[96..127]],resubmit(,oflow_in_table),bucket=bucket_id=1,weight:100,actions=load:2->xxreg0[[96..127]],resubmit(,oflow_in_table))
> > +
> >  reg0 = select(1=, 2);
> >      Syntax error at `,' expecting weight.
> >  reg0 = select(1=0, 2);
> > @@ -2125,6 +2135,14 @@ reg0 = select(1=123456, 2);
> >      Syntax error at `123456' expecting weight.
> >  reg0 = select(123);
> >      Syntax error at `;' expecting at least 2 group members.
> > +reg0 = select(values=1, 2);
> > +    Syntax error at `)' expecting id.
> > +reg0 = select(values=1, 2; test_fields="hello,world");
> > +    Syntax error at `test_fields' expecting hash_fields.
> > +reg0 = select(values=1, 2; hash_fields);
> > +    Syntax error at `)' invalid hash_fields.
> > +reg0 = select(values=1, 2; hash_fields=);
> > +    Syntax error at `)' invalid hash_fields.
> >  ip.proto = select(1, 2, 3);
> >      Field ip.proto is not modifiable.
> >  reg0[[0..14]] = select(1, 2, 3);
> > @@ -2132,12 +2150,12 @@ reg0[[0..14]] = select(1, 2, 3);
> >
> >  fwd_group(liveness=true, childports="eth0", "lsp1");
> >      formats as fwd_group(liveness="true", childports="eth0", "lsp1");
> > -    encodes as group:21
> > -    uses group: id(21), 
> > name(type=select,selection_method=dp_hash,bucket=watch_port:5,load=0x5->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT),bucket=watch_port:17,load=0x17->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT))
> > +    encodes as group:23
> > +    uses group: id(23), 
> > name(type=select,selection_method=dp_hash,bucket=watch_port:5,load=0x5->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT),bucket=watch_port:17,load=0x17->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT))
> >
> >  fwd_group(childports="eth0", "lsp1");
> > -    encodes as group:22
> > -    uses group: id(22), 
> > name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT),bucket=load=0x17->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT))
> > +    encodes as group:24
> > +    uses group: id(24), 
> > name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT),bucket=load=0x17->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT))
> >
> >  fwd_group(childports=eth0);
> >      Syntax error at `eth0' expecting logical switch port.
> > @@ -2146,8 +2164,8 @@ fwd_group();
> >      Syntax error at `)' expecting `;'.
> >
> >  fwd_group(childports="eth0", "lsp1");
> > -    encodes as group:22
> > -    uses group: id(22), 
> > name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT),bucket=load=0x17->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT))
> > +    encodes as group:24
> > +    uses group: id(24), 
> > name(type=select,selection_method=dp_hash,bucket=load=0x5->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT),bucket=load=0x17->NXM_NX_REG15[[0..15]],resubmit(,OFTABLE_SAVE_INPORT))
> >
> >  fwd_group(liveness=xyzzy, childports="eth0", "lsp1");
> >      Syntax error at `xyzzy' expecting true or false.
> > @@ -26586,6 +26604,229 @@ OVN_CLEANUP([hv1])
> >  AT_CLEANUP
> >  ])
> >
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ECMP static routes - custom hash])
> > +ovn_start
> > +
> > +# Logical network:
> > +# ls1 (192.168.1.0/24) - lr1 - ls2 (192.168.2.0/24)
> > +# lsl has lsp11 (192.168.1.11) and ls2 has lsp21 (192.168.2.21) and lsp22
> > +# (192.168.2.22)
> > +#
> > +# Static routes on lr1:
> > +# 10.0.0.0/24 nexthop 192.168.2.21
> > +# 10.0.0.0/24 nexthop 192.168.2.22
> > +#
> > +# ECMP hash on ip_proto,src_ip,dst_ip,dst_port
> > +#
> > +# Test:
> > +# lsp11 send packets to 10.0.0.100 with different source ports
> > +# Migrate all the VIFs to a hv2
> > +# Generate the same traffic from lsp11 again
> > +#
> > +# Expected result:
> > +# All packets should go out of a either lsp21 or lsp22 on hv1
> > +# All packets should go out of the same port even on hv2
> > +
> > +ovn-nbctl lr-add lr1
> > +
> > +ovn-nbctl ls-add ls1
> > +ovn-nbctl ls-add ls2
> > +
> > +for i in 1 2; do
> > +    ovn-nbctl lrp-add lr1 lrp-lr1-ls${i} 00:00:00:01:0${i}:01 
> > 192.168.${i}.1/24
> > +    ovn-nbctl lsp-add ls${i} lsp-ls${i}-lr1 -- lsp-set-type lsp-ls${i}-lr1 
> > router \
> > +        -- lsp-set-options lsp-ls${i}-lr1 router-port=lrp-lr1-ls${i} \
> > +        -- lsp-set-addresses lsp-ls${i}-lr1 router
> > +done
> > +
> > +#install static routes
> > +ovn-nbctl 
> > --ecmp-selection-fields="ip_proto,eth_src,ip_src,ip_dst,dst_port" 
> > lr-route-add lr1 10.0.0.0/24 192.168.2.21
> > +ovn-nbctl --ecmp 
> > --ecmp-selection-fields="ip_proto,eth_dst,ip_src,ip_dst,dst_port" 
> > lr-route-add lr1 10.0.0.0/24 192.168.2.22
> > +
> > +# Create logical ports
> > +ovn-nbctl lsp-add ls1 lsp11 -- \
> > +    lsp-set-addresses lsp11 "f0:00:00:00:01:11 192.168.1.11"
> > +ovn-nbctl lsp-add ls2 lsp21 -- \
> > +    lsp-set-addresses lsp21 "f0:00:00:00:02:21 192.168.2.21"
> > +ovn-nbctl lsp-add ls2 lsp22 -- \
> > +    lsp-set-addresses lsp22 "f0:00:00:00:02:22 192.168.2.22"
> > +
> > +net_add n1
> > +sim_add hv1
> > +as hv1
> > +ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.1
> > +
> > +sim_add hv2
> > +as hv2
> > +ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.2
> > +
> > +as hv1
> > +ovs-vsctl -- add-port br-int hv1-vif1 -- \
> > +    set interface hv1-vif1 external-ids:iface-id=lsp11 \
> > +    options:tx_pcap=hv1/vif1-tx.pcap \
> > +    options:rxq_pcap=hv1/vif1-rx.pcap \
> > +    ofport-request=1
> > +
> > +ovs-vsctl -- add-port br-int hv1-vif2 -- \
> > +    set interface hv1-vif2 external-ids:iface-id=lsp21 \
> > +    options:tx_pcap=hv1/vif2-tx.pcap \
> > +    options:rxq_pcap=hv1/vif2-rx.pcap \
> > +    ofport-request=2
> > +
> > +ovs-vsctl -- add-port br-int hv1-vif3 -- \
> > +    set interface hv1-vif3 external-ids:iface-id=lsp22 \
> > +    options:tx_pcap=hv1/vif3-tx.pcap \
> > +    options:rxq_pcap=hv1/vif3-rx.pcap \
> > +    ofport-request=3
> > +
> > +# wait for earlier changes to take effect
> > +check ovn-nbctl --wait=hv sync
> > +wait_for_ports_up
> > +
> > +ovn-sbctl dump-flows > sbflows
> > +AT_CAPTURE_FILE([sbflows])
> > +
> > +AT_CAPTURE_FILE([ofgroups])
> > +OVS_WAIT_FOR_OUTPUT([as hv1 ovs-ofctl dump-groups br-int > ofgroups
> > +    grep "selection_method=hash,fields" ofgroups | \
> > +    grep "nw_proto" | grep "ip_src" | grep "ip_dst" | wc -l], [0], [4
> > +])
> > +
> > +as hv1 ovs-ofctl dump-groups br-int > ofgroups
> > +AT_CHECK([grep "selection_method=hash,fields(ip_src,ip_dst,nw_proto)" 
> > ofgroups |  wc -l], [0], [1
> > +])
> > +AT_CHECK([grep 
> > "selection_method=hash,fields(ip_src,ip_dst,nw_proto,tcp_dst)" ofgroups |  
> > wc -l], [0], [1
> > +])
> > +AT_CHECK([grep 
> > "selection_method=hash,fields(ip_src,ip_dst,nw_proto,udp_dst)" ofgroups |  
> > wc -l], [0], [1
> > +])
> > +AT_CHECK([grep 
> > "selection_method=hash,fields(ip_src,ip_dst,nw_proto,sctp_dst)" ofgroups |  
> > wc -l], [0], [1
> > +])
> > +
> > +as hv1 ovs-ofctl dump-flows br-int > oflows
> > +AT_CAPTURE_FILE([oflows])
> > +
> > +for i in $(seq 5001 5010); do
> > +    packet="inport==\"lsp11\" && eth.src==f0:00:00:00:01:11 && 
> > eth.dst==00:00:00:01:01:01 &&
> > +            ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && 
> > ip4.dst==10.0.0.100 &&
> > +            tcp && tcp.src==$i && tcp.dst==80"
> > +    OVS_WAIT_UNTIL([as hv1 ovs-appctl -t ovn-controller inject-pkt 
> > "$packet"])
> > +
> > +    for j in 1 2; do
> > +        # Assume all packets go to lsp2${j}.
> > +        exp_packet="eth.src==00:00:00:01:02:01 && 
> > eth.dst==f0:00:00:00:02:2${j} &&
> > +                ip4 && ip.ttl==63 && ip4.src==192.168.1.11 && 
> > ip4.dst==10.0.0.100 &&
> > +                tcp && tcp.src==$i && tcp.dst==80"
> > +        echo $exp_packet | ovstest test-ovn expr-to-packets >> 
> > expected_lsp2${j}
> > +    done
> > +done
> > +
> > +# All packets should go out of a single port given the hashing is based on 
> > ip_proto,ip_src,ip_dst,dst_port which is fixed
> > +OVS_WAIT_UNTIL([
> > +    hv1_rcv_n1=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" 
> > hv1/vif2-tx.pcap > lsp21.packets && cat lsp21.packets | wc -l`
> > +    hv1_rcv_n2=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" 
> > hv1/vif3-tx.pcap > lsp22.packets && cat lsp22.packets | wc -l`
> > +    echo $hv1_rcv_n1 $hv1_rcv_n2
> > +    test $(($hv1_rcv_n1 + $hv1_rcv_n2)) -ge 10])
> > +
> > +if test $hv1_rcv_n1 = 0; then
> > +    AT_CHECK([test $hv1_rcv_n2 -ge 10], [0], [])
> > +else
> > +    AT_CHECK([test $hv1_rcv_n1 -ge 10], [0], [])
> > +fi
> > +
> > +# Move all VIFs to hv2 and send the same packets again
> > +as hv1
> > +ovs-vsctl del-port hv1-vif1
> > +ovs-vsctl del-port hv1-vif2
> > +ovs-vsctl del-port hv1-vif3
> > +
> > +wait_column "" Port_Binding chassis logical_port=lsp11
> > +wait_column "" Port_Binding chassis logical_port=lsp21
> > +wait_column "" Port_Binding chassis logical_port=lsp22
> > +
> > +as hv2
> > +ovs-vsctl -- add-port br-int hv2-vif1 -- \
> > +    set interface hv2-vif1 external-ids:iface-id=lsp11 \
> > +    options:tx_pcap=hv2/vif1-tx.pcap \
> > +    options:rxq_pcap=hv2/vif1-rx.pcap \
> > +    ofport-request=1
> > +
> > +ovs-vsctl -- add-port br-int hv2-vif2 -- \
> > +    set interface hv2-vif2 external-ids:iface-id=lsp21 \
> > +    options:tx_pcap=hv2/vif2-tx.pcap \
> > +    options:rxq_pcap=hv2/vif2-rx.pcap \
> > +    ofport-request=2
> > +
> > +ovs-vsctl -- add-port br-int hv2-vif3 -- \
> > +    set interface hv2-vif3 external-ids:iface-id=lsp22 \
> > +    options:tx_pcap=hv2/vif3-tx.pcap \
> > +    options:rxq_pcap=hv2/vif3-rx.pcap \
> > +    ofport-request=3
> > +
> > +# wait for earlier changes to take effect
> > +check ovn-nbctl --wait=hv sync
> > +wait_for_ports_up
> > +
> > +AT_CAPTURE_FILE([ofgroups])
> > +OVS_WAIT_FOR_OUTPUT([as hv2 ovs-ofctl dump-groups br-int > ofgroups
> > +    grep "selection_method=hash,fields" ofgroups | \
> > +    grep "nw_proto" | grep "ip_src" | grep "ip_dst" | wc -l], [0], [4
> > +])
> > +
> > +as hv2 ovs-ofctl dump-groups br-int > ofgroups
> > +AT_CHECK([grep "selection_method=hash,fields(ip_src,ip_dst,nw_proto)" 
> > ofgroups |  wc -l], [0], [1
> > +])
> > +AT_CHECK([grep 
> > "selection_method=hash,fields(ip_src,ip_dst,nw_proto,tcp_dst)" ofgroups |  
> > wc -l], [0], [1
> > +])
> > +AT_CHECK([grep 
> > "selection_method=hash,fields(ip_src,ip_dst,nw_proto,udp_dst)" ofgroups |  
> > wc -l], [0], [1
> > +])
> > +AT_CHECK([grep 
> > "selection_method=hash,fields(ip_src,ip_dst,nw_proto,sctp_dst)" ofgroups |  
> > wc -l], [0], [1
> > +])
> > +
> > +as hv2 ovs-ofctl dump-flows br-int > oflows
> > +AT_CAPTURE_FILE([oflows])
> > +
> > +for i in $(seq 5001 5010); do
> > +    packet="inport==\"lsp11\" && eth.src==f0:00:00:00:01:11 && 
> > eth.dst==00:00:00:01:01:01 &&
> > +            ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && 
> > ip4.dst==10.0.0.100 &&
> > +            tcp && tcp.src==$i && tcp.dst==80"
> > +    OVS_WAIT_UNTIL([as hv2 ovs-appctl -t ovn-controller inject-pkt 
> > "$packet"])
> > +
> > +    for j in 1 2; do
> > +        # Assume all packets go to lsp2${j}.
> > +        exp_packet="eth.src==00:00:00:01:02:01 && 
> > eth.dst==f0:00:00:00:02:2${j} &&
> > +                ip4 && ip.ttl==63 && ip4.src==192.168.1.11 && 
> > ip4.dst==10.0.0.100 &&
> > +                tcp && tcp.src==$i && tcp.dst==80"
> > +        echo $exp_packet | ovstest test-ovn expr-to-packets >> 
> > expected_lsp2${j}
> > +    done
> > +done
> > +
> > +# All packets should go out of a single port given the hashing is based on 
> > ip_proto,ip_src,ip_dst,dst_port which is fixed
> > +OVS_WAIT_UNTIL([
> > +    hv2_rcv_n1=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" 
> > hv2/vif2-tx.pcap > lsp21.packets && cat lsp21.packets | wc -l`
> > +    hv2_rcv_n2=`$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" 
> > hv2/vif3-tx.pcap > lsp22.packets && cat lsp22.packets | wc -l`
> > +    echo $hv2_rcv_n1 $hv2_rcv_n2
> > +    test $(($hv2_rcv_n1 + $hv2_rcv_n2)) -ge 10])
> > +
> > +if test $hv2_rcv_n1 = 0; then
> > +    AT_CHECK([test $hv2_rcv_n2 -ge 10], [0], [])
> > +else
> > +    AT_CHECK([test $hv2_rcv_n1 -ge 10], [0], [])
> > +fi
> > +
> > +# All packets should out of the same port on both hosts
> > +if test $hv1_rcv_n1 = 0; then
> > +    AT_CHECK([test $hv2_rcv_n1 -eq 0], [0], [])
> > +else
> > +    AT_CHECK([test $hv2_rcv_n2 -eq 0], [0], [])
> > +fi
> > +
> > +OVN_CLEANUP([hv1], [hv2])
> > +
> > +AT_CLEANUP
> > +])
> >
> >  OVN_FOR_EACH_NORTHD([
> >  AT_SETUP([route tables -- <main> route table routes])
> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
> > index d45be75c7..490edc492 100644
> > --- a/utilities/ovn-nbctl.c
> > +++ b/utilities/ovn-nbctl.c
> > @@ -4753,11 +4753,18 @@ nbctl_lr_route_add(struct ctl_context *ctx)
> >          nbrec_logical_router_static_route_set_route_table(route, 
> > route_table);
> >      }
> >
> > -    if (ecmp_symmetric_reply) {
> > -        const struct smap options = SMAP_CONST1(&options,
> > -                                                "ecmp_symmetric_reply",
> > -                                                "true");
> > +    const char *ecmp_selection_fields = shash_find_data(&ctx->options,
> > +                                            "--ecmp-selection-fields");
> > +    if (ecmp_symmetric_reply || ecmp_selection_fields) {
> > +        struct smap options = SMAP_INITIALIZER(&options);
> > +        if (ecmp_symmetric_reply) {
> > +            smap_add(&options, "ecmp_symmetric_reply", "true");
> > +        }
> > +        if (ecmp_selection_fields) {
> > +            smap_add(&options, "ecmp_selection_fields", 
> > ecmp_selection_fields);
> > +        }
> >          nbrec_logical_router_static_route_set_options(route, &options);
> > +        smap_destroy(&options);
> >      }
> >
> >      nbrec_logical_router_update_static_routes_addvalue(lr, route);
> > @@ -8090,7 +8097,7 @@ static const struct ctl_command_syntax 
> > nbctl_commands[] = {
> >      { "lr-route-add", 3, 4, "ROUTER PREFIX NEXTHOP [PORT]",
> >        nbctl_pre_lr_route_add, nbctl_lr_route_add, NULL,
> >        "--may-exist,--ecmp,--ecmp-symmetric-reply,--policy=,"
> > -      "--route-table=,--bfd?", RW },
> > +      "--route-table=,--bfd?,--ecmp-selection-fields=", RW },
> >      { "lr-route-del", 1, 4, "ROUTER [PREFIX [NEXTHOP [PORT]]]",
> >        nbctl_pre_lr_route_del, nbctl_lr_route_del, NULL,
> >        "--if-exists,--policy=,--route-table=", RW },
> > --
> > 2.22.3
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to