bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
@ 2021-09-29  2:06 Liu Jian
  2021-09-29  5:19 ` Cong Wang
  2021-09-30 22:48 ` John Fastabend
  0 siblings, 2 replies; 6+ messages in thread
From: Liu Jian @ 2021-09-29  2:06 UTC (permalink / raw)
  To: john.fastabend, daniel, jakub, lmb, davem, kuba, ast, andrii,
	kafai, songliubraving, yhs, kpsingh, netdev, bpf, xiyou.wangcong
  Cc: liujian56

If sockmap enable strparser, there are lose offset info in
sk_psock_skb_ingress. If the length determined by parse_msg function
is not skb->len, the skb will be converted to sk_msg multiple times,
and userspace app will get the data multiple times.

Fix this by get the offset and length from strp_msg.
And as Cong suggestion, add one bit in skb->_sk_redir to distinguish
enable or disable strparser.

Signed-off-by: Liu Jian <liujian56@huawei.com>
---
v1->v2: fix build error when disable CONFIG_BPF_STREAM_PARSER
v2->v3: Add one bit in skb->_sk_redir to distinguish enable or disable strparser
v3->v4: Remove "#if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)" code;
	and let "stm" have a more precise scope.

 include/linux/skmsg.h | 18 ++++++++++++++++--
 net/core/skmsg.c      | 40 ++++++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index 14ab0c0bc924..94e2a1f6e58d 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -508,8 +508,22 @@ static inline bool sk_psock_strp_enabled(struct sk_psock *psock)
 
 #if IS_ENABLED(CONFIG_NET_SOCK_MSG)
 
-/* We only have one bit so far. */
-#define BPF_F_PTR_MASK ~(BPF_F_INGRESS)
+#define BPF_F_STRPARSER	(1UL << 1)
+
+/* We only have two bits so far. */
+#define BPF_F_PTR_MASK ~(BPF_F_INGRESS | BPF_F_STRPARSER)
+
+static inline bool skb_bpf_strparser(const struct sk_buff *skb)
+{
+	unsigned long sk_redir = skb->_sk_redir;
+
+	return sk_redir & BPF_F_STRPARSER;
+}
+
+static inline void skb_bpf_set_strparser(struct sk_buff *skb)
+{
+	skb->_sk_redir |= BPF_F_STRPARSER;
+}
 
 static inline bool skb_bpf_ingress(const struct sk_buff *skb)
 {
diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index 2d6249b28928..e85b7f8491b9 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -494,6 +494,7 @@ static struct sk_msg *sk_psock_create_ingress_msg(struct sock *sk,
 }
 
 static int sk_psock_skb_ingress_enqueue(struct sk_buff *skb,
+					u32 off, u32 len,
 					struct sk_psock *psock,
 					struct sock *sk,
 					struct sk_msg *msg)
@@ -507,11 +508,11 @@ static int sk_psock_skb_ingress_enqueue(struct sk_buff *skb,
 	 */
 	if (skb_linearize(skb))
 		return -EAGAIN;
-	num_sge = skb_to_sgvec(skb, msg->sg.data, 0, skb->len);
+	num_sge = skb_to_sgvec(skb, msg->sg.data, off, len);
 	if (unlikely(num_sge < 0))
 		return num_sge;
 
-	copied = skb->len;
+	copied = len;
 	msg->sg.start = 0;
 	msg->sg.size = copied;
 	msg->sg.end = num_sge;
@@ -522,9 +523,11 @@ static int sk_psock_skb_ingress_enqueue(struct sk_buff *skb,
 	return copied;
 }
 
-static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb);
+static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb,
+				     u32 off, u32 len);
 
-static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb)
+static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb,
+				u32 off, u32 len)
 {
 	struct sock *sk = psock->sk;
 	struct sk_msg *msg;
@@ -535,7 +538,7 @@ static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb)
 	 * correctly.
 	 */
 	if (unlikely(skb->sk == sk))
-		return sk_psock_skb_ingress_self(psock, skb);
+		return sk_psock_skb_ingress_self(psock, skb, off, len);
 	msg = sk_psock_create_ingress_msg(sk, skb);
 	if (!msg)
 		return -EAGAIN;
@@ -547,7 +550,7 @@ static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb)
 	 * into user buffers.
 	 */
 	skb_set_owner_r(skb, sk);
-	err = sk_psock_skb_ingress_enqueue(skb, psock, sk, msg);
+	err = sk_psock_skb_ingress_enqueue(skb, off, len, psock, sk, msg);
 	if (err < 0)
 		kfree(msg);
 	return err;
@@ -557,7 +560,8 @@ static int sk_psock_skb_ingress(struct sk_psock *psock, struct sk_buff *skb)
  * skb. In this case we do not need to check memory limits or skb_set_owner_r
  * because the skb is already accounted for here.
  */
-static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb)
+static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb,
+				     u32 off, u32 len)
 {
 	struct sk_msg *msg = kzalloc(sizeof(*msg), __GFP_NOWARN | GFP_ATOMIC);
 	struct sock *sk = psock->sk;
@@ -567,7 +571,7 @@ static int sk_psock_skb_ingress_self(struct sk_psock *psock, struct sk_buff *skb
 		return -EAGAIN;
 	sk_msg_init(msg);
 	skb_set_owner_r(skb, sk);
-	err = sk_psock_skb_ingress_enqueue(skb, psock, sk, msg);
+	err = sk_psock_skb_ingress_enqueue(skb, off, len, psock, sk, msg);
 	if (err < 0)
 		kfree(msg);
 	return err;
@@ -581,7 +585,7 @@ static int sk_psock_handle_skb(struct sk_psock *psock, struct sk_buff *skb,
 			return -EAGAIN;
 		return skb_send_sock(psock->sk, skb, off, len);
 	}
-	return sk_psock_skb_ingress(psock, skb);
+	return sk_psock_skb_ingress(psock, skb, off, len);
 }
 
 static void sk_psock_skb_state(struct sk_psock *psock,
@@ -624,6 +628,12 @@ static void sk_psock_backlog(struct work_struct *work)
 	while ((skb = skb_dequeue(&psock->ingress_skb))) {
 		len = skb->len;
 		off = 0;
+		if (skb_bpf_strparser(skb)) {
+			struct strp_msg *stm = strp_msg(skb);
+
+			off = stm->offset;
+			len = stm->full_len;
+		}
 start:
 		ingress = skb_bpf_ingress(skb);
 		skb_bpf_redirect_clear(skb);
@@ -930,6 +940,7 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
 {
 	struct sock *sk_other;
 	int err = 0;
+	u32 len, off;
 
 	switch (verdict) {
 	case __SK_PASS:
@@ -949,7 +960,15 @@ static int sk_psock_verdict_apply(struct sk_psock *psock, struct sk_buff *skb,
 		 * retrying later from workqueue.
 		 */
 		if (skb_queue_empty(&psock->ingress_skb)) {
-			err = sk_psock_skb_ingress_self(psock, skb);
+			len = skb->len;
+			off = 0;
+			if (skb_bpf_strparser(skb)) {
+				struct strp_msg *stm = strp_msg(skb);
+
+				off = stm->offset;
+				len = stm->full_len;
+			}
+			err = sk_psock_skb_ingress_self(psock, skb, off, len);
 		}
 		if (err < 0) {
 			spin_lock_bh(&psock->ingress_lock);
@@ -1018,6 +1037,7 @@ static void sk_psock_strp_read(struct strparser *strp, struct sk_buff *skb)
 		ret = sk_psock_map_verd(ret, skb_bpf_redirect_fetch(skb));
 		skb->sk = NULL;
 	}
+	skb_bpf_set_strparser(skb);
 	sk_psock_verdict_apply(psock, skb, ret);
 out:
 	rcu_read_unlock();
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
  2021-09-29  2:06 [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress Liu Jian
@ 2021-09-29  5:19 ` Cong Wang
  2021-09-30 22:48 ` John Fastabend
  1 sibling, 0 replies; 6+ messages in thread
From: Cong Wang @ 2021-09-29  5:19 UTC (permalink / raw)
  To: Liu Jian
  Cc: John Fastabend, Daniel Borkmann, Jakub Sitnicki, Lorenz Bauer,
	David Miller, Jakub Kicinski, Alexei Starovoitov,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	KP Singh, Linux Kernel Network Developers, bpf

On Tue, Sep 28, 2021 at 7:06 PM Liu Jian <liujian56@huawei.com> wrote:
>
> If sockmap enable strparser, there are lose offset info in
> sk_psock_skb_ingress. If the length determined by parse_msg function
> is not skb->len, the skb will be converted to sk_msg multiple times,
> and userspace app will get the data multiple times.
>
> Fix this by get the offset and length from strp_msg.
> And as Cong suggestion, add one bit in skb->_sk_redir to distinguish
> enable or disable strparser.
>
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
> v1->v2: fix build error when disable CONFIG_BPF_STREAM_PARSER
> v2->v3: Add one bit in skb->_sk_redir to distinguish enable or disable strparser
> v3->v4: Remove "#if IS_ENABLED(CONFIG_BPF_STREAM_PARSER)" code;
>         and let "stm" have a more precise scope.

Looks much cleaner now.

Reviewed-by: Cong Wang <cong.wang@bytedance.com>

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
  2021-09-29  2:06 [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress Liu Jian
  2021-09-29  5:19 ` Cong Wang
@ 2021-09-30 22:48 ` John Fastabend
  2021-10-04  4:27   ` liujian (CE)
  2021-10-12  2:36   ` liujian (CE)
  1 sibling, 2 replies; 6+ messages in thread
From: John Fastabend @ 2021-09-30 22:48 UTC (permalink / raw)
  To: Liu Jian, john.fastabend, daniel, jakub, lmb, davem, kuba, ast,
	andrii, kafai, songliubraving, yhs, kpsingh, netdev, bpf,
	xiyou.wangcong
  Cc: liujian56

Liu Jian wrote:
> If sockmap enable strparser, there are lose offset info in
> sk_psock_skb_ingress. If the length determined by parse_msg function
> is not skb->len, the skb will be converted to sk_msg multiple times,
> and userspace app will get the data multiple times.
> 
> Fix this by get the offset and length from strp_msg.
> And as Cong suggestion, add one bit in skb->_sk_redir to distinguish
> enable or disable strparser.
> 
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---

Thanks. Please add Fixes tags so we can track these I've added it here.

This has been broken from the initial patches and after a quick glance I
suspect this will need manual backports if we need it. Also all the
I use and all the selftests set parser to a nop by returning skb->len.

Can you also create a test so we can ensure we don't break this
again?

Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
  2021-09-30 22:48 ` John Fastabend
@ 2021-10-04  4:27   ` liujian (CE)
  2021-10-12  2:36   ` liujian (CE)
  1 sibling, 0 replies; 6+ messages in thread
From: liujian (CE) @ 2021-10-04  4:27 UTC (permalink / raw)
  To: John Fastabend, daniel, jakub, lmb, davem, kuba, ast, andrii,
	kafai, songliubraving, yhs, kpsingh, netdev, bpf, xiyou.wangcong



> -----Original Message-----
> From: John Fastabend [mailto:john.fastabend@gmail.com]
> Sent: Friday, October 1, 2021 6:48 AM
> To: liujian (CE) <liujian56@huawei.com>; john.fastabend@gmail.com;
> daniel@iogearbox.net; jakub@cloudflare.com; lmb@cloudflare.com;
> davem@davemloft.net; kuba@kernel.org; ast@kernel.org;
> andrii@kernel.org; kafai@fb.com; songliubraving@fb.com; yhs@fb.com;
> kpsingh@kernel.org; netdev@vger.kernel.org; bpf@vger.kernel.org;
> xiyou.wangcong@gmail.com
> Cc: liujian (CE) <liujian56@huawei.com>
> Subject: RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
>
> Liu Jian wrote:
> > If sockmap enable strparser, there are lose offset info in
> > sk_psock_skb_ingress. If the length determined by parse_msg function
> > is not skb->len, the skb will be converted to sk_msg multiple times,
> > and userspace app will get the data multiple times.
> >
> > Fix this by get the offset and length from strp_msg.
> > And as Cong suggestion, add one bit in skb->_sk_redir to distinguish
> > enable or disable strparser.
> >
> > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > ---
>
> Thanks. Please add Fixes tags so we can track these I've added it here.
>
> This has been broken from the initial patches and after a quick glance I
> suspect this will need manual backports if we need it. Also all the I use and all
> the selftests set parser to a nop by returning skb->len.
>
> Can you also create a test so we can ensure we don't break this again?
Okay, I will do this after the holiday.
>
> Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg interface")
> Acked-by: John Fastabend <john.fastabend@gmail.com>
Thank you for reviewing this patch again.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
  2021-09-30 22:48 ` John Fastabend
  2021-10-04  4:27   ` liujian (CE)
@ 2021-10-12  2:36   ` liujian (CE)
  2021-10-12  5:27     ` John Fastabend
  1 sibling, 1 reply; 6+ messages in thread
From: liujian (CE) @ 2021-10-12  2:36 UTC (permalink / raw)
  To: John Fastabend, daniel, jakub, lmb, davem, kuba, ast, andrii,
	kafai, songliubraving, yhs, kpsingh, netdev, bpf, xiyou.wangcong



> -----Original Message-----
> From: liujian (CE)
> Sent: Monday, October 4, 2021 12:28 PM
> To: 'John Fastabend' <john.fastabend@gmail.com>; daniel@iogearbox.net;
> jakub@cloudflare.com; lmb@cloudflare.com; davem@davemloft.net;
> kuba@kernel.org; ast@kernel.org; andrii@kernel.org; kafai@fb.com;
> songliubraving@fb.com; yhs@fb.com; kpsingh@kernel.org;
> netdev@vger.kernel.org; bpf@vger.kernel.org; xiyou.wangcong@gmail.com
> Subject: RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
> 
> 
> 
> > -----Original Message-----
> > From: John Fastabend [mailto:john.fastabend@gmail.com]
> > Sent: Friday, October 1, 2021 6:48 AM
> > To: liujian (CE) <liujian56@huawei.com>; john.fastabend@gmail.com;
> > daniel@iogearbox.net; jakub@cloudflare.com; lmb@cloudflare.com;
> > davem@davemloft.net; kuba@kernel.org; ast@kernel.org;
> > andrii@kernel.org; kafai@fb.com; songliubraving@fb.com; yhs@fb.com;
> > kpsingh@kernel.org; netdev@vger.kernel.org; bpf@vger.kernel.org;
> > xiyou.wangcong@gmail.com
> > Cc: liujian (CE) <liujian56@huawei.com>
> > Subject: RE: [PATCH v4] skmsg: lose offset info in
> > sk_psock_skb_ingress
> >
> > Liu Jian wrote:
> > > If sockmap enable strparser, there are lose offset info in
> > > sk_psock_skb_ingress. If the length determined by parse_msg function
> > > is not skb->len, the skb will be converted to sk_msg multiple times,
> > > and userspace app will get the data multiple times.
> > >
> > > Fix this by get the offset and length from strp_msg.
> > > And as Cong suggestion, add one bit in skb->_sk_redir to distinguish
> > > enable or disable strparser.
> > >
> > > Signed-off-by: Liu Jian <liujian56@huawei.com>
> > > ---
> >
> > Thanks. Please add Fixes tags so we can track these I've added it here.
> >
> > This has been broken from the initial patches and after a quick glance
> > I suspect this will need manual backports if we need it. Also all the
> > I use and all the selftests set parser to a nop by returning skb->len.
> >
> > Can you also create a test so we can ensure we don't break this again?
> Okay, I will do this after the holiday.


Hi John, 
I checked selftests, there are have one test case named " test_txmsg_ingress_parser".
But with this patch and ktls, the test failed, this because ktls parser(tls_read_size) return value is 285 not 256.
the case like this: 
tls_sk1 --> redir_sk --> tls_sk2
tls_sk1 sent out 512 bytes data, after tls related processing redir_sk recved 570 btyes data,
and redirect 512 (skb_use_parser) bytes data to tls_sk2; but tls_sk2 needs 285 * 2 bytes data, receive timeout occurred.
I fix this as below:
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -1680,6 +1680,8 @@ static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt)
 {
        txmsg_pass = 1;
        skb_use_parser = 512;
+       if (ktls == 1)
+               skb_use_parser = 570;
        opt->iov_length = 256;
        opt->iov_count = 1;
        opt->rate = 2;


And i add one new test as below, is it ok?

--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -139,6 +139,7 @@ struct sockmap_options {
        bool sendpage;
        bool data_test;
        bool drop_expected;
+       bool check_recved_len;
        int iov_count;
        int iov_length;
        int rate;
@@ -556,8 +557,12 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
        int err, i, flags = MSG_NOSIGNAL;
        bool drop = opt->drop_expected;
        bool data = opt->data_test;
+       int iov_alloc_length = iov_length;
 
-       err = msg_alloc_iov(&msg, iov_count, iov_length, data, tx);
+       if (!tx && opt->check_recved_len)
+               iov_alloc_length *= 2;
+
+       err = msg_alloc_iov(&msg, iov_count, iov_alloc_length, data, tx);
        if (err)
                goto out_errno;
        if (peek_flag) {
@@ -665,6 +670,13 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
 
                        s->bytes_recvd += recv;
 
+                       if (opt->check_recved_len && s->bytes_recvd > total_bytes) {
+                               errno = EMSGSIZE;
+                               fprintf(stderr, "recv failed(), bytes_recvd:%zd, total_bytes:%f\n",
+                                               s->bytes_recvd, total_bytes);
+                               goto out_errno;
+                       }
+
                        if (data) {
                                int chunk_sz = opt->sendpage ?
                                                iov_length * cnt :
@@ -744,7 +756,8 @@ static int sendmsg_test(struct sockmap_options *opt)
 
        rxpid = fork();
        if (rxpid == 0) {
-               iov_buf -= (txmsg_pop - txmsg_start_pop + 1);
+               if (txmsg_pop || txmsg_start_pop)
+                       iov_buf -= (txmsg_pop - txmsg_start_pop + 1);
                if (opt->drop_expected || txmsg_ktls_skb_drop)
                        _exit(0);
 
@@ -1688,6 +1701,19 @@ static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt)
        test_exec(cgrp, opt);
 }
 
+static void test_txmsg_ingress_parser2(int cgrp, struct sockmap_options *opt)
+{
+       if (ktls == 1)
+               return;
+       skb_use_parser = 10;
+       opt->iov_length = 20;
+       opt->iov_count = 1;
+       opt->rate = 1;
+       opt->check_recved_len = true;
+       test_exec(cgrp, opt);
+       opt->check_recved_len = false;
+}
+
 char *map_names[] = {
        "sock_map",
        "sock_map_txmsg",
@@ -1786,7 +1812,8 @@ struct _test test[] = {
        {"txmsg test pull-data", test_txmsg_pull},
        {"txmsg test pop-data", test_txmsg_pop},
        {"txmsg test push/pop data", test_txmsg_push_pop},
-       {"txmsg text ingress parser", test_txmsg_ingress_parser},
+       {"txmsg test ingress parser", test_txmsg_ingress_parser},
+       {"txmsg test ingress parser2", test_txmsg_ingress_parser2},
 };

> >
> > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg
> > interface")
> > Acked-by: John Fastabend <john.fastabend@gmail.com>
> Thank you for reviewing this patch again.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
  2021-10-12  2:36   ` liujian (CE)
@ 2021-10-12  5:27     ` John Fastabend
  0 siblings, 0 replies; 6+ messages in thread
From: John Fastabend @ 2021-10-12  5:27 UTC (permalink / raw)
  To: liujian (CE),
	John Fastabend, daniel, jakub, lmb, davem, kuba, ast, andrii,
	kafai, songliubraving, yhs, kpsingh, netdev, bpf, xiyou.wangcong

[...]

> > > Thanks. Please add Fixes tags so we can track these I've added it here.
> > >
> > > This has been broken from the initial patches and after a quick glance
> > > I suspect this will need manual backports if we need it. Also all the
> > > I use and all the selftests set parser to a nop by returning skb->len.
> > >
> > > Can you also create a test so we can ensure we don't break this again?
> > Okay, I will do this after the holiday.
> 
> 
> Hi John, 
> I checked selftests, there are have one test case named " test_txmsg_ingress_parser".
> But with this patch and ktls, the test failed, this because ktls parser(tls_read_size) return value is 285 not 256.
> the case like this: 
> tls_sk1 --> redir_sk --> tls_sk2
> tls_sk1 sent out 512 bytes data, after tls related processing redir_sk recved 570 btyes data,
> and redirect 512 (skb_use_parser) bytes data to tls_sk2; but tls_sk2 needs 285 * 2 bytes data, receive timeout occurred.
> I fix this as below:

Ah good catch.

> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -1680,6 +1680,8 @@ static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt)
>  {
>         txmsg_pass = 1;
>         skb_use_parser = 512;
> +       if (ktls == 1)
> +               skb_use_parser = 570;
>         opt->iov_length = 256;
>         opt->iov_count = 1;
>         opt->rate = 2;
> 
> 
> And i add one new test as below, is it ok?


Yes looks good to me.

> 
> --- a/tools/testing/selftests/bpf/test_sockmap.c
> +++ b/tools/testing/selftests/bpf/test_sockmap.c
> @@ -139,6 +139,7 @@ struct sockmap_options {
>         bool sendpage;
>         bool data_test;
>         bool drop_expected;
> +       bool check_recved_len;
>         int iov_count;
>         int iov_length;
>         int rate;
> @@ -556,8 +557,12 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>         int err, i, flags = MSG_NOSIGNAL;
>         bool drop = opt->drop_expected;
>         bool data = opt->data_test;
> +       int iov_alloc_length = iov_length;
>  
> -       err = msg_alloc_iov(&msg, iov_count, iov_length, data, tx);
> +       if (!tx && opt->check_recved_len)
> +               iov_alloc_length *= 2;
> +
> +       err = msg_alloc_iov(&msg, iov_count, iov_alloc_length, data, tx);
>         if (err)
>                 goto out_errno;
>         if (peek_flag) {
> @@ -665,6 +670,13 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
>  
>                         s->bytes_recvd += recv;
>  
> +                       if (opt->check_recved_len && s->bytes_recvd > total_bytes) {
> +                               errno = EMSGSIZE;
> +                               fprintf(stderr, "recv failed(), bytes_recvd:%zd, total_bytes:%f\n",
> +                                               s->bytes_recvd, total_bytes);
> +                               goto out_errno;
> +                       }
> +
>                         if (data) {
>                                 int chunk_sz = opt->sendpage ?
>                                                 iov_length * cnt :
> @@ -744,7 +756,8 @@ static int sendmsg_test(struct sockmap_options *opt)
>  
>         rxpid = fork();
>         if (rxpid == 0) {
> -               iov_buf -= (txmsg_pop - txmsg_start_pop + 1);
> +               if (txmsg_pop || txmsg_start_pop)
> +                       iov_buf -= (txmsg_pop - txmsg_start_pop + 1);
>                 if (opt->drop_expected || txmsg_ktls_skb_drop)
>                         _exit(0);
>  
> @@ -1688,6 +1701,19 @@ static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt)
>         test_exec(cgrp, opt);
>  }
>  
> +static void test_txmsg_ingress_parser2(int cgrp, struct sockmap_options *opt)
> +{
> +       if (ktls == 1)
> +               return;
> +       skb_use_parser = 10;
> +       opt->iov_length = 20;
> +       opt->iov_count = 1;
> +       opt->rate = 1;
> +       opt->check_recved_len = true;
> +       test_exec(cgrp, opt);
> +       opt->check_recved_len = false;
> +}
> +
>  char *map_names[] = {
>         "sock_map",
>         "sock_map_txmsg",
> @@ -1786,7 +1812,8 @@ struct _test test[] = {
>         {"txmsg test pull-data", test_txmsg_pull},
>         {"txmsg test pop-data", test_txmsg_pop},
>         {"txmsg test push/pop data", test_txmsg_push_pop},
> -       {"txmsg text ingress parser", test_txmsg_ingress_parser},
> +       {"txmsg test ingress parser", test_txmsg_ingress_parser},
> +       {"txmsg test ingress parser2", test_txmsg_ingress_parser2},
>  };
> 

Great, please post as a series.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-10-12  5:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-29  2:06 [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress Liu Jian
2021-09-29  5:19 ` Cong Wang
2021-09-30 22:48 ` John Fastabend
2021-10-04  4:27   ` liujian (CE)
2021-10-12  2:36   ` liujian (CE)
2021-10-12  5:27     ` John Fastabend

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).