On Feb 23, Ales Musil wrote: > On Wed, Feb 22, 2023 at 4:58 PM Lorenzo Bianconi < > [email protected]> wrote: > > > Lorenzo Bianconi (2): > > northd: move defrag router pipeline stage forward > > northd: add flows to defrag IP traffic > > > > northd/northd.c | 50 ++-- > > northd/ovn-northd.8.xml | 60 +++-- > > tests/ovn-northd.at | 526 ++++++++++++++++++++-------------------- > > tests/ovn.at | 52 ++-- > > tests/system-ovn.at | 8 +- > > 5 files changed, 360 insertions(+), 336 deletions(-) > > > > -- > > 2.39.2 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > Hi Lorenzo, > > as discussed earlier, adding my opinion on this topic. > First of all I agree that we need a separate defrag stage, however > there I see two major drawbacks with your suggestion. > > 1) We will send to CT all packets that are fragmented even if they are > not to be load balanced.
yep, I am fine with it (we will remove some processing from datapath and move it to ovn-northd/ovn-controller). > 2) Fragmented traffic for LBs will go through CT twice (or three times if > it's a new connection). doing so we are going to break lb for ct.est connections since we are missing a ct_dnat in this case. I do not think we can reduce the number of ct here. Agree? > > I would suggest rearranging it slightly differently. > To solve both issues from above the defrag stage should apply only for > packets that are destined to LB VIP e.g. > > match=(ip && ip.dst == VIP), action(ct_next;) > > And the post-defrag/pre-dnat stage would populate all registers that are > required > for the dnat stage itself e.g. > > match=(ip && ip.dst == VIP && PROTO && PROTO.dst == VIP_PORT), > action=(reg00 = VIP, reg9 = PROTO.dst, next;) > > One thing that is not completely clear is why there is a ct_dnat; in the > current defrag stage as I think to squash the required defrag and dnat processing. Regards, Lorenzo > from my point of view it would be enough to have ct_next. Unless I'm > missing something > it should be completely fine to replace it with ct_next as there shouldn't > be any unDNAT > happening because this is for original traffic and not reply. > > Adding mainters to CC to get more insights especially why there is ct_dnat. > > Thanks, > Ales > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] IM: amusil > <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
