* [xdp-hints] [PATCH bpf-next] selftests/bpf: add options and ZC mode to xdp_hw_metadata
@ 2023-10-02 16:26 Larysa Zaremba
2023-10-02 16:46 ` [xdp-hints] " Stanislav Fomichev
0 siblings, 1 reply; 4+ messages in thread
From: Larysa Zaremba @ 2023-10-02 16:26 UTC (permalink / raw)
To: bpf
Cc: Larysa Zaremba, ast, daniel, andrii, martin.lau, song, yhs,
john.fastabend, kpsingh, sdf, haoluo, jolsa, David Ahern,
Jakub Kicinski, Willem de Bruijn, Jesper Dangaard Brouer,
Anatoly Burakov, Alexander Lobakin, Magnus Karlsson,
Maryam Tahhan, xdp-hints, netdev
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/
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.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"
+ " 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
^ permalink raw reply [flat|nested] 4+ messages in thread* [xdp-hints] Re: [PATCH bpf-next] selftests/bpf: add options and ZC mode to xdp_hw_metadata
2023-10-02 16:26 [xdp-hints] [PATCH bpf-next] selftests/bpf: add options and ZC mode to xdp_hw_metadata Larysa Zaremba
@ 2023-10-02 16:46 ` Stanislav Fomichev
2023-10-03 6:44 ` Larysa Zaremba
0 siblings, 1 reply; 4+ messages in thread
From: Stanislav Fomichev @ 2023-10-02 16:46 UTC (permalink / raw)
To: Larysa Zaremba
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, David Ahern, Jakub Kicinski,
Willem de Bruijn, Jesper Dangaard Brouer, Anatoly Burakov,
Alexander Lobakin, Magnus Karlsson, Maryam Tahhan, xdp-hints,
netdev
On Mon, Oct 2, 2023 at 9:35 AM Larysa Zaremba <larysa.zaremba@intel.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/
> Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [xdp-hints] Re: [PATCH bpf-next] selftests/bpf: add options and ZC mode to xdp_hw_metadata
2023-10-02 16:46 ` [xdp-hints] " Stanislav Fomichev
@ 2023-10-03 6:44 ` Larysa Zaremba
2023-10-03 16:09 ` Stanislav Fomichev
0 siblings, 1 reply; 4+ messages in thread
From: Larysa Zaremba @ 2023-10-03 6:44 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, David Ahern, Jakub Kicinski,
Willem de Bruijn, Jesper Dangaard Brouer, Anatoly Burakov,
Alexander Lobakin, Magnus Karlsson, Maryam Tahhan, xdp-hints,
netdev
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@intel.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@intel.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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread* [xdp-hints] Re: [PATCH bpf-next] selftests/bpf: add options and ZC mode to xdp_hw_metadata
2023-10-03 6:44 ` Larysa Zaremba
@ 2023-10-03 16:09 ` Stanislav Fomichev
0 siblings, 0 replies; 4+ messages in thread
From: Stanislav Fomichev @ 2023-10-03 16:09 UTC (permalink / raw)
To: Larysa Zaremba
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, David Ahern, Jakub Kicinski,
Willem de Bruijn, Jesper Dangaard Brouer, Anatoly Burakov,
Alexander Lobakin, Magnus Karlsson, Maryam Tahhan, xdp-hints,
netdev
On 10/03, Larysa Zaremba wrote:
> 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@intel.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.
SGTM, thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-03 16:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-02 16:26 [xdp-hints] [PATCH bpf-next] selftests/bpf: add options and ZC mode to xdp_hw_metadata Larysa Zaremba
2023-10-02 16:46 ` [xdp-hints] " Stanislav Fomichev
2023-10-03 6:44 ` Larysa Zaremba
2023-10-03 16:09 ` Stanislav Fomichev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox