On 5/13/20 11:51 PM, Han Zhou wrote:
> In commit c0bf32d the priorities of the regular routes were increased by
> 400, but ECMP routes were not updated. This patch fixes it. Since
> for ECMP routes the outport is unknown (there can be many different
> outports) the condition check of whether the outport is distributed
> gateway port is skipped.
> 
> Fixes: c0bf32d ("Manage ARP process locally in a DVR scenario")
> Cc: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
> Signed-off-by: Han Zhou <hz...@ovn.org>

Hi Han,

Should we also add a test for this scenario to avoid further regressions?

Thanks,
Dumitru

> ---
>  northd/ovn-northd.8.xml |  3 ++-
>  northd/ovn-northd.c     | 22 ++++++++++++----------
>  2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index 8f224b0..b0475d0 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2601,7 +2601,8 @@ next;
>            ids <var>MID1</var>, <var>MID2</var>, ..., a logical flow with 
> match
>            in CIDR notation <code>ip4.dst == <var>N</var>/<var>M</var></code>,
>            or <code>ip6.dst == <var>N</var>/<var>M</var></code>, whose 
> priority
> -          is the integer value of <var>M</var>, has the following actions:
> +          is <code>400</code> + the integer value of <var>M</var>, has the
> +          following actions:
>          </p>
>  
>          <pre>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index b25152d..c02e305 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -7337,7 +7337,8 @@ build_route_prefix_s(const struct v46_ip *prefix, 
> unsigned int plen)
>  }
>  
>  static void
> -build_route_match(const struct ovn_port *op_inport, const char *network_s,
> +build_route_match(const struct ovn_port *op_inport,
> +                  const struct ovn_port *op_outport, const char *network_s,
>                    int plen, bool is_src_route, bool is_ipv4, struct ds 
> *match,
>                    uint16_t *priority)
>  {
> @@ -7351,6 +7352,13 @@ build_route_match(const struct ovn_port *op_inport, 
> const char *network_s,
>          dir = "dst";
>          *priority = (plen * 2) + 1;
>      }
> +    /* traffic for internal IPs of logical switch ports must be sent to
> +     * the gw controller through the overlay tunnels
> +     */
> +    if (!op_outport ||
> +        (op_outport->nbrp && !op_outport->nbrp->n_gateway_chassis)) {
> +        priority += DROUTE_PRIO;
> +    }
>  
>      if (op_inport) {
>          ds_put_format(match, "inport == %s && ", op_inport->json_key);
> @@ -7434,8 +7442,8 @@ build_ecmp_route_flow(struct hmap *lflows, struct 
> ovn_datapath *od,
>      uint16_t priority;
>  
>      char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
> -    build_route_match(NULL, prefix_s, eg->plen, eg->is_src_route, is_ipv4,
> -                      &match, &priority);
> +    build_route_match(NULL, NULL, prefix_s, eg->plen, eg->is_src_route,
> +                      is_ipv4, &match, &priority);
>      free(prefix_s);
>  
>      struct ds actions = DS_EMPTY_INITIALIZER;
> @@ -7548,14 +7556,8 @@ add_route(struct hmap *lflows, const struct ovn_port 
> *op,
>              op_inport = op;
>          }
>      }
> -    build_route_match(op_inport, network_s, plen, is_src_route, is_ipv4,
> +    build_route_match(op_inport, op, network_s, plen, is_src_route, is_ipv4,
>                        &match, &priority);
> -    /* traffic for internal IPs of logical switch ports must be sent to
> -     * the gw controller through the overlay tunnels
> -     */
> -    if (op->nbrp && !op->nbrp->n_gateway_chassis) {
> -        priority += DROUTE_PRIO;
> -    }
>  
>      struct ds actions = DS_EMPTY_INITIALIZER;
>      ds_put_format(&actions, "ip.ttl--; "REG_ECMP_GROUP_ID" = 0; %sreg0 = ",
> 

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

Reply via email to