diff mbox series

[net-next,v3,04/14] net-timestamp: introduce TS_SCHED_OPT_CB to generate dev xmit timestamp

Message ID 20241028110535.82999-5-kerneljasonxing@gmail.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net-timestamp: bpf extension to equip applications transparently | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 204 this patch: 204
netdev/build_tools success Errors and warnings before: 1 (+0) this patch: 1 (+0)
netdev/cc_maintainers warning 1 maintainers not CCed: horms@kernel.org
netdev/build_clang success Errors and warnings before: 238 this patch: 238
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 6936 this patch: 6936
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 71 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 54 this patch: 54
netdev/source_inline success Was 0 now: 0

Commit Message

Jason Xing Oct. 28, 2024, 11:05 a.m. UTC 
From: Jason Xing <kernelxing@tencent.com>

Introduce BPF_SOCK_OPS_TS_SCHED_OPT_CB flag so that we can decide to
print timestamps when the skb just passes the dev layer.

Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 include/uapi/linux/bpf.h       |  5 +++++
 net/core/skbuff.c              | 31 ++++++++++++++++++++++++++++++-
 tools/include/uapi/linux/bpf.h |  5 +++++
 3 files changed, 40 insertions(+), 1 deletion(-)

Comments

kernel test robot Oct. 29, 2024, 12:23 a.m. UTC | #1 
Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Xing/net-timestamp-reorganize-in-skb_tstamp_tx_output/20241028-192036
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241028110535.82999-5-kerneljasonxing%40gmail.com
patch subject: [PATCH net-next v3 04/14] net-timestamp: introduce TS_SCHED_OPT_CB to generate dev xmit timestamp
config: arm-randconfig-001-20241029 (https://download.01.org/0day-ci/archive/20241029/202410290828.ZqgMO8Xc-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241029/202410290828.ZqgMO8Xc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410290828.ZqgMO8Xc-lkp@intel.com/

All errors (new ones prefixed by >>):

   net/core/skbuff.c: In function 'timestamp_call_bpf':
>> net/core/skbuff.c:5640:9: error: implicit declaration of function 'BPF_CGROUP_RUN_PROG_SOCK_OPS_SK'; did you mean 'BPF_CGROUP_RUN_PROG_SOCK_OPS'? [-Wimplicit-function-declaration]
    5640 |         BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |         BPF_CGROUP_RUN_PROG_SOCK_OPS


vim +5640 net/core/skbuff.c

  5624	
  5625	static void timestamp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
  5626	{
  5627		struct bpf_sock_ops_kern sock_ops;
  5628	
  5629		memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
  5630		if (sk_fullsock(sk)) {
  5631			sock_ops.is_fullsock = 1;
  5632			sock_owned_by_me(sk);
  5633		}
  5634	
  5635		sock_ops.sk = sk;
  5636		sock_ops.op = op;
  5637		if (nargs > 0)
  5638			memcpy(sock_ops.args, args, nargs * sizeof(*args));
  5639	
> 5640		BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk);
  5641	}
  5642
Willem de Bruijn Oct. 29, 2024, 1:02 a.m. UTC | #2 
Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> Introduce BPF_SOCK_OPS_TS_SCHED_OPT_CB flag so that we can decide to
> print timestamps when the skb just passes the dev layer.
> 
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  include/uapi/linux/bpf.h       |  5 +++++
>  net/core/skbuff.c              | 31 ++++++++++++++++++++++++++++++-
>  tools/include/uapi/linux/bpf.h |  5 +++++
>  3 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e8241b320c6d..324e9e40969c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -7013,6 +7013,11 @@ enum {
>  					 * by the kernel or the
>  					 * earlier bpf-progs.
>  					 */
> +	BPF_SOCK_OPS_TS_SCHED_OPT_CB,	/* Called when skb is passing through
> +					 * dev layer when SO_TIMESTAMPING
> +					 * feature is on. It indicates the
> +					 * recorded timestamp.
> +					 */
>  };
>  
>  /* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 39309f75e105..e6a5c883bdc6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -64,6 +64,7 @@
>  #include <linux/mpls.h>
>  #include <linux/kcov.h>
>  #include <linux/iov_iter.h>
> +#include <linux/bpf-cgroup.h>
>  
>  #include <net/protocol.h>
>  #include <net/dst.h>
> @@ -5621,13 +5622,41 @@ static void skb_tstamp_tx_output(struct sk_buff *orig_skb,
>  	__skb_complete_tx_timestamp(skb, sk, tstype, opt_stats);
>  }
>  
> +static void timestamp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
> +{
> +	struct bpf_sock_ops_kern sock_ops;
> +
> +	memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
> +	if (sk_fullsock(sk)) {
> +		sock_ops.is_fullsock = 1;
> +		sock_owned_by_me(sk);

Why this check?

This will usually be false, as timestamps are taken outside the
protocol layers.

> +	}
> +
> +	sock_ops.sk = sk;
> +	sock_ops.op = op;
> +	if (nargs > 0)
> +		memcpy(sock_ops.args, args, nargs * sizeof(*args));
> +
> +	BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk);
> +}
> +
kernel test robot Oct. 29, 2024, 1:04 a.m. UTC | #3 
Hi Jason,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Jason-Xing/net-timestamp-reorganize-in-skb_tstamp_tx_output/20241028-192036
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241028110535.82999-5-kerneljasonxing%40gmail.com
patch subject: [PATCH net-next v3 04/14] net-timestamp: introduce TS_SCHED_OPT_CB to generate dev xmit timestamp
config: arm64-randconfig-001-20241029 (https://download.01.org/0day-ci/archive/20241029/202410290852.PLcWZ1Yo-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241029/202410290852.PLcWZ1Yo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410290852.PLcWZ1Yo-lkp@intel.com/

All errors (new ones prefixed by >>):

>> net/core/skbuff.c:5640:2: error: call to undeclared function 'BPF_CGROUP_RUN_PROG_SOCK_OPS_SK'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    5640 |         BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk);
         |         ^
   1 error generated.


vim +/BPF_CGROUP_RUN_PROG_SOCK_OPS_SK +5640 net/core/skbuff.c

  5624	
  5625	static void timestamp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
  5626	{
  5627		struct bpf_sock_ops_kern sock_ops;
  5628	
  5629		memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
  5630		if (sk_fullsock(sk)) {
  5631			sock_ops.is_fullsock = 1;
  5632			sock_owned_by_me(sk);
  5633		}
  5634	
  5635		sock_ops.sk = sk;
  5636		sock_ops.op = op;
  5637		if (nargs > 0)
  5638			memcpy(sock_ops.args, args, nargs * sizeof(*args));
  5639	
> 5640		BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk);
  5641	}
  5642
Jason Xing Oct. 29, 2024, 1:30 a.m. UTC | #4 
On Tue, Oct 29, 2024 at 9:02 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > Introduce BPF_SOCK_OPS_TS_SCHED_OPT_CB flag so that we can decide to
> > print timestamps when the skb just passes the dev layer.
> >
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> >  include/uapi/linux/bpf.h       |  5 +++++
> >  net/core/skbuff.c              | 31 ++++++++++++++++++++++++++++++-
> >  tools/include/uapi/linux/bpf.h |  5 +++++
> >  3 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index e8241b320c6d..324e9e40969c 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -7013,6 +7013,11 @@ enum {
> >                                        * by the kernel or the
> >                                        * earlier bpf-progs.
> >                                        */
> > +     BPF_SOCK_OPS_TS_SCHED_OPT_CB,   /* Called when skb is passing through
> > +                                      * dev layer when SO_TIMESTAMPING
> > +                                      * feature is on. It indicates the
> > +                                      * recorded timestamp.
> > +                                      */
> >  };
> >
> >  /* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 39309f75e105..e6a5c883bdc6 100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -64,6 +64,7 @@
> >  #include <linux/mpls.h>
> >  #include <linux/kcov.h>
> >  #include <linux/iov_iter.h>
> > +#include <linux/bpf-cgroup.h>
> >
> >  #include <net/protocol.h>
> >  #include <net/dst.h>
> > @@ -5621,13 +5622,41 @@ static void skb_tstamp_tx_output(struct sk_buff *orig_skb,
> >       __skb_complete_tx_timestamp(skb, sk, tstype, opt_stats);
> >  }
> >
> > +static void timestamp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
> > +{
> > +     struct bpf_sock_ops_kern sock_ops;
> > +
> > +     memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
> > +     if (sk_fullsock(sk)) {
> > +             sock_ops.is_fullsock = 1;
> > +             sock_owned_by_me(sk);
>
> Why this check?

I imitated the use of BPF_CGROUP_RUN_PROG_SOCK_OPS.

>
> This will usually be false, as timestamps are taken outside the
> protocol layers.

I will remove this if branch.

Thanks,
Jason
diff mbox series

Patch

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e8241b320c6d..324e9e40969c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -7013,6 +7013,11 @@  enum {
 					 * by the kernel or the
 					 * earlier bpf-progs.
 					 */
+	BPF_SOCK_OPS_TS_SCHED_OPT_CB,	/* Called when skb is passing through
+					 * dev layer when SO_TIMESTAMPING
+					 * feature is on. It indicates the
+					 * recorded timestamp.
+					 */
 };
 
 /* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 39309f75e105..e6a5c883bdc6 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -64,6 +64,7 @@ 
 #include <linux/mpls.h>
 #include <linux/kcov.h>
 #include <linux/iov_iter.h>
+#include <linux/bpf-cgroup.h>
 
 #include <net/protocol.h>
 #include <net/dst.h>
@@ -5621,13 +5622,41 @@  static void skb_tstamp_tx_output(struct sk_buff *orig_skb,
 	__skb_complete_tx_timestamp(skb, sk, tstype, opt_stats);
 }
 
+static void timestamp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
+{
+	struct bpf_sock_ops_kern sock_ops;
+
+	memset(&sock_ops, 0, offsetof(struct bpf_sock_ops_kern, temp));
+	if (sk_fullsock(sk)) {
+		sock_ops.is_fullsock = 1;
+		sock_owned_by_me(sk);
+	}
+
+	sock_ops.sk = sk;
+	sock_ops.op = op;
+	if (nargs > 0)
+		memcpy(sock_ops.args, args, nargs * sizeof(*args));
+
+	BPF_CGROUP_RUN_PROG_SOCK_OPS_SK(&sock_ops, sk);
+}
+
 static void skb_tstamp_tx_output_bpf(struct sock *sk, int tstype)
 {
-	u32 tsflags;
+	u32 tsflags, cb_flag;
 
 	tsflags = READ_ONCE(sk->sk_tsflags_bpf);
 	if (!sk_tstamp_tx_flags(sk, tsflags, tstype))
 		return;
+
+	switch (tstype) {
+	case SCM_TSTAMP_SCHED:
+		cb_flag = BPF_SOCK_OPS_TS_SCHED_OPT_CB;
+		break;
+	default:
+		return;
+	}
+
+	timestamp_call_bpf(sk, cb_flag, 0, NULL);
 }
 
 void __skb_tstamp_tx(struct sk_buff *orig_skb,
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e8241b320c6d..324e9e40969c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -7013,6 +7013,11 @@  enum {
 					 * by the kernel or the
 					 * earlier bpf-progs.
 					 */
+	BPF_SOCK_OPS_TS_SCHED_OPT_CB,	/* Called when skb is passing through
+					 * dev layer when SO_TIMESTAMPING
+					 * feature is on. It indicates the
+					 * recorded timestamp.
+					 */
 };
 
 /* List of TCP states. There is a build check in net/ipv4/tcp.c to detect