XDP hardware hints discussion mail archive
 help / color / mirror / Atom feed
From: Larysa Zaremba <larysa.zaremba@intel.com>
To: Stanislav Fomichev <sdf@google.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
	haoluo@google.com, jolsa@kernel.org,
	David Ahern <dsahern@gmail.com>, Jakub Kicinski <kuba@kernel.org>,
	Willem de Bruijn <willemb@google.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Anatoly Burakov <anatoly.burakov@intel.com>,
	Alexander Lobakin <alexandr.lobakin@intel.com>,
	Magnus Karlsson <magnus.karlsson@gmail.com>,
	Maryam Tahhan <mtahhan@redhat.com>,
	xdp-hints@xdp-project.net, netdev@vger.kernel.org
Subject: [xdp-hints] Re: [PATCH bpf-next v2 18/20] selftests/bpf: Use AF_INET for TX in xdp_metadata
Date: Fri, 7 Jul 2023 18:58:00 +0200	[thread overview]
Message-ID: <ZKhEGK8J8MEp1iIS@lincoln> (raw)
In-Reply-To: <ZKhCL/YMo8dv4lqd@google.com>

On Fri, Jul 07, 2023 at 09:49:51AM -0700, Stanislav Fomichev wrote:
> On 07/07, Larysa Zaremba wrote:
> > On Thu, Jul 06, 2023 at 10:27:38AM -0700, Stanislav Fomichev wrote:
> > > On Thu, Jul 6, 2023 at 7:15 AM Larysa Zaremba <larysa.zaremba@intel.com> wrote:
> > > >
> > > > On Wed, Jul 05, 2023 at 10:39:35AM -0700, Stanislav Fomichev wrote:
> > > > > On 07/03, Larysa Zaremba wrote:
> > > > > > The easiest way to simulate stripped VLAN tag in veth is to send a packet
> > > > > > from VLAN interface, attached to veth. Unfortunately, this approach is
> > > > > > incompatible with AF_XDP on TX side, because VLAN interfaces do not have
> > > > > > such feature.
> > > > > >
> > > > > > Replace AF_XDP packet generation with sending the same datagram via
> > > > > > AF_INET socket.
> > > > > >
> > > > > > This does not change the packet contents or hints values with one notable
> > > > > > exception: rx_hash_type, which previously was expected to be 0, now is
> > > > > > expected be at least XDP_RSS_TYPE_L4.
> > > > > >
> > > > > > Also, usage of AF_INET requires a little more complicated namespace setup,
> > > > > > therefore open_netns() helper function is divided into smaller reusable
> > > > > > pieces.
> > > > >
> > > > > Ack, it's probably OK for now, but, FYI, I'm trying to extend this part
> > > > > with TX metadata:
> > > > > https://lore.kernel.org/bpf/20230621170244.1283336-10-sdf@google.com/
> > > > >
> > > > > So probably long-term I'll switch it back to AF_XDP but will add
> > > > > support for requesting vlan TX "offload" from the veth.
> > > > >
> > > >
> > > > My bad for not reading your series. Amazing work as always!
> > > >
> > > > So, 'requesting vlan TX "offload"' with new hints capabilities? This would be
> > > > pretty neat.
> > > >
> > > > But you think AF_INET TX is worth keeping for now, until TX hints are mature?
> > > >
> > > > > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
> > > > > > ---
> > > > > >  tools/testing/selftests/bpf/network_helpers.c |  37 +++-
> > > > > >  tools/testing/selftests/bpf/network_helpers.h |   3 +
> > > > > >  .../selftests/bpf/prog_tests/xdp_metadata.c   | 175 +++++++-----------
> > > > > >  3 files changed, 98 insertions(+), 117 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> > > > > > index a105c0cd008a..19463230ece5 100644
> > > > > > --- a/tools/testing/selftests/bpf/network_helpers.c
> > > > > > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > > > > > @@ -386,28 +386,51 @@ char *ping_command(int family)
> > > > > >     return "ping";
> > > > > >  }
> > > > > >
> > > > > > +int get_cur_netns(void)
> > > > > > +{
> > > > > > +   int nsfd;
> > > > > > +
> > > > > > +   nsfd = open("/proc/self/ns/net", O_RDONLY);
> > > > > > +   ASSERT_GE(nsfd, 0, "open /proc/self/ns/net");
> > > > > > +   return nsfd;
> > > > > > +}
> > > > > > +
> > > > > > +int get_netns(const char *name)
> > > > > > +{
> > > > > > +   char nspath[PATH_MAX];
> > > > > > +   int nsfd;
> > > > > > +
> > > > > > +   snprintf(nspath, sizeof(nspath), "%s/%s", "/var/run/netns", name);
> > > > > > +   nsfd = open(nspath, O_RDONLY | O_CLOEXEC);
> > > > > > +   ASSERT_GE(nsfd, 0, "open /proc/self/ns/net");
> > > > > > +   return nsfd;
> > > > > > +}
> > > > > > +
> > > > > > +int set_netns(int netns_fd)
> > > > > > +{
> > > > > > +   return setns(netns_fd, CLONE_NEWNET);
> > > > > > +}
> > > > >
> > > > > We have open_netns/close_netns in network_helpers.h that provide similar
> > > > > functionality, let's use them instead?
> > > > >
> > > >
> > > > I have divided open_netns() into smaller pieces (see below), because the code I
> > > > have added into xdp_metadata looked better with those smaller pieces (I had to
> > > > switch namespace several times).
> > > 
> > > Forgot to reply to this part. I missed the fact that you're extending
> > > network_helpers, sorry.
> > > But why do we need extra namespaces at all?
> > 
> > If veths are in the same namespace, AF_INET packets are not sent between them,
> > so XDP is skipped. So we need 2 test namespaces: for RX and TX.
> 
> Makes sense. But let's maybe use the existing helpers to jump to/from
> namespaces?
> 
> It might be a bit more verbose, but it makes it easy to annotate namespace
> being/end. (compared to random jumping around with setns)
> 
> tok = open_netns("tx");
> do_something();
> close_netns(tok);
> 
> tok = open_netns("rx");
> do_something_else();
> close_netns(tok);
> 
> Should be doable?

I guess you are right, will rewrite this part to use open_netns()/close_netns(), 
especially considering I have messed up namespace FD management according to CI.

  reply	other threads:[~2023-07-07 17:02 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-03 18:12 [xdp-hints] [PATCH bpf-next v2 00/20] XDP metadata via kfuncs for ice Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 01/20] ice: make RX hash reading code more reusable Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 02/20] ice: make RX HW timestamp " Larysa Zaremba
2023-07-04 10:04   ` [xdp-hints] " Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 03/20] ice: make RX checksum checking " Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 04/20] ice: Make ptype internal to descriptor info processing Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 05/20] ice: Introduce ice_xdp_buff Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 06/20] ice: Support HW timestamp hint Larysa Zaremba
2023-07-05 17:30   ` [xdp-hints] " Stanislav Fomichev
2023-07-06 14:22     ` Larysa Zaremba
2023-07-06 16:39       ` Stanislav Fomichev
2023-07-10 15:49         ` Larysa Zaremba
2023-07-10 18:12           ` Stanislav Fomichev
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 07/20] ice: Support RX hash XDP hint Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 08/20] ice: Support XDP hints in AF_XDP ZC mode Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 09/20] xdp: Add VLAN tag hint Larysa Zaremba
2023-07-03 20:15   ` [xdp-hints] " John Fastabend
2023-07-04  8:23     ` Larysa Zaremba
2023-07-04 10:23       ` Jesper Dangaard Brouer
2023-07-04 11:02         ` Larysa Zaremba
2023-07-04 14:18           ` Jesper Dangaard Brouer
2023-07-06 14:46             ` Larysa Zaremba
2023-07-07 13:57               ` Jesper Dangaard Brouer
2023-07-07 17:58                 ` Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 10/20] ice: Implement " Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 11/20] ice: use VLAN proto from ring packet context in skb path Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 12/20] xdp: Add checksum level hint Larysa Zaremba
2023-07-03 20:38   ` [xdp-hints] " John Fastabend
2023-07-04  9:24     ` Larysa Zaremba
2023-07-04 10:39       ` Jesper Dangaard Brouer
2023-07-04 11:19         ` Larysa Zaremba
2023-07-06  5:50           ` John Fastabend
2023-07-06  9:04             ` Jesper Dangaard Brouer
2023-07-06 12:38               ` Larysa Zaremba
2023-07-06 12:49                 ` Larysa Zaremba
2023-07-10 16:58                   ` Alexander Lobakin
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 13/20] ice: Implement " Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 14/20] selftests/bpf: Allow VLAN packets in xdp_hw_metadata Larysa Zaremba
2023-07-05 17:31   ` [xdp-hints] " Stanislav Fomichev
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 15/20] net, xdp: allow metadata > 32 Larysa Zaremba
2023-07-03 21:06   ` [xdp-hints] " John Fastabend
2023-07-06 14:51     ` Larysa Zaremba
2023-07-10 14:01       ` Alexander Lobakin
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 16/20] selftests/bpf: Add flags and new hints to xdp_hw_metadata Larysa Zaremba
2023-07-04 11:03   ` [xdp-hints] " Jesper Dangaard Brouer
2023-07-04 11:04     ` Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 17/20] veth: Implement VLAN tag and checksum level XDP hint Larysa Zaremba
2023-07-05 17:25   ` [xdp-hints] " Stanislav Fomichev
2023-07-06  9:57     ` Jesper Dangaard Brouer
2023-07-06 10:15       ` Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 18/20] selftests/bpf: Use AF_INET for TX in xdp_metadata Larysa Zaremba
2023-07-05 17:39   ` [xdp-hints] " Stanislav Fomichev
2023-07-06 14:11     ` Larysa Zaremba
2023-07-06 17:25       ` Stanislav Fomichev
2023-07-06 17:27       ` Stanislav Fomichev
2023-07-07  8:33         ` Larysa Zaremba
2023-07-07 16:49           ` Stanislav Fomichev
2023-07-07 16:58             ` Larysa Zaremba [this message]
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 19/20] selftests/bpf: Check VLAN tag and proto " Larysa Zaremba
2023-07-05 17:41   ` [xdp-hints] " Stanislav Fomichev
2023-07-06 10:10   ` Jesper Dangaard Brouer
2023-07-06 10:13     ` Larysa Zaremba
2023-07-03 18:12 ` [xdp-hints] [PATCH bpf-next v2 20/20] selftests/bpf: check checksum level " Larysa Zaremba
2023-07-05 17:41   ` [xdp-hints] " Stanislav Fomichev
2023-07-06 10:25   ` Jesper Dangaard Brouer
2023-07-06 12:02     ` Larysa Zaremba

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

  List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZKhEGK8J8MEp1iIS@lincoln \
    --to=larysa.zaremba@intel.com \
    --cc=alexandr.lobakin@intel.com \
    --cc=anatoly.burakov@intel.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brouer@redhat.com \
    --cc=daniel@iogearbox.net \
    --cc=dsahern@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=magnus.karlsson@gmail.com \
    --cc=martin.lau@linux.dev \
    --cc=mtahhan@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=willemb@google.com \
    --cc=xdp-hints@xdp-project.net \
    --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