> > > 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?"); ack, I am fine with it. @Alexei, Andrii: any hints about it? Regards, Lorenzo > > > > > Regards, > > Lorenzo > > > > [0] https://lore.kernel.org/all/202202031315.B425Ipe8-lkp@xxxxxxxxx/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); > > > > > > > } > > > > > > > > > > > > > >
Attachment:
signature.asc
Description: PGP signature
