On Wed, Aug 4, 2021 at 1:30 AM Lorenzo Bianconi <[email protected]> wrote: > > [...] > > > > I think REGBIT_EGRESS_LOOPBACK check is required so that the injected icmp4 > > packet generated from ovn-controller is skipped in this stage. > > Otherwise there will > > be recursion of the packet. > > > > @Lorenzo Bianconi Please correct me if I'm wrong. > > correct, REGBIT_EGRESS_LOOPBACK is used to skip re-injected packets > > > > > > > > > > > > > (It would be good if this is intentional because it would make my next > > > rebasing easier, but I just want to make sure I understand it correctly) > > > > > > > > > > I checked further on this change, and it seems the outport is not needed > > > for this flow because in the previous stage ( s_ROUTER_IN_CHK_PKT_LEN) it > > > is already matched. (although I am still not sure why adding the LOOPBACK > > > check) > > > However, for the ingress part, check_pkt_larger() is added to the stage > > > s_ROUTER_IN_ADMISSION regardless of whether it is a DGP, but based on > > > whether the gw_mtu option exists. I think that is reasonable. It didn't > > > need to distinguish DGP and ports on gateway routers. So I wonder if we > > > could do the same for the egress, i.e. combining the logic of DGP and ports > > > on gateway routers. This would simply the code and also the flows. I can > > > make that change if it sounds good. (It can also simplify my rebasing for > > > the multiple DGP support) > > > > I'm fine with it if you think it will not break the existing functionality. > > > > @Lorenzo Bianconi If you've any comments here please let us know. > > I am fine with the change if existing functionality is ok. > Btw I am not able to trigger any failure in "router - check packet length - icmp > defrag" in unit-test (I tried multiple times running all the tests and > each test individually). >
Thanks Numan and Lorenzo. I figured out that it is in fact incorrect without the outport matched. Please see the details in this patch I posted: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ The above mentioned refactoring is also included in this patch. Please take a look. For the test case failure on my machine, it happens at the step below which was added by this patch, and that's why it passed when I reverted the patch. AS_BOX([testing ingress traffic mtu 100 - IPv6]) test_ip6_packet_larger_ext 100 But I didn't have the time to dig further yet. Thanks, Han > > > > > > > > > > Regarding the test case, it seems to me it's flaky. When I run the > > > > > whole test suite with -j5, almost 100% of the > > > > > time the test fails, but it passes when run individually for me. > > > > > > > > > Thanks for confirming. This is interesting, completely reverse behavior > > > on my machine :) > > > > > > > > > The test case added in this patch series, does try to make sure that > > > > > both northd-c and northd-ddlog generate > > > > > the exact same flows. > > > > > > > > > > Thanks > > > > > Numan > > > > > > > > > > > > > > > > > Thanks > > > > > > Numan > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > Han > > > > > > > _______________________________________________ > > > > > > > dev mailing list > > > > > > > [email protected] > > > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > > > _______________________________________________ > > > dev mailing list > > > [email protected] > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
