On 3/10/23 11:07, Nobuhiro MIKI wrote: > On 2023/03/10 18:17, Simon Horman wrote: >> On Fri, Mar 10, 2023 at 02:49:06PM +0900, Nobuhiro MIKI wrote: >>> On 2023/01/17 18:41, Nobuhiro MIKI wrote: >>>> SRv6 (Segment Routing IPv6) tunnel vport is responsible >>>> for encapsulation and decapsulation the inner packets with >>>> IPv6 header and an extended header called SRH >>>> (Segment Routing Header). See spec in: >>>> >>>> https://datatracker.ietf.org/doc/html/rfc8754 >>>> >>>> This patch implements SRv6 tunneling in userspace datapath. >>>> It uses `remote_ip` and `local_ip` options as with existing >>>> tunnel protocols. It also adds a dedicated `srv6_segs` option >>>> to define a sequence of routers called segment list. >>>> >>>> Signed-off-by: Nobuhiro MIKI <[email protected]> >>> >>> Hi Ilya and OVS Community, >>> >>> Please let me discuss with you how to promote the development and >>> review of this huge patch. It has been 8 months since v1 was >>> submitted and only a few reviews and improvements have been done. >>> Perhaps it is because this patch is so huge that it affects about >>> 27 files and 700 lines. >>> >>> Would the review process go smoother if we split it into several >>> patches as follows? >>> >>> - Support two tunnel types (e.g. IPPROTO_IPIP and IPPRPTO_IPV6) >>> - SRv6 header generation and removal >>> - Parsing of extension headers for IPPRPTO_ROUTING in >>> parse_ipv6_ext_hdrs__() >>> - Parsing of Segment Lists >>> - Add OVS_VPORT_TYPE_SRV6 >>> >>> Thank you for always helping me out. Any comments would be greatly >>> appreciated. >> >> Only speaking for myself, but breaking up the change into >> separate, logically consistent, patches/patch-sets does >> make review significantly easier. > > Thanks for your comment. I'm feeling the same way now. > I will refactor this to split it into multiple patches that > make logical sense and submit again.
Thanks, Nobuhiro. Sorry I didn't get to properly review v7 still. Splitting it up will definitely make the task easier. I have a couple of small comments that you could address while working on a split: 1. Update docs and the NEWS file to point to 3.2 version, we missed the 3.1 release at this point. 2. Instead of creating a new file tests/system-userspace-traffic.at, it's better to add the test to a common tests/system-traffic.at alongside similar tunnel tests. To avoid failures with kernel datapath, you need to define OVS_CHECK_SRV6() macro separately for different testsuites. Move your current implementation of OVS_CHECK_SRV6() into tests/system-userspace-macros.at and define one that always skips the test in tests/system-kmod-macros.at. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
