[...]
> 
> 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).

Regards,
Lorenzo

> 
> Thanks
> Numan
> 
> >
> > > > 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
> > > > > > d...@openvswitch.org
> > > > > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> > > > > >
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
> 
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to