* [PATCH bpf-next v2 0/4] bpf: bidirectional VLAN support for bpf_fib_lookup()
@ 2026-06-16 22:34 Avinash Duduskar
2026-06-16 22:34 ` [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow Avinash Duduskar
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Avinash Duduskar @ 2026-06-16 22:34 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern,
Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang,
KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao,
Toke Høiland-Jørgensen, bpf, netdev, linux-kselftest,
linux-kernel
v1 added a single flag, BPF_FIB_LOOKUP_VLAN, to resolve a VLAN egress to
its underlying real device plus the VLAN tag. v2 fixes a QinQ bug the bpf
ci bot found, adds the input direction Toke asked for, adds selftests,
and prepends a fix for a pre-existing l3mdev/VRF lookup bug in the helper.
Patch 1 is an independent fix: bpf_fib_lookup() never initialized the
flow's flowi_l3mdev field, so on the fib-rules path it is read before it
is written. The VRF master is then not resolved and the l3mdev rule fails
to match, so a slave ingress can fail to select its VRF table, today,
with no part of this series. The helper already initializes every other
rules-path flow field (mark, tun_key, uid); l3mdev was added to that set
later and this one was missed. CONFIG_INIT_STACK_ALL_ZERO (the default)
masks it, which is why the VRF selftests in patch 4 pass with or without
it; built with CONFIG_INIT_STACK_ALL_PATTERN a plain bpf_fib_lookup over a
VRF slave returns NOT_FWDED without the patch and resolves with it. It is
first so the VRF behaviour the later patches document and test is well
defined. If you would rather take it through bpf or net on its own, I am
happy to send it separately. It will not apply cleanly before v6.18,
where the flowi4_dscp context line reads flowi4_tos, so a stable backport
needs a trivial context fixup.
Changes v1 -> v2:
- Fix QinQ handling (found by the bpf ci bot): resolve the immediate
parent with vlan_dev_priv(dev)->real_dev instead of
vlan_dev_real_dev() (which walks to the bottom of a stack), and only
swap when that parent is a real device; stacked VLANs are left
unchanged. The egress block is guarded with CONFIG_VLAN_8021Q.
- Add BPF_FIB_LOOKUP_VLAN_INPUT for the input direction (requested by
Toke): supply the packet tag, run the lookup on the matching VLAN
subinterface. Exclusive with BPF_FIB_LOOKUP_TBID (shared union) and
BPF_FIB_LOOKUP_OUTPUT (ingress-only); both return -EINVAL. Taking the
tag as lookup input follows the approach David Ahern suggested in the
2021 fwmark discussion:
https://lore.kernel.org/bpf/6248c547-ad64-04d6-fcec-374893cc1ef2@gmail.com/
- Both directions are network-namespace aware: a VLAN device can be
moved to another netns while registered on its parent, so the egress
swap is skipped (foreign parent ifindex is meaningless) and the input
resolution fails closed for a device in another netns.
- Add 36 selftest cases plus a cross-netns subtest in
prog_tests/fib_lookup.c, covering both directions, the neighbour path,
OUTPUT and DIRECT|TBID, VRF (rule and DIRECT), resolution semantics
(802.1ad, PCP/DEI, QinQ-inner, bond master and port), the frag-needed
mtu_result, the error returns on both families, and the netns boundary
in both directions.
- Document both flags and the now-bidirectional h_vlan_proto/h_vlan_TCI
fields.
Open questions (defaults chosen, noted here in case a maintainer prefers
otherwise):
1. An unmatched, down, or foreign-netns tag returns
BPF_FIB_LKUP_RET_NOT_FWDED, matching the DIRECT path when
fib_get_table() finds no table, rather than a new return code.
2. BPF_FIB_LOOKUP_OUTPUT | BPF_FIB_LOOKUP_VLAN_INPUT is rejected with
-EINVAL; restricting now keeps relaxing later backward-compatible.
3. The name BPF_FIB_LOOKUP_VLAN_INPUT reads oddly next to
BPF_FIB_LOOKUP_OUTPUT. A pair like _VLAN_EGRESS/_VLAN_INGRESS is an
option while nothing is merged.
4. With BPF_FIB_LOOKUP_VLAN, the tc-path mtu check that runs when
tot_len is not set follows params->ifindex, so after a swap it
checks against the parent device rather than the VLAN device (the
route-mtu path via tot_len is unaffected). Checking against the
VLAN device would preserve the pre-flag semantics if that is
preferred.
On the bot's comment-style note: the new comments keep the form that
prevails in net/core/filter.c, and checkpatch --strict is clean.
v1: https://lore.kernel.org/all/20260609172052.81613-1-avinash.duduskar@gmail.com/
Avinash Duduskar (4):
bpf: Initialize the l3mdev field for the fib lookup flow
bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper
bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT flag to bpf_fib_lookup() helper
selftests/bpf: Add bpf_fib_lookup() VLAN flag tests
include/uapi/linux/bpf.h | 63 ++-
net/core/filter.c | 119 ++++-
tools/include/uapi/linux/bpf.h | 63 ++-
.../selftests/bpf/prog_tests/fib_lookup.c | 494 +++++++++++++++++-
4 files changed, 726 insertions(+), 13 deletions(-)
base-commit: 140fa23df957b51385aa847986d44ad7f59b0563
--
2.54.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow 2026-06-16 22:34 [PATCH bpf-next v2 0/4] bpf: bidirectional VLAN support for bpf_fib_lookup() Avinash Duduskar @ 2026-06-16 22:34 ` Avinash Duduskar 2026-06-17 9:06 ` Toke Høiland-Jørgensen 2026-06-16 22:34 ` [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper Avinash Duduskar ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Avinash Duduskar @ 2026-06-16 22:34 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, Toke Høiland-Jørgensen, bpf, netdev, linux-kselftest, linux-kernel bpf_ipv4_fib_lookup() and bpf_ipv6_fib_lookup() build the flow key on the stack with a bare "struct flowi4 fl4;" / "struct flowi6 fl6;" and fill it field by field, but never set flowi4_l3mdev / flowi6_l3mdev. On the non-DIRECT path the lookup goes through the fib rules whenever the netns has custom rules, which a VRF installs: bpf_ipv4_fib_lookup() -> fib_lookup() -> __fib_lookup() -> l3mdev_update_flow() reads !fl->flowi_l3mdev -> fib_rules_lookup() -> fib_rule_match() -> l3mdev_fib_rule_match() uses fl->flowi_l3mdev l3mdev_update_flow() resolves the l3mdev master from the ingress device only while the field is still zero: if (fl->flowi_iif > LOOPBACK_IFINDEX && !fl->flowi_l3mdev) { dev = dev_get_by_index_rcu(net, fl->flowi_iif); if (dev) fl->flowi_l3mdev = l3mdev_master_ifindex_rcu(dev); } Left at a nonzero stack value the resolution is skipped, and l3mdev_fib_rule_match() then tests that value as an ifindex, so the VRF master is not resolved and the rule fails to match: an ingress enslaved to a VRF can fail to select its table. The same value is also read just before that, by FIB rules matching on an L3 master device (l3mdev_fib_rule_iif_match()/_oif_match()), so an "ip rule iif/oif <vrf>" mismatches the same way. The helper already initializes the other flow fields the rules path consumes (flowi4_mark, flowi4_tun_key.tun_id, flowi4_uid and the v6 counterparts); flowi*_l3mdev was added to that set afterwards and this helper was never updated to match. ip_route_input_slow() likewise zeroes the field before its input lookup. Do the same here. CONFIG_INIT_STACK_ALL_ZERO masks this by default, but it depends on compiler support (CC_HAS_AUTO_VAR_INIT_ZERO), so INIT_STACK_NONE builds, including older toolchains that fall back to it, are exposed. Built with INIT_STACK_ALL_PATTERN, a plain bpf_fib_lookup (no VLAN, no DIRECT) over a VRF slave whose destination is routed only in the VRF table returns BPF_FIB_LKUP_RET_NOT_FWDED, and resolves with this patch; reverting these two lines flips it back. The series' VRF selftests pass on the default config either way, so they do not exercise this fix. Fixes: 40867d74c374 ("net: Add l3mdev index to flow struct and avoid oif reset for port devices") Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> --- net/core/filter.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/core/filter.c b/net/core/filter.c index 9590877b0714..6fa172cb1348 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -6162,6 +6162,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params, fl4.flowi4_dscp = inet_dsfield_to_dscp(params->tos); fl4.flowi4_scope = RT_SCOPE_UNIVERSE; fl4.flowi4_flags = 0; + fl4.flowi4_l3mdev = 0; fl4.flowi4_proto = params->l4_protocol; fl4.daddr = params->ipv4_dst; @@ -6307,6 +6308,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, fl6.flowlabel = params->flowinfo; fl6.flowi6_scope = 0; fl6.flowi6_flags = 0; + fl6.flowi6_l3mdev = 0; fl6.mp_hash = 0; fl6.flowi6_proto = params->l4_protocol; base-commit: 140fa23df957b51385aa847986d44ad7f59b0563 -- 2.54.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow 2026-06-16 22:34 ` [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow Avinash Duduskar @ 2026-06-17 9:06 ` Toke Høiland-Jørgensen 0 siblings, 0 replies; 10+ messages in thread From: Toke Høiland-Jørgensen @ 2026-06-17 9:06 UTC (permalink / raw) To: Avinash Duduskar, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, bpf, netdev, linux-kselftest, linux-kernel > The helper already initializes the other flow fields the rules path > consumes (flowi4_mark, flowi4_tun_key.tun_id, flowi4_uid and the v6 > counterparts); flowi*_l3mdev was added to that set afterwards and this > helper was never updated to match. ip_route_input_slow() likewise zeroes > the field before its input lookup. Do the same here. So how about we explicitly zero-init the whole struct instead of adding more fields ad-hoc like this? Otherwise this seems like something that is likely to happen again if we ever add another field to the struct? -Toke ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper 2026-06-16 22:34 [PATCH bpf-next v2 0/4] bpf: bidirectional VLAN support for bpf_fib_lookup() Avinash Duduskar 2026-06-16 22:34 ` [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow Avinash Duduskar @ 2026-06-16 22:34 ` Avinash Duduskar 2026-06-16 22:47 ` sashiko-bot 2026-06-17 9:26 ` Toke Høiland-Jørgensen 2026-06-16 22:34 ` [PATCH bpf-next v2 3/4] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT " Avinash Duduskar 2026-06-16 22:34 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests Avinash Duduskar 3 siblings, 2 replies; 10+ messages in thread From: Avinash Duduskar @ 2026-06-16 22:34 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, Toke Høiland-Jørgensen, bpf, netdev, linux-kselftest, linux-kernel bpf_fib_lookup() returns the FIB-resolved egress ifindex straight from the fib result. When the egress is a VLAN device, the returned ifindex is the VLAN netdev's, which has no XDP xmit handler; XDP programs that want to forward the frame (e.g. xdp-forward) must instead target the underlying physical device and push the VLAN tag themselves. Today the program has no way to learn either the underlying ifindex or the VLAN tag without maintaining its own VLAN-to-ifindex map in userspace and refreshing it on netlink events. Add BPF_FIB_LOOKUP_VLAN. When the caller sets this flag and the fib result is a VLAN device whose immediate parent is a real (non-VLAN) device in the same network namespace, populate the existing output fields params->h_vlan_proto and params->h_vlan_TCI from the VLAN device and replace params->ifindex with the parent's ifindex. params->h_vlan_TCI carries the VID only, with PCP and DEI bits zero; a consumer wanting to set egress priority writes PCP itself. params->smac is the VLAN device's own address, which can differ from the parent's. Only the immediate parent is resolved, via vlan_dev_priv(dev)->real_dev and not vlan_dev_real_dev(), which walks to the bottom of a stack. For a stacked VLAN (QinQ) the immediate parent is itself a VLAN device; since one h_vlan_proto/h_vlan_TCI pair cannot describe two tags, ifindex is left unchanged and the vlan fields remain zero in that case. The swap is also skipped when the parent lives in another network namespace (a VLAN device can be moved while its parent stays), since its ifindex would be meaningless or match an unrelated device in the caller's namespace. The swap and the vlan fields are written only on success; other output fields keep their existing behaviour, so a frag-needed result still reports the route mtu in params->mtu_result. When the flag is not set, behaviour is unchanged: h_vlan_proto and h_vlan_TCI are zeroed and ifindex is left at the FIB result. The new block is compiled only under CONFIG_VLAN_8021Q since vlan_dev_priv() is not defined otherwise; without that config is_vlan_dev() is constant false and the flag is accepted but never acts. This lets an XDP redirect target the physical device and learn the tag to push in a single lookup, which xdp-forward's optional VLAN mode (xdp-project/xdp-tools#504) wants from the kernel side. The helper's input semantics are unchanged; the reverse direction (supplying a tag as lookup input) is added in the following patch. Suggested-by: Toke Høiland-Jørgensen <toke@redhat.com> Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> --- include/uapi/linux/bpf.h | 31 ++++++++++++++++++++++++++- net/core/filter.c | 39 ++++++++++++++++++++++++++++++---- tools/include/uapi/linux/bpf.h | 31 ++++++++++++++++++++++++++- 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 11dd610fa5fa..f77aa9472bf1 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -3527,6 +3527,31 @@ union bpf_attr { * Use the mark present in *params*->mark for the fib lookup. * This option should not be used with BPF_FIB_LOOKUP_DIRECT, * as it only has meaning for full lookups. + * **BPF_FIB_LOOKUP_VLAN** + * If the fib lookup resolves to a VLAN device whose + * parent is a real (non-VLAN) device, set + * *params*->h_vlan_proto and *params*->h_vlan_TCI from + * the VLAN device and replace *params*->ifindex with the + * parent's ifindex. This lets XDP programs that target + * the underlying physical device (VLAN devices have no + * XDP xmit) discover both the real egress ifindex and + * the VLAN tag to push in one call. *params*->h_vlan_TCI + * carries the VID only, with PCP and DEI bits zero; a + * consumer wanting to set egress priority writes PCP + * itself. *params*->smac is the VLAN device's own + * address, which can differ from the parent's. Only the + * immediate parent is resolved: for a stacked VLAN (QinQ) + * the parent is itself a VLAN device, and since one tag + * pair cannot describe two tags, *params*->ifindex is + * left unchanged and the vlan fields remain zero. The + * same applies when the parent is in another network + * namespace, where its ifindex would be meaningless. + * The swap and the vlan fields are written only on + * success; other output fields keep the helper's + * existing behaviour, so a frag-needed result still + * reports the route mtu in *params*->mtu_result, and on + * the tc path without tot_len the mtu check runs after + * the swap, against the parent device. * * *ctx* is either **struct xdp_md** for XDP programs or * **struct sk_buff** tc cls_act programs. @@ -7322,6 +7347,7 @@ enum { BPF_FIB_LOOKUP_TBID = (1U << 3), BPF_FIB_LOOKUP_SRC = (1U << 4), BPF_FIB_LOOKUP_MARK = (1U << 5), + BPF_FIB_LOOKUP_VLAN = (1U << 6), }; enum { @@ -7388,7 +7414,10 @@ struct bpf_fib_lookup { union { struct { - /* output */ + /* output with BPF_FIB_LOOKUP_VLAN: set from the + * resolved egress VLAN device (see the flag); zeroed + * on other successful lookups. + */ __be16 h_vlan_proto; __be16 h_vlan_TCI; }; diff --git a/net/core/filter.c b/net/core/filter.c index 6fa172cb1348..b37a12321fba 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -6119,10 +6119,40 @@ static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { #endif #if IS_ENABLED(CONFIG_INET) || IS_ENABLED(CONFIG_IPV6) -static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params, u32 mtu) +static int bpf_fib_set_fwd_params(struct net_device *dev, + struct bpf_fib_lookup *params, + u32 flags, u32 mtu) { params->h_vlan_TCI = 0; params->h_vlan_proto = 0; + +#if IS_ENABLED(CONFIG_VLAN_8021Q) + /* vlan_dev_priv() is only defined when 8021q is built in or as a + * module; under !CONFIG_VLAN_8021Q is_vlan_dev() is constant false + * so this would be dead, but it still has to compile. + */ + if ((flags & BPF_FIB_LOOKUP_VLAN) && is_vlan_dev(dev)) { + struct net_device *real_dev = vlan_dev_priv(dev)->real_dev; + + /* Resolve the immediate parent only. For a stacked VLAN + * (QinQ) the parent is itself a VLAN device, and a single + * h_vlan_proto/h_vlan_TCI pair cannot describe both tags; + * leave ifindex and the vlan fields untouched in that case + * rather than report the lower device with only one tag. + * The same applies when the parent lives in another netns + * (a VLAN device can be moved while its parent stays): + * its ifindex would be meaningless, or match an unrelated + * device, in the caller's namespace. + */ + if (!is_vlan_dev(real_dev) && + net_eq(dev_net(real_dev), dev_net(dev))) { + params->h_vlan_proto = vlan_dev_vlan_proto(dev); + params->h_vlan_TCI = htons(vlan_dev_vlan_id(dev)); + params->ifindex = real_dev->ifindex; + } + } +#endif + if (mtu) params->mtu_result = mtu; /* union with tot_len */ @@ -6266,7 +6296,7 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params, memcpy(params->smac, dev->dev_addr, ETH_ALEN); set_fwd_params: - return bpf_fib_set_fwd_params(params, mtu); + return bpf_fib_set_fwd_params(dev, params, flags, mtu); } #endif @@ -6406,13 +6436,14 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, memcpy(params->smac, dev->dev_addr, ETH_ALEN); set_fwd_params: - return bpf_fib_set_fwd_params(params, mtu); + return bpf_fib_set_fwd_params(dev, params, flags, mtu); } #endif #define BPF_FIB_LOOKUP_MASK (BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT | \ BPF_FIB_LOOKUP_SKIP_NEIGH | BPF_FIB_LOOKUP_TBID | \ - BPF_FIB_LOOKUP_SRC | BPF_FIB_LOOKUP_MARK) + BPF_FIB_LOOKUP_SRC | BPF_FIB_LOOKUP_MARK | \ + BPF_FIB_LOOKUP_VLAN) BPF_CALL_4(bpf_xdp_fib_lookup, struct xdp_buff *, ctx, struct bpf_fib_lookup *, params, int, plen, u32, flags) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 11dd610fa5fa..f77aa9472bf1 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -3527,6 +3527,31 @@ union bpf_attr { * Use the mark present in *params*->mark for the fib lookup. * This option should not be used with BPF_FIB_LOOKUP_DIRECT, * as it only has meaning for full lookups. + * **BPF_FIB_LOOKUP_VLAN** + * If the fib lookup resolves to a VLAN device whose + * parent is a real (non-VLAN) device, set + * *params*->h_vlan_proto and *params*->h_vlan_TCI from + * the VLAN device and replace *params*->ifindex with the + * parent's ifindex. This lets XDP programs that target + * the underlying physical device (VLAN devices have no + * XDP xmit) discover both the real egress ifindex and + * the VLAN tag to push in one call. *params*->h_vlan_TCI + * carries the VID only, with PCP and DEI bits zero; a + * consumer wanting to set egress priority writes PCP + * itself. *params*->smac is the VLAN device's own + * address, which can differ from the parent's. Only the + * immediate parent is resolved: for a stacked VLAN (QinQ) + * the parent is itself a VLAN device, and since one tag + * pair cannot describe two tags, *params*->ifindex is + * left unchanged and the vlan fields remain zero. The + * same applies when the parent is in another network + * namespace, where its ifindex would be meaningless. + * The swap and the vlan fields are written only on + * success; other output fields keep the helper's + * existing behaviour, so a frag-needed result still + * reports the route mtu in *params*->mtu_result, and on + * the tc path without tot_len the mtu check runs after + * the swap, against the parent device. * * *ctx* is either **struct xdp_md** for XDP programs or * **struct sk_buff** tc cls_act programs. @@ -7322,6 +7347,7 @@ enum { BPF_FIB_LOOKUP_TBID = (1U << 3), BPF_FIB_LOOKUP_SRC = (1U << 4), BPF_FIB_LOOKUP_MARK = (1U << 5), + BPF_FIB_LOOKUP_VLAN = (1U << 6), }; enum { @@ -7388,7 +7414,10 @@ struct bpf_fib_lookup { union { struct { - /* output */ + /* output with BPF_FIB_LOOKUP_VLAN: set from the + * resolved egress VLAN device (see the flag); zeroed + * on other successful lookups. + */ __be16 h_vlan_proto; __be16 h_vlan_TCI; }; -- 2.54.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper 2026-06-16 22:34 ` [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper Avinash Duduskar @ 2026-06-16 22:47 ` sashiko-bot 2026-06-17 9:26 ` Toke Høiland-Jørgensen 1 sibling, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-06-16 22:47 UTC (permalink / raw) To: Avinash Duduskar; +Cc: bpf Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Bypassing VLAN device MTU checks when BPF_FIB_LOOKUP_VLAN is used without tot_len creates a PMTU blackhole. - [Low] Multi-line comments introduced by the patch violate the BPF subsystem comment style guideline. -- commit d2aaa28c3503734e5c44721d9455da2a142a44cf Author: Avinash Duduskar <avinash.duduskar@gmail.com> bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper bpf_fib_lookup() returns the FIB-resolved egress ifindex straight from the fib result. When the egress is a VLAN device, the returned ifindex is the VLAN netdev's, which has no XDP xmit handler; XDP programs that want to forward the frame (e.g. xdp-forward) must instead target the underlying physical device and push the VLAN tag themselves. > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 11dd610fa5fae..f77aa9472bf1e 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h [ ... ] > @@ -7388,7 +7414,10 @@ struct bpf_fib_lookup { > > union { > struct { > - /* output */ > + /* output with BPF_FIB_LOOKUP_VLAN: set from the > + * resolved egress VLAN device (see the flag); zeroed > + * on other successful lookups. > + */ [Severity: Low] This isn't a bug, but does this violate the BPF subsystem multi-line comment style? The guidelines prefer the opening /* to be on its own line: /* * output with BPF_FIB_LOOKUP_VLAN: set from the * ... */ > __be16 h_vlan_proto; > __be16 h_vlan_TCI; > }; > diff --git a/net/core/filter.c b/net/core/filter.c > index 6fa172cb13480..b37a12321fba3 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c [ ... ] > @@ -6119,10 +6119,40 @@ static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { > #endif > > #if IS_ENABLED(CONFIG_INET) || IS_ENABLED(CONFIG_IPV6) > -static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params, u32 mtu) > +static int bpf_fib_set_fwd_params(struct net_device *dev, > + struct bpf_fib_lookup *params, > + u32 flags, u32 mtu) > { > params->h_vlan_TCI = 0; > params->h_vlan_proto = 0; > + > +#if IS_ENABLED(CONFIG_VLAN_8021Q) > + /* vlan_dev_priv() is only defined when 8021q is built in or as a > + * module; under !CONFIG_VLAN_8021Q is_vlan_dev() is constant false > + * so this would be dead, but it still has to compile. > + */ [Severity: Low] This isn't a bug, but this multi-line comment also starts with text on the same line as the opening /*. > + if ((flags & BPF_FIB_LOOKUP_VLAN) && is_vlan_dev(dev)) { > + struct net_device *real_dev = vlan_dev_priv(dev)->real_dev; > + > + /* Resolve the immediate parent only. For a stacked VLAN > + * (QinQ) the parent is itself a VLAN device, and a single > + * h_vlan_proto/h_vlan_TCI pair cannot describe both tags; > + * leave ifindex and the vlan fields untouched in that case > + * rather than report the lower device with only one tag. > + * The same applies when the parent lives in another netns > + * (a VLAN device can be moved while its parent stays): > + * its ifindex would be meaningless, or match an unrelated > + * device, in the caller's namespace. > + */ [Severity: Low] This isn't a bug, but this multi-line comment also starts with text on the same line as the opening /*. > + if (!is_vlan_dev(real_dev) && > + net_eq(dev_net(real_dev), dev_net(dev))) { > + params->h_vlan_proto = vlan_dev_vlan_proto(dev); > + params->h_vlan_TCI = htons(vlan_dev_vlan_id(dev)); > + params->ifindex = real_dev->ifindex; [Severity: High] Will this cause PMTU blackholes when bpf_skb_fib_lookup() is called without tot_len? When tot_len is not provided, the MTU check is deferred and done in bpf_skb_fib_lookup() after bpf_fib_set_fwd_params() returns. Because params->ifindex is swapped to the parent device here, the deferred MTU check looks up the parent device: net/core/filter.c:bpf_skb_fib_lookup() { ... dev = dev_get_by_index_rcu(net, params->ifindex); if (unlikely(!dev)) return -ENODEV; if (!is_skb_forwardable(dev, skb)) rc = BPF_FIB_LKUP_RET_FRAG_NEEDED; params->mtu_result = dev->mtu; ... } If the VLAN device has a smaller MTU than the parent, will this incorrectly pass oversized packets against the parent's MTU, or return the parent's larger MTU for ICMP Fragmentation Needed responses? > + } > + } > +#endif > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h > index 11dd610fa5fae..f77aa9472bf1e 100644 > --- a/tools/include/uapi/linux/bpf.h > +++ b/tools/include/uapi/linux/bpf.h [ ... ] > @@ -7388,7 +7414,10 @@ struct bpf_fib_lookup { > > union { > struct { > - /* output */ > + /* output with BPF_FIB_LOOKUP_VLAN: set from the > + * resolved egress VLAN device (see the flag); zeroed > + * on other successful lookups. > + */ [Severity: Low] This isn't a bug, but this copied UAPI multi-line comment also starts with text on the same line as the opening /*. > __be16 h_vlan_proto; > __be16 h_vlan_TCI; > }; -- Sashiko AI review · https://sashiko.dev/#/patchset/20260616223426.3568080-1-avinash.duduskar@gmail.com?part=2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper 2026-06-16 22:34 ` [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper Avinash Duduskar 2026-06-16 22:47 ` sashiko-bot @ 2026-06-17 9:26 ` Toke Høiland-Jørgensen 1 sibling, 0 replies; 10+ messages in thread From: Toke Høiland-Jørgensen @ 2026-06-17 9:26 UTC (permalink / raw) To: Avinash Duduskar, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, bpf, netdev, linux-kselftest, linux-kernel Avinash Duduskar <avinash.duduskar@gmail.com> writes: > bpf_fib_lookup() returns the FIB-resolved egress ifindex straight > from the fib result. When the egress is a VLAN device, the returned > ifindex is the VLAN netdev's, which has no XDP xmit handler; XDP > programs that want to forward the frame (e.g. xdp-forward) must > instead target the underlying physical device and push the VLAN tag > themselves. Today the program has no way to learn either the > underlying ifindex or the VLAN tag without maintaining its own > VLAN-to-ifindex map in userspace and refreshing it on netlink > events. > > Add BPF_FIB_LOOKUP_VLAN. When the caller sets this flag and the fib > result is a VLAN device whose immediate parent is a real (non-VLAN) > device in the same network namespace, populate the existing output > fields params->h_vlan_proto and params->h_vlan_TCI from the VLAN > device and replace params->ifindex with the parent's ifindex. > params->h_vlan_TCI carries the VID only, with PCP and DEI bits zero; a > consumer wanting to set egress priority writes PCP itself. > params->smac is the VLAN device's own address, which can differ from > the parent's. > > Only the immediate parent is resolved, via vlan_dev_priv(dev)->real_dev > and not vlan_dev_real_dev(), which walks to the bottom of a stack. For a > stacked VLAN (QinQ) the immediate parent is itself a VLAN device; since > one h_vlan_proto/h_vlan_TCI pair cannot describe two tags, ifindex is > left unchanged and the vlan fields remain zero in that case. The swap > is also skipped when the parent lives in another network namespace (a > VLAN device can be moved while its parent stays), since its ifindex > would be meaningless or match an unrelated device in the caller's > namespace. The swap and the vlan fields are written only on success; > other output fields keep their existing behaviour, so a frag-needed > result still reports the route mtu in params->mtu_result. When the > flag is not set, behaviour is unchanged: h_vlan_proto and h_vlan_TCI > are zeroed and ifindex is left at the FIB result. > > The new block is compiled only under CONFIG_VLAN_8021Q since > vlan_dev_priv() is not defined otherwise; without that config > is_vlan_dev() is constant false and the flag is accepted but never > acts. > > This lets an XDP redirect target the physical device and learn the > tag to push in a single lookup, which xdp-forward's optional VLAN > mode (xdp-project/xdp-tools#504) wants from the kernel side. > > The helper's input semantics are unchanged; the reverse direction > (supplying a tag as lookup input) is added in the following patch. > > Suggested-by: Toke Høiland-Jørgensen <toke@redhat.com> > Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> > --- > include/uapi/linux/bpf.h | 31 ++++++++++++++++++++++++++- > net/core/filter.c | 39 ++++++++++++++++++++++++++++++---- > tools/include/uapi/linux/bpf.h | 31 ++++++++++++++++++++++++++- > 3 files changed, 95 insertions(+), 6 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 11dd610fa5fa..f77aa9472bf1 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -3527,6 +3527,31 @@ union bpf_attr { > * Use the mark present in *params*->mark for the fib lookup. > * This option should not be used with BPF_FIB_LOOKUP_DIRECT, > * as it only has meaning for full lookups. > + * **BPF_FIB_LOOKUP_VLAN** > + * If the fib lookup resolves to a VLAN device whose > + * parent is a real (non-VLAN) device, set > + * *params*->h_vlan_proto and *params*->h_vlan_TCI from > + * the VLAN device and replace *params*->ifindex with the > + * parent's ifindex. This lets XDP programs that target > + * the underlying physical device (VLAN devices have no > + * XDP xmit) discover both the real egress ifindex and > + * the VLAN tag to push in one call. *params*->h_vlan_TCI > + * carries the VID only, with PCP and DEI bits zero; a > + * consumer wanting to set egress priority writes PCP > + * itself. *params*->smac is the VLAN device's own > + * address, which can differ from the parent's. Only the > + * immediate parent is resolved: for a stacked VLAN (QinQ) > + * the parent is itself a VLAN device, and since one tag > + * pair cannot describe two tags, *params*->ifindex is > + * left unchanged and the vlan fields remain zero. The > + * same applies when the parent is in another network > + * namespace, where its ifindex would be meaningless. > + * The swap and the vlan fields are written only on > + * success; other output fields keep the helper's > + * existing behaviour, so a frag-needed result still > + * reports the route mtu in *params*->mtu_result, and on > + * the tc path without tot_len the mtu check runs after > + * the swap, against the parent device. This comment is quite long, please trim. At the very least drop: "This lets XDP programs that target the underlying physical device (VLAN devices have no XDP xmit) discover both the real egress ifindex and the VLAN tag to push in one call." and shorten: "Only the immediate parent is resolved: for a stacked VLAN (QinQ) the parent is itself a VLAN device, and since one tag pair cannot describe two tags, *params*->ifindex is left unchanged and the vlan fields remain zero. The same applies when the parent is in another network namespace, where its ifindex would be meaningless." to: "The lookup only resolves the immediate parent (QinQ is not supported), and fails if the parent is in a different namespace." > * > * *ctx* is either **struct xdp_md** for XDP programs or > * **struct sk_buff** tc cls_act programs. > @@ -7322,6 +7347,7 @@ enum { > BPF_FIB_LOOKUP_TBID = (1U << 3), > BPF_FIB_LOOKUP_SRC = (1U << 4), > BPF_FIB_LOOKUP_MARK = (1U << 5), > + BPF_FIB_LOOKUP_VLAN = (1U << 6), > }; > > enum { > @@ -7388,7 +7414,10 @@ struct bpf_fib_lookup { > > union { > struct { > - /* output */ > + /* output with BPF_FIB_LOOKUP_VLAN: set from the > + * resolved egress VLAN device (see the flag); zeroed > + * on other successful lookups. > + */ > __be16 h_vlan_proto; > __be16 h_vlan_TCI; > }; > diff --git a/net/core/filter.c b/net/core/filter.c > index 6fa172cb1348..b37a12321fba 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -6119,10 +6119,40 @@ static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = { > #endif > > #if IS_ENABLED(CONFIG_INET) || IS_ENABLED(CONFIG_IPV6) > -static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params, u32 mtu) > +static int bpf_fib_set_fwd_params(struct net_device *dev, > + struct bpf_fib_lookup *params, > + u32 flags, u32 mtu) > { > params->h_vlan_TCI = 0; > params->h_vlan_proto = 0; > + > +#if IS_ENABLED(CONFIG_VLAN_8021Q) > + /* vlan_dev_priv() is only defined when 8021q is built in or as a > + * module; under !CONFIG_VLAN_8021Q is_vlan_dev() is constant false > + * so this would be dead, but it still has to compile. > + */ Superfluous comment - please drop. > + if ((flags & BPF_FIB_LOOKUP_VLAN) && is_vlan_dev(dev)) { > + struct net_device *real_dev = vlan_dev_priv(dev)->real_dev; > + > + /* Resolve the immediate parent only. For a stacked VLAN > + * (QinQ) the parent is itself a VLAN device, and a single > + * h_vlan_proto/h_vlan_TCI pair cannot describe both tags; > + * leave ifindex and the vlan fields untouched in that case > + * rather than report the lower device with only one tag. > + * The same applies when the parent lives in another netns > + * (a VLAN device can be moved while its parent stays): > + * its ifindex would be meaningless, or match an unrelated > + * device, in the caller's namespace. > + */ And this one - it's redundant with the flag description (and commit message). -Toke ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 3/4] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT flag to bpf_fib_lookup() helper 2026-06-16 22:34 [PATCH bpf-next v2 0/4] bpf: bidirectional VLAN support for bpf_fib_lookup() Avinash Duduskar 2026-06-16 22:34 ` [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow Avinash Duduskar 2026-06-16 22:34 ` [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper Avinash Duduskar @ 2026-06-16 22:34 ` Avinash Duduskar 2026-06-17 9:42 ` Toke Høiland-Jørgensen 2026-06-16 22:34 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests Avinash Duduskar 3 siblings, 1 reply; 10+ messages in thread From: Avinash Duduskar @ 2026-06-16 22:34 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, Toke Høiland-Jørgensen, bpf, netdev, linux-kselftest, linux-kernel BPF_FIB_LOOKUP_VLAN resolves a VLAN egress. The reverse is also useful: an XDP program receiving a VLAN-tagged frame on a physical device wants the lookup to behave as if the packet had arrived on the corresponding VLAN subinterface, so iif-based policy routing and VRF table selection use the right ingress. Add BPF_FIB_LOOKUP_VLAN_INPUT. When set, params->h_vlan_proto and params->h_vlan_TCI are read as an input VLAN tag and the matching VLAN device of params->ifindex is resolved with __vlan_find_dev_deep_rcu(). The device must be up and in the same network namespace as params->ifindex (a VLAN device can be moved to another netns while registered on its parent; receive would deliver into that other namespace, which a lookup here cannot represent). If params->ifindex is itself a VLAN device, its inner (QinQ) subinterface is matched. For a bond or team, a tag on a port matches no device and returns NOT_FWDED; pass the master's ifindex. The lookup then runs with the resolved device as the ingress; params->ifindex itself is not modified on the input side. When the resolved device is enslaved to a VRF, both the full lookup (via the l3mdev rule) and BPF_FIB_LOOKUP_DIRECT (via l3mdev_fib_table_rcu()) select the VRF's table from the resolved ingress. That follows from feeding the resolved device to the flow as the ingress (fl4.flowi4_iif = dev->ifindex), which is what makes l3mdev resolve the VRF master from the subinterface rather than from params->ifindex. The two failure classes get different treatment on purpose. A h_vlan_proto other than 802.1Q/802.1ad is API misuse and returns -EINVAL, since it would otherwise reach the WARN in vlan_proto_idx() with a program-controlled value. An unmatched VID, a device that is down, or one in another namespace is a data outcome and returns BPF_FIB_LKUP_RET_NOT_FWDED, matching the DIRECT path when fib_get_table() finds no table and mirroring real ingress, where the receive path drops such frames. A VID of 0 (a priority tag) is looked up literally and normally fails the same way; receive instead processes such frames untagged, so callers should not set the flag for priority tags. Proceeding on the physical device for any of these would be fail-open for the policy-routing cases above. The h_vlan fields share a union with tbid, so the flag cannot be combined with BPF_FIB_LOOKUP_TBID. It describes ingress, so it also cannot be combined with BPF_FIB_LOOKUP_OUTPUT. Both combinations return -EINVAL; restricting now keeps a later relaxation backward compatible. Combining with BPF_FIB_LOOKUP_VLAN is allowed: the tag is consumed on the ingress side and the egress tag is written on success. Under !CONFIG_VLAN_8021Q the __vlan_find_dev_deep_rcu() stub returns NULL, so every lookup with the flag returns NOT_FWDED, which is correct since no VLAN device can exist. Suggested-by: Toke Høiland-Jørgensen <toke@redhat.com> Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> --- include/uapi/linux/bpf.h | 34 ++++++++++++++- net/core/filter.c | 80 +++++++++++++++++++++++++++++++--- tools/include/uapi/linux/bpf.h | 34 ++++++++++++++- 3 files changed, 141 insertions(+), 7 deletions(-) diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index f77aa9472bf1..57e28da3336a 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -3552,6 +3552,35 @@ union bpf_attr { * reports the route mtu in *params*->mtu_result, and on * the tc path without tot_len the mtu check runs after * the swap, against the parent device. + * **BPF_FIB_LOOKUP_VLAN_INPUT** + * Treat *params*->h_vlan_proto and *params*->h_vlan_TCI + * as an input VLAN tag (e.g. parsed from the packet) and + * run the lookup as if ingress had happened on the VLAN + * subinterface carrying that tag for *params*->ifindex, + * rather than on *params*->ifindex itself. The VID is the + * low 12 bits of *params*->h_vlan_TCI; + * *params*->h_vlan_proto must be ETH_P_8021Q or + * ETH_P_8021AD in network byte order (any other value + * returns **-EINVAL**). The + * subinterface is the one configured for that tag on + * *params*->ifindex; if *params*->ifindex is itself a + * VLAN device, its inner (QinQ) subinterface is matched. + * For a bond or team, a tag on a port matches no + * device and returns NOT_FWDED; pass the master's + * ifindex. + * If no matching subinterface exists, or it is not up, + * or it was moved to another network namespace, the + * lookup returns **BPF_FIB_LKUP_RET_NOT_FWDED**, + * mirroring real ingress, which drops a frame whose tag + * is unconfigured or whose VLAN device is down. A VID of + * 0 (a priority-tagged frame) is looked up literally like + * any other VID; receive instead processes such frames + * untagged on the device itself, so do not set this flag + * for priority tags. + * Cannot be combined with **BPF_FIB_LOOKUP_TBID** (both + * use the same input fields) or **BPF_FIB_LOOKUP_OUTPUT** + * (this flag is ingress-only); doing so returns + * **-EINVAL**. * * *ctx* is either **struct xdp_md** for XDP programs or * **struct sk_buff** tc cls_act programs. @@ -7348,6 +7377,7 @@ enum { BPF_FIB_LOOKUP_SRC = (1U << 4), BPF_FIB_LOOKUP_MARK = (1U << 5), BPF_FIB_LOOKUP_VLAN = (1U << 6), + BPF_FIB_LOOKUP_VLAN_INPUT = (1U << 7), }; enum { @@ -7416,7 +7446,9 @@ struct bpf_fib_lookup { struct { /* output with BPF_FIB_LOOKUP_VLAN: set from the * resolved egress VLAN device (see the flag); zeroed - * on other successful lookups. + * on other successful lookups. input with + * BPF_FIB_LOOKUP_VLAN_INPUT: the VLAN tag to scope + * the lookup by. */ __be16 h_vlan_proto; __be16 h_vlan_TCI; diff --git a/net/core/filter.c b/net/core/filter.c index b37a12321fba..cfbdd842ce61 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -6158,6 +6158,41 @@ static int bpf_fib_set_fwd_params(struct net_device *dev, return 0; } + +/* With BPF_FIB_LOOKUP_VLAN_INPUT the caller passes the packet's VLAN tag in + * params->h_vlan_proto and params->h_vlan_TCI; the lookup is done as if + * ingress had happened on the matching VLAN subinterface of *dev. Resolve + * it and store it in *dev. params is not modified. + * + * A protocol other than 802.1Q/802.1AD is API misuse (it would otherwise + * reach the WARN in vlan_proto_idx()), so it is rejected with -EINVAL. An + * unmatched VID, a matching device that is down, or one that was moved + * to another netns (receive would deliver into that netns' stack, which + * a lookup here cannot represent) is a data outcome, reported as + * NOT_FWDED, the same way the DIRECT path reports a missing table. Under + * !CONFIG_VLAN_8021Q __vlan_find_dev_deep_rcu() returns NULL, so every + * call returns NOT_FWDED, which is correct since no subinterface can + * exist. + */ +static int bpf_fib_vlan_input_dev(struct net_device **dev, + const struct bpf_fib_lookup *params) +{ + __be16 proto = params->h_vlan_proto; + struct net_device *vlan_dev; + u16 vid; + + if (proto != htons(ETH_P_8021Q) && proto != htons(ETH_P_8021AD)) + return -EINVAL; + + vid = ntohs(params->h_vlan_TCI) & VLAN_VID_MASK; + vlan_dev = __vlan_find_dev_deep_rcu(*dev, proto, vid); + if (!vlan_dev || !(vlan_dev->flags & IFF_UP) || + !net_eq(dev_net(vlan_dev), dev_net(*dev))) + return BPF_FIB_LKUP_RET_NOT_FWDED; + + *dev = vlan_dev; + return 0; +} #endif #if IS_ENABLED(CONFIG_INET) @@ -6177,6 +6212,12 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params, if (unlikely(!dev)) return -ENODEV; + if (flags & BPF_FIB_LOOKUP_VLAN_INPUT) { + err = bpf_fib_vlan_input_dev(&dev, params); + if (err) + return err; + } + /* verify forwarding is enabled on this interface */ in_dev = __in_dev_get_rcu(dev); if (unlikely(!in_dev || !IN_DEV_FORWARD(in_dev))) @@ -6186,7 +6227,10 @@ static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params, fl4.flowi4_iif = 1; fl4.flowi4_oif = params->ifindex; } else { - fl4.flowi4_iif = params->ifindex; + /* dev->ifindex, not params->ifindex: VLAN_INPUT may have + * resolved dev to a subinterface above. + */ + fl4.flowi4_iif = dev->ifindex; fl4.flowi4_oif = 0; } fl4.flowi4_dscp = inet_dsfield_to_dscp(params->tos); @@ -6323,6 +6367,12 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, if (unlikely(!dev)) return -ENODEV; + if (flags & BPF_FIB_LOOKUP_VLAN_INPUT) { + err = bpf_fib_vlan_input_dev(&dev, params); + if (err) + return err; + } + idev = __in6_dev_get_safely(dev); if (unlikely(!idev || !READ_ONCE(idev->cnf.forwarding))) return BPF_FIB_LKUP_RET_FWD_DISABLED; @@ -6331,7 +6381,11 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, fl6.flowi6_iif = 1; oif = fl6.flowi6_oif = params->ifindex; } else { - oif = fl6.flowi6_iif = params->ifindex; + /* dev->ifindex, not params->ifindex: VLAN_INPUT may have + * resolved dev to a subinterface above. + */ + oif = dev->ifindex; + fl6.flowi6_iif = oif; fl6.flowi6_oif = 0; strict = RT6_LOOKUP_F_HAS_SADDR; } @@ -6443,7 +6497,23 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params, #define BPF_FIB_LOOKUP_MASK (BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_OUTPUT | \ BPF_FIB_LOOKUP_SKIP_NEIGH | BPF_FIB_LOOKUP_TBID | \ BPF_FIB_LOOKUP_SRC | BPF_FIB_LOOKUP_MARK | \ - BPF_FIB_LOOKUP_VLAN) + BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_VLAN_INPUT) + +static bool bpf_fib_lookup_flags_ok(u32 flags) +{ + if (flags & ~BPF_FIB_LOOKUP_MASK) + return false; + + /* VLAN_INPUT reads h_vlan_proto/h_vlan_TCI, which alias tbid, so it + * cannot be combined with TBID. It is also ingress-only, so it + * cannot be combined with the egress-perspective OUTPUT flag. + */ + if ((flags & BPF_FIB_LOOKUP_VLAN_INPUT) && + (flags & (BPF_FIB_LOOKUP_TBID | BPF_FIB_LOOKUP_OUTPUT))) + return false; + + return true; +} BPF_CALL_4(bpf_xdp_fib_lookup, struct xdp_buff *, ctx, struct bpf_fib_lookup *, params, int, plen, u32, flags) @@ -6451,7 +6521,7 @@ BPF_CALL_4(bpf_xdp_fib_lookup, struct xdp_buff *, ctx, if (plen < sizeof(*params)) return -EINVAL; - if (flags & ~BPF_FIB_LOOKUP_MASK) + if (!bpf_fib_lookup_flags_ok(flags)) return -EINVAL; switch (params->family) { @@ -6489,7 +6559,7 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb, if (plen < sizeof(*params)) return -EINVAL; - if (flags & ~BPF_FIB_LOOKUP_MASK) + if (!bpf_fib_lookup_flags_ok(flags)) return -EINVAL; if (params->tot_len) diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index f77aa9472bf1..57e28da3336a 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -3552,6 +3552,35 @@ union bpf_attr { * reports the route mtu in *params*->mtu_result, and on * the tc path without tot_len the mtu check runs after * the swap, against the parent device. + * **BPF_FIB_LOOKUP_VLAN_INPUT** + * Treat *params*->h_vlan_proto and *params*->h_vlan_TCI + * as an input VLAN tag (e.g. parsed from the packet) and + * run the lookup as if ingress had happened on the VLAN + * subinterface carrying that tag for *params*->ifindex, + * rather than on *params*->ifindex itself. The VID is the + * low 12 bits of *params*->h_vlan_TCI; + * *params*->h_vlan_proto must be ETH_P_8021Q or + * ETH_P_8021AD in network byte order (any other value + * returns **-EINVAL**). The + * subinterface is the one configured for that tag on + * *params*->ifindex; if *params*->ifindex is itself a + * VLAN device, its inner (QinQ) subinterface is matched. + * For a bond or team, a tag on a port matches no + * device and returns NOT_FWDED; pass the master's + * ifindex. + * If no matching subinterface exists, or it is not up, + * or it was moved to another network namespace, the + * lookup returns **BPF_FIB_LKUP_RET_NOT_FWDED**, + * mirroring real ingress, which drops a frame whose tag + * is unconfigured or whose VLAN device is down. A VID of + * 0 (a priority-tagged frame) is looked up literally like + * any other VID; receive instead processes such frames + * untagged on the device itself, so do not set this flag + * for priority tags. + * Cannot be combined with **BPF_FIB_LOOKUP_TBID** (both + * use the same input fields) or **BPF_FIB_LOOKUP_OUTPUT** + * (this flag is ingress-only); doing so returns + * **-EINVAL**. * * *ctx* is either **struct xdp_md** for XDP programs or * **struct sk_buff** tc cls_act programs. @@ -7348,6 +7377,7 @@ enum { BPF_FIB_LOOKUP_SRC = (1U << 4), BPF_FIB_LOOKUP_MARK = (1U << 5), BPF_FIB_LOOKUP_VLAN = (1U << 6), + BPF_FIB_LOOKUP_VLAN_INPUT = (1U << 7), }; enum { @@ -7416,7 +7446,9 @@ struct bpf_fib_lookup { struct { /* output with BPF_FIB_LOOKUP_VLAN: set from the * resolved egress VLAN device (see the flag); zeroed - * on other successful lookups. + * on other successful lookups. input with + * BPF_FIB_LOOKUP_VLAN_INPUT: the VLAN tag to scope + * the lookup by. */ __be16 h_vlan_proto; __be16 h_vlan_TCI; -- 2.54.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 3/4] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT flag to bpf_fib_lookup() helper 2026-06-16 22:34 ` [PATCH bpf-next v2 3/4] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT " Avinash Duduskar @ 2026-06-17 9:42 ` Toke Høiland-Jørgensen 0 siblings, 0 replies; 10+ messages in thread From: Toke Høiland-Jørgensen @ 2026-06-17 9:42 UTC (permalink / raw) To: Avinash Duduskar, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, bpf, netdev, linux-kselftest, linux-kernel Avinash Duduskar <avinash.duduskar@gmail.com> writes: > BPF_FIB_LOOKUP_VLAN resolves a VLAN egress. The reverse is also > useful: an XDP program receiving a VLAN-tagged frame on a physical > device wants the lookup to behave as if the packet had arrived on the > corresponding VLAN subinterface, so iif-based policy routing and VRF > table selection use the right ingress. > > Add BPF_FIB_LOOKUP_VLAN_INPUT. When set, params->h_vlan_proto and > params->h_vlan_TCI are read as an input VLAN tag and the matching VLAN > device of params->ifindex is resolved with __vlan_find_dev_deep_rcu(). > The device must be up and in the same network namespace as > params->ifindex (a VLAN device can be moved to another netns while > registered on its parent; receive would deliver into that other > namespace, which a lookup here cannot represent). If params->ifindex > is itself a VLAN device, its inner (QinQ) subinterface is matched. > For a bond or team, a tag on a port matches no device and returns > NOT_FWDED; pass the master's ifindex. > The lookup then runs with the resolved device as the ingress; > params->ifindex itself is not modified on the input side. When the > resolved device is enslaved to a VRF, both the full lookup (via the > l3mdev rule) and BPF_FIB_LOOKUP_DIRECT (via l3mdev_fib_table_rcu()) > select the VRF's table from the resolved ingress. That follows from > feeding the resolved device to the flow as the ingress > (fl4.flowi4_iif = dev->ifindex), which is what makes l3mdev resolve > the VRF master from the subinterface rather than from > params->ifindex. > > The two failure classes get different treatment on purpose. A > h_vlan_proto other than 802.1Q/802.1ad is API misuse and returns > -EINVAL, since it would otherwise reach the WARN in vlan_proto_idx() > with a program-controlled value. An unmatched VID, a device that is > down, or one in another namespace is a data outcome and returns > BPF_FIB_LKUP_RET_NOT_FWDED, matching the DIRECT path when > fib_get_table() finds no table and mirroring real ingress, where the > receive path drops such frames. A VID of 0 (a priority tag) is looked > up literally and normally fails the same way; receive instead > processes such frames untagged, so callers should not set the flag for > priority tags. Proceeding on the physical device for any of these > would be fail-open for the policy-routing cases above. > > The h_vlan fields share a union with tbid, so the flag cannot be > combined with BPF_FIB_LOOKUP_TBID. It describes ingress, so it also > cannot be combined with BPF_FIB_LOOKUP_OUTPUT. Both combinations > return -EINVAL; restricting now keeps a later relaxation backward > compatible. Combining with BPF_FIB_LOOKUP_VLAN is allowed: the tag is > consumed on the ingress side and the egress tag is written on > success. > > Under !CONFIG_VLAN_8021Q the __vlan_find_dev_deep_rcu() stub returns > NULL, so every lookup with the flag returns NOT_FWDED, which is > correct since no VLAN device can exist. > > Suggested-by: Toke Høiland-Jørgensen <toke@redhat.com> > Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> > --- > include/uapi/linux/bpf.h | 34 ++++++++++++++- > net/core/filter.c | 80 +++++++++++++++++++++++++++++++--- > tools/include/uapi/linux/bpf.h | 34 ++++++++++++++- > 3 files changed, 141 insertions(+), 7 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index f77aa9472bf1..57e28da3336a 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -3552,6 +3552,35 @@ union bpf_attr { > * reports the route mtu in *params*->mtu_result, and on > * the tc path without tot_len the mtu check runs after > * the swap, against the parent device. > + * **BPF_FIB_LOOKUP_VLAN_INPUT** > + * Treat *params*->h_vlan_proto and *params*->h_vlan_TCI > + * as an input VLAN tag (e.g. parsed from the packet) and > + * run the lookup as if ingress had happened on the VLAN > + * subinterface carrying that tag for *params*->ifindex, > + * rather than on *params*->ifindex itself. The VID is the > + * low 12 bits of *params*->h_vlan_TCI; > + * *params*->h_vlan_proto must be ETH_P_8021Q or > + * ETH_P_8021AD in network byte order (any other value > + * returns **-EINVAL**). The > + * subinterface is the one configured for that tag on > + * *params*->ifindex; if *params*->ifindex is itself a > + * VLAN device, its inner (QinQ) subinterface is matched. > + * For a bond or team, a tag on a port matches no > + * device and returns NOT_FWDED; pass the master's > + * ifindex. > + * If no matching subinterface exists, or it is not up, > + * or it was moved to another network namespace, the > + * lookup returns **BPF_FIB_LKUP_RET_NOT_FWDED**, > + * mirroring real ingress, which drops a frame whose tag > + * is unconfigured or whose VLAN device is down. A VID of > + * 0 (a priority-tagged frame) is looked up literally like > + * any other VID; receive instead processes such frames > + * untagged on the device itself, so do not set this flag > + * for priority tags. > + * Cannot be combined with **BPF_FIB_LOOKUP_TBID** (both > + * use the same input fields) or **BPF_FIB_LOOKUP_OUTPUT** > + * (this flag is ingress-only); doing so returns > + * **-EINVAL**. This comment is also overly long - please trim. > * > * *ctx* is either **struct xdp_md** for XDP programs or > * **struct sk_buff** tc cls_act programs. > @@ -7348,6 +7377,7 @@ enum { > BPF_FIB_LOOKUP_SRC = (1U << 4), > BPF_FIB_LOOKUP_MARK = (1U << 5), > BPF_FIB_LOOKUP_VLAN = (1U << 6), > + BPF_FIB_LOOKUP_VLAN_INPUT = (1U << 7), > }; > > enum { > @@ -7416,7 +7446,9 @@ struct bpf_fib_lookup { > struct { > /* output with BPF_FIB_LOOKUP_VLAN: set from the > * resolved egress VLAN device (see the flag); zeroed > - * on other successful lookups. > + * on other successful lookups. input with > + * BPF_FIB_LOOKUP_VLAN_INPUT: the VLAN tag to scope > + * the lookup by. > */ > __be16 h_vlan_proto; > __be16 h_vlan_TCI; > diff --git a/net/core/filter.c b/net/core/filter.c > index b37a12321fba..cfbdd842ce61 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -6158,6 +6158,41 @@ static int bpf_fib_set_fwd_params(struct net_device *dev, > > return 0; > } > + > +/* With BPF_FIB_LOOKUP_VLAN_INPUT the caller passes the packet's VLAN tag in > + * params->h_vlan_proto and params->h_vlan_TCI; the lookup is done as if > + * ingress had happened on the matching VLAN subinterface of *dev. Resolve > + * it and store it in *dev. params is not modified. > + * > + * A protocol other than 802.1Q/802.1AD is API misuse (it would otherwise > + * reach the WARN in vlan_proto_idx()), so it is rejected with -EINVAL. An > + * unmatched VID, a matching device that is down, or one that was moved > + * to another netns (receive would deliver into that netns' stack, which > + * a lookup here cannot represent) is a data outcome, reported as > + * NOT_FWDED, the same way the DIRECT path reports a missing table. Under > + * !CONFIG_VLAN_8021Q __vlan_find_dev_deep_rcu() returns NULL, so every > + * call returns NOT_FWDED, which is correct since no subinterface can > + * exist. > + */ As in the previous patch, please drop this comment. > +static int bpf_fib_vlan_input_dev(struct net_device **dev, > + const struct bpf_fib_lookup *params) > +{ Just return the dev pointer and use ERR_PTR for errors? That's what we usually do for these kinds of functions. -Toke ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next v2 4/4] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests 2026-06-16 22:34 [PATCH bpf-next v2 0/4] bpf: bidirectional VLAN support for bpf_fib_lookup() Avinash Duduskar ` (2 preceding siblings ...) 2026-06-16 22:34 ` [PATCH bpf-next v2 3/4] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT " Avinash Duduskar @ 2026-06-16 22:34 ` Avinash Duduskar 2026-06-16 22:40 ` sashiko-bot 3 siblings, 1 reply; 10+ messages in thread From: Avinash Duduskar @ 2026-06-16 22:34 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis, John Fastabend, Stanislav Fomichev, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, David Ahern, Shuah Khan, Jesper Dangaard Brouer, Mykyta Yatsenko, Leon Hwang, KP Singh, Anton Protopopov, Amery Hung, Eyal Birger, Rong Tao, Toke Høiland-Jørgensen, bpf, netdev, linux-kselftest, linux-kernel Cover both directions of the new VLAN flags in the fib_lookup test, 36 table cases plus a dedicated cross-netns subtest. For BPF_FIB_LOOKUP_VLAN the egress cases assert: without the flag the lookup returns the VLAN netdev's ifindex and zeroed vlan fields, with the flag it returns the parent's ifindex plus the tag (including via a neighbour resolved on the VLAN device, in OUTPUT mode, over a bond, and through a DIRECT|TBID table), with the flag on a non-VLAN egress it changes nothing, for a stacked VLAN it leaves ifindex untouched with the vlan fields zero, and a frag-needed return reports the route mtu in mtu_result while leaving the swap unwritten. For BPF_FIB_LOOKUP_VLAN_INPUT, an iif rule on the subinterface routes the same destination to a different gateway, so the asserted gateway shows which device the lookup used as ingress: without the flag the main table answers, with a matching tag the subinterface's table does, with or without SKIP_NEIGH, and BPF_FIB_LOOKUP_SRC selects the subinterface's address. A VRF-enslaved subinterface selects the VRF table through the l3mdev rule and, with DIRECT, through l3mdev_fib_table_rcu(). One case sets BPF_FIB_LOOKUP_VLAN as well and asserts both directions work in a single lookup. Resolution semantics are pinned: an 802.1ad tag resolves its device, PCP and DEI bits in h_vlan_TCI are ignored, a VLAN ifindex resolves the inner QinQ device, a tag on a bond master resolves while the same tag on the bond port does not. The error cases assert -EINVAL for an invalid h_vlan_proto on both address families, for the TBID and OUTPUT flag combinations and for an unknown flag bit, and BPF_FIB_LKUP_RET_NOT_FWDED for a VID with no configured device on both families, for a VID-0 priority tag and for a device that exists but is down. The failure cases also assert that params is left untouched. A separate subtest moves a VLAN device into a second netns while it stays registered on its parent, and checks both directions refuse to cross the boundary: the input flag fails closed with the tag and ifindex untouched, and the egress flag does not publish the foreign parent's ifindex. The tbid read-back check is skipped for DIRECT cases that set BPF_FIB_LOOKUP_VLAN, since a successful swap packs the vlan fields into the union the check reads. Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com> --- .../selftests/bpf/prog_tests/fib_lookup.c | 494 +++++++++++++++++- 1 file changed, 491 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c index bd7658958004..42107d60c9ca 100644 --- a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c +++ b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c @@ -2,6 +2,7 @@ /* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */ #include <linux/rtnetlink.h> +#include <linux/if_ether.h> #include <sys/types.h> #include <net/if.h> @@ -37,6 +38,41 @@ #define IPV6_LOCAL "fd01::3" #define IPV6_GW1 "fd01::1" #define IPV6_GW2 "fd01::2" +#define VLAN_ID 100 +#define VLAN_IFACE "veth1.100" +#define VLAN_ID_DOWN 102 +#define VLAN_IFACE_DOWN "veth1.102" +#define QINQ_OUTER_IFACE "veth1.200" +#define QINQ_INNER_IFACE "veth1.200.300" +#define VLAN_TABLE "300" +#define IPV4_VLAN_IFACE_ADDR "10.5.0.254" +#define IPV4_VLAN_EGRESS_DST "10.5.0.2" +#define IPV4_QINQ_DST "10.7.0.2" +#define IPV4_VLAN_DST "10.6.0.2" +#define IPV4_VLAN_GW "10.5.0.1" +#define IPV6_VLAN_IFACE_ADDR "fd02::254" +#define IPV6_VLAN_EGRESS_DST "fd02::2" +#define IPV6_VLAN_DST "fd03::2" +#define IPV6_VLAN_GW "fd02::1" +#define VLAN_VID_UNUSED 999 +#define VRF_IFACE "vrf-blue" +#define VRF_TABLE "1000" +#define VRF_VLAN_ID 101 +#define VRF_VLAN_IFACE "veth1.101" +#define IPV4_VRF_IFACE_ADDR "10.8.0.254" +#define IPV4_VRF_GW "10.8.0.1" +#define IPV4_VRF_DST "10.9.0.2" +#define TBID_VLAN_ID 50 +#define TBID_VLAN_IFACE "veth2.50" +#define IPV4_TBID_VLAN_DST "172.2.0.2" +#define IPV4_BOND_VLAN_DST "10.11.0.2" +#define IPV4_VLAN_MTU_DST "10.5.9.2" +#define QINQ_AD_VLAN_ID 200 +#define QINQ_INNER_VLAN_ID 300 +#define BOND_IFACE "bond99" +#define BOND_PORT "veth3" +#define BOND_PORT_PEER "veth4" +#define BOND_VLAN_ID 500 #define DMAC "11:11:11:11:11:11" #define DMAC_INIT { 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, } #define DMAC2 "01:01:01:01:01:01" @@ -52,6 +88,16 @@ struct fib_lookup_test { __u32 tbid; __u8 dmac[6]; __u32 mark; + /* input tag with BPF_FIB_LOOKUP_VLAN_INPUT; expected output tag + * with BPF_FIB_LOOKUP_VLAN (checked when check_vlan is set) + */ + __u16 vlan_proto; + __u16 vlan_id; + bool check_vlan; + const char *expected_dev; /* expected params->ifindex after lookup */ + const char *iif; /* override the default veth1 input device */ + __u16 tot_len; /* triggers the in-lookup mtu check when set */ + __u16 expected_mtu; /* expected mtu_result (union with tot_len) */ }; static const struct fib_lookup_test tests[] = { @@ -142,6 +188,204 @@ static const struct fib_lookup_test tests[] = { .expected_dst = IPV6_GW1, .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, .mark = MARK, }, + /* vlan egress resolution */ + { .desc = "IPv4 VLAN egress, no flag", + .daddr = IPV4_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = VLAN_IFACE, .check_vlan = true, }, + { .desc = "IPv4 VLAN egress, single VLAN", + .daddr = IPV4_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + /* skb path without tot_len: mtu_result follows params->ifindex, so the + * swap moves it from the VLAN device's mtu (1400) to the parent's (1500) + */ + { .desc = "IPv4 VLAN egress, skb-path mtu is the VLAN device's without the flag", + .daddr = IPV4_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = VLAN_IFACE, .check_vlan = true, .expected_mtu = 1400, }, + { .desc = "IPv4 VLAN egress, skb-path mtu is the parent's after the swap", + .daddr = IPV4_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, .expected_mtu = 1500, }, + { .desc = "IPv4 VLAN egress, flag set but egress is not a VLAN", + .daddr = IPV4_NUD_FAILED_ADDR, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, }, + { .desc = "IPv4 VLAN egress, stacked VLAN untouched", + .daddr = IPV4_QINQ_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = QINQ_INNER_IFACE, .check_vlan = true, }, + { .desc = "IPv6 VLAN egress, single VLAN", + .daddr = IPV6_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN egress, neighbour on the VLAN device", + .daddr = IPV4_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN, + .expected_dev = "veth1", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, .dmac = DMAC_INIT, }, + { .desc = "IPv4 VLAN egress in OUTPUT mode", + .daddr = IPV4_VLAN_EGRESS_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .iif = VLAN_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_OUTPUT | BPF_FIB_LOOKUP_VLAN | + BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN egress over a bond", + .daddr = IPV4_BOND_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = BOND_IFACE, .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = BOND_VLAN_ID, }, + { .desc = "IPv4 VLAN egress via TBID table", + .daddr = IPV4_TBID_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .lookup_flags = BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_TBID | + BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .tbid = 100, + .expected_dev = "veth2", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = TBID_VLAN_ID, }, + { .desc = "IPv4 VLAN egress, success writes mtu_result with the swap", + .daddr = IPV4_VLAN_MTU_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .tot_len = 500, .expected_mtu = 1000, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN egress, FRAG_NEEDED reports mtu, swap unwritten", + .daddr = IPV4_VLAN_MTU_DST, .expected_ret = BPF_FIB_LKUP_RET_FRAG_NEEDED, + .tot_len = 1400, .expected_mtu = 1000, + .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH, + .expected_dev = "veth1", .check_vlan = true, }, + /* vlan tag as lookup input */ + { .desc = "IPv4 VLAN input, no flag", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_GW1, + .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, }, + { .desc = "IPv4 VLAN input, tag selects subinterface route", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_VLAN_GW, .expected_dev = VLAN_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv6 VLAN input, tag selects subinterface route", + .daddr = IPV6_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV6_VLAN_GW, .expected_dev = VLAN_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN input and egress combined", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_VLAN_GW, .expected_dev = "veth1", + .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_VLAN | + BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN input, neighbour resolved on the route", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_VLAN_GW, .expected_dev = VLAN_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, .dmac = DMAC_INIT2, }, + { .desc = "IPv4 VLAN input, source address from the subinterface", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_src = IPV4_VLAN_IFACE_ADDR, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SRC | + BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + /* VRF: the resolved subinterface is enslaved, so the l3mdev rule + * (full lookup) and l3mdev_fib_table_rcu() (DIRECT) must select + * the VRF table from the resolved ingress + */ + { .desc = "IPv4 VLAN input, VRF subinterface, no flag", + .daddr = IPV4_VRF_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_GW1, + .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH, }, + { .desc = "IPv4 VLAN input, tag selects VRF table", + .daddr = IPV4_VRF_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_VRF_GW, .expected_dev = VRF_VLAN_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VRF_VLAN_ID, }, + { .desc = "IPv4 VLAN input, DIRECT uses VRF table from resolved ingress", + .daddr = IPV4_VRF_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_VRF_GW, .expected_dev = VRF_VLAN_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_DIRECT | + BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VRF_VLAN_ID, }, + /* failure arms also assert params is left untouched: ifindex still + * names the physical device and the input tag bytes survive + */ + { .desc = "IPv4 VLAN input, invalid proto", + .daddr = IPV4_VLAN_DST, .expected_ret = -EINVAL, + .expected_dev = "veth1", .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = 0x1234, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN input, unmatched VID", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_NOT_FWDED, + .expected_dev = "veth1", .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_VID_UNUSED, }, + { .desc = "IPv4 VLAN input, subinterface down", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_NOT_FWDED, + .expected_dev = "veth1", .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID_DOWN, }, + /* the resolver runs before the forwarding check, so on devices + * with forwarding off FWD_DISABLED (not NOT_FWDED) proves the tag + * resolved to that device and the lookup used it as ingress + */ + { .desc = "IPv4 VLAN input, 802.1ad tag", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_FWD_DISABLED, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021AD, .vlan_id = QINQ_AD_VLAN_ID, }, + { .desc = "IPv4 VLAN input, PCP and DEI bits ignored in TCI", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS, + .expected_dst = IPV4_VLAN_GW, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = 0xe000 | VLAN_ID, }, + { .desc = "IPv4 VLAN input, inner QinQ device from VLAN ifindex", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_FWD_DISABLED, + .iif = QINQ_OUTER_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = QINQ_INNER_VLAN_ID, }, + /* bonding: the VLANs live on the master, as on receive, where the + * frame is steered to the master before VLAN processing; a port + * ifindex does not match (ports carry vid state but no VLAN devs) + */ + { .desc = "IPv4 VLAN input, tag on bond master resolves", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_FWD_DISABLED, + .iif = BOND_IFACE, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = BOND_VLAN_ID, }, + { .desc = "IPv4 VLAN input, tag on bond port does not match", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_NOT_FWDED, + .iif = BOND_PORT, .expected_dev = BOND_PORT, .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = BOND_VLAN_ID, }, + { .desc = "IPv6 VLAN input, invalid proto", + .daddr = IPV6_VLAN_DST, .expected_ret = -EINVAL, + .expected_dev = "veth1", .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = 0x1234, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN input, VID 0 priority tag fails closed", + .daddr = IPV4_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_NOT_FWDED, + .expected_dev = "veth1", .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = 0, }, + { .desc = "IPv6 VLAN input, unmatched VID", + .daddr = IPV6_VLAN_DST, .expected_ret = BPF_FIB_LKUP_RET_NOT_FWDED, + .expected_dev = "veth1", .check_vlan = true, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_SKIP_NEIGH, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_VID_UNUSED, }, + { .desc = "unknown flag bit rejected", + .daddr = IPV4_VLAN_DST, .expected_ret = -EINVAL, + .lookup_flags = (1 << 14) | BPF_FIB_LOOKUP_SKIP_NEIGH, }, + { .desc = "IPv4 VLAN input rejected with TBID", + .daddr = IPV4_VLAN_DST, .expected_ret = -EINVAL, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_TBID, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, + { .desc = "IPv4 VLAN input rejected with OUTPUT", + .daddr = IPV4_VLAN_DST, .expected_ret = -EINVAL, + .lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | BPF_FIB_LOOKUP_OUTPUT, + .vlan_proto = ETH_P_8021Q, .vlan_id = VLAN_ID, }, }; static int setup_netns(void) @@ -204,6 +448,105 @@ static int setup_netns(void) SYS(fail, "ip rule add prio 2 fwmark %d lookup %s", MARK, MARK_TABLE); SYS(fail, "ip -6 rule add prio 2 fwmark %d lookup %s", MARK, MARK_TABLE); + /* Setup for vlan tests: a subinterface for egress resolution and + * tag-as-input, a QinQ stack, and an iif rule so the input tests + * observe which device the lookup used as ingress. + */ + SYS(fail, "ip link add link veth1 name %s type vlan id %d", + VLAN_IFACE, VLAN_ID); + SYS(fail, "ip link set dev %s up", VLAN_IFACE); + /* lower than the veth1 parent (1500): the skb-path mtu check follows + * params->ifindex, so the egress swap makes mtu_result jump from this + * value to the parent's, which two arms below pin + */ + SYS(fail, "ip link set dev %s mtu 1400", VLAN_IFACE); + SYS(fail, "ip addr add %s/24 dev %s", IPV4_VLAN_IFACE_ADDR, VLAN_IFACE); + SYS(fail, "ip addr add %s/64 dev %s nodad", IPV6_VLAN_IFACE_ADDR, VLAN_IFACE); + + /* stays down: the input flag must treat its tag the way real + * ingress treats a frame arriving on a down VLAN device (drop) + */ + SYS(fail, "ip link add link veth1 name %s type vlan id %d", + VLAN_IFACE_DOWN, VLAN_ID_DOWN); + + err = write_sysctl("/proc/sys/net/ipv4/conf/" VLAN_IFACE "/forwarding", "1"); + if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf." VLAN_IFACE ".forwarding)")) + goto fail; + + err = write_sysctl("/proc/sys/net/ipv6/conf/" VLAN_IFACE "/forwarding", "1"); + if (!ASSERT_OK(err, "write_sysctl(net.ipv6.conf." VLAN_IFACE ".forwarding)")) + goto fail; + + SYS(fail, "ip link add link veth1 name %s type vlan proto 802.1ad id 200", + QINQ_OUTER_IFACE); + SYS(fail, "ip link add link %s name %s type vlan id 300", + QINQ_OUTER_IFACE, QINQ_INNER_IFACE); + SYS(fail, "ip link set dev %s up", QINQ_OUTER_IFACE); + SYS(fail, "ip link set dev %s up", QINQ_INNER_IFACE); + SYS(fail, "ip route add %s/32 dev %s", IPV4_QINQ_DST, QINQ_INNER_IFACE); + + SYS(fail, "ip route add %s/32 via %s", IPV4_VLAN_DST, IPV4_GW1); + SYS(fail, "ip route add table %s %s/32 via %s", + VLAN_TABLE, IPV4_VLAN_DST, IPV4_VLAN_GW); + SYS(fail, "ip rule add prio 3 iif %s lookup %s", VLAN_IFACE, VLAN_TABLE); + SYS(fail, "ip -6 route add %s/128 via %s", IPV6_VLAN_DST, IPV6_GW1); + SYS(fail, "ip -6 route add table %s %s/128 via %s", + VLAN_TABLE, IPV6_VLAN_DST, IPV6_VLAN_GW); + SYS(fail, "ip -6 rule add prio 3 iif %s lookup %s", VLAN_IFACE, VLAN_TABLE); + + /* a bond with one port and a VLAN on the bond: VLANs on a bond + * live on the master, so resolution succeeds for the master's + * ifindex and fails closed for a port's, matching receive, which + * steers the frame to the master before VLAN processing + */ + SYS(fail, "ip link add %s type bond", BOND_IFACE); + SYS(fail, "ip link add %s type veth peer name %s", BOND_PORT, BOND_PORT_PEER); + SYS(fail, "ip link set %s master %s", BOND_PORT, BOND_IFACE); + SYS(fail, "ip link set dev %s up", BOND_IFACE); + SYS(fail, "ip link set dev %s up", BOND_PORT); + SYS(fail, "ip link add link %s name %s.%d type vlan id %d", + BOND_IFACE, BOND_IFACE, BOND_VLAN_ID, BOND_VLAN_ID); + SYS(fail, "ip link set dev %s.%d up", BOND_IFACE, BOND_VLAN_ID); + SYS(fail, "ip route add %s/32 dev %s.%d", + IPV4_BOND_VLAN_DST, BOND_IFACE, BOND_VLAN_ID); + + /* a VRF with its own dedicated subinterface (the iif rules above + * must not see it), for the table-selection-by-ingress cases + */ + SYS(fail, "ip link add %s type vrf table %s", VRF_IFACE, VRF_TABLE); + SYS(fail, "ip link set dev %s up", VRF_IFACE); + SYS(fail, "ip link add link veth1 name %s type vlan id %d", + VRF_VLAN_IFACE, VRF_VLAN_ID); + SYS(fail, "ip link set %s master %s", VRF_VLAN_IFACE, VRF_IFACE); + SYS(fail, "ip link set dev %s up", VRF_VLAN_IFACE); + SYS(fail, "ip addr add %s/24 dev %s", IPV4_VRF_IFACE_ADDR, VRF_VLAN_IFACE); + err = write_sysctl("/proc/sys/net/ipv4/conf/" VRF_VLAN_IFACE "/forwarding", "1"); + if (!ASSERT_OK(err, "write_sysctl(net.ipv4.conf." VRF_VLAN_IFACE ".forwarding)")) + goto fail; + SYS(fail, "ip route add %s/32 via %s", IPV4_VRF_DST, IPV4_GW1); + SYS(fail, "ip route add table %s %s/32 via %s", + VRF_TABLE, IPV4_VRF_DST, IPV4_VRF_GW); + + /* neighbours on the VLAN subinterface for the non-SKIP_NEIGH cases */ + err = write_sysctl("/proc/sys/net/ipv4/neigh/" VLAN_IFACE "/gc_stale_time", "900"); + if (!ASSERT_OK(err, "write_sysctl(net.ipv4.neigh." VLAN_IFACE ".gc_stale_time)")) + goto fail; + SYS(fail, "ip neigh add %s dev %s lladdr %s nud stale", + IPV4_VLAN_EGRESS_DST, VLAN_IFACE, DMAC); + SYS(fail, "ip neigh add %s dev %s lladdr %s nud stale", + IPV4_VLAN_GW, VLAN_IFACE, DMAC2); + + /* a VLAN on veth2 with a route in the tbid test table */ + SYS(fail, "ip link add link veth2 name %s type vlan id %d", + TBID_VLAN_IFACE, TBID_VLAN_ID); + SYS(fail, "ip link set dev %s up", TBID_VLAN_IFACE); + SYS(fail, "ip route add table 100 %s/32 dev %s", + IPV4_TBID_VLAN_DST, TBID_VLAN_IFACE); + + /* a locked-mtu route via the subinterface for the FRAG_NEEDED case */ + SYS(fail, "ip route add %s/32 dev %s mtu lock 1000", + IPV4_VLAN_MTU_DST, VLAN_IFACE); + return 0; fail: return -1; @@ -218,9 +561,16 @@ static int set_lookup_params(struct bpf_fib_lookup *params, memset(params, 0, sizeof(*params)); params->l4_protocol = IPPROTO_TCP; - params->ifindex = ifindex; + params->ifindex = test->iif ? if_nametoindex(test->iif) : ifindex; params->tbid = test->tbid; params->mark = test->mark; + params->tot_len = test->tot_len; + + /* h_vlan_proto/h_vlan_TCI union with tbid */ + if (test->lookup_flags & BPF_FIB_LOOKUP_VLAN_INPUT) { + params->h_vlan_proto = htons(test->vlan_proto); + params->h_vlan_TCI = htons(test->vlan_id); + } if (inet_pton(AF_INET6, test->daddr, params->ipv6_dst) == 1) { params->family = AF_INET6; @@ -352,6 +702,21 @@ void test_fib_lookup(void) if (tests[i].expected_dst) assert_dst_ip(fib_params, tests[i].expected_dst); + if (tests[i].expected_dev) + ASSERT_EQ(fib_params->ifindex, + if_nametoindex(tests[i].expected_dev), "ifindex"); + + if (tests[i].expected_mtu) + ASSERT_EQ(fib_params->mtu_result, tests[i].expected_mtu, + "mtu_result"); + + if (tests[i].check_vlan) { + ASSERT_EQ(fib_params->h_vlan_proto, + htons(tests[i].vlan_proto), "h_vlan_proto"); + ASSERT_EQ(fib_params->h_vlan_TCI, + htons(tests[i].vlan_id), "h_vlan_TCI"); + } + ret = memcmp(tests[i].dmac, fib_params->dmac, sizeof(tests[i].dmac)); if (!ASSERT_EQ(ret, 0, "dmac not match")) { char expected[18], actual[18]; @@ -361,8 +726,12 @@ void test_fib_lookup(void) printf("dmac expected %s actual %s ", expected, actual); } - // ensure tbid is zero'd out after fib lookup. - if (tests[i].lookup_flags & BPF_FIB_LOOKUP_DIRECT) { + /* ensure tbid is zero'd out after fib lookup. With + * BPF_FIB_LOOKUP_VLAN the union holds the packed vlan + * fields instead, so skip the check for those. + */ + if ((tests[i].lookup_flags & BPF_FIB_LOOKUP_DIRECT) && + !(tests[i].lookup_flags & BPF_FIB_LOOKUP_VLAN)) { if (!ASSERT_EQ(skel->bss->fib_params.tbid, 0, "expected fib_params.tbid to be zero")) goto fail; @@ -375,3 +744,122 @@ void test_fib_lookup(void) SYS_NOFAIL("ip netns del " NS_TEST); fib_lookup__destroy(skel); } + +#define NS_VLAN_A "fib_lookup_vlan_ns_a" +#define NS_VLAN_B "fib_lookup_vlan_ns_b" + +/* A VLAN device can be moved to another netns while staying registered + * on its parent. Neither direction may then cross the boundary: the + * egress flag must not publish the foreign parent's ifindex, and the + * input flag must fail closed rather than use a foreign ingress. + */ +void test_fib_lookup_vlan_netns(void) +{ + struct bpf_fib_lookup *fib_params; + struct nstoken *nstoken = NULL; + struct __sk_buff skb = { }; + struct fib_lookup *skel = NULL; + int prog_fd, err, parent_idx, vlan_idx; + + LIBBPF_OPTS(bpf_test_run_opts, run_opts, + .data_in = &pkt_v6, + .data_size_in = sizeof(pkt_v6), + .ctx_in = &skb, + .ctx_size_in = sizeof(skb), + ); + + skel = fib_lookup__open_and_load(); + if (!ASSERT_OK_PTR(skel, "skel open_and_load")) + return; + prog_fd = bpf_program__fd(skel->progs.fib_lookup); + fib_params = &skel->bss->fib_params; + + SYS(fail, "ip netns add %s", NS_VLAN_A); + SYS(fail, "ip netns add %s", NS_VLAN_B); + + nstoken = open_netns(NS_VLAN_A); + if (!ASSERT_OK_PTR(nstoken, "open_netns(a)")) + goto fail; + + SYS(fail, "ip link add veth7 type veth peer name veth8"); + SYS(fail, "ip link set dev veth7 up"); + SYS(fail, "ip link add link veth7 name veth7.66 type vlan id 66"); + SYS(fail, "ip link set veth7.66 netns %s", NS_VLAN_B); + + parent_idx = if_nametoindex("veth7"); + if (!ASSERT_NEQ(parent_idx, 0, "if_nametoindex(veth7)")) + goto fail; + + /* input: the moved device is still in veth7's VLAN group, but it + * lives in another netns, so the lookup must fail closed + */ + skb.ifindex = parent_idx; + memset(fib_params, 0, sizeof(*fib_params)); + fib_params->family = AF_INET; + fib_params->l4_protocol = IPPROTO_TCP; + fib_params->ifindex = parent_idx; + fib_params->h_vlan_proto = htons(ETH_P_8021Q); + fib_params->h_vlan_TCI = htons(66); + if (!ASSERT_EQ(inet_pton(AF_INET, "10.66.0.2", &fib_params->ipv4_dst), + 1, "inet_pton(dst)")) + goto fail; + + skel->bss->fib_lookup_ret = -1; + skel->bss->lookup_flags = BPF_FIB_LOOKUP_VLAN_INPUT | + BPF_FIB_LOOKUP_SKIP_NEIGH; + err = bpf_prog_test_run_opts(prog_fd, &run_opts); + if (!ASSERT_OK(err, "test_run(input)")) + goto fail; + ASSERT_EQ(skel->bss->fib_lookup_ret, BPF_FIB_LKUP_RET_NOT_FWDED, + "input across netns fails closed"); + ASSERT_EQ(fib_params->ifindex, parent_idx, "ifindex untouched"); + ASSERT_EQ(fib_params->h_vlan_TCI, htons(66), "tag untouched"); + + close_netns(nstoken); + nstoken = open_netns(NS_VLAN_B); + if (!ASSERT_OK_PTR(nstoken, "open_netns(b)")) + goto fail; + + /* egress: the fib result is the VLAN device here, but its parent + * is in the other netns, so the swap must not happen + */ + SYS(fail, "ip link set dev veth7.66 up"); + SYS(fail, "ip addr add 10.66.0.1/24 dev veth7.66"); + err = write_sysctl("/proc/sys/net/ipv4/conf/veth7.66/forwarding", "1"); + if (!ASSERT_OK(err, "write_sysctl(forwarding)")) + goto fail; + + vlan_idx = if_nametoindex("veth7.66"); + if (!ASSERT_NEQ(vlan_idx, 0, "if_nametoindex(veth7.66)")) + goto fail; + + skb.ifindex = vlan_idx; + memset(fib_params, 0, sizeof(*fib_params)); + fib_params->family = AF_INET; + fib_params->l4_protocol = IPPROTO_TCP; + fib_params->ifindex = vlan_idx; + if (!ASSERT_EQ(inet_pton(AF_INET, "10.66.0.2", &fib_params->ipv4_dst), + 1, "inet_pton(dst)") || + !ASSERT_EQ(inet_pton(AF_INET, "10.66.0.1", &fib_params->ipv4_src), + 1, "inet_pton(src)")) + goto fail; + + skel->bss->fib_lookup_ret = -1; + skel->bss->lookup_flags = BPF_FIB_LOOKUP_VLAN | + BPF_FIB_LOOKUP_SKIP_NEIGH; + err = bpf_prog_test_run_opts(prog_fd, &run_opts); + if (!ASSERT_OK(err, "test_run(egress)")) + goto fail; + ASSERT_EQ(skel->bss->fib_lookup_ret, BPF_FIB_LKUP_RET_SUCCESS, + "egress lookup succeeds"); + ASSERT_EQ(fib_params->ifindex, vlan_idx, + "foreign parent not published"); + ASSERT_EQ(fib_params->h_vlan_TCI, 0, "vlan fields zero"); + +fail: + if (nstoken) + close_netns(nstoken); + SYS_NOFAIL("ip netns del " NS_VLAN_A); + SYS_NOFAIL("ip netns del " NS_VLAN_B); + fib_lookup__destroy(skel); +} -- 2.54.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests 2026-06-16 22:34 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests Avinash Duduskar @ 2026-06-16 22:40 ` sashiko-bot 0 siblings, 0 replies; 10+ messages in thread From: sashiko-bot @ 2026-06-16 22:40 UTC (permalink / raw) To: Avinash Duduskar; +Cc: bpf Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Multi-line comments do not follow the mandatory BPF subsystem modern comment style. -- commit ce4915a9efe53d10db6f263bbf7bb1a929ea9dbd Author: Avinash Duduskar <avinash.duduskar@gmail.com> selftests/bpf: Add bpf_fib_lookup() VLAN flag tests This commit adds extensive test coverage for the new BPF_FIB_LOOKUP_VLAN and BPF_FIB_LOOKUP_VLAN_INPUT flags. It introduces 36 table cases and a new cross-netns subtest to verify both the egress and input direction logic. > diff --git a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c > --- a/tools/testing/selftests/bpf/prog_tests/fib_lookup.c > +++ b/tools/testing/selftests/bpf/prog_tests/fib_lookup.c [ ... ] > @@ -52,6 +88,16 @@ struct fib_lookup_test { > __u32 tbid; > __u8 dmac[6]; > __u32 mark; > + /* input tag with BPF_FIB_LOOKUP_VLAN_INPUT; expected output tag > + * with BPF_FIB_LOOKUP_VLAN (checked when check_vlan is set) > + */ [Severity: Low] This isn't a bug, but does this follow the required BPF multi-line comment style? The BPF subsystem prefers having the opening /* on its own line, with the comment text starting on the next line. [ ... ] > @@ -204,6 +448,105 @@ static int setup_netns(void) > SYS(fail, "ip rule add prio 2 fwmark %d lookup %s", MARK, MARK_TABLE); > SYS(fail, "ip -6 rule add prio 2 fwmark %d lookup %s", MARK, MARK_TABLE); > > + /* Setup for vlan tests: a subinterface for egress resolution and > + * tag-as-input, a QinQ stack, and an iif rule so the input tests > + * observe which device the lookup used as ingress. > + */ [Severity: Low] This isn't a bug, but could we also adjust this block in setup_netns() to place the opening /* on an empty line? There are several other multi-line comments introduced in this patch that follow this older pattern. [ ... ] > +#define NS_VLAN_A "fib_lookup_vlan_ns_a" > +#define NS_VLAN_B "fib_lookup_vlan_ns_b" > + > +/* A VLAN device can be moved to another netns while staying registered > + * on its parent. Neither direction may then cross the boundary: the > + * egress flag must not publish the foreign parent's ifindex, and the > + * input flag must fail closed rather than use a foreign ingress. > + */ > +void test_fib_lookup_vlan_netns(void) [Severity: Low] This isn't a bug, but could we apply the modern BPF multi-line comment style here as well before test_fib_lookup_vlan_netns() is declared? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260616223426.3568080-1-avinash.duduskar@gmail.com?part=4 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-06-17 9:43 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-16 22:34 [PATCH bpf-next v2 0/4] bpf: bidirectional VLAN support for bpf_fib_lookup() Avinash Duduskar 2026-06-16 22:34 ` [PATCH bpf-next v2 1/4] bpf: Initialize the l3mdev field for the fib lookup flow Avinash Duduskar 2026-06-17 9:06 ` Toke Høiland-Jørgensen 2026-06-16 22:34 ` [PATCH bpf-next v2 2/4] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper Avinash Duduskar 2026-06-16 22:47 ` sashiko-bot 2026-06-17 9:26 ` Toke Høiland-Jørgensen 2026-06-16 22:34 ` [PATCH bpf-next v2 3/4] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT " Avinash Duduskar 2026-06-17 9:42 ` Toke Høiland-Jørgensen 2026-06-16 22:34 ` [PATCH bpf-next v2 4/4] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests Avinash Duduskar 2026-06-16 22:40 ` sashiko-bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox