On 2/18/25 12:06 PM, Ales Musil wrote: > On Tue, Feb 18, 2025 at 12:06 PM Ales Musil <[email protected]> wrote: > >> The flag to check if the usnat is not tracked is not needed, if the >> traffic is considered as ct.new we should commit anyway. >> >> Suggested-by: Han Zhou <[email protected]> >> Signed-off-by: Ales Musil <[email protected]> >> ---
Hi Ales, Han, This patch looks good to me: Acked-by: Dumitru Ceara <[email protected]> Han, do you have any concerns with this change? I'm thinking of applying this patch to main and 25.03 sometime next week (before the 25.03.0 release) - if you don't beat me to it that is. :) Regards, Dumitru >> I was able to do additional testing and this change looks safe, if >> possible it should be backported to 25.03. >> --- >> northd/northd.c | 4 ++-- >> tests/ovn-northd.at | 8 ++++---- >> 2 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/northd/northd.c b/northd/northd.c >> index f1f1ede43..c2ba3d139 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -16817,7 +16817,7 @@ build_gw_lrouter_commit_all(const struct >> ovn_datapath *od, >> "ip && (!ct.trk || !ct.rpl) && flags.unsnat_new == 1", >> "ct_commit_to_zone(snat);", lflow_ref); >> ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 10, >> - "ip && ct.new && flags.unsnat_not_tracked == 1", >> + "ip && ct.new", >> "ct_commit_to_zone(snat);", lflow_ref); >> } >> >> @@ -16894,7 +16894,7 @@ build_dgp_lrouter_commit_all(const struct >> ovn_datapath *od, >> ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 10, ds_cstr(match), >> "ct_commit_to_zone(snat);", lflow_ref); >> ds_clear(match); >> - ds_put_format(match, "ip && ct.new && flags.unsnat_not_tracked == 1 >> && " >> + ds_put_format(match, "ip && ct.new && " >> "outport == %s && is_chassis_resident(%s)", >> l3dgw_port->json_key, l3dgw_port->cr_port->json_key); >> ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 10, ds_cstr(match), >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index 0ddb12027..c2fcfe19d 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -15758,7 +15758,7 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) >> table=??(lr_out_snat ), priority=10 , match=(ip && (!ct.trk || >> !ct.rpl) && flags.unsnat_new == 1 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_commit_to_zone(snat);) >> - table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> flags.unsnat_not_tracked == 1 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), >> action=(ct_commit_to_zone(snat);) >> table=??(lr_out_snat ), priority=120 , match=(nd_ns), >> action=(next;) >> table=??(lr_out_snat ), priority=153 , match=(ip && ip4.src == >> 10.0.0.0/24 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), >> action=(ct_snat(172.168.0.10);) >> ]) >> @@ -15806,7 +15806,7 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) >> table=??(lr_out_snat ), priority=10 , match=(ip && (!ct.trk || >> !ct.rpl) && flags.unsnat_new == 1 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_commit_to_zone(snat);) >> - table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> flags.unsnat_not_tracked == 1 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), >> action=(ct_commit_to_zone(snat);) >> table=??(lr_out_snat ), priority=120 , match=(nd_ns), >> action=(next;) >> ]) >> >> @@ -15850,7 +15850,7 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) >> table=??(lr_out_snat ), priority=10 , match=(ip && (!ct.trk || >> !ct.rpl) && flags.unsnat_new == 1 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_commit_to_zone(snat);) >> - table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> flags.unsnat_not_tracked == 1 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public")), action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> outport == "lr0-public" && is_chassis_resident("cr-lr0-public")), >> action=(ct_commit_to_zone(snat);) >> table=??(lr_out_snat ), priority=120 , match=(nd_ns), >> action=(next;) >> table=??(lr_out_snat ), priority=153 , match=(ip && ip4.src == >> 10.0.0.0/24 && outport == "lr0-public" && >> is_chassis_resident("cr-lr0-public") && (!ct.trk || !ct.rpl)), >> action=(ct_snat(172.168.0.10);) >> ]) >> @@ -15896,7 +15896,7 @@ AT_CHECK([grep "lr_out_post_undnat" lr0flows | >> ovn_strip_lflows], [0], [dnl >> AT_CHECK([grep "lr_out_snat" lr0flows | ovn_strip_lflows], [0], [dnl >> table=??(lr_out_snat ), priority=0 , match=(1), action=(next;) >> table=??(lr_out_snat ), priority=10 , match=(ip && (!ct.trk || >> !ct.rpl) && flags.unsnat_new == 1), action=(ct_commit_to_zone(snat);) >> - table=??(lr_out_snat ), priority=10 , match=(ip && ct.new && >> flags.unsnat_not_tracked == 1), action=(ct_commit_to_zone(snat);) >> + table=??(lr_out_snat ), priority=10 , match=(ip && ct.new), >> action=(ct_commit_to_zone(snat);) >> table=??(lr_out_snat ), priority=120 , match=(nd_ns), >> action=(next;) >> table=??(lr_out_snat ), priority=25 , match=(ip && ip4.src == >> 10.0.0.0/24 && (!ct.trk || !ct.rpl)), action=(ct_snat(172.168.0.10);) >> ]) >> -- >> 2.48.1 >> >> > Ah sorry about the v7, it's a completely new patch. > > Regards, > Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
