On Thu, Feb 8, 2024 at 1:49 PM <[email protected]> wrote:
>
> From: Numan Siddique <[email protected]>
>
> The default drop flow in lr_out_delivery stage is generated
> for every router port of a logical router.  This results in the
> lflow_table_add_lflow() to be called multiple times for the
> same match and actions and the ovn_lflow to have multiple
> dp_refcnts.  Fix this by generating this lflow only once for
> each router.
>
> Fixes: 27a92cc272aa ("northd: make default drops explicit")
> Signed-off-by: Numan Siddique <[email protected]>
> ---
>  northd/northd.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index a174a4dcd1..a5d5e67117 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -13470,9 +13470,6 @@ build_egress_delivery_flows_for_lrouter_port(
>      ds_put_format(match, "outport == %s", op->json_key);
>      ovn_lflow_add(lflows, op->od, S_ROUTER_OUT_DELIVERY, 100,
>                    ds_cstr(match), "output;", lflow_ref);
> -
> -    ovn_lflow_add_default_drop(lflows, op->od, S_ROUTER_OUT_DELIVERY,
> -                               lflow_ref);
>  }
>
>  static void
> @@ -14838,9 +14835,9 @@ lrouter_check_nat_entry(const struct ovn_datapath
*od,
>  }
>
>  /* NAT, Defrag and load balancing. */
> -static void build_lr_nat_defrag_and_lb_default_flows(struct ovn_datapath
*od,
> -                                                struct lflow_table
*lflows,
> -                                                struct lflow_ref
*lflow_ref)
> +static void build_lr_nat_defrag_and_lb_default_flows(
> +    struct ovn_datapath *od, struct lflow_table *lflows,
> +    struct lflow_ref *lflow_ref)
>  {
>      ovs_assert(od->nbr);
>
> @@ -14866,6 +14863,12 @@ static void
build_lr_nat_defrag_and_lb_default_flows(struct ovn_datapath *od,
>       * packet would go through conntrack - which is not required. */
>      ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120, "nd_ns", "next;",
>                    lflow_ref);
> +
> +    /* Default drop rule in lr_out_delivery stage.  See
> +     * build_egress_delivery_flows_for_lrouter_port() which adds a rule
> +     * for each router port. */
> +    ovn_lflow_add_default_drop(lflows, od, S_ROUTER_OUT_DELIVERY,
> +                               lflow_ref);
>  }
>
>  static void
> --
> 2.43.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Numan. The patch LGTM except that the function
build_lr_nat_defrag_and_lb_default_flows() doesn't seem to be the right
place to add the flow. I'd either add a new function, or just call the
ovn_lflow_add_default_drop directly in
build_lswitch_and_lrouter_iterate_by_lr for this flow.
With this addressed:

Acked-by: Han Zhou <[email protected]>

Regards,
Han
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to