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@xxxxxxxxx> 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@xxxxxxxxxx/ > > > > > > > > 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@xxxxxxxxx> > > > > > --- > > > > > 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?
