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]> > --- > 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
