On Fri, Jul 9, 2021 at 2:02 PM Numan Siddique <[email protected]> wrote: > > 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
Unfortunately there are constant system test failures: 141: ECMP IPv6 symmetric reply -- ovn-northd -- dp-groups=yes FAILED ( system-ovn.at:5752) 142: ECMP IPv6 symmetric reply -- ovn-northd -- dp-groups=no FAILED ( system-ovn.at:5752) Sorry that I didn't verify the system test before merging. Mark/Numan, would you take a look? Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
