* Re: [PATCH bpf-next v3 01/15] bpf: Remove __rcu tagging in st_link->map
From: Emil Tsalapatis @ 2026-07-13 19:02 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, shakeel.butt, roman.gushchin, kuniyu, kerneljasonxing,
kernel-team
In-Reply-To: <20260706171918.317102-2-ameryhung@gmail.com>
On Mon, Jul 6, 2026 at 2:59 PM Amery Hung <ameryhung@gmail.com> wrote:
>
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> st_link->map is always written under update_mutex. The paths that read
> st_link->map with rcu_read_lock() are not in the fast path, so they can
> simply take update_mutex instead. Remove the __rcu annotation and replace
> all RCU accessors with direct pointer reads under update_mutex. Use
> READ_ONCE() in bpf_struct_ops_map_link_poll() which reads the pointer
> without holding update_mutex.
>
> It is a simplification change.
>
> Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
>
> kernel/bpf/bpf_struct_ops.c | 29 ++++++++++++++---------------
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 51b16e5f5534..d06b3d9bcc13 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -57,7 +57,7 @@ struct bpf_struct_ops_map {
>
> struct bpf_struct_ops_link {
> struct bpf_link link;
> - struct bpf_map __rcu *map;
> + struct bpf_map *map;
> wait_queue_head_t wait_hup;
> };
>
> @@ -1257,8 +1257,7 @@ static void bpf_struct_ops_map_link_dealloc(struct bpf_link *link)
> struct bpf_struct_ops_map *st_map;
>
> st_link = container_of(link, struct bpf_struct_ops_link, link);
> - st_map = (struct bpf_struct_ops_map *)
> - rcu_dereference_protected(st_link->map, true);
> + st_map = (struct bpf_struct_ops_map *)st_link->map;
> if (st_map) {
> st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data, link);
> bpf_map_put(&st_map->map);
> @@ -1273,11 +1272,11 @@ static void bpf_struct_ops_map_link_show_fdinfo(const struct bpf_link *link,
> struct bpf_map *map;
>
> st_link = container_of(link, struct bpf_struct_ops_link, link);
> - rcu_read_lock();
> - map = rcu_dereference(st_link->map);
> + mutex_lock(&update_mutex);
> + map = st_link->map;
> if (map)
> seq_printf(seq, "map_id:\t%d\n", map->id);
> - rcu_read_unlock();
> + mutex_unlock(&update_mutex);
> }
>
> static int bpf_struct_ops_map_link_fill_link_info(const struct bpf_link *link,
> @@ -1287,11 +1286,11 @@ static int bpf_struct_ops_map_link_fill_link_info(const struct bpf_link *link,
> struct bpf_map *map;
>
> st_link = container_of(link, struct bpf_struct_ops_link, link);
> - rcu_read_lock();
> - map = rcu_dereference(st_link->map);
> + mutex_lock(&update_mutex);
> + map = st_link->map;
> if (map)
> info->struct_ops.map_id = map->id;
> - rcu_read_unlock();
> + mutex_unlock(&update_mutex);
> return 0;
> }
>
> @@ -1314,7 +1313,7 @@ static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct bpf_map
>
> mutex_lock(&update_mutex);
>
> - old_map = rcu_dereference_protected(st_link->map, lockdep_is_held(&update_mutex));
> + old_map = st_link->map;
> if (!old_map) {
> err = -ENOLINK;
> goto err_out;
> @@ -1336,7 +1335,7 @@ static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct bpf_map
> goto err_out;
>
> bpf_map_inc(new_map);
> - rcu_assign_pointer(st_link->map, new_map);
> + WRITE_ONCE(st_link->map, new_map);
> bpf_map_put(old_map);
>
> err_out:
> @@ -1353,7 +1352,7 @@ static int bpf_struct_ops_map_link_detach(struct bpf_link *link)
>
> mutex_lock(&update_mutex);
>
> - map = rcu_dereference_protected(st_link->map, lockdep_is_held(&update_mutex));
> + map = st_link->map;
> if (!map) {
> mutex_unlock(&update_mutex);
> return 0;
> @@ -1362,7 +1361,7 @@ static int bpf_struct_ops_map_link_detach(struct bpf_link *link)
>
> st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data, link);
>
> - RCU_INIT_POINTER(st_link->map, NULL);
> + WRITE_ONCE(st_link->map, NULL);
> /* Pair with bpf_map_get() in bpf_struct_ops_link_create() or
> * bpf_map_inc() in bpf_struct_ops_map_link_update().
> */
> @@ -1382,7 +1381,7 @@ static __poll_t bpf_struct_ops_map_link_poll(struct file *file,
>
> poll_wait(file, &st_link->wait_hup, pts);
>
> - return rcu_access_pointer(st_link->map) ? 0 : EPOLLHUP;
> + return READ_ONCE(st_link->map) ? 0 : EPOLLHUP;
> }
>
> static const struct bpf_link_ops bpf_struct_ops_map_lops = {
> @@ -1438,7 +1437,7 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
> link = NULL;
> goto err_out;
> }
> - RCU_INIT_POINTER(link->map, map);
> + link->map = map;
> mutex_unlock(&update_mutex);
>
> return bpf_link_settle(&link_primer);
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 02/15] bpf: Make struct_ops tasks_rcu grace period optional
From: Emil Tsalapatis @ 2026-07-13 19:01 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, shakeel.butt, roman.gushchin, kuniyu, kerneljasonxing,
kernel-team
In-Reply-To: <20260706171918.317102-3-ameryhung@gmail.com>
On Mon Jul 6, 2026 at 1:19 PM EDT, Amery Hung wrote:
> From: Martin KaFai Lau <martin.lau@kernel.org>
>
> bpf_struct_ops_map_free() currently waits for both a regular RCU grace
> period and a tasks RCU grace period for every struct_ops map through
> synchronize_rcu_mult(call_rcu, call_rcu_tasks).
>
> A regular RCU grace period is still required for all struct_ops maps
> because the struct_ops trampoline ksyms requires a rcu grace period
> (take a look at the list_del_rcu in __bpf_ksym_del).
> Add a map_free_pre_rcu() callback so the struct_ops map can remove
> ksyms before bpf_map_put() wait for the regular rcu grace period.
>
> The tasks RCU grace period is only needed by tcp_congestion_ops.
> Add free_after_tasks_rcu_gp only to struct bpf_struct_ops instead
> of the bpf_map.
>
> When CONFIG_TASKS_RCU=n, synchronize_rcu_tasks() is the same as
> synchronize_rcu(). Since all struct_ops maps now complete a regular RCU
> grace period before bpf_struct_ops_map_free() runs, skip the extra
> synchronize_rcu_tasks() call in this case.
>
> This cleanup prepares for a later patch that needs to support
> free_after_mult_rcu_gp.
>
> Reviewed-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
The Sashiko issue looks spurious to me. bpf_prog_dissoc_progs() is not
related to the availability of the struct_ops methods. It is valid for
it to be after the end of the grace period.
> ---
> include/linux/bpf.h | 7 +++++++
> kernel/bpf/bpf_struct_ops.c | 31 +++++++++++++------------------
> kernel/bpf/syscall.c | 3 +++
> net/ipv4/bpf_tcp_ca.c | 16 ++++++++++++++++
> 4 files changed, 39 insertions(+), 18 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 7719f6528445..7ac8873839f4 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -90,6 +90,7 @@ struct bpf_map_ops {
> struct bpf_map *(*map_alloc)(union bpf_attr *attr);
> void (*map_release)(struct bpf_map *map, struct file *map_file);
> void (*map_free)(struct bpf_map *map);
> + void (*map_free_pre_rcu)(struct bpf_map *map);
> int (*map_get_next_key)(struct bpf_map *map, void *key, void *next_key);
> void (*map_release_uref)(struct bpf_map *map);
> void *(*map_lookup_elem_sys_only)(struct bpf_map *map, void *key);
> @@ -2099,6 +2100,11 @@ struct btf_member;
> * unloaded while in use.
> * @name: The name of the struct bpf_struct_ops object.
> * @func_models: Func models
> + * @free_after_tasks_rcu_gp: Set to true if it needs the bpf core to wait for
> + * a tasks_rcu gp before freeing the struct_ops map
> + * and its progs. It is unnecessary if the @unreg
> + * has waited for the correct rcu gp or the @unreg
> + * has ensured all struct_ops prog has finished running.
> */
> struct bpf_struct_ops {
> const struct bpf_verifier_ops *verifier_ops;
> @@ -2117,6 +2123,7 @@ struct bpf_struct_ops {
> struct module *owner;
> const char *name;
> struct btf_func_model func_models[BPF_STRUCT_OPS_MAX_NR_MEMBERS];
> + bool free_after_tasks_rcu_gp;
> };
>
> /* Every member of a struct_ops type has an instance even a member is not
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index d06b3d9bcc13..c422ce41873e 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -984,9 +984,18 @@ static void __bpf_struct_ops_map_free(struct bpf_map *map)
> bpf_map_area_free(st_map);
> }
>
> +static void bpf_struct_ops_map_free_pre_rcu(struct bpf_map *map)
> +{
> + struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
> +
> + bpf_struct_ops_map_del_ksyms(st_map);
> +}
> +
> static void bpf_struct_ops_map_free(struct bpf_map *map)
> {
> struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
> + struct bpf_struct_ops *st_ops = st_map->st_ops_desc->st_ops;
> + bool tasks_rcu = st_ops->free_after_tasks_rcu_gp;
>
> /* st_ops->owner was acquired during map_alloc to implicitly holds
> * the btf's refcnt. The acquire was only done when btf_is_module()
> @@ -997,24 +1006,8 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
>
> bpf_struct_ops_map_dissoc_progs(st_map);
>
> - bpf_struct_ops_map_del_ksyms(st_map);
> -
> - /* The struct_ops's function may switch to another struct_ops.
> - *
> - * For example, bpf_tcp_cc_x->init() may switch to
> - * another tcp_cc_y by calling
> - * setsockopt(TCP_CONGESTION, "tcp_cc_y").
> - * During the switch, bpf_struct_ops_put(tcp_cc_x) is called
> - * and its refcount may reach 0 which then free its
> - * trampoline image while tcp_cc_x is still running.
> - *
> - * A vanilla rcu gp is to wait for all bpf-tcp-cc prog
> - * to finish. bpf-tcp-cc prog is non sleepable.
> - * A rcu_tasks gp is to wait for the last few insn
> - * in the tramopline image to finish before releasing
> - * the trampoline image.
> - */
> - synchronize_rcu_mult(call_rcu, call_rcu_tasks);
> + if (tasks_rcu && IS_ENABLED(CONFIG_TASKS_RCU))
> + synchronize_rcu_tasks();
>
> __bpf_struct_ops_map_free(map);
> }
> @@ -1123,6 +1116,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
>
> mutex_init(&st_map->lock);
> bpf_map_init_from_attr(map, attr);
> + map->free_after_rcu_gp = true;
>
> return map;
>
> @@ -1155,6 +1149,7 @@ const struct bpf_map_ops bpf_struct_ops_map_ops = {
> .map_alloc_check = bpf_struct_ops_map_alloc_check,
> .map_alloc = bpf_struct_ops_map_alloc,
> .map_free = bpf_struct_ops_map_free,
> + .map_free_pre_rcu = bpf_struct_ops_map_free_pre_rcu,
> .map_get_next_key = bpf_struct_ops_map_get_next_key,
> .map_lookup_elem = bpf_struct_ops_map_lookup_elem,
> .map_delete_elem = bpf_struct_ops_map_delete_elem,
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6db306d23b47..b07acf37ad1d 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -956,6 +956,9 @@ void bpf_map_put(struct bpf_map *map)
> /* bpf_map_free_id() must be called first */
> bpf_map_free_id(map);
>
> + if (map->ops->map_free_pre_rcu)
> + map->ops->map_free_pre_rcu(map);
> +
> WARN_ON_ONCE(atomic64_read(&map->sleepable_refcnt));
> /* RCU tasks trace grace period implies RCU grace period. */
> if (READ_ONCE(map->free_after_mult_rcu_gp))
> diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
> index 791e15063237..e224ecafbd69 100644
> --- a/net/ipv4/bpf_tcp_ca.c
> +++ b/net/ipv4/bpf_tcp_ca.c
> @@ -339,6 +339,22 @@ static struct bpf_struct_ops bpf_tcp_congestion_ops = {
> .validate = bpf_tcp_ca_validate,
> .name = "tcp_congestion_ops",
> .cfi_stubs = &__bpf_ops_tcp_congestion_ops,
> + /* The struct_ops's function may switch to another struct_ops.
> + *
> + * For example, bpf_tcp_cc_x->init() may switch to
> + * another tcp_cc_y by calling
> + * setsockopt(TCP_CONGESTION, "tcp_cc_y").
> + * During the switch, bpf_struct_ops_put(tcp_cc_x) is called
> + * and its refcount may reach 0 which then free its
> + * trampoline image while tcp_cc_x is still running.
> + *
> + * A vanilla rcu gp is to wait for all bpf-tcp-cc prog
> + * to finish. bpf-tcp-cc prog is non sleepable.
> + * A rcu_tasks gp is to wait for the last few insn
> + * in the tramopline image to finish before releasing
> + * the trampoline image.
> + */
> + .free_after_tasks_rcu_gp = true,
> .owner = THIS_MODULE,
> };
>
^ permalink raw reply
* Re: [PATCH v9 12/14] wifi: ath12k: Switch to generic PAS TZ APIs
From: Jeff Johnson @ 2026-07-13 18:58 UTC (permalink / raw)
To: Sumit Garg, andersson, konradybcio
Cc: linux-arm-msm, devicetree, dri-devel, freedreno, linux-media,
netdev, linux-wireless, ath12k, linux-remoteproc, robh, krzk+dt,
conor+dt, robin.clark, sean, akhilpo, lumag, abhinav.kumar,
jesszhan0024, marijn.suijten, airlied, simona, vikash.garodia,
bod, mchehab, elder, andrew+netdev, davem, edumazet, kuba, pabeni,
jjohnson, mathieu.poirier, trilokkumar.soni, mukesh.ojha,
pavan.kondeti, jorge.ramirez, tonyh, vignesh.viswanathan,
srinivas.kandagatla, amirreza.zarrabi, jenswi, op-tee, apurupa,
skare, linux-kernel, Sumit Garg
In-Reply-To: <20260702115835.167602-13-sumit.garg@kernel.org>
On 7/2/2026 4:58 AM, Sumit Garg wrote:
> From: Sumit Garg <sumit.garg@oss.qualcomm.com>
>
> Switch ath12k client driver over to generic PAS TZ APIs. Generic PAS TZ
> service allows to support multiple TZ implementation backends like QTEE
> based SCM PAS service, OP-TEE based PAS service and any further future TZ
> backend service.
>
> Acked-by: Jeff Johnson <jjohnson@kernel.org>
> Signed-off-by: Sumit Garg <sumit.garg@oss.qualcomm.com>
> ---
> drivers/net/wireless/ath/ath12k/Kconfig | 2 +-
> drivers/net/wireless/ath/ath12k/ahb.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/Kconfig b/drivers/net/wireless/ath/ath12k/Kconfig
> index 4a2b240f967a..0d5d1c55bfc1 100644
> --- a/drivers/net/wireless/ath/ath12k/Kconfig
> +++ b/drivers/net/wireless/ath/ath12k/Kconfig
> @@ -18,7 +18,7 @@ config ATH12K_AHB
> bool "Qualcomm ath12k AHB support"
> depends on ATH12K && REMOTEPROC
> select QCOM_MDT_LOADER
> - select QCOM_SCM
> + select QCOM_PAS
> help
> Enable support for Ath12k AHB bus chipsets, example IPQ5332.
>
> diff --git a/drivers/net/wireless/ath/ath12k/ahb.c b/drivers/net/wireless/ath/ath12k/ahb.c
> index 30733a244454..69e21214e629 100644
> --- a/drivers/net/wireless/ath/ath12k/ahb.c
> +++ b/drivers/net/wireless/ath/ath12k/ahb.c
> @@ -5,7 +5,7 @@
> */
>
> #include <linux/dma-mapping.h>
> -#include <linux/firmware/qcom/qcom_scm.h>
> +#include <linux/firmware/qcom/qcom_pas.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> @@ -420,7 +420,7 @@ static int ath12k_ahb_power_up(struct ath12k_base *ab)
>
> if (ab_ahb->scm_auth_enabled) {
> /* Authenticate FW image using peripheral ID */
> - ret = qcom_scm_pas_auth_and_reset(pasid);
> + ret = qcom_pas_auth_and_reset(pasid);
> if (ret) {
> ath12k_err(ab, "failed to boot the remote processor %d\n", ret);
> goto err_fw2;
> @@ -485,10 +485,10 @@ static void ath12k_ahb_power_down(struct ath12k_base *ab, bool is_suspend)
> pasid = (u32_encode_bits(ab_ahb->userpd_id, ATH12K_USERPD_ID_MASK)) |
> ATH12K_AHB_UPD_SWID;
> /* Release the firmware */
> - ret = qcom_scm_pas_shutdown(pasid);
> + ret = qcom_pas_shutdown(pasid);
> if (ret)
> - ath12k_err(ab, "scm pas shutdown failed for userPD%d\n",
> - ab_ahb->userpd_id);
> + ath12k_err(ab, "PAS shutdown failed for userPD%d: %d\n",
> + ab_ahb->userpd_id, ret);
> }
> }
>
My code review agent is flagging:
**Missing probe-defer guard** (`ahb.c:422`) — `qcom_pas_is_available()` is
explicitly documented as mandatory before any PAS call. The OP-TEE backend
registers its ops asynchronously; without an `if (!qcom_pas_is_available())
return -EPROBE_DEFER` in the probe path, firmware auth silently returns
`-ENODEV` with no retry.
Is it an existing deficiency in ath12k that there is no probe deferral?
Or did the qcom_scm_*() calls somehow guarantee something that is no longer
true with the qcom_pas_*() calls?
And also for future cleanup:
**Misleading field name** (`ahb.c:384`) — `scm_auth_enabled` should be
`pas_auth_enabled` to match the backend-agnostic API it now guards.
I plan on taking this patch as-is through the ath tree since it is currently
just simple API changes. Any additional changes can come separately.
/jeff
^ permalink raw reply
* [PATCH nf] netfilter: nft_fib: bail out if input device is missing
From: Xiang Mei (Microsoft) @ 2026-07-13 18:36 UTC (permalink / raw)
To: Florian Westphal, Pablo Neira Ayuso, Phil Sutter,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: netfilter-devel, coreteam, netdev, linux-kernel,
AutonomousCodeSecurity, tgopinath, kys, Xiang Mei (Microsoft)
nft_fib_can_skip() dereferences the input device (indev->ifindex, and
in->flags via nft_fib_is_loopback()) without a NULL check, assuming the
hook switch only admits PRE_ROUTING/INGRESS/LOCAL_IN. But NF_NETDEV_EGRESS
== NF_INET_LOCAL_IN == 1, so a netdev-family base chain on the egress hook
passes both the switch and nft_fib_validate() (which also keys only on the
hook number). Egress packets have no input device, so nft_fib_can_skip()
dereferences NULL.
KASAN: null-ptr-deref in range [0x00000000000000b0-0x00000000000000b7]
RIP: 0010:nft_fib4_eval (net/netfilter/nft_fib.h:19)
nft_do_chain (net/netfilter/nf_tables_core.c:285)
nft_do_chain_netdev (net/netfilter/nft_chain_filter.c:307)
nf_hook_slow (net/netfilter/core.c:619)
__dev_queue_xmit (net/core/dev.c:4799)
...
Kernel panic - not syncing: Fatal exception in interrupt
The eval path touched the device only via l3mdev_master_ifindex_rcu(),
which tolerates NULL, until
commit eaaff9b6702e ("netfilter: fib: avoid lookup if socket is available")
added the sk/indev dereference. Restore the old behaviour by returning
early when indev is NULL, so the packet takes the regular FIB lookup.
Receive-side hooks always have an input device and are unaffected.
Fixes: eaaff9b6702e ("netfilter: fib: avoid lookup if socket is available")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
---
include/net/netfilter/nft_fib.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/net/netfilter/nft_fib.h b/include/net/netfilter/nft_fib.h
index e0422456f27b..d53e5a5214fe 100644
--- a/include/net/netfilter/nft_fib.h
+++ b/include/net/netfilter/nft_fib.h
@@ -33,6 +33,9 @@ static inline bool nft_fib_can_skip(const struct nft_pktinfo *pkt)
return false;
}
+ if (!indev)
+ return false;
+
sk = pkt->skb->sk;
if (sk && sk_fullsock(sk))
return sk->sk_rx_dst_ifindex == indev->ifindex;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v4 00/15][pull request] Introduce iXD driver
From: Larysa Zaremba @ 2026-07-13 18:29 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
przemyslaw.kitszel, aleksander.lobakin, sridhar.samudrala,
michal.swiatkowski, maciej.fijalkowski, emil.s.tantilov,
madhu.chittim, joshua.a.hay, jacob.e.keller,
jayaprakash.shanmugam, jiri, horms, corbet, richardcochran,
linux-doc
In-Reply-To: <20260710215313.1475803-1-anthony.l.nguyen@intel.com>
I have addressed Sashiko's feedback for each patch.
Those are the only 2 important changes that came out of that.
Please, notice that the first one is not a functional regression, but a memory
usage issue and I tested the change.
commit d7a772efb77228a1b12558dced6b1fdfeef123c4
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Mon Jul 13 17:02:01 2026 +0200
fixup! libie: add bookkeeping support for control queue messages
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index c043c07dbb89..8e7b2783b86b 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -720,7 +720,7 @@ static int libie_ctlq_xn_init_dma(struct device *dev,
goto dealloc_dma;
dma_mem->va = libie_cp_alloc_dma_mem(dev, dma_mem,
- LIBIE_CTLQ_MAX_BUF_LEN);
+ LIBIE_CP_TX_COPYBREAK);
if (!dma_mem->va) {
kfree(dma_mem);
goto dealloc_dma;
commit 740e3b2dc9fab1ad24ee7fa1420d0c7025bc89ac
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Mon Jul 13 20:25:21 2026 +0200
fixup! libie: add control queue support
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index 0392608856c8..c043c07dbb89 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -509,6 +509,7 @@ void libie_ctlq_send(struct libie_ctlq_info *ctlq, u32 num_q_msg)
if (unlikely(++ntu == ctlq->ring_len))
ntu = 0;
}
+ dma_wmb();
writel(ntu, ctlq->reg.tail);
ctlq->next_to_use = ntu;
}
Other changes are of much lesser importance. Look at particular patches for
details on nice-to-haves and false positives. Here is overall nice-to-have diff:
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 5a191644b28e..184d30c12abb 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -265,8 +265,9 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err = idpf_dev_init(adapter, ent);
if (err) {
- dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
- ent->device);
+ dev_err(&pdev->dev,
+ "Failed to initialize device (ID 0x%x): %d\n",
+ ent->device, err);
goto err_free;
}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 04a0421c1b77..d768b63700c8 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -2931,6 +2931,8 @@ int idpf_init_dflt_mbx(struct idpf_adapter *adapter)
adapter->arq = libie_find_ctlq(ctx, LIBIE_CTLQ_TYPE_RX,
LIBIE_CTLQ_MBX_ID);
if (!adapter->asq || !adapter->arq) {
+ adapter->asq = NULL;
+ adapter->arq = NULL;
libie_ctlq_xn_deinit(params.xnm, ctx);
return -ENOENT;
}
@@ -3993,7 +3995,7 @@ int idpf_set_promiscuous(struct idpf_adapter *adapter,
* @send_msg: message to send
* @msg_size: size of message to send
* @recv_msg: message to populate on reception of response
- * @recv_len: length of message copied into recv_msg or 0 on error
+ * @recv_len: length of message copied into recv_msg
*
* Return: 0 on success or error code on failure.
*/
diff --git a/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c b/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c
index 66049d1b1d15..5b04769443b9 100644
--- a/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c
+++ b/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c
@@ -120,8 +120,11 @@ static int ixd_handle_vc_ver(struct ixd_adapter *adapter, void *recv_buff,
return -EBADMSG;
recv_ver = recv_buff;
- if (le32_to_cpu(need_ver.major) > le32_to_cpu(recv_ver->major))
+ if (le32_to_cpu(need_ver.major) != le32_to_cpu(recv_ver->major))
return -EOPNOTSUPP;
+ if (le32_to_cpu(recv_ver->minor) != le32_to_cpu(need_ver.minor))
+ dev_warn(ixd_to_dev(adapter),
+ "Virtchnl minor version does not match, proceed with caution\n");
adapter->vc_ver.major = le32_to_cpu(recv_ver->major);
adapter->vc_ver.minor = le32_to_cpu(recv_ver->minor);
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index 885b4437b4f0..8e7b2783b86b 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -327,7 +327,8 @@ libie_ctlq_add(struct libie_ctlq_ctx *ctx,
{
struct libie_ctlq_info *ctlq;
- if (qinfo->id != LIBIE_CTLQ_MBX_ID)
+ if (qinfo->id != LIBIE_CTLQ_MBX_ID ||
+ qinfo->len > FIELD_MAX(LIBIE_CTLQ_MBX_ATQ_LEN))
return ERR_PTR(-EOPNOTSUPP);
/* libie_ctlq_init was not called */
@@ -493,8 +494,6 @@ EXPORT_SYMBOL_NS_GPL(libie_ctlq_send_desc_avail, "LIBIE_CP");
* The caller must hold ctlq->lock. The intended pattern is to first check
* the number of descriptors available, then fill in the messages and perform
* send within a single critical section.
- *
- * Return: %0 on success, -%errno on failure.
*/
void libie_ctlq_send(struct libie_ctlq_info *ctlq, u32 num_q_msg)
{
^ permalink raw reply related
* Re: [PATCH net-next v4 14/15] ixd: add the core initialization
From: Larysa Zaremba @ 2026-07-13 18:13 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
przemyslaw.kitszel, aleksander.lobakin, sridhar.samudrala,
michal.swiatkowski, maciej.fijalkowski, emil.s.tantilov,
madhu.chittim, joshua.a.hay, jacob.e.keller,
jayaprakash.shanmugam, jiri, horms, corbet, richardcochran,
linux-doc, Bharath R
In-Reply-To: <20260710215313.1475803-15-anthony.l.nguyen@intel.com>
Sashiko expressed concerns with virtchnl version handling in ixd. I think it is
fine as-is, but here is how it can be addressed:
commit 736215457094a287aa8f40af322e0bc7ab94dddc
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Mon Jul 13 15:44:55 2026 +0200
fixup! ixd: add the core initialization
diff --git a/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c b/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c
index 66049d1b1d15..5b04769443b9 100644
--- a/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c
+++ b/drivers/net/ethernet/intel/ixd/ixd_virtchnl.c
@@ -120,8 +120,11 @@ static int ixd_handle_vc_ver(struct ixd_adapter *adapter, void *recv_buff,
return -EBADMSG;
recv_ver = recv_buff;
- if (le32_to_cpu(need_ver.major) > le32_to_cpu(recv_ver->major))
+ if (le32_to_cpu(need_ver.major) != le32_to_cpu(recv_ver->major))
return -EOPNOTSUPP;
+ if (le32_to_cpu(recv_ver->minor) != le32_to_cpu(need_ver.minor))
+ dev_warn(ixd_to_dev(adapter),
+ "Virtchnl minor version does not match, proceed with caution\n");
adapter->vc_ver.major = le32_to_cpu(recv_ver->major);
adapter->vc_ver.minor = le32_to_cpu(recv_ver->minor);
Everything else:
[...]
> > + libie_ctlq_xn_shutdown(adapter->xnm);
> > + ixd_trigger_reset(adapter);
> > + ixd_deinit_dflt_mbx(adapter);
> Could this sequence trigger a race condition causing PCIe AER fatal errors?
> The hardware reset is triggered here before the mailbox polling task
> (mbx_task) is canceled inside ixd_deinit_dflt_mbx().
> If mbx_task runs concurrently with or immediately after the reset trigger,
> it might access MMIO doorbell registers while the device is actively in a
> global reset.
>
Not an issue with available HW
[...]
> > ixd_trigger_reset(adapter);
> > ixd_deinit_dflt_mbx(adapter);
> Similarly to ixd_init_task(), does triggering the reset before
> ixd_deinit_dflt_mbx() here risk the mbx_task touching MMIO during reset?
> Perhaps the newly added polling task should be canceled before the hardware
> reset is triggered?
>
This is not an issue with available HW, and consistent with existing idpf flow.
[...]
> > +static int ixd_req_vc_caps(struct ixd_adapter *adapter)
> > +{
> > + const struct ixd_ctlq_req req = {
> > + .opcode = VIRTCHNL2_OP_GET_CAPS,
> > + .send_size = sizeof(struct virtchnl2_get_capabilities),
> > + .ctx = NULL,
> > + .send_buff_init = NULL,
> > + .recv_process = ixd_handle_caps,
> > + };
> > +
> > + return ixd_ctlq_do_req(adapter, &req);
> > +}
> Because send_buff_init is NULL, will the driver send an all-zero request
> payload for capabilities and resources?
> Requesting 0 max vectors and 0 RX queues might cause the Control Plane to
> respond with only the default mailbox vector, starving the driver of
> interrupts for its networking queues.
>
We do not currently have any queues
[...]
^ permalink raw reply related
* Re: [PATCH net-next v4 09/15] idpf: refactor idpf to use libie control queues
From: Larysa Zaremba @ 2026-07-13 17:55 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
Pavan Kumar Linga, przemyslaw.kitszel, aleksander.lobakin,
sridhar.samudrala, michal.swiatkowski, maciej.fijalkowski,
emil.s.tantilov, joshua.a.hay, jacob.e.keller,
jayaprakash.shanmugam, jiri, horms, corbet, richardcochran,
linux-doc, Aleksandr Loktionov, Samuel Salin
In-Reply-To: <20260710215313.1475803-10-anthony.l.nguyen@intel.com>
Here are some minor changes that came out of Sashiko's review.
The first one is not needed, but does make code more consistent and robust.
The second one is updating kdoc, where Sashiko was concerned about a functional
change. I have checked that the functional change itself does not affect
any callers.
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index 04a0421c1b77..d768b63700c8 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -2931,6 +2931,8 @@ int idpf_init_dflt_mbx(struct idpf_adapter *adapter)
adapter->arq = libie_find_ctlq(ctx, LIBIE_CTLQ_TYPE_RX,
LIBIE_CTLQ_MBX_ID);
if (!adapter->asq || !adapter->arq) {
+ adapter->asq = NULL;
+ adapter->arq = NULL;
libie_ctlq_xn_deinit(params.xnm, ctx);
return -ENOENT;
}
@@ -3993,7 +3995,7 @@ int idpf_set_promiscuous(struct idpf_adapter *adapter,
* @send_msg: message to send
* @msg_size: size of message to send
* @recv_msg: message to populate on reception of response
- * @recv_len: length of message copied into recv_msg or 0 on error
+ * @recv_len: length of message copied into recv_msg
*
* Return: 0 on success or error code on failure.
*/
Everything else:
> commit 9ebd6c429379be8477841f5180023c78d16732d0
> Author: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> idpf: refactor idpf to use libie control queues
> This refactors idpf to use the libie control-queue API, removing the
> in-tree idpf_controlq.[ch] and idpf_controlq_setup.c and reworking every
> virtchnl send/receive path to use libie_ctlq_xn_send_params /
> libie_ctlq_release_rx_buf(). A few observations below.
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > index 357ff3e86b2e..388075a07af7 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> [ ... ]
> > @@ -4128,9 +3731,9 @@ static void idpf_set_mac_type(const u8 *default_mac_addr,
> >
> > /**
> > * idpf_mac_filter_async_handler - Async callback for mac filters
> [ ... ]
> > - */
> > -static int idpf_mac_filter_async_handler(struct idpf_adapter *adapter,
> > - struct idpf_vc_xn *xn,
> > - const struct idpf_ctlq_msg *ctlq_msg)
> > +static void idpf_mac_filter_async_handler(void *ctx,
> > + struct kvec *buff,
> > + int status)
> > {
> > struct virtchnl2_mac_addr_list *ma_list;
> > struct idpf_vport_config *vport_config;
> > struct virtchnl2_mac_addr *mac_addr;
> > + struct idpf_adapter *adapter = ctx;
> > struct idpf_mac_filter *f, *tmp;
> > struct list_head *ma_list_head;
> > struct idpf_vport *vport;
> > @@ -4152,18 +3756,18 @@ static int idpf_mac_filter_async_handler(struct idpf_adapter *adapter,
> > int i;
> >
> > /* if success we're done, we're only here if something bad happened */
> > - if (!ctlq_msg->cookie.mbx.chnl_retval)
> > - return 0;
> > + if (!status || status == -ETIMEDOUT)
> > + return;
> Should -ETIMEDOUT really be treated the same as success here?
> In the old code the async handler only ran when a reply was received,
> so a timeout could not reach this callback. With libie the async path
> now delivers -ETIMEDOUT via libie_xn_check_async_timeout(), and this
> new short-circuit returns immediately without reconciling the driver's
> mac_filter_list with what the CP actually applied.
> Note that idpf_add_del_mac_filters() clears f->add / f->remove for each
> entry before sending:
> if (add && f->add) {
> ether_addr_copy(mac_addr[i].addr, f->macaddr);
> idpf_set_mac_type(default_mac_addr, &mac_addr[i]);
> i++;
> f->add = false;
> So on -ETIMEDOUT those entries look "applied" to the driver while the
> CP never acknowledged them. Would it be safer to only skip work when
> status == 0 and fall through to the list_del reconciliation on
> -ETIMEDOUT the same way as for any other error?
>
On -ETIMEDOUT we do not have any buffer to handle.
In this case it could be either added or not, we do not know.
Both options lead us into a suboptimal situation, but treating it
as a success is simpler. Also, 2 minute timeout probably means you
should reload something anyway.
[...]
> > int idpf_send_dealloc_vectors_msg(struct idpf_adapter *adapter)
> > {
> > struct virtchnl2_alloc_vectors *ac = adapter->req_vec_chunks;
> > - struct virtchnl2_vector_chunks *vcs = &ac->vchunks;
> > - struct idpf_vc_xn_params xn_params = {};
> > - ssize_t reply_sz;
> > - int buf_size;
> > + struct libie_ctlq_xn_send_params xn_params = {
> > + .timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
> > + .chnl_opcode = VIRTCHNL2_OP_DEALLOC_VECTORS,
> > + };
> > + struct virtchnl2_vector_chunks *vcs;
> > + int buf_size, err;
> >
> > - buf_size = struct_size(vcs, vchunks, le16_to_cpu(vcs->num_vchunks));
> > + buf_size = struct_size(&ac->vchunks, vchunks,
> > + le16_to_cpu(ac->vchunks.num_vchunks));
> > + vcs = kmemdup(&ac->vchunks, buf_size, GFP_KERNEL);
> > + if (!vcs)
> > + return -ENOMEM;
> >
> > - xn_params.vc_op = VIRTCHNL2_OP_DEALLOC_VECTORS;
> > - xn_params.send_buf.iov_base = vcs;
> > - xn_params.send_buf.iov_len = buf_size;
> > - xn_params.timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC;
> > - reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> > - if (reply_sz < 0)
> > - return reply_sz;
> > + err = idpf_send_mb_msg_kfree(adapter, &xn_params, vcs, buf_size);
> > + if (err)
> > + return err;
> >
> > kfree(adapter->req_vec_chunks);
> > adapter->req_vec_chunks = NULL;
> Does this code leak adapter->req_vec_chunks on the error path?
> On success the kfree()/NULL of adapter->req_vec_chunks runs after
> idpf_send_mb_msg_kfree(). On error the function returns err early
> while adapter->req_vec_chunks is still allocated.
> Its caller is idpf_intr_rel():
> void idpf_intr_rel(struct idpf_adapter *adapter)
> {
> ...
> idpf_send_dealloc_vectors_msg(adapter);
> idpf_deinit_vector_stack(adapter);
> kfree(adapter->msix_entries);
> adapter->msix_entries = NULL;
> kfree(adapter->rdma_msix_entries);
> adapter->rdma_msix_entries = NULL;
> }
> which does not free req_vec_chunks either. Is there another site
> that frees adapter->req_vec_chunks when the dealloc virtchnl fails
> (for example on a mailbox timeout during teardown), or does it
> accumulate across reset/unload attempts?
>
Seems so, but this is a preexistent issue
[...]
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> > index 8d8fb498e092..1d8dd67e299d 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl_ptp.c
> [ ... ]
> > @@ -234,23 +244,18 @@ int idpf_ptp_set_dev_clk_time(struct idpf_adapter *adapter, u64 time)
> > struct virtchnl2_ptp_set_dev_clk_time set_dev_clk_time_msg = {
> > .dev_time_ns = cpu_to_le64(time),
> > };
> > - struct idpf_vc_xn_params xn_params = {
> > - .vc_op = VIRTCHNL2_OP_PTP_SET_DEV_CLK_TIME,
> > - .send_buf.iov_base = &set_dev_clk_time_msg,
> > - .send_buf.iov_len = sizeof(set_dev_clk_time_msg),
> > - .recv_buf.iov_base = &set_dev_clk_time_msg,
> > - .recv_buf.iov_len = sizeof(set_dev_clk_time_msg),
> > + struct libie_ctlq_xn_send_params xn_params = {
> > + .chnl_opcode = VIRTCHNL2_OP_PTP_SET_DEV_CLK_TIME,
> > .timeout_ms = IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC,
> > };
> > - int reply_sz;
> > + int err;
> >
> > - reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> > - if (reply_sz < 0)
> > - return reply_sz;
> > - if (reply_sz != sizeof(set_dev_clk_time_msg))
> > - return -EIO;
> > + err = idpf_send_mb_msg(adapter, &xn_params, &set_dev_clk_time_msg,
> > + sizeof(set_dev_clk_time_msg));
> > + if (!err)
> > + libie_ctlq_release_rx_buf(&xn_params.recv_mem);
> >
> > - return 0;
> > + return err;
> > }
> Is dropping the reply-size check here intentional?
> The pre-patch code explicitly rejected a malformed reply:
> if (reply_sz != sizeof(set_dev_clk_time_msg))
> return -EIO;
> The refactored helper only surfaces the return code of
> idpf_send_mb_msg() and unconditionally releases the Rx buffer, so a
> short/truncated payload from the CP is silently accepted as success.
> The same reply-size check has also been dropped in
> idpf_ptp_adj_dev_clk_time() and idpf_ptp_adj_dev_clk_fine():
> > @@ -267,23 +272,18 @@ int idpf_ptp_adj_dev_clk_time(struct idpf_adapter *adapter, s64 delta)
> > struct virtchnl2_ptp_adj_dev_clk_time adj_dev_clk_time_msg = {
> > .delta = cpu_to_le64(delta),
> > };
> [ ... ]
> > - reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> > - if (reply_sz < 0)
> > - return reply_sz;
> > - if (reply_sz != sizeof(adj_dev_clk_time_msg))
> > - return -EIO;
> > + err = idpf_send_mb_msg(adapter, &xn_params, &adj_dev_clk_time_msg,
> > + sizeof(adj_dev_clk_time_msg));
> > + if (!err)
> > + libie_ctlq_release_rx_buf(&xn_params.recv_mem);
> > @@ -301,23 +301,18 @@ int idpf_ptp_adj_dev_clk_fine(struct idpf_adapter *adapter, u64 incval)
> > struct virtchnl2_ptp_adj_dev_clk_fine adj_dev_clk_fine_msg = {
> > .incval = cpu_to_le64(incval),
> > };
> [ ... ]
> > - reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> > - if (reply_sz < 0)
> > - return reply_sz;
> > - if (reply_sz != sizeof(adj_dev_clk_fine_msg))
> > - return -EIO;
> > + err = idpf_send_mb_msg(adapter, &xn_params, &adj_dev_clk_fine_msg,
> > + sizeof(adj_dev_clk_fine_msg));
> > + if (!err)
> > + libie_ctlq_release_rx_buf(&xn_params.recv_mem);
> Should these three helpers keep validating that
> xn_params.recv_mem.iov_len matches the expected reply size, the way
> idpf_ptp_get_caps() and idpf_ptp_get_dev_clk_time() still do?
No need to validate size, if we do not use the buffer.
^ permalink raw reply related
* Re: [PATCH net-next v4 08/15] idpf: refactor idpf to use libie_pci APIs
From: Larysa Zaremba @ 2026-07-13 17:46 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
Pavan Kumar Linga, przemyslaw.kitszel, aleksander.lobakin,
sridhar.samudrala, michal.swiatkowski, maciej.fijalkowski,
emil.s.tantilov, joshua.a.hay, jacob.e.keller,
jayaprakash.shanmugam, jiri, horms, corbet, richardcochran,
linux-doc, Samuel Salin
In-Reply-To: <20260710215313.1475803-9-anthony.l.nguyen@intel.com>
There is one nice-to-have change that came out of Sashiko's review:
commit 5238b5986775c0c76881ed594a7b09216ee20107
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Mon Jul 13 17:07:10 2026 +0200
fixup! idpf: refactor idpf to use libie_pci APIs
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 5a191644b28e..184d30c12abb 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -265,8 +265,9 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err = idpf_dev_init(adapter, ent);
if (err) {
- dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
- ent->device);
+ dev_err(&pdev->dev,
+ "Failed to initialize device (ID 0x%x): %d\n",
+ ent->device, err);
goto err_free;
}
Everything else:
> commit 461758d19f17656a1ed2f88e0867b4610ec38b43
> Author: Pavan Kumar Linga <pavan.kumar.linga@intel.com>
> idpf: refactor idpf to use libie_pci APIs
> This patch converts the idpf driver over to the shared libie_pci init
> and MMIO helpers, routes mailbox and register accesses through
> libie_pci_get_mmio_addr(), and adds a struct libie_ctlq_ctx to the
> adapter for the mmio_info list. struct idpf_hw is kept around because
> it still houses the control queues.
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_controlq.h b/drivers/net/ethernet/intel/idpf/idpf_controlq.h
> > index de4ece40c2ff..acf595e9265f 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_controlq.h
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_controlq.h
> > @@ -109,8 +109,6 @@ struct idpf_mmio_reg {
> > * Align to ctlq_hw_info
> > */
> > struct idpf_hw {
> > - struct idpf_mmio_reg mbx;
> > - struct idpf_mmio_reg rstat;
> > /* Array of remaining LAN BAR regions */
> > int num_lan_regs;
> > struct idpf_mmio_reg *lan_regs;
> After this hunk, do num_lan_regs, lan_regs, struct idpf_mmio_reg, and
> IDPF_MMIO_MAP_FALLBACK_MAX_REMAINING still have any users? A grep of
> the tree shows only their own declarations in idpf_controlq.h with no
> readers or writers after this commit.
> The same question applies to the mailbox helper macros in
> drivers/net/ethernet/intel/idpf/idpf_mem.h, which are left in place
> but still reference the just-removed mbx field:
> #define idpf_mbx_wr32(a, reg, value) writel((value), ((a)->mbx.vaddr + (reg)))
> #define idpf_mbx_rd32(a, reg) readl((a)->mbx.vaddr + (reg))
> #define idpf_mbx_wr64(a, reg, value) writeq((value), ((a)->mbx.vaddr + (reg)))
> #define idpf_mbx_rd64(a, reg) readq((a)->mbx.vaddr + (reg))
> Would it make sense to drop these together with the field removal
> above so that any future accidental use fails at review rather than
> silently reintroducing dead paths?
>
struct idpf_hw and those macros are dropped completely in the next patch
>
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_idc.c b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> > index b7d6b08fc89e..0a7edb783758 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_idc.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_idc.c
> > @@ -416,9 +416,12 @@ idpf_idc_init_msix_data(struct idpf_adapter *adapter)
> > int idpf_idc_init_aux_core_dev(struct idpf_adapter *adapter,
> > enum iidc_function_type ftype)
> > {
> > + struct libie_mmio_info *mmio = &adapter->ctlq_ctx.mmio_info;
> > struct iidc_rdma_core_dev_info *cdev_info;
> > struct iidc_rdma_priv_dev_info *privd;
> > - int err, i;
> > + struct libie_pci_mmio_region *mr;
> > + size_t num_mem_regions;
> > + int err, i = 0;
> >
> > adapter->cdev_info = kzalloc_obj(*cdev_info);
> > if (!adapter->cdev_info)
> > @@ -436,22 +439,37 @@ int idpf_idc_init_aux_core_dev(struct idpf_adapter *adapter,
> > cdev_info->rdma_protocol = IIDC_RDMA_PROTOCOL_ROCEV2;
> > privd->ftype = ftype;
> >
> > + num_mem_regions = list_count_nodes(&mmio->mmio_list);
> > + if (num_mem_regions <= IDPF_MMIO_REG_NUM_STATIC) {
> > + err = -EINVAL;
> > + goto err_plug_aux_dev;
> > + }
> > +
> > + num_mem_regions -= IDPF_MMIO_REG_NUM_STATIC;
> > privd->mapped_mem_regions =
> > kzalloc_objs(struct iidc_rdma_lan_mapped_mem_region,
> > - adapter->hw.num_lan_regs);
> > + num_mem_regions);
> > if (!privd->mapped_mem_regions) {
> > err = -ENOMEM;
> > goto err_plug_aux_dev;
> > }
> >
> > - privd->num_memory_regions = cpu_to_le16(adapter->hw.num_lan_regs);
> > - for (i = 0; i < adapter->hw.num_lan_regs; i++) {
> > - privd->mapped_mem_regions[i].region_addr =
> > - adapter->hw.lan_regs[i].vaddr;
> > - privd->mapped_mem_regions[i].size =
> > - cpu_to_le64(adapter->hw.lan_regs[i].addr_len);
> > - privd->mapped_mem_regions[i].start_offset =
> > - cpu_to_le64(adapter->hw.lan_regs[i].addr_start);
> > + privd->num_memory_regions = cpu_to_le16(num_mem_regions);
> > + list_for_each_entry(mr, &mmio->mmio_list, list) {
> > + struct resource *static_regs = adapter->dev_ops.static_reg_info;
> > + bool is_static = false;
> > +
> > + for (uint j = 0; j < IDPF_MMIO_REG_NUM_STATIC; j++)
> > + if (mr->offset == static_regs[j].start)
> > + is_static = true;
> > +
> > + if (is_static)
> > + continue;
> > +
> > + privd->mapped_mem_regions[i].region_addr = mr->addr;
> > + privd->mapped_mem_regions[i].size = cpu_to_le64(mr->size);
> > + privd->mapped_mem_regions[i++].start_offset =
> > + cpu_to_le64(mr->offset);
> > }
> Is a semantic change to what is reported to the IIDC RDMA aux device
> intended here?
> Previously idpf_calc_remaining_mmio_regs() always reported
> IDPF_MMIO_MAP_FALLBACK_MAX_REMAINING (3) regions, including any that
> happened to have addr_len == 0, and firmware-supplied regions with
> size == 0 were forwarded verbatim.
> After this patch:
> - idpf_map_remaining_mmio_regs() skips zero-sized regions via the
> !size || short-circuit before libie_pci_map_mmio_region().
> - idpf_cfg_lan_memory_regions() skips zero-sized firmware regions via
> if (len && !libie_pci_map_mmio_region(mmio, offset, len)).
> - idpf_idc_init_aux_core_dev() now fails outright with -EINVAL when
> list_count_nodes(&mmio->mmio_list) <= IDPF_MMIO_REG_NUM_STATIC.
> If a valid platform has, for example, the mailbox at BAR offset 0
> and rstat abutting the end of BAR0, are all three fallback regions
> zero-sized, and would that make the RDMA aux init unconditionally
> fail on that configuration? Would it be worth calling this ABI
> change out in the commit message?
> Separately, is the count/write-index invariant in this loop enforced
> anywhere?
> The code computes:
> num_mem_regions = list_count_nodes(&mmio->mmio_list);
> ...
> num_mem_regions -= IDPF_MMIO_REG_NUM_STATIC;
> privd->mapped_mem_regions =
> kzalloc_objs(struct iidc_rdma_lan_mapped_mem_region,
> num_mem_regions);
> ...
> privd->num_memory_regions = cpu_to_le16(num_mem_regions);
> list_for_each_entry(mr, &mmio->mmio_list, list) {
> ...
> privd->mapped_mem_regions[i++].start_offset =
> cpu_to_le64(mr->offset);
> }
> The loop increments i for every non-static entry, but there is no
> i < num_mem_regions guard. It relies on exactly
> IDPF_MMIO_REG_NUM_STATIC entries in the list having mr->offset equal
> to a static_reg_info[j].start. If, for any reason, fewer than
> IDPF_MMIO_REG_NUM_STATIC entries match (a future coalescing in
> libie_pci_map_mmio_region(), or a firmware-supplied region whose
> offset happens to coincide with a static offset without being added
> as a duplicate), does the loop overrun the mapped_mem_regions
> allocation?
> Would an explicit if (i >= num_mem_regions) break; inside the loop
> make this invariant local and enforceable?
>
Static regions are *static*, we can absolutely rely on at least
static regions to be valid
>
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> > index ab3c409e587b..93b11fb1609f 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> > @@ -246,32 +265,21 @@ static int idpf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > adapter->req_rx_splitq = true;
> >
> > adapter->pdev = pdev;
> > - err = pcim_enable_device(pdev);
> > - if (err)
> > - goto err_free;
> >
> > - err = pcim_request_region(pdev, 0, pci_name(pdev));
> > + err = idpf_dev_init(adapter, ent);
> > if (err) {
> > - pci_err(pdev, "pcim_request_region failed %pe\n", ERR_PTR(err));
> > -
> > + dev_err(&pdev->dev, "Unexpected dev ID 0x%x in idpf probe\n",
> > + ent->device);
> > goto err_free;
> > }
> >
> > - err = pci_enable_ptm(pdev);
> > - if (err)
> > - pci_dbg(pdev, "PCIe PTM is not supported by PCIe bus/controller\n");
> > -
> > - /* set up for high or low dma */
> > - err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> > + err = idpf_cfg_device(adapter);
> > if (err) {
> > - pci_err(pdev, "DMA configuration failed: %pe\n", ERR_PTR(err));
> > -
> > + pci_err(pdev, "Failed to configure device specific resources: %pe\n",
> > + ERR_PTR(err));
> > goto err_free;
> > }
> Does the "Unexpected dev ID" message still accurately describe every
> failure mode of idpf_dev_init() after this change?
> idpf_dev_init() now calls libie_pci_init_dev() first, which wraps
> pcim_enable_device(), pcim_request_region(), and
> dma_set_mask_and_coherent(). Failures in any of those (for example
> -EIO or -ENOMEM from BAR request, or DMA mask failures) will now be
> reported as "Unexpected dev ID 0x%x in idpf probe" instead of the
> prior specific messages ("pcim_request_region failed %pe", "DMA
> configuration failed: %pe"). Would it be worth propagating err via
> %pe alongside the device ID, or letting libie_pci_init_dev() log its
> own reason?
>
Valid, look at diff at the start.
>
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > index a3c17f0e14f3..357ff3e86b2e 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> > @@ -3445,35 +3450,30 @@ int idpf_vc_core_init(struct idpf_adapter *adapter)
> > }
> >
> > if (idpf_is_cap_ena(adapter, IDPF_OTHER_CAPS, VIRTCHNL2_CAP_LAN_MEMORY_REGIONS)) {
> > - err = idpf_send_get_lan_memory_regions(adapter);
> > + err = idpf_cfg_lan_memory_regions(adapter);
> > if (err) {
> > - dev_err(&adapter->pdev->dev, "Failed to get LAN memory regions: %d\n",
> > + dev_err(&adapter->pdev->dev, "Failed to configure LAN memory regions: %d\n",
> > err);
> > return -EINVAL;
> > }
> > } else {
> > /* Fallback to mapping the remaining regions of the entire BAR */
> > - err = idpf_calc_remaining_mmio_regs(adapter);
> > + err = idpf_map_remaining_mmio_regs(adapter);
> > if (err) {
> > - dev_err(&adapter->pdev->dev, "Failed to allocate BAR0 region(s): %d\n",
> > + dev_err(&adapter->pdev->dev, "Failed to configure BAR0 region(s): %d\n",
> > err);
> > - return -ENOMEM;
> > + return err;
> > }
> > }
> Is the mmio_info->mmio_list traversed while it may be concurrently
> mutated?
> libie_pci_map_mmio_region() calls list_add_tail() and
> libie_pci_unmap_fltr_regs()/libie_pci_unmap_all_mmio_regions() call
> list_del() + kvfree(), while __libie_pci_get_mmio_addr() traverses
> the list with plain list_for_each_entry() (no lock, no RCU).
> In idpf_init_hard_reset() the mbx_task workqueue is kicked before
> idpf_vc_core_init() runs the memory-region setup:
> queue_delayed_work(adapter->mbx_wq, &adapter->mbx_task, 0);
> ...
> err = idpf_vc_core_init(adapter);
> And idpf_vc_core_init() then reaches idpf_cfg_lan_memory_regions()
> (which calls libie_pci_map_mmio_region() in a loop) or its error
> path decfg_regions: which calls idpf_decfg_lan_memory_regions() ->
> libie_pci_unmap_fltr_regs().
> Meanwhile mbx_task -> idpf_ctlq_post_rx_buffs()/idpf_ctlq_send() ends
> up in libie_pci_get_mmio_addr(&hw->back->ctlq_ctx.mmio_info,
> cq->reg.tail), which walks the same mmio_list.
> Under current code the mailbox entry is the first one added at probe
> time and is never removed until unmap-all, so iteration terminates
> before touching the tail being mutated. Is that invariant intended
> to be relied on going forward? Would an explicit lock or a comment
> documenting the read/write ordering constraint help protect against
> a future addition (for example a PTP or ITR register lookup from a
> work item) that would need to walk past the mailbox entry?
>
This is fine, we can rely on the mailbox register to be valid
while mailbox communication is going. MBX is a static region,
so it is expected to be unmapped only once we are shutting down for good
>
^ permalink raw reply related
* Re: [PATCH net-next v4 05/15] libie: add bookkeeping support for control queue messages
From: Larysa Zaremba @ 2026-07-13 17:42 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
Phani R Burra, przemyslaw.kitszel, aleksander.lobakin,
sridhar.samudrala, anjali.singhai, michal.swiatkowski,
maciej.fijalkowski, emil.s.tantilov, madhu.chittim, joshua.a.hay,
jacob.e.keller, jayaprakash.shanmugam, jiri, horms, corbet,
richardcochran, linux-doc, Bharath R, Samuel Salin
In-Reply-To: <20260710215313.1475803-6-anthony.l.nguyen@intel.com>
Sashiko has some concerns about this patch.
I have found one (very) valid concern there, and concerns memory usage.
commit d7a772efb77228a1b12558dced6b1fdfeef123c4
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Mon Jul 13 17:02:01 2026 +0200
fixup! libie: add bookkeeping support for control queue messages
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index c043c07dbb89..8e7b2783b86b 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -720,7 +720,7 @@ static int libie_ctlq_xn_init_dma(struct device *dev,
goto dealloc_dma;
dma_mem->va = libie_cp_alloc_dma_mem(dev, dma_mem,
- LIBIE_CTLQ_MAX_BUF_LEN);
+ LIBIE_CP_TX_COPYBREAK);
if (!dma_mem->va) {
kfree(dma_mem);
goto dealloc_dma;
Now regarding other concerns:
* xn cookie overflow. The flow is consistent with what was in idpf beforehand,
lengthy testing also did not detect any problems
* Compromised CP accessing other CP message
> Regarding cross-transaction disclosure in the pre-allocated small-send
> buffer: in libie_ctlq_xn_process_send() the small path memcpys only
> buf_len bytes into a LIBIE_CTLQ_MAX_BUF_LEN buffer, leaving prior
> transaction data in the tail. The descriptor advertises
> data_len = buf_len so a well-behaved device only reads that many bytes,
> but could a compromised or buggy CP firmware read past data_len and
> observe prior control-plane message content?
> Would a memset of the unused tail (or of the whole buffer before
> memcpy) close this at negligible cost?
>
I think zeroing would be excessive
* Both Sashikos reported this one:
> Regarding bitmap access without the bitmap lock: this iterates
> xnm->free_xns_bm via for_each_clear_bit() without holding
> xnm->free_xns_bm_lock, while __set_bit()/__clear_bit() elsewhere
> (non-atomic RMW) mutate the same bitmap under that lock.
> Is that intentional? The comment on free_xns_bm_lock says it protects
> get/check entries, so reading the bitmap outside the lock appears to
> break the invariant even if the subsequent xn_lock re-check masks the
> functional impact today.
>
If the xn becomes free in the meantime, we catch that under xn->xn_lock
If the xn becomes taken in the meantime, we just check it next time
* Also reported by both:
> Regarding async timeout enforcement: libie_xn_check_async_timeout()
> runs only from libie_ctlq_xn_recv(). If the driver stops polling recv
> (link down, reset, NAPI suspended, interrupts masked, no incoming
> ctlq traffic, budget exhausted repeatedly), do async transactions
> ever fire their -ETIMEDOUT callback?
>
This is intended
>
> There is no independent watchdog inside the xnm (delayed_work,
> hrtimer, etc.), while the sync path is self-contained via
> wait_for_completion_timeout(). Is timeout_ms meant to be a hard
> guarantee, or caller-responsibility to keep recv polling? Either the
> kerneldoc should say so, or an independent timer inside the xnm would
> back the guarantee.
>
Caller's responsibility, timer would be excessive
* BH-related concern
> If a driver invokes xn_recv from NAPI/softirq on the same CPU that
> another thread has entered xn_send in process context and grabbed one
> of these locks, can the softirq spin-deadlock trying to reacquire the
> lock?
> Should these paths use spin_lock_bh() (or spin_lock_irqsave() if the
> recv side may run from hardirq)?
>
Both send and receive are in non-BH and non-IRQ context
* DMA direction
> For the small-message path here, direction is not set on xn->dma_mem,
> so it stays at whatever kzalloc left it (DMA_BIDIRECTIONAL). The copy
> of dma_mem into ctlq_msg->send_mem then carries that value. Today
> libie_ctlq_xn_send_clean() only calls dma_unmap_single on the
> non-onstack path, so this is not consumed, but should direction be
> set consistently to avoid a future consumer of msg->send_mem.direction
> misbehaving?
dma->direction is not expected to be access by users outside of libeth.
Library uses the field correctly
* "For the NULL-iov_base large-buffer case" - I think this can be considered a
driver's programming error, if this happens
* XN_SHUTDOWN
> The SHUTDOWN early-exit here jumps to unlock_xn, bypassing release_xn
> which is the only place that resets state to IDLE and calls
> reinit_completion(). So the xn is pushed back to free with
> state == SHUTDOWN and a stale completion count.
XN_SHUTDOWN means shutdown has already reclaimed the transaction and it cannot
be reused, hence can be dirty.
* 0-data messages
> The kerneldoc says "if force is set, then clear all the outstanding
> send messages irrespective their send status", but the loop breaks
> early on the first slot with data_len == 0. Should the wording be
> adjusted to reflect the actual behavior?
>
The only data_len = 0 message is VF reset, which can only be sent
as the last message, so we can safely assume no further buffers
need cleaning, if we encounter that
* dma_wmb() + dma_rmb() = dma_mb()
> Would dma_wmb() suffice here instead of dma_mb()? Only a write-side
> ordering constraint is needed for the desc->qword0 = 0 store, and
> dma_mb() is significantly more expensive on weakly-ordered
> architectures.
> libie_ctlq_recv() pairs its DD check with dma_rmb() before consuming
> the rest of the descriptor. Here, DD is read via le64_to_cpu() with
> no dma_rmb() before any further use. Today nothing device-updated is
> read after the DD check, so this is safe, but should the same barrier
> pattern be used for symmetry so a future descriptor field consumed
> after DD is not silently unordered?
>
Both read and write berriers are needed, therefore I have dma_mb().
* Same answer as for the previous version:
> Async callers of xn_send() typically pass a send_ctx that owns
> resources released only by resp_cb. When shutdown races with in-flight
> async transactions, does send_ctx (and anything it owns) leak?
> Would calling xn->resp_cb(xn->send_ctx, NULL, -ESHUTDOWN) (or
> -ECANCELED / -ETIMEDOUT) here, matching the timeout path, close this?
>
Callers currently do not rely on those callbacks for cleanup,
and they certainly should not do whatever handling they do,
if we are shutting down
* Clearing the queue before shutdown
> Regarding TX cleanup on deinit: libie_ctlq_xn_deinit() calls
> xn_shutdown() then xn_deinit_dma() then libie_ctlq_deinit(), none of
> which iterate the TX ring calling libie_cp_unmap_dma_mem() or
> rel_tx_buf() on outstanding sends.
Both callers force-clean the queue beforehand.
* Small return value concern:
> The ctlq_deinit label unconditionally returns -ENOMEM, discarding the
> ret from libie_ctlq_xn_init_dma() and papering over the kzalloc-failure
> path with the same constant. init_dma today only returns -ENOMEM so it
> works, but would "return ret;" (with ret set to -ENOMEM in the kzalloc
> branch) be more robust against a future error code addition?
>
Those are all memory-related functions, so this is very unlikely they return anything else.
* xn->send_ctx
> The send_ctx field here is never consumed by libie_ctlq_xn_send_clean()
> or by the rel_tx_buf callback signature (which only takes buf_va).
> Should send_ctx be removed, or should rel_tx_buf's signature be
> updated to receive it?
>
This field is used for async transactions and is usually
a rather persistent object, like HW structure.
xn->resp_cb(xn->send_ctx, response, status);
* Slightly outdated commit message
> Is this accurate? Only messages up to LIBIE_CP_TX_COPYBREAK (128 bytes)
> use libie's pre-allocated per-slot DMA buffers via
> libie_cp_can_send_onstack(); larger messages are caller-allocated and
> freed via the caller-supplied rel_tx_buf callback. Could the wording
> be tightened to describe this mixed ownership model?2
>
This is correct interpretation, but the kdoc is outdated
* Calling resp_cb() under xn_lock
> Is it safe to invoke the opaque resp_cb() while holding xn->xn_lock?
> If the caller attempts to submit another message from within the callback via
> libie_ctlq_xn_send(), it will attempt to acquire free_xns_bm_lock and another
> xn->xn_lock. Could this cause recursive locking or deadlocks?
>
idpf and ixd do not behave that way
* "memory leaks" for small buffers
> > + bool free_send = !libie_cp_can_send_onstack(params->send_buf.iov_len);
> Does this logic cause memory leaks for dynamically allocated small buffers?
> If the message size is <= 128 bytes (LIBIE_CP_TX_COPYBREAK), the data is
> copied to the pre-allocated DMA buffer, and free_send is set to false. If the
> caller provided a dynamically allocated buffer and passed a rel_tx_buf
> callback to free it, the callback is never executed because of this flag.
This is by design
* transaction reuse
> Can a timed-out transaction overwrite active DMA memory?
> If a message <= 128 bytes times out, it breaks out of the switch here and is
> pushed back to the free list in release_xn below.
> However, the hardware descriptor in the transmit ring might still point to
> xn->dma_mem->pa. If a new caller allocates this transaction entry and writes
> its payload, it will overwrite the buffer while the hardware might still be
> processing the old descriptor. Could this lead to hardware data corruption
> or invalid firmware commands?
>
Highly unlikely
^ permalink raw reply related
* Re: [PATCH net-next 1/2] dt-bindings: net: add DAPU Telecom DAP8211R(I) PHY binding
From: Rob Herring @ 2026-07-13 17:22 UTC (permalink / raw)
To: Artem Shimko
Cc: netdev, Andrew Lunn, Heiner Kallweit, Russell King,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Krzysztof Kozlowski, Conor Dooley, linux-kernel, devicetree
In-Reply-To: <20260713131223.279555-2-a.shimko.dev@gmail.com>
On Mon, Jul 13, 2026 at 04:12:22PM +0300, Artem Shimko wrote:
> Add device tree binding documentation for the DAPU Telecom DAP8211R(I)
> Gigabit Ethernet PHY.
>
> The PHY supports TX and RX clock delays in 150 ps steps from 0 to 2250 ps,
> with a default of 1950 ps if not specified. The tx-inverted-clk flag
> provides a vendor-specific extension for boards where PCB trace length or
> MAC requirements necessitate 180-degree clock phase shift.
>
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> ---
> .../bindings/net/dapu,dap8211r.yaml | 78 +++++++++++++++++++
> 1 file changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml b/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
> new file mode 100644
> index 000000000000..208a82f779d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/dapu,dap8211r.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/dapu,dap8211r.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: DAPU Telecom DAP8211R(I) Gigabit Ethernet PHY
> +
> +maintainers:
> + - Artem Shimko <a.shimko.dev@gmail.com>
> +
> +description: |
> + The DAP8211R(I) is a Gigabit Ethernet PHY with RGMII interface,
> + supporting IEEE 802.3az Energy Efficient Ethernet, IEEE 1588 SyncE,
> + and an internal packet generator for diagnostics.
> +
> + Specifications:
> + - 10BASE-Te, 100BASE-TX, 1000BASE-T
> + - RGMII with configurable TX/RX clock delays (150 ps steps, 0-2250 ps)
> + - IEEE 802.3az-2010 Energy Efficient Ethernet
> + - IEEE 1588 SyncE support
> + - Internal packet generator and checker for link diagnostics
> +
> +allOf:
> + - $ref: ethernet-phy.yaml#
> +
> +properties:
> + compatible:
> + const: ethernet-phy-id0008.011b
> +
> + reg:
> + maxItems: 1
> +
> + rx-internal-delay-ps:
> + description:
> + RGMII RX clock delay in picoseconds. The PHY supports 150 ps steps
> + from 0 to 2250 ps. If not specified, defaults to 1950 ps. If the
> + requested value does not exactly match a supported step, the driver
> + selects the nearest supported value and issues a warning.
> + enum: [0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500,
> + 1650, 1800, 1950, 2100, 2250]
> + default: 1950
> +
> + tx-internal-delay-ps:
> + description:
> + RGMII TX clock delay in picoseconds. The PHY supports 150 ps steps
> + from 0 to 2250 ps. If not specified, defaults to 1950 ps. If the
> + requested value does not exactly match a supported step, the driver
> + selects the nearest supported value and issues a warning.
> + enum: [0, 150, 300, 450, 600, 750, 900, 1050, 1200, 1350, 1500,
> + 1650, 1800, 1950, 2100, 2250]
This would also work:
multipleOf: 150
maximum: 2250
> + default: 1950
> +
> + tx-inverted-clk:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If present, the RGMII TX clock to the MAC is inverted (180 degree
> + phase shift relative to the data lines). This is a vendor-specific
> + extension for boards where PCB trace length or MAC requirements
> + necessitate clock inversion. Only use this property after hardware
> + signal integrity validation.
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + rx-internal-delay-ps = <1050>;
> + tx-internal-delay-ps = <1150>;
> + tx-inverted-clk;
> + };
> + };
> \ No newline at end of file
With this fixed,
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* [PATCH net v3] net: stmmac: intel: skip SerDes reconfig when rate is unchanged
From: Markus Breitenberger @ 2026-07-13 17:16 UTC (permalink / raw)
To: netdev
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, maxime.chevallier,
mcoquelin.stm32, alexandre.torgue, rmk+kernel, yong.liang.choong,
linux-stm32, linux-arm-kernel, stable, Markus Breitenberger
From: Markus Breitenberger <bre@keba.com>
intel_mac_finish() is registered as the phylink mac_finish()
callback for the Elkhart Lake SGMII ports. phylink calls it at
the end of every major link reconfiguration, including the
initial one during probe.
The callback selects the PMC ModPHY LCPLL programming for the
requested MAC-side interface and then power-cycles the SerDes.
On Elkhart Lake that ModPHY is also used by the on-die AHCI
SATA PHY. Reapplying the programming during the initial
boot-time link-up disturbs the shared analog block while it is
still driving SATA, so the SATA link fails to train:
ata1: SATA link down (SStatus 1 SControl 300)
The disk carrying the root filesystem is never detected and the
system hangs at rootwait. Ethernet itself comes up normally,
which makes the failure look unrelated to the network driver.
Before mac_finish() runs, the legacy SerDes power-up path has
already programmed SERDES_GCR0 for the current interface. The
1G and 2.5G ModPHY tables selected by mac_finish() correspond
to the SerDes lane rate, so read that rate back from SERDES_GCR0
and skip the PMC reprogramming and SerDes power-cycle when it
already matches the selected interface.
This keeps the disruptive reprogramming out of the boot path
when the SerDes is configured correctly, while preserving the
previous behavior when a real SGMII/1000BASE-X to 2500BASE-X
rate change is needed. If the register read fails, reconfigure
as before.
Fixes: a42f6b3f1cc1 ("net: stmmac: configure SerDes according to the interface mode")
Cc: stable@vger.kernel.org
Assisted-by: GitHub-Copilot:claude-opus-4.8
Signed-off-by: Markus Breitenberger <bre@keba.com>
---
v3:
- Update priv->plat->phy_interface before skipping SerDes reconfiguration,
so SGMII <-> 1000BASE-X changes still update the cached interface.
- Rename subject to "net: stmmac: intel: skip SerDes reconfig when rate is unchanged".
v2: https://lore.kernel.org/netdev/20260709190329.124432-1-bre@breiti.cc/
- Read current SerDes lane rate from SERDES_GCR0 instead of comparing
against cached phy_interface state.
- Rework commit message.
- Keep previous behavior if SERDES_GCR0 read fails.
v1: https://lore.kernel.org/netdev/20260706061954.94842-1-bre@breiti.cc/
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index b8d467ba6d72..4d207f41a43b 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -525,6 +525,32 @@ static int intel_set_reg_access(const struct pmc_serdes_regs *regs, int max_regs
return ret;
}
+/*
+ * Return true if the SerDes lane rate must change to serve @interface.
+ * If the current rate cannot be determined, reconfigure as before.
+ */
+static bool intel_serdes_needs_reconfig(struct stmmac_priv *priv,
+ struct intel_priv_data *intel_priv,
+ phy_interface_t interface)
+{
+ u32 cur_rate, want_rate;
+ int data;
+
+ if (!intel_priv->mdio_adhoc_addr)
+ return true;
+
+ data = mdiobus_read(priv->mii, intel_priv->mdio_adhoc_addr,
+ SERDES_GCR0);
+ if (data < 0)
+ return true;
+
+ cur_rate = (data & SERDES_RATE_MASK) >> SERDES_RATE_PCIE_SHIFT;
+ want_rate = interface == PHY_INTERFACE_MODE_2500BASEX ?
+ SERDES_RATE_PCIE_GEN2 : SERDES_RATE_PCIE_GEN1;
+
+ return cur_rate != want_rate;
+}
+
static int intel_mac_finish(struct net_device *ndev,
void *intel_data,
unsigned int mode,
@@ -536,6 +562,11 @@ static int intel_mac_finish(struct net_device *ndev,
int max_regs = 0;
int ret = 0;
+ if (!intel_serdes_needs_reconfig(priv, intel_priv, interface)) {
+ priv->plat->phy_interface = interface;
+ return 0;
+ }
+
ret = intel_tsn_lane_is_available(ndev, intel_priv);
if (ret < 0) {
netdev_info(priv->dev, "No TSN lane available to set the registers.\n");
--
2.55.0
^ permalink raw reply related
* net/mlx5: duplicate kmem_cache name warning
From: Christian Borntraeger @ 2026-07-13 17:12 UTC (permalink / raw)
To: Yevgeny Kliteynik, Saeed Mahameed, Leon Romanovsky, Tariq Toukan,
Mark Bloch
Cc: Network Development
Hi,
on a CONFIG_DEBUG_VM kernel I get the following warning in our CI for the vdpa testcase
WARNING: mm/slab_common.c:111 at __kmem_cache_create_args+0xca/0x480, CPU#18: devlink/331372
Modules linked in: act_mirred act_skbedit cls_matchall act_gact cls_flower sch_ingress vhost_vdpa veth nfnetlink_cttimeout openvswitch macvtap macvlan vfio_ap kvm nf_nat_tftp nf_conntrack_tftp nsh nf_conncount vfio_pci_core irqbypass scsi_debug vhost_net tap tun vhost_vsock vmw_vsock_virtio_transport_common vsock vhost nft_masq nft_reject_ipv4 act_csum cls_u32 sch_htb smc_diag smc ppp_deflate bsd_comp ppp_async crc_ccitt ppp_generic slhc loop algif_hash af_alg nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables mlx5_vdpa vdpa mlx5_ib dm_service_time ib_uverbs_support vringh ib_core vhost_iotlb mlx5_core s390_trng eadm_sch vfio_ccw mdev vfio_iommu_type1 vfio sch_fq_codel drm i2c_core dm_multipath drm_panel_orientation_quirks uvdevice diag288_wdt watchdog hmac_s390 prng aes_s390 zfcp scsi_transport_fc pkey_pckmo pkey_cca pkey_ep11 zcrypt paes_s390 phmac_s390 rng_core
scsi_dh_alua pkey scsi_dh_rdac scsi_dh_emc crypto_engine autofs4 ecdsa_generic ecc sha512 [last unloaded: openvswitch]
CPU: 18 UID: 0 PID: 331372 Comm: devlink Tainted: G W 7.2.0-20260712.rc2.git0.e3321fa3034d.300.fc44.s390x+debug #1 PREEMPT
Tainted: [W]=WARN
Hardware name: IBM 9175 ME1 701 (LPAR)
Krnl PSW : 0704c00180000000 0000038139a6771a (__kmem_cache_create_args+0xda/0x480)
R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:0 PM:0 RI:0 EA:3
Krnl GPRS: 0000000000000000 0000000000000000 000003813b7de578 00000380b9eb8974
00000276c417f690 00000380b9eb8974 0000030144623348 00000277234a1660
0000000000000020 00000380b9eb8974 00000277234a1600 000003813b686d30
0000000000000000 00000380b9e9a810 0000038139a6771a 0000030144623238
Krnl Code: 0000038139a6770a: c02000ebb737 larl %r2,000003813b7de578
0000038139a67710: b9040039 lgr %r3,%r9
*0000038139a67714: c0e5006e2eb2 brasl %r14,000003813a82d478
>0000038139a6771a: a7390020 lghi %r3,32
0000038139a6771e: b9040029 lgr %r2,%r9
0000038139a67722: c0e5006cf98f brasl %r14,000003813a806a40
0000038139a67728: ec26018e007c cgij %r2,0,6,0000038139a67a44
0000038139a6772e: 58d0f0a4 l %r13,164(%r15)
Call Trace:
[<0000038139a6771a>] __kmem_cache_create_args+0xda/0x480
([<0000038139a676a2>] __kmem_cache_create_args+0x62/0x480)
[<00000380b9da1c70>] dr_domain_init_mem_resources+0x80/0x240 [mlx5_core]
[<00000380b9da226e>] dr_domain_init_resources.constprop.0+0x7e/0x2c0 [mlx5_core]
[<00000380b9da28b2>] mlx5dr_domain_create+0x132/0x250 [mlx5_core]
[<00000380b9dc1e20>] mlx5_cmd_dr_create_ns+0x30/0x90 [mlx5_core]
[<00000380b9cd8dbe>] mlx5_flow_namespace_set_mode+0x6e/0x130 [mlx5_core]
[<00000380b9d846ec>] esw_create_offloads_fdb_tables+0xac/0x5a0 [mlx5_core]
[<00000380b9d865b6>] esw_offloads_steering_init+0x1c6/0x480 [mlx5_core]
[<00000380b9d86e8e>] esw_offloads_enable+0x13e/0x410 [mlx5_core]
[<00000380b9d7b04a>] mlx5_eswitch_enable_locked+0x36a/0x540 [mlx5_core]
[<00000380b9d84ff0>] esw_offloads_start+0x50/0x1d0 [mlx5_core]
[<00000380b9d8774a>] mlx5_devlink_eswitch_mode_set+0x35a/0x3f0 [mlx5_core]
[<000003813a791e68>] devlink_nl_eswitch_set_doit+0x88/0x120
[<000003813a5b93ea>] genl_family_rcv_msg_doit+0xea/0x150
[<000003813a5b95c2>] genl_family_rcv_msg+0x172/0x210
[<000003813a5b96c2>] genl_rcv_msg+0x62/0xc0
[<000003813a5b7cac>] netlink_rcv_skb+0x5c/0x120
[<000003813a5b8f0c>] genl_rcv+0x3c/0x50
[<000003813a5b74a4>] netlink_unicast+0x1f4/0x2b0
[<000003813a5b783c>] netlink_sendmsg+0x2dc/0x460
[<000003813a4c9764>] __sock_sendmsg+0x64/0xd0
[<000003813a4cc878>] __sys_sendto+0x108/0x160
[<000003813a4cdf50>] __do_sys_socketcall+0x350/0x460
[<000003813a8184d2>] __do_syscall+0x172/0x750
[<000003813a82d5d2>] system_call+0x72/0x90
Looks like Each software-steering domain creates its own slab caches with fixed
global names in dr_domain_init_mem_resources():
dmn->chunks_kmem_cache = kmem_cache_create("mlx5_dr_chunks", ...);
dmn->htbls_kmem_cache = kmem_cache_create("mlx5_dr_htbls", ...);
so as soon as we have 2 domains (e.g. a second PF being switched to switchdev mode
while the first one already is), the second kmem_cache_create() trips over this.
^ permalink raw reply
* Re: [PATCH net-next v4 04/15] libie: add control queue support
From: Larysa Zaremba @ 2026-07-13 17:10 UTC (permalink / raw)
To: Tony Nguyen
Cc: davem, kuba, pabeni, edumazet, andrew+netdev, netdev,
Phani R Burra, przemyslaw.kitszel, aleksander.lobakin,
sridhar.samudrala, anjali.singhai, michal.swiatkowski,
maciej.fijalkowski, emil.s.tantilov, madhu.chittim, joshua.a.hay,
jacob.e.keller, jayaprakash.shanmugam, jiri, horms, corbet,
richardcochran, linux-doc, Samuel Salin, Bharath R
In-Reply-To: <20260710215313.1475803-5-anthony.l.nguyen@intel.com>
Sashiko has some concerns about this patch.
There are some improvements that I think are nice to have based on that:
commit 1da5bb5c7a7be66fc6226afa94c0f25bb52a57a0
Author: Larysa Zaremba <larysa.zaremba@intel.com>
Date: Mon Jul 13 15:59:55 2026 +0200
fixup! libie: add control queue support
diff --git a/drivers/net/ethernet/intel/libie/controlq.c b/drivers/net/ethernet/intel/libie/controlq.c
index 885b4437b4f0..c043c07dbb89 100644
--- a/drivers/net/ethernet/intel/libie/controlq.c
+++ b/drivers/net/ethernet/intel/libie/controlq.c
@@ -327,7 +327,8 @@ libie_ctlq_add(struct libie_ctlq_ctx *ctx,
{
struct libie_ctlq_info *ctlq;
- if (qinfo->id != LIBIE_CTLQ_MBX_ID)
+ if (qinfo->id != LIBIE_CTLQ_MBX_ID ||
+ qinfo->len > FIELD_MAX(LIBIE_CTLQ_MBX_ATQ_LEN))
return ERR_PTR(-EOPNOTSUPP);
/* libie_ctlq_init was not called */
@@ -493,8 +494,6 @@ EXPORT_SYMBOL_NS_GPL(libie_ctlq_send_desc_avail, "LIBIE_CP");
* The caller must hold ctlq->lock. The intended pattern is to first check
* the number of descriptors available, then fill in the messages and perform
* send within a single critical section.
- *
- * Return: %0 on success, -%errno on failure.
*/
void libie_ctlq_send(struct libie_ctlq_info *ctlq, u32 num_q_msg)
{
@@ -510,6 +509,7 @@ void libie_ctlq_send(struct libie_ctlq_info *ctlq, u32 num_q_msg)
if (unlikely(++ntu == ctlq->ring_len))
ntu = 0;
}
+ dma_wmb();
writel(ntu, ctlq->reg.tail);
ctlq->next_to_use = ntu;
}
Other than that, I would put feedback into the following categories:
* worrying about patterns not employed by idpf or ixd
> libie_ctlq_recv() only breaks the loop on DD=0, but DD is never
> cleared on consumption. It is cleared only when the slot is
> re-posted by libie_ctlq_post_rx_buffs(), and even there the zeroing
> happens for descriptors that get freshly filled:
> while (num_to_post--) {
> ...
> ctlq->descs[ntp] = (struct libie_ctlq_desc) {};
> ...
> libie_ctlq_prep_rx_desc(&ctlq->descs[ntp], addr, fq.truesize);
> if (unlikely(++ntp == ctlq->ring_len))
> ntp = 0;
> }
> The final barrier slot between next_to_clean and next_to_post is
> never zeroed, so it retains DD=1 from the previous rotation.
> With ring_len = 4, after one full cycle ntc=3 and ntp=3; post_rx_buffs
> refills slots 3, 0, 1 and leaves slot 2 (the new barrier) with stale
> DD=1. If a caller then calls libie_ctlq_recv() with
> num_q_msg >= ring_len, the loop can wrap and re-process the stale
> descriptor at slot 2.
>
If caller calls libie_ctlq_recv() with num_q_msg >= ring_len, this is caller's
programming error, direct ctlq APIs are not exactly safe, and queue cannot
contain more than ring_len - 1 messages. So maybe worth mentioning in the kdoc.
So the stale slot is not a valid concern.
[...]
> Can this returned pointer be freed concurrently by another thread?
> The pointer is returned without acquiring a reference, and the lock that
> protected it (ctlqs_lock) is released immediately before returning.
> If a concurrent thread initiates teardown via libie_ctlq_deinit(), it could
> remove and free the queue while the caller of libie_find_ctlq() is actively
> accessing it, causing a use-after-free.
>
idpf and ixd do not do that
* Worrying about impossible libeth configurations
> If the page pool uses compound pages and the offset places iov_base into a
> subsequent 4K frame, calling virt_to_netmem(rx_buf->iov_base) yields the tail
> page rather than the head page.
> Page pool metadata is only valid on the head page, so calling
> page_pool_put_full_netmem() on this tail page could read garbage data.
>
page_pool in libie_cp is created in a way that makes compound pages impossible
> If the page pool ever returns a smaller truesize (for example via
> libeth_rx_page_pool_params_zc), would a hardware-provided data_len
> up to 4K let iov_len point past the end of the actual buffer?
>
page_pool via libeth is configured exactly in a way that disables any headroom
or tailroom, so truesize will never be less than data_len.
* Some parts of code that I agree look semi-ugly (like defines vs virtchnl2
enums, limiting id to LIBIE_CTLQ_MBX_ID, and lack of low-level completion
function for Tx control queue), but those I think are best addressed withing
non-default-mailbox-ctlq development, as currently the best way is unclear, but
how is looks currently is perfectly acceptable.
* Concerns about HW doorbells and such. Same as with previous versions, the flow
is consistent with what was in idpf beforehand.
* This one is an outlier:
> if (unlikely(msg->data_len > LIBIE_CTLQ_MAX_BUF_LEN)) {
> msg->data_len = LIBIE_CTLQ_MAX_BUF_LEN;
> msg->chnl_retval = U32_MAX;
> }
> Can callers distinguish "hardware returned U32_MAX" from "libie
> truncated the buffer"?
>
Yes, U32_MAX was chosen, so that it never intersects with the valid HW codes
^ permalink raw reply related
* Re: [PATCH] ieee802154: hwsim: serialize pib updates to fix double-free
From: Miquel Raynal @ 2026-07-13 16:59 UTC (permalink / raw)
To: David Carlier
Cc: alex.aring, stable, syzbot+60332fd095f8bb2946ad, Stefan Schmidt,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-wpan, netdev, linux-kernel, Yousef Alhouseen
In-Reply-To: <20260709221858.158063-1-devnexen@gmail.com>
Hello David,
On 09/07/2026 at 23:18:58 +01, David Carlier <devnexen@gmail.com> wrote:
> hwsim_update_pib() does an unserialized read-swap-free of phy->pib:
>
> pib_old = rtnl_dereference(phy->pib);
> ...
> rcu_assign_pointer(phy->pib, pib);
> kfree_rcu(pib_old, rcu);
>
> It assumes the RTNL is held, but ->set_channel is not always called
> under it: the mac802154 scan worker changes channels via
> drv_set_channel() without the RTNL. Such an update can race an
> RTNL-held one on the same phy; both read the same pib_old and both
> kfree_rcu() it, double-freeing the object. With SLUB percpu sheaves
> batching kfree_rcu(), this surfaces as a KASAN invalid-free in
> rcu_free_sheaf().
>
> struct hwsim_phy has no lock for pib. Add one and make the swap atomic
> with rcu_replace_pointer() under it, dropping the misleading
> rtnl_dereference().
>
> Reported-by: syzbot+60332fd095f8bb2946ad@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=60332fd095f8bb2946ad
> Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> Cc: <stable@vger.kernel.org>
Thank you for the patch, but I think Yousef already provided a similar
patch:
https://lore.kernel.org/all/20260627235805.17310-1-alhouseenyousef@gmail.com/
Yousef, can you confirm you will send v2 soon?
Thanks,
Miquèl
^ permalink raw reply
* Re: [PATCH net-next v3 14/15] net: macb: use context swapping in .set_ringparam()
From: Théo Lebrun @ 2026-07-13 16:41 UTC (permalink / raw)
To: Nicolai Buchwitz
Cc: Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Richard Cochran, Russell King,
netdev, linux-kernel, Nicolas Ferre, Claudiu Beznea,
Paolo Valerio, Vladimir Kondratiev, Gregory CLEMENT,
Benoît Monin, Tawfik Bayouk, Thomas Petazzoni,
Maxime Chevallier
In-Reply-To: <be3fb533fb7a93a809e71efef04edb91@tipi-net.de>
On Thu Jul 2, 2026 at 12:37 PM CEST, Nicolai Buchwitz wrote:
> On 1.7.2026 17:59, Théo Lebrun wrote:
>> ethtool_ops.set_ringparam() is implemented using the primitive close /
>> update ring size / reopen sequence. Under memory pressure this does not
>> fly: we free our buffers at close and cannot reallocate new ones at
>> open. Also, it triggers a slow PHY reinit.
>>
>> Instead, exploit the new context mechanism and improve our sequence to:
>> - allocate a new context (including buffers) first
>> - if it fails, early return without any impact to the interface
>> - stop interface
>> - update global state (bp, netdev, etc)
>> - pass buffer pointers to the hardware
>> - start interface
>> - free old context.
>>
>> The HW disable sequence is inspired by macb_reset_hw() but avoids
>> (1) setting NCR bit CLRSTAT and (2) clearing register PBUFRXCUT.
>>
>> The HW re-enable sequence is inspired by macb_mac_link_up(), skipping
>> over register writes which would be redundant (because values have not
>> changed).
>>
>> The generic context swapping parts are isolated into helper functions
>> macb_context_swap_start|end(), reusable by other operations
>> (change_mtu,
>> set_channels, etc).
>>
>> Introduce a new locking primitive (mac_cfg_lock mutex) to serialise
>> swap
>> with phylink MAC callbacks. Avoid stopping phylink to avoid a slow PHY
>> retrain. Those callbacks grab phydev->lock if it exists so we could
>> imagine grabbing that from the swap op, but phydev->lock doesn't exist
>> in the SFP case.
>>
>> AT91 EMAC is handled differently as their buffer management is separate
>> and they don't do NAPI. We refuse them (-EBUSY) to avoid implementing
>> context swapping for them.
>>
>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
>> ---
>> drivers/net/ethernet/cadence/macb.h | 2 +
>> drivers/net/ethernet/cadence/macb_main.c | 142
>> +++++++++++++++++++++++++++++--
>
>> [...]
>
>> +static void macb_context_swap_start(struct macb *bp)
>> +{
>> + struct macb_queue *queue;
>> + unsigned long flags;
>> + unsigned int q;
>> + u32 ctrl;
>> +
>> + mutex_lock(&bp->mac_cfg_lock);
>> +
>> + /* Mask interrupts before disabling BH features. */
>> + spin_lock_irqsave(&bp->lock, flags);
>> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>> + queue_writel(queue, IDR, -1);
>> + queue_readl(queue, ISR);
>> + macb_queue_isr_clear(bp, queue, -1);
>> + }
>> + spin_unlock_irqrestore(&bp->lock, flags);
>> +
>> + /* Drain BH features. HW is still active and usable at this point. */
>> +
>> + cancel_work_sync(&bp->hresp_err_bh_work);
>> + cancel_delayed_work_sync(&bp->tx_lpi_work);
>> +
>> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
>> + napi_disable(&queue->napi_rx);
>> + napi_disable(&queue->napi_tx);
>> + cancel_work_sync(&queue->tx_error_task);
>> + netdev_tx_reset_queue(netdev_get_tx_queue(bp->netdev, q));
>> + }
>
> Can this deadlock against a pending tx_error_task?
>
> AFAIU macb_tx_error_task() does napi_disable(&queue->napi_tx) and later
> napi_enable() on the same napi, and it can already be queued
> (macb_interrupt()
> schedules it on a TX error) by the time the swap runs:
>
> swap_start: napi_disable(napi_tx) /* sets SCHED, returns */
> worker: tx_error_task: napi_disable(napi_tx) /* spins on SCHED */
> swap_start: cancel_work_sync(tx_error_task) /* waits on worker
> */
>
> napi_disable() spins until napi_enable() clears SCHED, but here the swap
> won't
> re-enable until macb_context_swap_end(), and cancel_work_sync() is
> what's
> holding it up. Nothing clears it.
>
> Maybe cancel_work_sync() before the napi_disable() calls would work
> instead? IRQs
> are masked just above, so AFAICT nothing can reschedule tx_error_task by
> then.
Ah yes, good catch. macb_tx_error_task() should never enter if NAPI is
disabled, else its napi_disable() will hang.
The fix sounds easy enough, as you indicated. We'll move the
cancel_work_sync(queue->tx_error_task) call to be above our swap start
napi_disable().
Thanks,
--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v12 nf-next 3/7] netfilter: nf_flow_table_offload: Add nf_flow_rule_bridge()
From: Pablo Neira Ayuso @ 2026-07-13 16:37 UTC (permalink / raw)
To: Eric Woudstra
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Florian Westphal, Phil Sutter,
Nikolay Aleksandrov, Ido Schimmel, Kuniyuki Iwashima,
Stanislav Fomichev, Samiullah Khawaja, Hangbin Liu, Krishna Kumar,
Martin Karsten, netdev, netfilter-devel, bridge
In-Reply-To: <12451f0d-7e5b-4697-a734-8200944b204f@gmail.com>
Hi,
On Fri, Jul 10, 2026 at 05:16:44PM +0200, Eric Woudstra wrote:
>
>
> On 7/10/26 11:26 AM, Pablo Neira Ayuso wrote:
> > Hi Eric,
> >
> > On Wed, Jul 08, 2026 at 08:36:11PM +0200, Eric Woudstra wrote:
> >> On 7/8/26 11:48 AM, Pablo Neira Ayuso wrote:
> >>> On Tue, Jul 07, 2026 at 11:10:41AM +0200, Eric Woudstra wrote:
> >>>> Add nf_flow_rule_bridge().
> >>>>
> >>>> It only calls the common rule and adds the redirect.
> >>>
> >>> I decided to use the new _unsupp() function, so we don't pretend
> >>> bridge hw offload is already supported. We will need a driver before
> >>> we can add this, this stub does not provide much. I guess your goal
> >>> was just to avoid a crash here.
> >>
> >> No, I am already using hw_offload between bridged interfaces
> >> on the mt7986 succesfully for almost 2 years.
> >> It works dsa-port to direct interface (lan1 to eth1 on Bananapi R3) and
> >> between direct interfaces (eth0 to eth1 on Bananapi-R3-mini)
> >
> > Do you utilize the existing mt7986 driver in-tree without changes to
> > achive this hardware offload? Or you have still have out-of-tree
> > patches that need to be merged to achive this?
>
> I do not change anything about the mediatek drivers to achieve hardware
> offload. No patch needed to fix hardware offload.
Good. I would suggest you follow up to replace my _unsupp() function
with the .action function once the initial flowtable bridge support
gets merged upstream, explaining what drivers you have tested, it
would be nice for the record.
But hold on a bit until initial steps are made to upstream the initial
infrastructure, please.
> However I do have a small fix for the offloading towards the mediatek
> wifi interface. This is a fix for the software fastpath already.
> Also with hardware offload to wifi interface, once the software fastpath
> is setup correctly (needs this patch), then the hardware offload functions
> correctly without any further patch. See patch:
>
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260317101525.358016-1-ericwouds@gmail.com/
>
> It is not reviewed yet.
I'm reading the commit description, info.indev = NULL is not returned
anymore, a rebase, review and re-post once initial bridge supports
gets added would be good.
This is to build a fast path between the bridge ports and wifi through
the SoC.
> >> It can also be tested with my bridge_fastpath.sh selftest script.
> >> This script uses veth-device pairs to test the software fastpath.
> >> It can also use 2 real interfaces interconnected in a loop of copper,
> >> when chosen with commandline arguments. Then it tests software- and
> >> hardware-fastpath. It also tests many different scenarios.
> >>
> >> So this is why I've added it, as it is already functional. If a software
> >> fastpath is setup correctly, the hardware fastpath is also functional.
> >
> > Thanks for explaining.
> >
> > I am targetting at a minimal subset of the flowtable bridge support at
> > this stage. There is a need to make progress with the
> > nf_conntrack_bridge counterpart before the flowtable bridge can get
> > more features (namely, bridge vlan filtering support).
>
> I did send a newer version of my patch-set for nf_conntrack_bridge,
> last version also adding support to defrag/refrag. See:
>
> https://patchwork.ozlabs.org/project/netfilter-devel/cover/20260512103347.102746-1-ericwouds@gmail.com/
I'm taking a look to the nf_conntrack_bridge side.
> I've added testcases for defrag/refrag to the bridge_fastpath.sh selftest
> script (v5), so I know it is functional.
>
> For proper vlan filtering support, I do also believe you will need to
> introduce DEV_PATH_BR_VLAN_KEEP_HW, or do something similar. See:
>
> https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260317101722.358640-1-ericwouds@gmail.com/
OK, this will be useful once bridge vlan filtering gets supported.
The existing proposal that extends the bridge fill_forward_path relies
uniquely on one single bridge port to decide whether keep, untag or
tag? Should this look for the pvid at the ingress bridge port (tag or
keep vlan), then look at the egress bridge port (for untagging).
^ permalink raw reply
* [PATCH nf-next v3] netfilter: ipset: skip extension destroy on hash resize replay
From: Weiming Shi @ 2026-07-13 16:33 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
Phil Sutter
Cc: netfilter-devel, coreteam, netdev, linux-kernel, Xiang Mei,
Weiming Shi
During a hash set resize, mtype_resize() copies each element into the
new table with memcpy(), so the new-table element shares the old-table
element's comment extension. An xt_SET delete on the old table during
the resize destroys that shared comment via ip_set_ext_destroy() and
queues a replayed delete on h->ad. After the table swap mtype_resize()
replays it with mtype_del() on the new table, whose copy still points at
the freed comment, so ip_set_ext_destroy() frees it a second time:
ODEBUG: activate active (active state 1) object: ... object type: rcu_head
WARNING: CPU: 3 PID: 5311 at lib/debugobjects.c:514 debug_print_object
Call Trace:
<IRQ>
kvfree_call_rcu (kernel/rcu/tree.c:3825)
ip_set_comment_free (net/netfilter/ipset/ip_set_core.c:397)
hash_ip4_del (net/netfilter/ipset/ip_set_hash_gen.h:1098)
hash_ip4_kadt (net/netfilter/ipset/ip_set_hash_ip.c:96)
ip_set_del (net/netfilter/ipset/ip_set_core.c:813)
set_target_v3 (net/netfilter/xt_set.c:412)
ipt_do_table (net/ipv4/netfilter/ip_tables.c:346)
__ip_local_out (net/ipv4/ip_output.c:119)
icmp_push_reply (net/ipv4/icmp.c:397)
__icmp_send (net/ipv4/icmp.c:804)
__udp4_lib_rcv (net/ipv4/udp.c:2521)
ip_local_deliver (net/ipv4/ip_input.c:254)
ip_rcv (net/ipv4/ip_input.c:569)
</IRQ>
The replay passes a NULL ext (the kernel-side delete that queued it
already destroyed the extensions), so skip ip_set_ext_destroy() when ext
is NULL.
Reachable from an unprivileged user namespace.
Fixes: f66ee0410b1c ("netfilter: ipset: Fix \"INFO: rcu detected stall in hash_xxx\" reports")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v3: Repost as a new thread; v2 was sent in reply to v1.
v2: Rebase onto nf-next; drop the second hunk (already fixed there).
net/netfilter/ipset/ip_set_hash_gen.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 8231317b0..d15530241 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1112,7 +1112,9 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
mtype_del_cidr(set, h,
NCIDR_PUT(DCIDR_GET(d->cidr, j)), j);
#endif
- ip_set_ext_destroy(set, data);
+ /* On a resize replay the extensions were already destroyed. */
+ if (ext)
+ ip_set_ext_destroy(set, data);
if (t->resizing && ext && ext->target) {
/* Resize is in process and kernel side del,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net v2 1/2] sctp: avoid auth_enable sysctl UAF during netns teardown
From: Xin Long @ 2026-07-13 16:31 UTC (permalink / raw)
To: Ren Wei
Cc: linux-sctp, netdev, marcelo.leitner, davem, edumazet, pabeni,
horms, matttbe, yuantan098, yifanwucs, tomapufckgml, bird,
tpluszz77, roxy520tt, sashiko-bot
In-Reply-To: <2e48ecffe7fa9e983203a22f68e603cd8530c5d1.1782745545.git.roxy520tt@gmail.com>
On Sat, Jul 11, 2026 at 12:22 AM Ren Wei <n05ec@lzu.edu.cn> wrote:
>
> From: Zhiling Zou <roxy520tt@gmail.com>
>
> proc_sctp_do_auth() updates the SCTP control socket after changing
> net.sctp.auth_enable. The handler gets the per-net SCTP state from
> ctl->data, so an already opened sysctl file can still target a network
> namespace while that namespace is being torn down.
>
> SCTP previously registered its per-net sysctls from sctp_defaults_init(),
> while the control socket is created later from sctp_ctrlsock_init(). This
> exposed a window during initialization where auth_enable was writable
> before net->sctp.ctl_sock existed, and a teardown window where auth_enable
> stayed writable after inet_ctl_sock_destroy() had released the control
> socket.
>
> Move the per-net SCTP sysctl registration into sctp_ctrlsock_init() after
> sctp_ctl_sock_init() succeeds, and unregister the sysctl table before
> destroying the control socket in sctp_ctrlsock_exit(). If sysctl
> registration fails after the control socket was created, destroy the
> control socket in the same init path.
>
> Make sctp_sysctl_net_unregister() tolerate a missing header and clear the
> saved pointer so init-error and exit paths can safely share the unregister
> helper.
>
> Fixes: 15649fd5415e ("sctp: sysctl: auth_enable: avoid using current->nsproxy")
> Cc: stable@vger.kernel.org
> Reported-by: Yuan Tan <yuantan098@gmail.com>
> Reported-by: Yifan Wu <yifanwucs@gmail.com>
> Reported-by: Juefei Pu <tomapufckgml@gmail.com>
> Reported-by: Xin Liu <bird@lzu.edu.cn>
> Co-developed-by: Qi Tang <tpluszz77@gmail.com>
> Signed-off-by: Qi Tang <tpluszz77@gmail.com>
> Signed-off-by: Zhiling Zou <roxy520tt@gmail.com>
> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
> ---
> net/sctp/protocol.c | 17 ++++++++++-------
> net/sctp/sysctl.c | 9 +++++++--
> 2 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 587b0017a67d..f5fe6ddf0d7d 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1382,10 +1382,6 @@ static int __net_init sctp_defaults_init(struct net *net)
> net->sctp.l3mdev_accept = 1;
> #endif
>
> - status = sctp_sysctl_net_register(net);
> - if (status)
> - goto err_sysctl_register;
> -
> /* Allocate and initialise sctp mibs. */
> status = init_sctp_mibs(net);
> if (status)
> @@ -1419,8 +1415,6 @@ static int __net_init sctp_defaults_init(struct net *net)
> cleanup_sctp_mibs(net);
> #endif
> err_init_mibs:
> - sctp_sysctl_net_unregister(net);
> -err_sysctl_register:
> return status;
> }
>
> @@ -1435,7 +1429,6 @@ static void __net_exit sctp_defaults_exit(struct net *net)
> net->sctp.proc_net_sctp = NULL;
> #endif
> cleanup_sctp_mibs(net);
> - sctp_sysctl_net_unregister(net);
> }
>
> static struct pernet_operations sctp_defaults_ops = {
> @@ -1451,14 +1444,24 @@ static int __net_init sctp_ctrlsock_init(struct net *net)
> status = sctp_ctl_sock_init(net);
> if (status)
> pr_err("Failed to initialize the SCTP control sock\n");
> + else
> + status = sctp_sysctl_net_register(net);
> +
> + if (status && net->sctp.ctl_sock) {
> + inet_ctl_sock_destroy(net->sctp.ctl_sock);
> + net->sctp.ctl_sock = NULL;
> + }
I think the Linux style here should be:
/* Initialize the control inode/socket for handling OOTB packets. */
status = sctp_ctl_sock_init(net);
if (status) {
pr_err("Failed to initialize the SCTP control sock\n");
return status;
}
status = sctp_sysctl_net_register(net);
if (status) {
inet_ctl_sock_destroy(net->sctp.ctl_sock);
net->sctp.ctl_sock = NULL;
}
Thanks.
>
> return status;
> }
>
> static void __net_exit sctp_ctrlsock_exit(struct net *net)
> {
> + sctp_sysctl_net_unregister(net);
> +
> /* Free the control endpoint. */
> inet_ctl_sock_destroy(net->sctp.ctl_sock);
> + net->sctp.ctl_sock = NULL;
> }
>
> static struct pernet_operations sctp_ctrlsock_ops = {
> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> index 15e7db9a3ab2..fca840484ebf 100644
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -615,11 +615,16 @@ int sctp_sysctl_net_register(struct net *net)
>
> void sctp_sysctl_net_unregister(struct net *net)
> {
> + struct ctl_table_header *header = net->sctp.sysctl_header;
> const struct ctl_table *table;
>
> - table = net->sctp.sysctl_header->ctl_table_arg;
> - unregister_net_sysctl_table(net->sctp.sysctl_header);
> + if (!header)
> + return;
> +
> + table = header->ctl_table_arg;
> + unregister_net_sysctl_table(header);
> kfree(table);
> + net->sctp.sysctl_header = NULL;
> }
>
> static struct ctl_table_header *sctp_sysctl_header;
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH bpf-next v7 3/3] selftests/bpf: Add bpf_fib_lookup() VLAN flag tests
From: Avinash Duduskar @ 2026-07-13 16:23 UTC (permalink / raw)
To: ast, daniel, andrii
Cc: eddyz87, memxor, martin.lau, song, yonghong.song, jolsa, emil,
john.fastabend, sdf, davem, edumazet, kuba, pabeni, horms, shuah,
hawk, yatsenko, leon.hwang, kpsingh, a.s.protopopov, ameryhung,
rongtao, eyal.birger, bpf, netdev, linux-kernel, linux-kselftest,
toke, dsahern
In-Reply-To: <20260713162305.1237211-1-avinash.duduskar@gmail.com>
Cover both new VLAN flags in the fib_lookup test. BPF_FIB_LOOKUP_VLAN
reduces a VLAN egress to its physical parent plus the tag, and
BPF_FIB_LOOKUP_VLAN_INPUT scopes the lookup to a VLAN subinterface.
BPF_FIB_LOOKUP_VLAN is XDP-only, since VLAN devices have no XDP xmit; the
tc helper rejects it with -EINVAL, which the table runner asserts for
every flag arm, and the egress result is checked through
bpf_xdp_fib_lookup(). Non-VLAN cases run through both helpers and assert
the path-independent results match; the XDP loop also checks dmac and,
for the tot_len cases, the route mtu_result, so the VLAN-egress dmac and
frag-needed coverage stays even though the tc path no longer reaches it.
The egress arms pin the reduction (parent ifindex plus tag, including
via a neighbour on the VLAN device, in OUTPUT mode, over a bond, and
through a DIRECT|TBID table) and the failure contract: a stacked-VLAN
(QinQ) egress returns BPF_FIB_LKUP_RET_VLAN_FAILURE with params->ifindex
left at the input. That is distinct from a no-neighbour return, which
reports the egress ifindex; only VLAN_FAILURE rewinds params->ifindex,
and a guard arm whose input and egress devices differ pins the
distinction. The VLAN_FAILURE arms are IPv4; the IPv6 path reaches it
through the same shared code, so an IPv6 arm would only re-test that.
The input arms use an iif rule that routes one destination to two
gateways, so the asserted gateway reveals which device the lookup used
as ingress, including VRF table selection through the l3mdev rule and
l3mdev_fib_table_rcu(). The VRF arms are IPv4-only: the l3mdev match
and table resolution are family-independent core shared by both rule
paths, and the IPv6 iif feed is pinned by the IPv6 VLAN input arm. A
cross-netns subtest moves a VLAN device into a second netns while it
stays registered on its parent and checks both directions fail closed
at the boundary.
A live-frames subtest (test_fib_lookup_vlan_redirect, with
BPF_F_TEST_XDP_LIVE_FRAMES) drives real frames through the native
xdp_do_redirect() / xdp_do_flush() path: a reducible egress is
redirected to the parent and delivered to its peer, while a QinQ egress
is passed to the stack, since redirecting to the VLAN device would drop
the frame at flush (no ndo_xdp_xmit).
The remaining per-case assertions are in the test table: resolution
semantics, the -EINVAL and NOT_FWDED error arms, and the SRC/SKIP_NEIGH
combinations.
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com>
---
v6 -> v7:
- local defines for the netns subtest addresses (Emil's review)
- the netns input arm brings the moved device up before the lookup, so
the namespace check is the only condition it can fail on
- the live-frames subtest uses its own netns name, so it cannot collide
with test_fib_lookup under test_progs -j, and counts only the test's
TCP frames, so background traffic cannot satisfy the delivery
assertion
- a stale mtu comment corrected
.../selftests/bpf/prog_tests/fib_lookup.c | 720 +++++++++++++++++-
.../testing/selftests/bpf/progs/fib_lookup.c | 57 ++
2 files changed, 773 insertions(+), 4 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..f7361f9a3459 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>
@@ -23,6 +24,7 @@
#define IPV4_TBID_ADDR "172.0.0.254"
#define IPV4_TBID_NET "172.0.0.0"
#define IPV4_TBID_DST "172.0.0.2"
+#define IPV4_TBID_NONEIGH_DST "172.0.0.5"
#define IPV6_TBID_ADDR "fd00::FFFF"
#define IPV6_TBID_NET "fd00::"
#define IPV6_TBID_DST "fd00::2"
@@ -37,6 +39,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 +89,17 @@ 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[] = {
@@ -79,6 +127,17 @@ static const struct fib_lookup_test tests[] = {
.daddr = IPV4_TBID_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS,
.lookup_flags = BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_TBID, .tbid = 100,
.dmac = DMAC_INIT2, },
+ /*
+ * An error that returns after the egress device is resolved must
+ * report the egress ifindex, not the input. This routes from input
+ * veth1 via veth2 (table 100) to a dst with no neighbour, so
+ * input != egress, pinning NO_NEIGH to the egress device.
+ */
+ { .desc = "IPv4 NO_NEIGH reports the egress ifindex, not the input",
+ .daddr = IPV4_TBID_NONEIGH_DST,
+ .expected_ret = BPF_FIB_LKUP_RET_NO_NEIGH,
+ .lookup_flags = BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_TBID, .tbid = 100,
+ .expected_dev = "veth2", },
{ .desc = "IPv6 TBID lookup failure",
.daddr = IPV6_TBID_DST, .expected_ret = BPF_FIB_LKUP_RET_NOT_FWDED,
.lookup_flags = BPF_FIB_LOOKUP_DIRECT | BPF_FIB_LOOKUP_TBID,
@@ -142,6 +201,218 @@ static const struct fib_lookup_test tests[] = {
.expected_dst = IPV6_GW1,
.lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH,
.mark = MARK, },
+ /* vlan egress resolution */
+ /*
+ * Invariant the VLAN-egress arms jointly enforce: a
+ * BPF_FIB_LOOKUP_VLAN SUCCESS always carries a physical,
+ * xmit-capable ifindex; no SUCCESS ever returns a VLAN-device
+ * ifindex. Reducible arms pin ifindex == the physical parent; the
+ * QinQ and foreign-netns arms pin VLAN_FAILURE with params->ifindex
+ * left at the input, so a regression to best-effort (SUCCESS + the
+ * VLAN ifindex) fails one.
+ */
+ { .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 is the VLAN device's mtu
+ * (1400), not 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, 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, QinQ not reducible (VLAN_FAILURE)",
+ .daddr = IPV4_QINQ_DST,
+ .expected_ret = BPF_FIB_LKUP_RET_VLAN_FAILURE,
+ .lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH,
+ .expected_dev = "veth1", .check_vlan = true, },
+ { .desc = "IPv4 QinQ egress without the flag (escape hatch)",
+ .daddr = IPV4_QINQ_DST, .expected_ret = BPF_FIB_LKUP_RET_SUCCESS,
+ .lookup_flags = BPF_FIB_LOOKUP_SKIP_NEIGH,
+ .expected_dev = QINQ_INNER_IFACE, },
+ { .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 +475,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 uses the
+ * FIB result (VLAN) device, so mtu_result is this value, which the
+ * no-flag arm below pins
+ */
+ 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 */
+ 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 +588,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;
@@ -298,7 +675,7 @@ void test_fib_lookup(void)
struct nstoken *nstoken = NULL;
struct __sk_buff skb = { };
struct fib_lookup *skel;
- int prog_fd, err, ret, i;
+ int prog_fd, xdp_fd, err, ret, i;
/* The test does not use the skb->data, so
* use pkt_v6 for both v6 and v4 test.
@@ -309,11 +686,16 @@ void test_fib_lookup(void)
.ctx_in = &skb,
.ctx_size_in = sizeof(skb),
);
+ LIBBPF_OPTS(bpf_test_run_opts, xdp_opts,
+ .data_in = &pkt_v6,
+ .data_size_in = sizeof(pkt_v6),
+ );
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);
+ xdp_fd = bpf_program__fd(skel->progs.fib_lookup_xdp);
SYS(fail, "ip netns add %s", NS_TEST);
@@ -343,6 +725,16 @@ void test_fib_lookup(void)
if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
continue;
+ /*
+ * BPF_FIB_LOOKUP_VLAN is XDP-only; the tc helper rejects it.
+ * These cases are exercised on the XDP path below.
+ */
+ if (tests[i].lookup_flags & BPF_FIB_LOOKUP_VLAN) {
+ ASSERT_EQ(skel->bss->fib_lookup_ret, -EINVAL,
+ "tc rejects BPF_FIB_LOOKUP_VLAN");
+ continue;
+ }
+
ASSERT_EQ(skel->bss->fib_lookup_ret, tests[i].expected_ret,
"fib_lookup_ret");
@@ -352,6 +744,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,17 +768,322 @@ 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;
}
}
+ /*
+ * Re-run the cases through bpf_xdp_fib_lookup(). test_run uses the
+ * current netns' loopback for ctx->rxq->dev, so dev_net() is NS_TEST
+ * and the lookup runs against its FIB. The path-independent results
+ * (return code, swapped ifindex, vlan tag, gateway) must match the skb
+ * path; the no-tot_len mtu_result is skb-specific and not rechecked.
+ */
+ for (i = 0; i < ARRAY_SIZE(tests); i++) {
+ if (set_lookup_params(fib_params, &tests[i], skb.ifindex))
+ continue;
+
+ skel->bss->fib_lookup_ret = -1;
+ skel->bss->lookup_flags = tests[i].lookup_flags;
+
+ err = bpf_prog_test_run_opts(xdp_fd, &xdp_opts);
+ if (!ASSERT_OK(err, "xdp test_run"))
+ continue;
+
+ if (!ASSERT_EQ(skel->bss->fib_lookup_ret, tests[i].expected_ret,
+ "xdp fib_lookup_ret"))
+ printf("(xdp) %s\n", tests[i].desc);
+
+ if (tests[i].expected_dev)
+ ASSERT_EQ(fib_params->ifindex,
+ if_nametoindex(tests[i].expected_dev),
+ "xdp ifindex");
+
+ if (tests[i].expected_dst)
+ assert_dst_ip(fib_params, tests[i].expected_dst);
+
+ if (tests[i].check_vlan) {
+ ASSERT_EQ(fib_params->h_vlan_proto,
+ htons(tests[i].vlan_proto), "xdp h_vlan_proto");
+ ASSERT_EQ(fib_params->h_vlan_TCI,
+ htons(tests[i].vlan_id), "xdp h_vlan_TCI");
+ }
+
+ ret = memcmp(tests[i].dmac, fib_params->dmac, sizeof(tests[i].dmac));
+ ASSERT_EQ(ret, 0, "xdp dmac");
+
+ /*
+ * mtu_result from a tot_len lookup is the route mtu and is
+ * path-independent; the no-tot_len arm reads dev->mtu and is
+ * skb-only, so gate on tot_len
+ */
+ if (tests[i].expected_mtu && tests[i].tot_len)
+ ASSERT_EQ(fib_params->mtu_result, tests[i].expected_mtu,
+ "xdp mtu_result");
+ }
+
fail:
if (nstoken)
close_netns(nstoken);
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"
+#define IPV4_VLAN_NETNS_ADDR "10.66.0.1"
+#define IPV4_VLAN_NETNS_DST "10.66.0.2"
+
+/*
+ * 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, xdp_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),
+ );
+ LIBBPF_OPTS(bpf_test_run_opts, xdp_opts,
+ .data_in = &pkt_v6,
+ .data_size_in = sizeof(pkt_v6),
+ );
+
+ 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);
+ xdp_fd = bpf_program__fd(skel->progs.fib_lookup_xdp);
+ 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);
+ /*
+ * up it in B before the input lookup: the move closed it, and a
+ * down device fails the resolver on IFF_UP before reaching the
+ * netns check this subtest exists to pin
+ */
+ SYS(fail, "ip -n %s link set dev veth7.66 up", 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, IPV4_VLAN_NETNS_DST, &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 addr add %s/24 dev veth7.66", IPV4_VLAN_NETNS_ADDR);
+ 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;
+
+ 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, IPV4_VLAN_NETNS_DST, &fib_params->ipv4_dst),
+ 1, "inet_pton(dst)") ||
+ !ASSERT_EQ(inet_pton(AF_INET, IPV4_VLAN_NETNS_ADDR, &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(xdp_fd, &xdp_opts);
+ if (!ASSERT_OK(err, "test_run(egress)"))
+ goto fail;
+ ASSERT_EQ(skel->bss->fib_lookup_ret, BPF_FIB_LKUP_RET_VLAN_FAILURE,
+ "egress returns VLAN_FAILURE");
+ 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);
+}
+
+#define REDIRECT_NPKTS 1000
+#define NS_REDIRECT "fib_lookup_redirect_ns"
+
+/*
+ * The egress flag exists so an XDP program can redirect to the physical
+ * parent. A redirect that lands on a VLAN device is dropped at
+ * xdp_do_flush(), because a VLAN device has no ndo_xdp_xmit. Drive real
+ * frames with BPF_F_TEST_XDP_LIVE_FRAMES, which runs the native
+ * xdp_do_redirect() + xdp_do_flush() path: a reducible VLAN egress
+ * resolves to veth1 and is delivered to its peer veth2, while a QinQ
+ * egress returns VLAN_FAILURE and is passed to the stack instead of
+ * redirected to a device that would silently drop it.
+ */
+void test_fib_lookup_vlan_redirect(void)
+{
+ int redirect_fd, err, veth1_idx, veth2_idx = -1;
+ struct bpf_fib_lookup *fib_params;
+ struct nstoken *nstoken = NULL;
+ struct fib_lookup *skel = NULL;
+ bool xdp_attached = false;
+
+ LIBBPF_OPTS(bpf_test_run_opts, lf_opts,
+ .data_in = &pkt_v4,
+ .data_size_in = sizeof(pkt_v4),
+ .flags = BPF_F_TEST_XDP_LIVE_FRAMES,
+ .repeat = REDIRECT_NPKTS,
+ );
+
+ skel = fib_lookup__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel open_and_load"))
+ return;
+ redirect_fd = bpf_program__fd(skel->progs.fib_lookup_redirect);
+ fib_params = &skel->bss->fib_params;
+
+ SYS(fail, "ip netns add %s", NS_REDIRECT);
+ nstoken = open_netns(NS_REDIRECT);
+ if (!ASSERT_OK_PTR(nstoken, "open_netns"))
+ goto fail;
+ if (setup_netns())
+ goto fail;
+
+ veth1_idx = if_nametoindex("veth1");
+ veth2_idx = if_nametoindex("veth2");
+ if (!ASSERT_NEQ(veth1_idx, 0, "if_nametoindex(veth1)") ||
+ !ASSERT_NEQ(veth2_idx, 0, "if_nametoindex(veth2)"))
+ goto fail;
+
+ /*
+ * A redirect to veth1 is delivered to its peer veth2. veth_xdp_xmit()
+ * only accepts the frame if veth2's NAPI is up, which on veth means
+ * veth2 carries an XDP program; xdp_count tallies what arrives.
+ */
+ err = bpf_xdp_attach(veth2_idx, bpf_program__fd(skel->progs.xdp_count),
+ XDP_FLAGS_DRV_MODE, NULL);
+ if (!ASSERT_OK(err, "attach xdp_count on veth2"))
+ goto fail;
+ xdp_attached = true;
+
+ /* reducible VLAN egress: resolves to the physical parent veth1 */
+ memset(fib_params, 0, sizeof(*fib_params));
+ fib_params->family = AF_INET;
+ fib_params->l4_protocol = IPPROTO_TCP;
+ fib_params->ifindex = veth1_idx;
+ if (!ASSERT_EQ(inet_pton(AF_INET, IPV4_IFACE_ADDR, &fib_params->ipv4_src),
+ 1, "inet_pton(src)") ||
+ !ASSERT_EQ(inet_pton(AF_INET, IPV4_VLAN_EGRESS_DST, &fib_params->ipv4_dst),
+ 1, "inet_pton(reducible dst)"))
+ goto fail;
+ skel->bss->lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH;
+ skel->bss->redirected = 0;
+ skel->bss->passed = 0;
+ skel->bss->delivered = 0;
+
+ err = bpf_prog_test_run_opts(redirect_fd, &lf_opts);
+ if (!ASSERT_OK(err, "test_run(reducible egress)"))
+ goto fail;
+ ASSERT_EQ(skel->bss->redirected, REDIRECT_NPKTS, "reducible egress redirected");
+ ASSERT_EQ(skel->bss->passed, 0, "reducible egress not passed");
+ ASSERT_GT(skel->bss->delivered, 0, "reducible egress delivered to veth2");
+
+ /*
+ * QinQ egress: not reducible, so the lookup returns VLAN_FAILURE and
+ * the program passes the frame instead of redirecting to the inner
+ * VLAN device. redirected == 0 is the assertion that matters: the
+ * program did not redirect to a device that would drop the frame at
+ * xdp_do_flush(). veth2's delivered count is not checked here, since
+ * a passed frame can still reach veth2 through the stack's forwarding
+ * path, which is unrelated to the redirect under test.
+ */
+ memset(fib_params, 0, sizeof(*fib_params));
+ fib_params->family = AF_INET;
+ fib_params->l4_protocol = IPPROTO_TCP;
+ fib_params->ifindex = veth1_idx;
+ if (!ASSERT_EQ(inet_pton(AF_INET, IPV4_IFACE_ADDR, &fib_params->ipv4_src),
+ 1, "inet_pton(src)") ||
+ !ASSERT_EQ(inet_pton(AF_INET, IPV4_QINQ_DST, &fib_params->ipv4_dst),
+ 1, "inet_pton(qinq dst)"))
+ goto fail;
+ skel->bss->lookup_flags = BPF_FIB_LOOKUP_VLAN | BPF_FIB_LOOKUP_SKIP_NEIGH;
+ skel->bss->redirected = 0;
+ skel->bss->passed = 0;
+
+ err = bpf_prog_test_run_opts(redirect_fd, &lf_opts);
+ if (!ASSERT_OK(err, "test_run(qinq egress)"))
+ goto fail;
+ ASSERT_EQ(skel->bss->passed, REDIRECT_NPKTS, "qinq egress passed");
+ ASSERT_EQ(skel->bss->redirected, 0, "qinq egress not redirected");
+
+fail:
+ if (xdp_attached)
+ bpf_xdp_detach(veth2_idx, XDP_FLAGS_DRV_MODE, NULL);
+ if (nstoken)
+ close_netns(nstoken);
+ SYS_NOFAIL("ip netns del " NS_REDIRECT);
+ fib_lookup__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/fib_lookup.c b/tools/testing/selftests/bpf/progs/fib_lookup.c
index 7b5dd2214ff4..36b7218d9ae2 100644
--- a/tools/testing/selftests/bpf/progs/fib_lookup.c
+++ b/tools/testing/selftests/bpf/progs/fib_lookup.c
@@ -4,7 +4,11 @@
#include <linux/types.h>
#include <linux/bpf.h>
#include <linux/pkt_cls.h>
+#include <linux/if_ether.h>
+#include <linux/ip.h>
+#include <linux/in.h>
#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
struct bpf_fib_lookup fib_params = {};
int fib_lookup_ret = 0;
@@ -19,4 +23,57 @@ int fib_lookup(struct __sk_buff *skb)
return TC_ACT_SHOT;
}
+SEC("xdp")
+int fib_lookup_xdp(struct xdp_md *ctx)
+{
+ fib_lookup_ret = bpf_fib_lookup(ctx, &fib_params, sizeof(fib_params),
+ lookup_flags);
+
+ return XDP_DROP;
+}
+
+int redirected = 0;
+int passed = 0;
+int delivered = 0;
+
+SEC("xdp")
+int fib_lookup_redirect(struct xdp_md *ctx)
+{
+ struct bpf_fib_lookup params = fib_params;
+ long ret;
+
+ ret = bpf_fib_lookup(ctx, ¶ms, sizeof(params), lookup_flags);
+ if (ret == BPF_FIB_LKUP_RET_SUCCESS) {
+ redirected++;
+ return bpf_redirect(params.ifindex, 0);
+ }
+
+ passed++;
+ return XDP_PASS;
+}
+
+SEC("xdp")
+int xdp_count(struct xdp_md *ctx)
+{
+ void *data = (void *)(long)ctx->data;
+ void *data_end = (void *)(long)ctx->data_end;
+ struct ethhdr *eth = data;
+ struct iphdr *iph;
+
+ /*
+ * count only the test's TCP frames: the netns has live
+ * link-local traffic (DAD, MLD) that would satisfy a bare
+ * counter
+ */
+ if ((void *)(eth + 1) > data_end ||
+ eth->h_proto != bpf_htons(ETH_P_IP))
+ return XDP_DROP;
+ iph = (void *)(eth + 1);
+ if ((void *)(iph + 1) > data_end || iph->protocol != IPPROTO_TCP)
+ return XDP_DROP;
+
+ delivered++;
+ return XDP_DROP;
+}
+
char _license[] SEC("license") = "GPL";
--
2.54.0
^ permalink raw reply related
* [PATCH bpf-next v7 2/3] bpf: Add BPF_FIB_LOOKUP_VLAN_INPUT flag to bpf_fib_lookup() helper
From: Avinash Duduskar @ 2026-07-13 16:23 UTC (permalink / raw)
To: ast, daniel, andrii
Cc: eddyz87, memxor, martin.lau, song, yonghong.song, jolsa, emil,
john.fastabend, sdf, davem, edumazet, kuba, pabeni, horms, shuah,
hawk, yatsenko, leon.hwang, kpsingh, a.s.protopopov, ameryhung,
rongtao, eyal.birger, bpf, netdev, linux-kernel, linux-kselftest,
toke, dsahern
In-Reply-To: <20260713162305.1237211-1-avinash.duduskar@gmail.com>
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 a valid proto returns NOT_FWDED, which is
correct since no VLAN device can exist.
Suggested-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com>
---
v6 -> v7:
- no code change; commit message correction: under !CONFIG_VLAN_8021Q
an invalid proto still returns -EINVAL (the proto check precedes the
stub), so only valid-proto lookups return NOT_FWDED
include/uapi/linux/bpf.h | 21 ++++++++++-
net/core/filter.c | 66 +++++++++++++++++++++++++++++++---
tools/include/uapi/linux/bpf.h | 21 ++++++++++-
3 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4ce1491e4ea6..ff03019abe35 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3557,6 +3557,22 @@ union bpf_attr {
* This flag is only valid for XDP programs; tc programs
* receive -EINVAL since they can redirect to the VLAN
* device directly.
+ * **BPF_FIB_LOOKUP_VLAN_INPUT**
+ * Treat *params*->h_vlan_proto and *params*->h_vlan_TCI
+ * as an input VLAN tag and run the lookup as if ingress
+ * had happened on the VLAN subinterface carrying that tag
+ * on *params*->ifindex. 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, else
+ * **-EINVAL**. If *params*->ifindex is itself a VLAN
+ * device, its inner (QinQ) subinterface is matched; for a
+ * bond or team, pass the master's ifindex. An unmatched
+ * tag, a down device, or one in another namespace returns
+ * **BPF_FIB_LKUP_RET_NOT_FWDED**, mirroring real ingress.
+ * A VID of 0 is looked up literally, so do not set this
+ * flag for priority-tagged frames. Cannot be combined with
+ * **BPF_FIB_LOOKUP_TBID** or **BPF_FIB_LOOKUP_OUTPUT**
+ * (returns **-EINVAL**).
*
* *ctx* is either **struct xdp_md** for XDP programs or
* **struct sk_buff** tc cls_act programs.
@@ -7353,6 +7369,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 {
@@ -7423,7 +7440,9 @@ struct bpf_fib_lookup {
/*
* 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 b5a45485a54b..0ea362fa4287 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6229,6 +6229,25 @@ static int bpf_fib_set_fwd_params(struct net_device *dev,
return 0;
}
+
+static struct net_device *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 ERR_PTR(-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 NULL;
+
+ return vlan_dev;
+}
#endif
#if IS_ENABLED(CONFIG_INET)
@@ -6249,6 +6268,14 @@ 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) {
+ dev = bpf_fib_vlan_input_dev(dev, params);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+ if (!dev)
+ return BPF_FIB_LKUP_RET_NOT_FWDED;
+ }
+
/* verify forwarding is enabled on this interface */
in_dev = __in_dev_get_rcu(dev);
if (unlikely(!in_dev || !IN_DEV_FORWARD(in_dev)))
@@ -6258,7 +6285,11 @@ 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);
@@ -6395,6 +6426,14 @@ 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) {
+ dev = bpf_fib_vlan_input_dev(dev, params);
+ if (IS_ERR(dev))
+ return PTR_ERR(dev);
+ if (!dev)
+ return BPF_FIB_LKUP_RET_NOT_FWDED;
+ }
+
idev = __in6_dev_get_safely(dev);
if (unlikely(!idev || !READ_ONCE(idev->cnf.forwarding)))
return BPF_FIB_LKUP_RET_FWD_DISABLED;
@@ -6403,7 +6442,12 @@ 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;
}
@@ -6514,7 +6558,19 @@ 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;
+
+ 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)
@@ -6522,7 +6578,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) {
@@ -6560,7 +6616,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 (flags & BPF_FIB_LOOKUP_VLAN)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 4ce1491e4ea6..ff03019abe35 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3557,6 +3557,22 @@ union bpf_attr {
* This flag is only valid for XDP programs; tc programs
* receive -EINVAL since they can redirect to the VLAN
* device directly.
+ * **BPF_FIB_LOOKUP_VLAN_INPUT**
+ * Treat *params*->h_vlan_proto and *params*->h_vlan_TCI
+ * as an input VLAN tag and run the lookup as if ingress
+ * had happened on the VLAN subinterface carrying that tag
+ * on *params*->ifindex. 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, else
+ * **-EINVAL**. If *params*->ifindex is itself a VLAN
+ * device, its inner (QinQ) subinterface is matched; for a
+ * bond or team, pass the master's ifindex. An unmatched
+ * tag, a down device, or one in another namespace returns
+ * **BPF_FIB_LKUP_RET_NOT_FWDED**, mirroring real ingress.
+ * A VID of 0 is looked up literally, so do not set this
+ * flag for priority-tagged frames. Cannot be combined with
+ * **BPF_FIB_LOOKUP_TBID** or **BPF_FIB_LOOKUP_OUTPUT**
+ * (returns **-EINVAL**).
*
* *ctx* is either **struct xdp_md** for XDP programs or
* **struct sk_buff** tc cls_act programs.
@@ -7353,6 +7369,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 {
@@ -7423,7 +7440,9 @@ struct bpf_fib_lookup {
/*
* 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
* [PATCH bpf-next v7 1/3] bpf: Add BPF_FIB_LOOKUP_VLAN flag to bpf_fib_lookup() helper
From: Avinash Duduskar @ 2026-07-13 16:23 UTC (permalink / raw)
To: ast, daniel, andrii
Cc: eddyz87, memxor, martin.lau, song, yonghong.song, jolsa, emil,
john.fastabend, sdf, davem, edumazet, kuba, pabeni, horms, shuah,
hawk, yatsenko, leon.hwang, kpsingh, a.s.protopopov, ameryhung,
rongtao, eyal.birger, bpf, netdev, linux-kernel, linux-kselftest,
toke, dsahern
In-Reply-To: <20260713162305.1237211-1-avinash.duduskar@gmail.com>
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. When
the immediate parent is not a real device in the same namespace, the
lookup returns BPF_FIB_LKUP_RET_VLAN_FAILURE and leaves params->ifindex
at the input. This covers a stacked VLAN (QinQ), where the immediate
parent is itself a VLAN device and one h_vlan_proto/h_vlan_TCI pair
cannot describe two tags, and a parent in another network namespace (a
VLAN device can be moved while its parent stays), whose ifindex would
be meaningless in the caller's namespace. A program that wants the
VLAN device's own ifindex re-issues the lookup, with a re-initialized
params, without BPF_FIB_LOOKUP_VLAN, so the unreducible case stays
distinct from a physical egress. That distinction matters for XDP: a
program cannot xmit on a VLAN device, so a success carrying the VLAN
ifindex would make it redirect to a device with no ndo_xdp_xmit and
drop the frame at xdp_do_flush(). The swap and the vlan fields are
written only on the reduce path; other output fields keep their
existing behaviour, so a frag-needed result still reports the route
mtu in params->mtu_result.
BPF_FIB_LOOKUP_VLAN is only useful to XDP, which cannot redirect to a
VLAN device. A tc program can redirect to the VLAN device directly, so
bpf_skb_fib_lookup() rejects the flag with -EINVAL; bpf_xdp_fib_lookup()
accepts it. 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. That is safe because no VLAN device can exist there, so every
egress is already physical.
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>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: David Ahern <dsahern@kernel.org>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com>
---
v6 -> v7:
- uapi doc: repeating the lookup after BPF_FIB_LKUP_RET_VLAN_FAILURE
needs a re-initialized params, since output fields overwrite the
inputs they share storage with; commit message aligned
include/uapi/linux/bpf.h | 33 ++++++++++++++++++++++++++++++++-
net/core/filter.c | 33 +++++++++++++++++++++++++++++----
tools/include/uapi/linux/bpf.h | 33 ++++++++++++++++++++++++++++++++-
3 files changed, 93 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 89b36de5fdbb..4ce1491e4ea6 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3532,6 +3532,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. *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; if it
+ * is itself a VLAN device (QinQ) or in another namespace,
+ * the egress cannot be reduced to a physical device plus
+ * one tag and the lookup returns
+ * **BPF_FIB_LKUP_RET_VLAN_FAILURE** with *params*->ifindex
+ * left at the input. To obtain the VLAN device's own
+ * ifindex, repeat the lookup without
+ * **BPF_FIB_LOOKUP_VLAN**, re-initializing *params*
+ * first: output fields overwrite the inputs they share
+ * storage with. 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.
+ * This flag is only valid for XDP programs; tc programs
+ * receive -EINVAL since they can redirect to the VLAN
+ * device directly.
*
* *ctx* is either **struct xdp_md** for XDP programs or
* **struct sk_buff** tc cls_act programs.
@@ -7327,6 +7352,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 {
@@ -7340,6 +7366,7 @@ enum {
BPF_FIB_LKUP_RET_NO_NEIGH, /* no neighbor entry for nh */
BPF_FIB_LKUP_RET_FRAG_NEEDED, /* fragmentation required to fwd */
BPF_FIB_LKUP_RET_NO_SRC_ADDR, /* failed to derive IP src addr */
+ BPF_FIB_LKUP_RET_VLAN_FAILURE, /* VLAN egress, parent unresolvable */
};
struct bpf_fib_lookup {
@@ -7393,7 +7420,11 @@ 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 2e96b4b847ce..b5a45485a54b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6201,10 +6201,29 @@ 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, u32 in_ifindex)
{
params->h_vlan_TCI = 0;
params->h_vlan_proto = 0;
+
+#if IS_ENABLED(CONFIG_VLAN_8021Q)
+ if ((flags & BPF_FIB_LOOKUP_VLAN) && is_vlan_dev(dev)) {
+ struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
+
+ 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;
+ } else {
+ params->ifindex = in_ifindex;
+ return BPF_FIB_LKUP_RET_VLAN_FAILURE;
+ }
+ }
+#endif
+
if (mtu)
params->mtu_result = mtu; /* union with tot_len */
@@ -6216,6 +6235,7 @@ static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params, u32 mtu)
static int bpf_ipv4_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
u32 flags, bool check_mtu)
{
+ u32 in_ifindex = params->ifindex;
struct neighbour *neigh = NULL;
struct fib_nh_common *nhc;
struct in_device *in_dev;
@@ -6347,7 +6367,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, in_ifindex);
}
#endif
@@ -6357,6 +6377,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
{
struct in6_addr *src = (struct in6_addr *) params->ipv6_src;
struct in6_addr *dst = (struct in6_addr *) params->ipv6_dst;
+ u32 in_ifindex = params->ifindex;
struct fib6_result res = {};
struct neighbour *neigh;
struct net_device *dev;
@@ -6486,13 +6507,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, in_ifindex);
}
#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)
@@ -6541,6 +6563,9 @@ BPF_CALL_4(bpf_skb_fib_lookup, struct sk_buff *, skb,
if (flags & ~BPF_FIB_LOOKUP_MASK)
return -EINVAL;
+ if (flags & BPF_FIB_LOOKUP_VLAN)
+ return -EINVAL;
+
if (params->tot_len)
check_mtu = true;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 89b36de5fdbb..4ce1491e4ea6 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3532,6 +3532,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. *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; if it
+ * is itself a VLAN device (QinQ) or in another namespace,
+ * the egress cannot be reduced to a physical device plus
+ * one tag and the lookup returns
+ * **BPF_FIB_LKUP_RET_VLAN_FAILURE** with *params*->ifindex
+ * left at the input. To obtain the VLAN device's own
+ * ifindex, repeat the lookup without
+ * **BPF_FIB_LOOKUP_VLAN**, re-initializing *params*
+ * first: output fields overwrite the inputs they share
+ * storage with. 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.
+ * This flag is only valid for XDP programs; tc programs
+ * receive -EINVAL since they can redirect to the VLAN
+ * device directly.
*
* *ctx* is either **struct xdp_md** for XDP programs or
* **struct sk_buff** tc cls_act programs.
@@ -7327,6 +7352,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 {
@@ -7340,6 +7366,7 @@ enum {
BPF_FIB_LKUP_RET_NO_NEIGH, /* no neighbor entry for nh */
BPF_FIB_LKUP_RET_FRAG_NEEDED, /* fragmentation required to fwd */
BPF_FIB_LKUP_RET_NO_SRC_ADDR, /* failed to derive IP src addr */
+ BPF_FIB_LKUP_RET_VLAN_FAILURE, /* VLAN egress, parent unresolvable */
};
struct bpf_fib_lookup {
@@ -7393,7 +7420,11 @@ 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
* [PATCH bpf-next v7 0/3] bpf: bidirectional VLAN support for bpf_fib_lookup()
From: Avinash Duduskar @ 2026-07-13 16:23 UTC (permalink / raw)
To: ast, daniel, andrii
Cc: eddyz87, memxor, martin.lau, song, yonghong.song, jolsa, emil,
john.fastabend, sdf, davem, edumazet, kuba, pabeni, horms, shuah,
hawk, yatsenko, leon.hwang, kpsingh, a.s.protopopov, ameryhung,
rongtao, eyal.birger, bpf, netdev, linux-kernel, linux-kselftest,
toke, dsahern
This series adds VLAN awareness to bpf_fib_lookup() in both directions.
BPF_FIB_LOOKUP_VLAN resolves a VLAN egress to its underlying real device
plus the VLAN tag (XDP programs need this because VLAN devices have no
XDP xmit), and BPF_FIB_LOOKUP_VLAN_INPUT runs the lookup as if a tagged
frame had arrived on the matching VLAN subinterface, for iif policy
routing and VRF table selection.
BPF_FIB_LOOKUP_VLAN opts in to replacing params->ifindex, whose value
existing programs consume since d1c362e1dd68 ("bpf: Always return
target ifindex in bpf_fib_lookup"); without it the output is unchanged.
An egress that does not reduce to a real device plus one tag (a QinQ
stack, or a parent in another network namespace) returns
BPF_FIB_LKUP_RET_VLAN_FAILURE with params->ifindex left at the input;
repeating the lookup without the flag, with a re-initialized params,
returns the VLAN device's own ifindex. A VLAN on a bond reduces to the
bond, which picks its egress slave at xmit.
The new return code is appended after BPF_FIB_LKUP_RET_NO_SRC_ADDR
(nothing renumbered, tools/ mirror updated) and is returned only when
the flag is set, so no existing caller can observe it.
Changes v6 -> v7:
- Patch 1 (BPF_FIB_LOOKUP_VLAN: resolve a VLAN egress to its real
device plus the tag): uapi doc clarified, repeating the lookup after
BPF_FIB_LKUP_RET_VLAN_FAILURE needs a re-initialized params, since
output fields overwrite the inputs they share storage with. No
functional change.
- Patch 2 (BPF_FIB_LOOKUP_VLAN_INPUT: run the lookup as if the tagged
frame arrived on the matching VLAN subinterface): no code change; a
commit message correction (an invalid proto returns -EINVAL under
!CONFIG_VLAN_8021Q too).
- Patch 3 (selftests for both flags, tc and XDP paths): local defines
for the netns subtest addresses (Emil's review); the netns input arm
brings the moved device up first, so the namespace check is the only
condition it can fail on; the live-frames subtest uses its own netns
name (no collision under test_progs -j) and counts only the test's
TCP frames, so background traffic cannot satisfy the delivery
assertion; a stale mtu comment corrected.
v6: https://lore.kernel.org/all/20260704092159.1256823-1-avinash.duduskar@gmail.com/
v5: https://lore.kernel.org/all/20260624030530.3342884-1-avinash.duduskar@gmail.com/
v4: https://lore.kernel.org/all/20260623025147.1001664-1-avinash.duduskar@gmail.com/
v3: https://lore.kernel.org/all/20260617224729.1428662-1-avinash.duduskar@gmail.com/
v2: https://lore.kernel.org/all/20260616223426.3568080-1-avinash.duduskar@gmail.com/
v1: https://lore.kernel.org/all/20260609172052.81613-1-avinash.duduskar@gmail.com/
Avinash Duduskar (3):
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 | 52 +-
net/core/filter.c | 97 ++-
tools/include/uapi/linux/bpf.h | 52 +-
.../selftests/bpf/prog_tests/fib_lookup.c | 720 +++++++++++++++++-
.../testing/selftests/bpf/progs/fib_lookup.c | 57 ++
5 files changed, 964 insertions(+), 14 deletions(-)
base-commit: a975094bf98ca97be9146f9d3b5681a6f9cf5ce3
--
2.54.0
^ permalink raw reply
* Re: [PATCH nf] netfilter: ipset: skip extension destroy on hash resize replay
From: Weiming Shi @ 2026-07-13 16:18 UTC (permalink / raw)
To: Jozsef Kadlecsik
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, netfilter-devel, coreteam,
netdev, linux-kernel, Xiang Mei
In-Reply-To: <e3bb8ad1-cb24-5d74-6ca6-a7a1e41fb133@blackhole.kfki.hu>
Jozsef Kadlecsik <kadlec@blackhole.kfki.hu> 于2026年7月13日周一 20:59写道：
>
> Hi,
>
> On Fri, 3 Jul 2026, Weiming Shi wrote:
>
> > During a hash set resize, mtype_resize() copies each element into the
> > new table with memcpy(), so the new-table element shares the old-table
> > element's comment extension. An xt_SET delete on the old table during
> > the resize destroys that shared comment via ip_set_ext_destroy() and
> > queues a replayed delete on h->ad. After the table swap mtype_resize()
> > replays it with mtype_del() on the new table, whose copy still points at
> > the freed comment, so ip_set_ext_destroy() frees it a second time:
> >
> > ODEBUG: activate active (active state 1) object: ... object type: rcu_head
> > WARNING: CPU: 3 PID: 5311 at lib/debugobjects.c:514 debug_print_object
> > Call Trace:
> > <IRQ>
> > kvfree_call_rcu (kernel/rcu/tree.c:3825)
> > ip_set_comment_free (net/netfilter/ipset/ip_set_core.c:397)
> > hash_ip4_del (net/netfilter/ipset/ip_set_hash_gen.h:1098)
> > hash_ip4_kadt (net/netfilter/ipset/ip_set_hash_ip.c:96)
> > ip_set_del (net/netfilter/ipset/ip_set_core.c:813)
> > set_target_v3 (net/netfilter/xt_set.c:412)
> > ipt_do_table (net/ipv4/netfilter/ip_tables.c:346)
> > __ip_local_out (net/ipv4/ip_output.c:119)
> > icmp_push_reply (net/ipv4/icmp.c:397)
> > __icmp_send (net/ipv4/icmp.c:804)
> > __udp4_lib_rcv (net/ipv4/udp.c:2521)
> > ip_local_deliver (net/ipv4/ip_input.c:254)
> > ip_rcv (net/ipv4/ip_input.c:569)
> > </IRQ>
> >
> > The replay passes a NULL ext (the kernel-side delete that queued it
> > already destroyed the extensions), so skip ip_set_ext_destroy() when ext
> > is NULL. This also avoids the NULL ext->target dereference that was only
> > kept safe by the new table's ref being zero.
> >
> > Reachable from an unprivileged user namespace.
> >
> > Fixes: f66ee0410b1c ("netfilter: ipset: Fix \"INFO: rcu detected stall in hash_xxx\" reports")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> > net/netfilter/ipset/ip_set_hash_gen.h | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
> > index 5e4453e9e..bc909ae2d 100644
> > --- a/net/netfilter/ipset/ip_set_hash_gen.h
> > +++ b/net/netfilter/ipset/ip_set_hash_gen.h
> > @@ -1080,9 +1080,11 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
> > mtype_del_cidr(set, h,
> > NCIDR_PUT(DCIDR_GET(d->cidr, j)), j);
> > #endif
> > - ip_set_ext_destroy(set, data);
> > + /* On a resize replay the extensions were already destroyed. */
> > + if (ext)
> > + ip_set_ext_destroy(set, data);
> >
> > - if (atomic_read(&t->ref) && ext->target) {
> > + if (ext && atomic_read(&t->ref) && ext->target) {
> > /* Resize is in process and kernel side del,
> > * save values
> > */
>
> Please rebase your patch against the nf-next tree
> (git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git): the
> second chunk of your patch is not needed as it has been fixed.
>
> Thank you and best regards,
> Jozsef
Hi,
Thanks for your review. v2 sent.
Best regards,
Weiming Shi
^ permalink raw reply
* [PATCH nf-next v2] netfilter: ipset: skip extension destroy on hash resize replay
From: Weiming Shi @ 2026-07-13 16:15 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
Phil Sutter
Cc: netfilter-devel, coreteam, netdev, linux-kernel, Xiang Mei,
Weiming Shi
In-Reply-To: <PASTE-JOZSEF-MESSAGE-ID-HERE>
During a hash set resize, mtype_resize() copies each element into the
new table with memcpy(), so the new-table element shares the old-table
element's comment extension. An xt_SET delete on the old table during
the resize destroys that shared comment via ip_set_ext_destroy() and
queues a replayed delete on h->ad. After the table swap mtype_resize()
replays it with mtype_del() on the new table, whose copy still points at
the freed comment, so ip_set_ext_destroy() frees it a second time:
ODEBUG: activate active (active state 1) object: ... object type: rcu_head
WARNING: CPU: 3 PID: 5311 at lib/debugobjects.c:514 debug_print_object
Call Trace:
<IRQ>
kvfree_call_rcu (kernel/rcu/tree.c:3825)
ip_set_comment_free (net/netfilter/ipset/ip_set_core.c:397)
hash_ip4_del (net/netfilter/ipset/ip_set_hash_gen.h:1098)
hash_ip4_kadt (net/netfilter/ipset/ip_set_hash_ip.c:96)
ip_set_del (net/netfilter/ipset/ip_set_core.c:813)
set_target_v3 (net/netfilter/xt_set.c:412)
ipt_do_table (net/ipv4/netfilter/ip_tables.c:346)
__ip_local_out (net/ipv4/ip_output.c:119)
icmp_push_reply (net/ipv4/icmp.c:397)
__icmp_send (net/ipv4/icmp.c:804)
__udp4_lib_rcv (net/ipv4/udp.c:2521)
ip_local_deliver (net/ipv4/ip_input.c:254)
ip_rcv (net/ipv4/ip_input.c:569)
</IRQ>
The replay passes a NULL ext (the kernel-side delete that queued it
already destroyed the extensions), so skip ip_set_ext_destroy() when ext
is NULL.
Reachable from an unprivileged user namespace.
Fixes: f66ee0410b1c ("netfilter: ipset: Fix \"INFO: rcu detected stall in hash_xxx\" reports")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v2: Rebase onto nf-next; drop the second hunk (already fixed there).
net/netfilter/ipset/ip_set_hash_gen.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 5e4453e9e..bc909ae2d 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1080,7 +1080,9 @@ mtype_del(struct ip_set *set, void *value, const struct ip_set_ext *ext,
mtype_del_cidr(set, h,
NCIDR_PUT(DCIDR_GET(d->cidr, j)), j);
#endif
- ip_set_ext_destroy(set, data);
+ /* On a resize replay the extensions were already destroyed. */
+ if (ext)
+ ip_set_ext_destroy(set, data);
if (t->resizing && ext && ext->target) {
/* Resize is in process and kernel side del,
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net v2] tls: Fix race condition in tls_sw_cancel_work_tx()
From: Alexander Martyniuk @ 2026-07-13 16:07 UTC (permalink / raw)
To: imv4bel
Cc: davem, edumazet, horms, john.fastabend, kuba, netdev, pabeni, sd,
Alexander Martyniuk
In-Reply-To: <aZgsFO6nfylfvLE7@v4bel>
> To prevent this race condition, cancel_delayed_work_sync() is
> replaced with disable_delayed_work_sync().
In lower stable kernel versions no such function as
disable_delayed_work_sync(). So could it be fixed by
testing BIT_TX_CLOSING in tls_sw_write_space() before
calling schedule_delayed_work() ?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox