On Thu, Feb 23, 2023 at 7:29 AM Ales Musil <[email protected]> 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. > 2) Fragmented traffic for LBs will go through CT twice (or three times if > it's a new connection). > > 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 > 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. > Ah scratch that idea, I just completely ignored the fact that the ct_dnat is there for already established traffic. However we should still address the first point. > > Thanks, > Ales > > -- > > Ales Musil > > Senior Software Engineer - OVN Core > > Red Hat EMEA <https://www.redhat.com> > > [email protected] IM: amusil > <https://red.ht/sig> > -- 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
