On 5/30/23 20:27, Mark Michelson wrote: > Thanks Ihar and Dumitru. > > I applied this series to main and branch-23.06. I encountered conflicts
Thanks, Mark! Unfortunately, I had missed the fact that a new test had been added in the meantime and that we had to update the table numbers for that one too. CI was failing because of that. I posted a fix for it: https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/ Regards, Dumitru > when trying to apply this to branch-23.03, specifically with patch 4. I > suspect a lot of this is due to the fact that the tiered ACL changes are > not present in branch-23.03, so > > * The table numbers don't work out the same in some cases. > * ACL flow actions are different prior to 23.06. > > If you can post backport versions of the patches for 23.03 and below, > that would be great, Ihar. > > Thanks, > Mark Michelson > > On 5/23/23 17:55, Ihar Hrachyshka wrote: >> This series fixes a non-optimal behavior with some multichassis ports. >> >> Specifically, >> >> - when a multichassis port belongs to a switch that also has a localnet >> port, >> - because ingress and egress traffic for the port is funnelled through >> tunnels to guarantee delivery of packets to all chassis that host the >> port, >> - because tunnels add overhead to each funnelled packet, >> - leaving less space for actual packets, >> >> ... the effective MTU of the path for multichassis ports is reduced by >> the size that takes the tunnel to wrap a packet around. (This depends on >> the type of tunnel, also on IP version of the tunnel.) >> >> When this happens, the port and its peers are not notified about the >> reduced MTU for the path to the port, effectively blackholing some of >> the larger packets. >> >> This patch series makes OVN detect these scenarios and handle them as >> follows: >> >> - for all multichassis ports, 4 flows are added - for inport and >> outport matches - to detect "too-big" IP packets; >> - for all "too-big" packets, 2 flows are added to produce a ICMP >> Fragmentation Needed (for IPv4) or Too Big (for IPv6) packet and >> return it to the offending sender. >> >> The proposed implementation is isolated in ovn-controller. An >> alternative would be to implement flows producing ICMP errors via the >> existing Logical_Flow action icmp4_error, as is done for router ports. >> In this case, the 4 flows that detect "too-big" packets would still have >> to reside in ovn-controller because of limitations of the current OVN >> port model, namely lack of `mtu` as an attribute of OVN logical port. >> >> Caveats: this works for IP traffic only. The party receiving the ICMP >> error should handle it by temporarily lowering the MTU used to reach the >> port. Behavior may depend on protocol implementation of the offending >> peer as well as its networking stack configuration. >> >> This patch series was successfully tested with Linux kernel 6.0.8. >> >> This patch series is designed to simplify backporting process. It is >> split in logical pieces to simplify cherry-picking. I consider the >> original behavior described at the start of the cover letter a bug and >> worth a backport. >> >> --- >> v1: - initial version. >> v2: - more system test cases adjusted to new table numbers for egress >> pipeline; >> - switch from xxd to coreutils (tr, head) tools to avoid a new >> dependency; >> - adjusted a comment in the new test case to avoid "migrator" >> terminology that makes no sense outside live migration context - >> which is not the only potential use case for multichassis ports; >> - guard the new test case with HAVE_SCAPY; >> - nit: rewrapped some lines in patch 6/7 to avoid unnecessary line >> changes; >> - added a NEWS entry for patch 6/7 that implements the core of the >> feature; >> - rebased to latest main. >> v3: - expose and reuse lib/actions.c:encode_[start|finish]_controller_op >> functions; >> - reuse ETH_HEADER_LEN, IP_HEADER_LEN, and IPV6_HEADER_LEN from >> ovs:lib/packets.h; >> - add a comment for REGBIT_PKT_LARGER that mentions that the >> register is also used in OFTABLE_OUTPUT_LARGE_PKT_DETECT table; >> - squash the patch that makes if-status-mgr track changes to >> interface mtu into the patch that introduces the mtu field in the >> manager; >> - doc: don't describe path discovery flows in the patch that >> introduces new tables (only in the patch that actually implements >> the flows); >> - nit: use MAX to shave off several lines of code; >> - nit: remove redundant if_status_mgr_iface_get_mtu declaration from >> a header file; >> - rebased to latest main; >> - updated acks based on latest review comments. >> v4: - fix compilation issue in the middle of the series due to previous >> commit rearrangement. >> v5: - introduce a new if-status-mgr input to pflow_output, then process >> OVS interface updates inside if-status-mgr handlers. >> - if-status-mgr: remove set_mtu public function, instead other >> modules should call to if_status_mgr_iface_update. This allows the >> callers to not care which particular fields if-status-mgr would >> like to persist from the OVS record, keeping concerns separated. >> v6: - update more tests in ovn-controller.at to reflect new table >> numbers. >> v7: - remove ovs submodule bump that creeped in by mistake. >> --- >> >> Ihar Hrachyshka (5): >> Track ip version of tunnel in chassis_tunnel struct >> Track interface MTU in if-status-mgr >> if-status: track interfaces for additional chassis >> Add new egress tables to accommodate for too-big packets handling >> Implement MTU Path Discovery for multichassis ports >> >> NEWS | 6 + >> controller/binding.c | 48 +-- >> controller/binding.h | 4 + >> controller/if-status.c | 48 ++- >> controller/if-status.h | 8 +- >> controller/lflow.c | 4 +- >> controller/lflow.h | 49 +-- >> controller/local_data.c | 2 + >> controller/local_data.h | 1 + >> controller/ovn-controller.c | 122 +++++++ >> controller/ovsport.c | 9 + >> controller/ovsport.h | 2 + >> controller/physical.c | 337 ++++++++++++++++++-- >> controller/physical.h | 2 + >> controller/pinctrl.c | 8 +- >> include/ovn/actions.h | 3 + >> lib/actions.c | 4 +- >> lib/ovn-util.h | 7 + >> northd/northd.c | 2 + >> ovn-architecture.7.xml | 76 ++--- >> tests/ovn-controller.at | 298 +++++++++--------- >> tests/ovn.at | 611 +++++++++++++++++++++++++++--------- >> tests/system-ovn-kmod.at | 2 +- >> tests/system-ovn.at | 8 +- >> 24 files changed, 1241 insertions(+), 420 deletions(-) >> > > _______________________________________________ > 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
