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@xxxxxxxxx>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
