From: Yonghong Song <yhs@fb.com>
To: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Cc: Lorenzo Bianconi <lorenzo@kernel.org>, <bpf@vger.kernel.org>,
<ast@kernel.org>, <daniel@iogearbox.net>, <brouer@redhat.com>,
<toke@redhat.com>, <andrii@kernel.org>, <netdev@vger.kernel.org>
Subject: Re: [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags
Date: Mon, 7 Feb 2022 09:08:33 -0800 [thread overview]
Message-ID: <c03f0c81-8e56-6c92-d31a-03a5394b9388@fb.com> (raw)
In-Reply-To: <Yf15n2GJG70JrxX6@lore-desk>
On 2/4/22 11:08 AM, Lorenzo Bianconi wrote:
>>>
>>
>> [...]
>>
>>>>>
>>>>> In kernel, the nr_frags checking is against MAX_SKB_FRAGS,
>>>>> but if /proc/sys/net/core/max_skb_flags is 2 or more less
>>>>> than MAX_SKB_FRAGS, the test won't fail, right?
>>>>
>>>> yes, you are right. Should we use the same definition used in
>>>> include/linux/skbuff.h instead? Something like:
>>>>
>>>> if (65536 / page_size + 1 < 16)
>>>> max_skb_flags = 16;
>>>> else
>>>> max_skb_flags = 65536/page_size + 1;
>>>
>>> The maximum packet size limit 64KB won't change anytime soon.
>>> So the above should work. Some comments to explain why using
>>> the above formula will be good.
>>
>> ack, I will do in v2.
>
> I can see there is a on-going discussion here [0] about increasing
> MAX_SKB_FRAGS. I guess we can put on-hold this patch and see how
> MAX_SKB_FRAGS will be changed.
Thanks for the link. The new patch is going to increase
MAX_SKB_FRAGS and it is possible that will be changed again
(maybe under some config options).
The default value for
/proc/sys/net/core/max_skb_flags is MAX_SKB_FRAGS and I suspect
anybody is bothering to change it. So your patch is okay to me.
Maybe change a little bit -ENOMEM error message. current,
ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
to
ASSERT_EQ(err, -ENOMEM, "unsupported buffer size, possible
non-default /proc/sys/net/core/max_skb_flags?");
>
> Regards,
> Lorenzo
>
> [0] https://lore.kernel.org/all/202202031315.B425Ipe8-lkp@intel.com/t/#ma1b2c7e71fe9bc69e24642a62dadf32fda7d5f03
>
>>
>> Regards,
>> Lorenzo
>>
>>>
>>>>
>>>> Regards,
>>>> Lorenzo
>>>>
>>>>>
>>>>>> +
>>>>>> + num = fscanf(f, "%d", &max_skb_frags);
>>>>>> + fclose(f);
>>>>>> +
>>>>>> + if (!ASSERT_EQ(num, 1, "max_skb_frags read failed"))
>>>>>> + goto out;
>>>>>> +
>>>>>> + /* xdp_buff linear area size is always set to 4096 in the
>>>>>> + * bpf_prog_test_run_xdp routine.
>>>>>> + */
>>>>>> + buf_size = 4096 + (max_skb_frags + 1) * sysconf(_SC_PAGE_SIZE);
>>>>>> + buf = malloc(buf_size);
>>>>>> + if (!ASSERT_OK_PTR(buf, "alloc buf"))
>>>>>> + goto out;
>>>>>> +
>>>>>> + memset(buf, 0, buf_size);
>>>>>> + offset = (__u32 *)buf;
>>>>>> + *offset = 16;
>>>>>> + buf[*offset] = 0xaa;
>>>>>> + buf[*offset + 15] = 0xaa;
>>>>>> +
>>>>>> + topts.data_in = buf;
>>>>>> + topts.data_out = buf;
>>>>>> + topts.data_size_in = buf_size;
>>>>>> + topts.data_size_out = buf_size;
>>>>>> +
>>>>>> + err = bpf_prog_test_run_opts(prog_fd, &topts);
>>>>>> + ASSERT_EQ(err, -ENOMEM, "unsupported buffer size");
>>>>>> + free(buf);
>>>>>> out:
>>>>>> bpf_object__close(obj);
>>>>>> }
>>>>>
>>>
>
>
next prev parent reply other threads:[~2022-02-07 17:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-04 13:58 [PATCH bpf-next] selftest/bpf: check invalid length in test_xdp_update_frags Lorenzo Bianconi
2022-02-04 17:37 ` Yonghong Song
2022-02-04 17:52 ` Lorenzo Bianconi
2022-02-04 18:14 ` Yonghong Song
2022-02-04 18:30 ` Lorenzo Bianconi
2022-02-04 19:08 ` Lorenzo Bianconi
2022-02-07 17:08 ` Yonghong Song [this message]
2022-02-07 17:53 ` Lorenzo Bianconi
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=c03f0c81-8e56-6c92-d31a-03a5394b9388@fb.com \
--to=yhs@fb.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brouer@redhat.com \
--cc=daniel@iogearbox.net \
--cc=lorenzo.bianconi@redhat.com \
--cc=lorenzo@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=toke@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.