netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Jason Xing <kerneljasonxing@gmail.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, dsahern@kernel.org,
	willemdebruijn.kernel@gmail.com, willemb@google.com,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me,
	haoluo@google.com, jolsa@kernel.org, bpf@vger.kernel.org,
	netdev@vger.kernel.org, Jason Xing <kernelxing@tencent.com>
Subject: Re: [PATCH net-next v4 10/11] net-timestamp: export the tskey for TCP bpf extension
Date: Fri, 10 Jan 2025 12:35:57 -0800	[thread overview]
Message-ID: <13c5a76b-0635-42ed-8dfa-4f656a03a564@linux.dev> (raw)
In-Reply-To: <CAL+tcoCSrBBaW3Rg1hD0mBAGu_ZTCTfjVBGe_7B=_JB+uJTuYA@mail.gmail.com>

On 1/7/25 8:21 PM, Jason Xing wrote:
> Hi Martin,
> 
>>> -     bpf_skops_tx_timestamping(sk, skb, op, 2, args);
>>> +     if (sk_is_tcp(sk))
>>> +             args[2] = skb_shinfo(skb)->tskey;
>>
>> Instead of only passing one info "skb_shinfo(skb)->tskey" of a skb, pass the
>> whole skb ptr to the bpf prog. Take a look at bpf_skops_init_skb. Lets start
>> with end_offset = 0 for now so that the bpf prog won't use it to read the
>> skb->data. It can be revisited later.
>>
>>          bpf_skops_init_skb(&sock_ops, skb, 0);
>>
>> The bpf prog can use bpf_cast_to_kern_ctx() and bpf_core_cast() to get to the
>> skb_shinfo(skb). Take a look at the md_skb example in type_cast.c.
> 
> In recent days, I've been working on this part. It turns out to be
> infeasible to pass "struct __sk_buff *skb" as the second parameter in
> skops_sockopt() in patch [11/11]. I cannot find a way to acquire the
> skb itself

I didn't mean to pass skb in sock_ops_kern->args[] or pass skb to the bpf prog
"SEC("sockops") skops_sockopt(struct bpf_sock_ops *skops, struct sk_buff *skb)".
The bpf prog can only take one ctx argument which is
"struct bpf_sock_ops *skops" here.

I meant to have kernel initializing the sock_ops_kern->skb by doing
"bpf_skops_init_skb(&sock_ops, skb, 0);" before running the bpf prog.

The bpf prog can read the skb by using bpf_cast_to_kern_ctx() and bpf_core_cast().

Something like the following. I directly change the existing test_tcp_hdr_options.c.
It has not been changed to use the vmlinux.h, so I need to redefine some parts of
the sk_buff, skb_shared_info, and bpf_sock_ops_kern. Your new test should directly
include <vmlinux.h> and no need to redefine them.

Untested code:

diff --git i/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c w/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
index 5f4e87ee949a..c98ebe71f6ba 100644
--- i/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
+++ w/tools/testing/selftests/bpf/progs/test_tcp_hdr_options.c
@@ -12,8 +12,10 @@
  #include <linux/types.h>
  #include <bpf/bpf_helpers.h>
  #include <bpf/bpf_endian.h>
+#include <bpf/bpf_core_read.h>
  #define BPF_PROG_TEST_TCP_HDR_OPTIONS
  #include "test_tcp_hdr_options.h"
+#include "bpf_kfuncs.h"
  
  #ifndef sizeof_field
  #define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER))
@@ -348,9 +350,63 @@ static int current_mss_opt_len(struct bpf_sock_ops *skops)
  	return CG_OK;
  }
  
+struct sk_buff {
+	unsigned int		end;
+	unsigned char		*head;
+} __attribute__((preserve_access_index));
+
+struct skb_shared_info {
+	__u8		flags;
+	__u8		meta_len;
+	__u8		nr_frags;
+	__u8		tx_flags;
+	unsigned short	gso_size;
+	unsigned short	gso_segs;
+	unsigned int	gso_type;
+	__u32		tskey;
+} __attribute__((preserve_access_index));
+
+struct bpf_sock_ops_kern {
+	struct	sock *sk;
+	union {
+		__u32 args[4];
+		__u32 reply;
+		__u32 replylong[4];
+	};
+	struct sk_buff	*syn_skb;
+	struct sk_buff	*skb;
+	void	*skb_data_end;
+	__u8	op;
+	__u8	is_fullsock;
+	__u8	remaining_opt_len;
+	__u64	temp;			/* temp and everything after is not
+					 * initialized to 0 before calling
+					 * the BPF program. New fields that
+					 * should be initialized to 0 should
+					 * be inserted before temp.
+					 * temp is scratch storage used by
+					 * sock_ops_convert_ctx_access
+					 * as temporary storage of a register.
+					 */
+} __attribute__((preserve_access_index));
+
  static int handle_hdr_opt_len(struct bpf_sock_ops *skops)
  {
  	__u8 tcp_flags = skops_tcp_flags(skops);
+	struct bpf_sock_ops_kern *skops_kern;
+	struct skb_shared_info *shared_info;
+	struct sk_buff *skb;
+
+	skops_kern = bpf_cast_to_kern_ctx(skops);
+	skb = skops_kern->skb;
+
+	if (skb) {
+		shared_info = bpf_core_cast(skb->head + skb->end, struct skb_shared_info);
+		/* printk as an example. don't do that in selftests. */
+		bpf_printk("tskey %u gso_size %u gso_segs %u gso_type %u flags %x\n",
+			   shared_info->tskey, shared_info->gso_size,
+			   shared_info->gso_segs, shared_info->gso_type, shared_info->flags);
+	}
  
  	if ((tcp_flags & TCPHDR_SYNACK) == TCPHDR_SYNACK)
  		return synack_opt_len(skops);



  parent reply	other threads:[~2025-01-10 20:36 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-07 17:37 [PATCH net-next v4 00/11] net-timestamp: bpf extension to equip applications transparently Jason Xing
2024-12-07 17:37 ` [PATCH net-next v4 01/11] net-timestamp: add support for bpf_setsockopt() Jason Xing
2024-12-09  4:28   ` kernel test robot
2024-12-09  4:31   ` kernel test robot
2024-12-12 19:34   ` Martin KaFai Lau
2024-12-13 14:12     ` Jason Xing
2024-12-07 17:37 ` [PATCH net-next v4 02/11] net-timestamp: prepare for bpf prog use Jason Xing
2024-12-11  2:02   ` Martin KaFai Lau
2024-12-11  9:17     ` Jason Xing
2024-12-13  1:41       ` Martin KaFai Lau
2024-12-13 14:42         ` Jason Xing
2024-12-13 22:26           ` Martin KaFai Lau
2024-12-13 23:56             ` Jason Xing
2024-12-07 17:37 ` [PATCH net-next v4 03/11] net-timestamp: reorganize in skb_tstamp_tx_output() Jason Xing
2024-12-09 14:37   ` Willem de Bruijn
2024-12-09 14:57     ` Jason Xing
2024-12-07 17:37 ` [PATCH net-next v4 04/11] net-timestamp: support SCM_TSTAMP_SCHED for bpf extension Jason Xing
2024-12-07 17:37 ` [PATCH net-next v4 05/11] net-timestamp: support SCM_TSTAMP_SND " Jason Xing
2024-12-07 17:37 ` [PATCH net-next v4 06/11] net-timestamp: support SCM_TSTAMP_ACK " Jason Xing
2024-12-12 22:36   ` Martin KaFai Lau
2024-12-13 14:49     ` Jason Xing
2024-12-13 22:46       ` Martin KaFai Lau
2024-12-07 17:37 ` [PATCH net-next v4 07/11] net-timestamp: support hwtstamp print " Jason Xing
2024-12-12 23:25   ` Martin KaFai Lau
2024-12-13  6:28     ` Martin KaFai Lau
2024-12-13 15:13     ` Jason Xing
2024-12-13 23:15       ` Martin KaFai Lau
2024-12-14  0:02         ` Jason Xing
2024-12-14  0:17           ` Martin KaFai Lau
2024-12-14  0:48             ` Jason Xing
2024-12-07 17:38 ` [PATCH net-next v4 08/11] net-timestamp: make TCP tx timestamp bpf extension work Jason Xing
2024-12-07 17:38 ` [PATCH net-next v4 09/11] net-timestamp: introduce cgroup lock to avoid affecting non-bpf cases Jason Xing
2024-12-07 17:38 ` [PATCH net-next v4 10/11] net-timestamp: export the tskey for TCP bpf extension Jason Xing
2024-12-13  0:28   ` Martin KaFai Lau
2024-12-13 15:44     ` Jason Xing
2024-12-13 23:55       ` Martin KaFai Lau
2024-12-14  0:09         ` Jason Xing
2025-01-08  4:21     ` Jason Xing
2025-01-08 12:55       ` Jason Xing
2025-01-10 20:35       ` Martin KaFai Lau [this message]
2025-01-10 22:46         ` Jason Xing
2024-12-07 17:38 ` [PATCH net-next v4 11/11] bpf: add simple bpf tests in the tx path for so_timstamping feature Jason Xing
2024-12-09 14:45   ` Willem de Bruijn
2024-12-09 14:58     ` Jason Xing
2024-12-13  1:14   ` Martin KaFai Lau
2024-12-13 16:02     ` Jason Xing
2024-12-14  0:14       ` Martin KaFai Lau
2024-12-14  0:45         ` Jason Xing

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=13c5a76b-0635-42ed-8dfa-4f656a03a564@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=eddyz87@gmail.com \
    --cc=edumazet@google.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kerneljasonxing@gmail.com \
    --cc=kernelxing@tencent.com \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).