On 2/13/24 07:44, Han Zhou wrote: > 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.
I agree with Han, this should be moved somewhere else but that can happen when the patch is merged. > With this addressed: > > Acked-by: Han Zhou <[email protected]> > Acked-by: Dumitru Ceara <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
