netdev - Re: [PATCH net-next] selftests: net: ioam: add tunsrc support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e9cf1bf8-2d77-4165-bf64-b41f3b8b880d@uliege.be>
Date: Mon, 23 Sep 2024 11:22:12 +0200
From: Justin Iurman <justin.iurman@...ege.be>
To: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 shuah@...nel.org, linux-kernel@...r.kernel.org,
 linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: net: ioam: add tunsrc support

On 9/19/24 09:57, Paolo Abeni wrote:
> On 9/7/24 18:42, Justin Iurman wrote:
>> TL;DR This patch comes from a discussion we had with Jakub and Paolo.
>>
>> This patch updates the IOAM selftests to support the new "tunsrc"
>> feature of IOAM. As a consequence, some changes were required. For
>> example, the IPv6 header must be accessed to check some fields (i.e.,
>> the source address for the "tunsrc" feature), which is not possible
>> AFAIK with IPv6 raw sockets. The latter is currently used with
>> IPV6_RECVHOPOPTS and was introduced by commit 187bbb6968af ("selftests:
>> ioam: refactoring to align with the fix") to fix an issue. But, we
>> really need packet sockets actually... which is one of the changes in
>> this patch (see the description of the topology at the top of ioam6.sh
>> for explanations). Another change is that all IPv6 addresses used in the
>> topology are now based on the documentation prefix (2001:db8::/32).
>> Also, the tests have been improved and there are now many more of them.
>> Overall, the script is more robust.
>>
>> The diff is kind of a mess. Since it's "just" a selftests patch, I
>> didn't bother having a series of two patches (one to remove it, one to
>> add the new one back). Let me know if you think it's necessary for
>> readability.
>>
>> Note: this patch needs this [1] iproute2-next patch to be merged
>> (waiting for David to do so, should be done soon).
>>
>>    [1] https://patchwork.kernel.org/project/netdevbpf/list/?series=884653
>>
>> Signed-off-by: Justin Iurman <justin.iurman@...ege.be>
> 
> Unfortunatelly we was unable to process this patch before the merge 
> window and net-next is currently closed. You will need to repost it is ~2w.

Hi Paolo,

Sure, no worries.

> Strictly speaking about the patch contents, any chance you could 
> refactor the change in a more 'incremental' way?
> The current format is very hard to review, and even self-tests patches 
> deserve some love ;)

I think the best incremental way I can come up with right now (so that 
it also makes my life easier) is to have a series that (i) removes the 
ioam selftests (patch #1) and (ii) re-adds the new one (patch #2). Would 
you all agree with this?

Cheers,
Justin

> Thanks,
> 
> Paolo
>