On Fri, Jul 9, 2021 at 4:17 PM Han Zhou <[email protected]> wrote: > > On Wed, Jul 7, 2021 at 6:23 PM Han Zhou <[email protected]> wrote: > > > > > > > > On Wed, Jul 7, 2021 at 1:28 AM Mark Gray <[email protected]> wrote: > > > > > > This patch addresses a number of interconnected issues with Gateway > Routers > > > that have Load Balancing enabled: > > > > > > 1) In the router pipeline, we have the following stages to handle > > > dnat and unsnat. > > > > > > - Stage 4 : lr_in_defrag (dnat zone) > > > - Stage 5 : lr_in_unsnat (snat zone) > > > - Stage 6 : lr_in_dnat (dnat zone) > > > > > > In the reply direction, the order of traversal of the tables > > > "lr_in_defrag", "lr_in_unsnat" and "lr_in_dnat" adds incorrect > > > datapath flows that check ct_state in the wrong conntrack zone. > > > This is illustrated below where reply trafic enters the physical host > > > port (6) and traverses DNAT zone (14), SNAT zone (default), back to the > > > DNAT zone and then on to Logical Switch Port zone (22). The third > > > flow is incorrectly checking the state from the SNAT zone instead > > > of the DNAT zone. > > > > > > recirc_id(0),in_port(6),ct_state(-new-est-rel-rpl-trk) > actions:ct_clear,ct(zone=14),recirc(0xf) > > > recirc_id(0xf),in_port(6) actions:ct(nat),recirc(0x10) > > > recirc_id(0x10),in_port(6),ct_state(-new+est+trk) > actions:ct(zone=14,nat),recirc(0x11) > > > recirc_id(0x11),in_port(6),ct_state(+new-est-rel-rpl+trk) actions: > ct(zone=22,nat),recirc(0x12) > > > recirc_id(0x12),in_port(6),ct_state(-new+est-rel+rpl+trk) actions:5 > > > > > > Update the order of these tables to resolve this. > > > > > > 2) Efficiencies can be gained by using the ct_dnat action in the > > > table "lr_in_defrag" instead of ct_next. This removes the need for the > > > ct_dnat action for established Load Balancer flows avoiding a > > > recirculation. > > > > > > 3) On a Gateway router with DNAT flows configured, the router will > translate > > > the destination IP address from (A) to (B). Reply packets from (B) are > > > correctly UNDNATed in the reverse direction. > > > > > > However, if a new connection is established from (B), this flow is never > > > committed to conntrack and, as such, is never established. This will > > > cause OVS datapath flows to be added that match on the ct.new flag. > > > > > > For software-only datapaths this is not a problem. However, for > > > datapaths that offload these flows to hardware, this may be problematic > > > as some devices are unable to offload flows that match on ct.new. > > > > > > This patch resolves this by committing these flows to the DNAT zone in > > > the new "lr_out_post_undnat" stage. Although this could be done in the > > > DNAT zone, by doing this in the new zone we can avoid a recirculation. > > > > > > This patch also generalizes these changes to distributed routers with > > > gateway ports. > > > > > > Co-authored-by: Numan Siddique <[email protected]> > > > Signed-off-by: Mark Gray <[email protected]> > > > Signed-off-by: Numan Siddique <[email protected]> > > > Reported-at: https://bugzilla.redhat.com/1956740 > > > Reported-at: https://bugzilla.redhat.com/1953278 > > > --- > > > > > > Notes: > > > v2: Addressed Han's comments > > > * fixed ovn-northd.8.xml > > > * added 'is_gw_router' to all cases where relevant > > > * refactor add_router_lb_flow() > > > * added ct_commit/ct_dnat to gateway ports case > > > * updated flows like "ct.new && ip && <register> to specify > ip4/ip6 instead of ip > > > * increment ovn_internal_version > > > v4: Fix line length errors from 0-day > > > v5: Add "Reported-at" tag > > > > > > Thanks Mark! > > Acked-by: Han Zhou <[email protected]> > > Thanks Ariel (in CC) for testing. I added you with Tested-by. I could wait > for Numan to merge himself, but since this has been hanging for long and > the rebase conflict was big (thanks Mark and Numan again). To avoid further > rebasing effort, I just applied it to the master branch, with the minor > diff below (spotted during another minor rebase).
Sounds good to me. Thanks for the reviews. Numan > > Thanks, > Han > > =========================================== > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index e81175937..2a7905b43 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -3858,6 +3858,7 @@ AT_CHECK([grep -w "ls_in_dhcp_options" sw0flows | > sort], [0], [dnl > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > AT_SETUP([ovn -- LR NAT flows]) > ovn_start > _______________________________________________ > 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
