From: John Fastabend <john.fastabend@gmail.com>
To: "liujian (CE)" <liujian56@huawei.com>,
John Fastabend <john.fastabend@gmail.com>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"jakub@cloudflare.com" <jakub@cloudflare.com>,
"lmb@cloudflare.com" <lmb@cloudflare.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"kuba@kernel.org" <kuba@kernel.org>,
"ast@kernel.org" <ast@kernel.org>,
"andrii@kernel.org" <andrii@kernel.org>,
"kafai@fb.com" <kafai@fb.com>,
"songliubraving@fb.com" <songliubraving@fb.com>,
"yhs@fb.com" <yhs@fb.com>,
"kpsingh@kernel.org" <kpsingh@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
"xiyou.wangcong@gmail.com" <xiyou.wangcong@gmail.com>
Subject: RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
Date: Mon, 11 Oct 2021 22:27:01 -0700 [thread overview]
Message-ID: <61651ca5cbf46_2215c2088b@john-XPS-13-9370.notmuch> (raw)
In-Reply-To: <e60cf9bd85924ead98bb5bfd5b7e4919@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:
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.
prev parent reply other threads:[~2021-10-12 5:27 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
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=61651ca5cbf46_2215c2088b@john-XPS-13-9370.notmuch \
--to=john.fastabend@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=jakub@cloudflare.com \
--cc=kafai@fb.com \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=liujian56@huawei.com \
--cc=lmb@cloudflare.com \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=xiyou.wangcong@gmail.com \
--cc=yhs@fb.com \
/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).