netdev - Re: [PATCH net-next v4 3/3] net: exthdrs: ioam6: send trace event

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: <da6a308f-8053-4744-8b81-67b63232a526@uliege.be>
Date: Fri, 23 Feb 2024 19:20:04 +0100
From: Justin Iurman <justin.iurman@...ege.be>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net, dsahern@...nel.org, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, linux-kernel@...r.kernel.org,
 justin.iurman@...ege.be
Subject: Re: [PATCH net-next v4 3/3] net: exthdrs: ioam6: send trace event

On 2/23/24 15:41, Justin Iurman wrote:
> If we're processing an IOAM Pre-allocated Trace Option-Type (the only
> one supported currently), and if we're the destination, then send the
> trace as an ioam6 event to the multicast group. This way, user space
> apps will be able to collect IOAM data (for a trace, it only makes sense
> to send events if we're the destination).
> 
> Signed-off-by: Justin Iurman <justin.iurman@...ege.be>
> ---
>   net/ipv6/exthdrs.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 02e9ffb63af1..003f35d9b42b 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -50,6 +50,7 @@
>   #endif
>   #include <net/rpl.h>
>   #include <linux/ioam6.h>
> +#include <linux/ioam6_genl.h>
>   #include <net/ioam6.h>
>   #include <net/dst_metadata.h>
>   
> @@ -954,6 +955,11 @@ static bool ipv6_hop_ioam(struct sk_buff *skb, int optoff)
>   						   + optoff + sizeof(*hdr));
>   
>   		ioam6_fill_trace_data(skb, ns, trace, true);
> +
> +		if (skb_dst(skb)->dev->flags & IFF_LOOPBACK)

Hmm, now that I think about this a bit more, I'm not sure we should 
force a node to be the destination in order to send the event. Instead, 
we could just remove the "am I the destination" check and directly call 
ioam6_event(), which does check if there is at least one listener before 
sending the event. I see two reasons for the change: (a) one may want 
(for whatever reason) to capture IOAM traces on transit nodes, or (b) 
one may use inline (instead of encap) IOAM injection where the 
destination may be outside the IOAM domain (i.e., destination != 
decap/egress IOAM node), which is not RFC8200-compliant and requires 
removing the IOAM Option-Type in-flight from the HbH, but that's another 
story.

Any opinion?

> +			ioam6_event(IOAM6_EVENT_TRACE, dev_net(skb->dev),
> +				    GFP_ATOMIC, (void *)trace,
> +				    hdr->opt_len - 2);
>   		break;
>   	default:
>   		break;