On 2023/03/14 6:20, Ilya Maximets wrote:
> 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.

Hi Ilya, thanks for taking the time to reply.

OK. I'll address these items while I split this patch.

Best Regards,
Nobuhiro MIKI
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to