| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 3 Oct 2023 08:44:08 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Stanislav Fomichev <sdf@...gle.com>
CC: <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>,
<andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>, <yhs@...com>,
<john.fastabend@...il.com>, <kpsingh@...nel.org>, <haoluo@...gle.com>,
<jolsa@...nel.org>, David Ahern <dsahern@...il.com>, Jakub Kicinski
<kuba@...nel.org>, Willem de Bruijn <willemb@...gle.com>, "Jesper Dangaard
Brouer" <brouer@...hat.com>, Anatoly Burakov <anatoly.burakov@...el.com>,
Alexander Lobakin <alexandr.lobakin@...el.com>, Magnus Karlsson
<magnus.karlsson@...il.com>, Maryam Tahhan <mtahhan@...hat.com>,
<xdp-hints@...-project.net>, <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: add options and ZC mode to
xdp_hw_metadata
On Mon, Oct 02, 2023 at 09:46:08AM -0700, Stanislav Fomichev wrote:
> On Mon, Oct 2, 2023 at 9:35 AM Larysa Zaremba <larysa.zaremba@...el.com> wrote:
> >
> > By default, xdp_hw_metadata runs in AF_XDP copy mode. However, hints are
> > also supposed to be supported in ZC mode, which is usually implemented
> > separately in driver, and so needs to be tested too.
> >
> > Add an option to run xdp_hw_metadata in ZC mode.
> >
> > As for now, xdp_hw_metadata accepts no options, so add simple option
> > parsing logic and a help message.
> >
> > For quick reference, also add an ingress packet generation command to the
> > help message. The command comes from [0].
> >
> > [0] https://lore.kernel.org/all/20230119221536.3349901-18-sdf@google.com/
>
> I did similar changes in my pending [0], but I made the zerocopy, not
> the copy mode, the default.
> If you want to get this in faster (my series will probably need
> another iteration), let's maybe do the same here?
> ZC as a default feels better.
>
> 0: https://lore.kernel.org/bpf/20230914210452.2588884-9-sdf@google.com/
I do not need those changes in tree ASAP, that is just something I had locally
for some time and decided to send. So I think I can wait for your series. This
way it is less work for both of us.
>
>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
> > ---
> > tools/testing/selftests/bpf/xdp_hw_metadata.c | 59 ++++++++++++++++---
> > 1 file changed, 52 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdp_hw_metadata.c b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> > index 613321eb84c1..c1d1b161a964 100644
> > --- a/tools/testing/selftests/bpf/xdp_hw_metadata.c
> > +++ b/tools/testing/selftests/bpf/xdp_hw_metadata.c
> > @@ -26,6 +26,7 @@
> > #include <linux/sockios.h>
> > #include <sys/mman.h>
> > #include <net/if.h>
> > +#include <ctype.h>
> > #include <poll.h>
> > #include <time.h>
> >
> > @@ -49,6 +50,7 @@ struct xsk {
> > struct xdp_hw_metadata *bpf_obj;
> > struct xsk *rx_xsk;
> > const char *ifname;
> > +bool zero_copy;
> > int ifindex;
> > int rxq;
> >
> > @@ -60,7 +62,7 @@ static int open_xsk(int ifindex, struct xsk *xsk, __u32 queue_id)
> > const struct xsk_socket_config socket_config = {
> > .rx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > .tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > - .bind_flags = XDP_COPY,
> > + .bind_flags = zero_copy ? XDP_ZEROCOPY : XDP_COPY,
> > };
> > const struct xsk_umem_config umem_config = {
> > .fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > @@ -404,6 +406,54 @@ static void timestamping_enable(int fd, int val)
> > error(1, errno, "setsockopt(SO_TIMESTAMPING)");
> > }
> >
> > +static void print_usage(void)
> > +{
> > + const char *usage =
> > + " Usage: xdp_hw_metadata [OPTIONS] [IFNAME]\n"
>
> Maybe [OPTIONS] <IFNAME> to mark ifname as required?
>
> > + " Options:\n"
> > + " -z Run AF_XDP in ZC mode (copy mode is used by default)\n"
> > + " -h Display this help and exit\n\n"
> > + " Generate test packets on other machine with:\n"
> > + " echo -n xdp | nc -u -q1 <dst_ip> 9091\n";
> > +
> > + printf("%s", usage);
> > +}
> > +
> > +static void read_args(int argc, char *argv[])
> > +{
> > + char opt;
> > +
> > + while ((opt = getopt(argc, argv, "zh")) != -1) {
> > + switch (opt) {
> > + case 'z':
> > + zero_copy = true;
> > + break;
> > + case 'h':
> > + print_usage();
> > + exit(0);
> > + case '?':
> > + if (isprint(optopt))
> > + fprintf(stderr, "Unknown option: -%c\n", optopt);
> > + fallthrough;
> > + default:
> > + print_usage();
> > + error(-1, opterr, "Command line options error");
> > + }
> > + }
> > +
> > + if (optind >= argc) {
> > + fprintf(stderr, "No device name provided\n");
> > + print_usage();
> > + exit(-1);
> > + }
> > +
> > + ifname = argv[optind];
> > + ifindex = if_nametoindex(ifname);
> > +
> > + if (!ifname)
> > + error(-1, errno, "Invalid interface name");
> > +}
> > +
> > int main(int argc, char *argv[])
> > {
> > clockid_t clock_id = CLOCK_TAI;
> > @@ -413,13 +463,8 @@ int main(int argc, char *argv[])
> >
> > struct bpf_program *prog;
> >
> > - if (argc != 2) {
> > - fprintf(stderr, "pass device name\n");
> > - return -1;
> > - }
> > + read_args(argc, argv);
> >
> > - ifname = argv[1];
> > - ifindex = if_nametoindex(ifname);
> > rxq = rxq_num(ifname);
> >
> > printf("rxq: %d\n", rxq);
> > --
> > 2.41.0
> >
Powered by blists - more mailing lists