* Re: [PATCH net-next 2/4] net: usb: centralize usbnet_cdc_zte_rx_fixup in usbnet
From: Paolo Abeni @ 2026-07-09 13:56 UTC (permalink / raw)
To: Oliver Neukum, Manuel Ebner, andrew+netdev, davem, edumazet, kuba,
shaoxul, netdev, linux-usb, linux-kernel
In-Reply-To: <cbb51e43-ad79-4b6f-b38b-3ec2221de814@suse.com>
On 7/6/26 2:00 PM, Oliver Neukum wrote:
> On 02.07.26 20:19, Manuel Ebner wrote:
>> On Thu, 2026-07-02 at 16:25 +0200, Oliver Neukum wrote:
>
>>>
>>> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
>>> index 5544af1f4aa5..7beea6d0e731 100644
>>> --- a/drivers/net/usb/usbnet.c
>>> +++ b/drivers/net/usb/usbnet.c
>>> @@ -2347,6 +2347,25 @@ void usbnet_cdc_status(struct usbnet *dev, struct urb *urb)
>>> }
>>> }
>>> EXPORT_SYMBOL_GPL(usbnet_cdc_status);
>>> +
>>> +/* Make sure packets have correct destination MAC address
>> /*
>> * Make sure packets have the correct destination MAC address
>
> This sort of puts me into a conundrum. Does networking code use
> the short style of comments or not? The documentation says that it does.
> I am fine either way, but I need a clear rule.
Networking code used to require
/* for
* multiline comments
*/
but somewhat recently we switched to accept also the more common
/*
* multiline comment
*/
Checkpatch accepts both; it's not a deal breaker either ways.
/P
^ permalink raw reply
* Re: [PATCH] docs: packing: fix brackets
From: Andrew Lunn @ 2026-07-09 13:58 UTC (permalink / raw)
To: Manuel Ebner
Cc: Vladimir Oltean, Jonathan Corbet, Shuah Khan, netdev, linux-doc,
linux-kernel
In-Reply-To: <20260709121427.391749-2-manuelebner@mailbox.org>
On Thu, Jul 09, 2026 at 02:14:27PM +0200, Manuel Ebner wrote:
> Add two ')' to nested functions in code block.
>
> Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
Please add a Fixes tag.
With that: Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
---
pw-bot: cr
^ permalink raw reply
* Re: [PATCH v2 03/18] dt-bindings: clock: mediatek: Make '#clock-cells' required for MT8195
From: AngeloGioacchino Del Regno @ 2026-07-09 14:01 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-3-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Both MT8195 clock controllers dt-bindings (clock and sys-clock)
> document the '#clock-cells' property but do not enforce it as required
> property.
> As clock provider nodes should define this property in devicetrees, add
> it to the required property list in both its dt-bindings files.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 02/18] dt-bindings: clock: mediatek: Make '#clock-cells' required for MT8192
From: AngeloGioacchino Del Regno @ 2026-07-09 14:01 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-2-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Both MT8192 clock controllers dt-bindings (clock and sys-clock)
> document the '#clock-cells' property but do not enforce it as required
> property.
> As clock provider nodes should define this property in devicetrees, add
> it to the required property list in both its dt-bindings files.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 01/18] dt-bindings: clock: mediatek: Make '#clock-cells' required for MT8186
From: AngeloGioacchino Del Regno @ 2026-07-09 14:01 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-1-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Both MT8186 clock controllers dt-bindings (clock and sys-clock)
> document the '#clock-cells' property but do not enforce it as required
> property.
> As clock provider nodes should define this property in devicetrees, add
> it to the required property list in both its dt-bindings files.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Though you could've done that in one commit, or just skipped the additions in all
of mt8186/92/95 and went directly for the new one... IMO, it's ok as you're showing
that there was an actual mistake on the bindings for clock controllers.
Same comment applies for patch 2 and 3, btw.
Anyway:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 04/18] dt-bindings: clock: mediatek: reorder MT8186 compatibles
From: AngeloGioacchino Del Regno @ 2026-07-09 14:03 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-4-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> In order to prepare regrouping several Mediatek SoC clock controller
> dt-bindings files into the MT8186 ones, reorder the MT8186 clock
> controller compatibles so they are sorted alphanumerically.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 05/18] dt-bindings: clock: mediatek: regroup MT8188 dt-bindings into MT8186
From: AngeloGioacchino Del Regno @ 2026-07-09 14:03 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-5-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Regroup the MT8188 clock and system clock dt-bindings into MT8186 ones
> to ease maintainability and have common files for several currently
> supported SoC or new future ones, that have the same kind of clock
> controller design.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
> .../bindings/clock/mediatek,mt8186-clock.yaml | 50 +++++++++++-
> .../bindings/clock/mediatek,mt8186-sys-clock.yaml | 6 +-
> .../bindings/clock/mediatek,mt8188-clock.yaml | 93 ----------------------
> .../bindings/clock/mediatek,mt8188-sys-clock.yaml | 58 --------------
> 4 files changed, 54 insertions(+), 153 deletions(-)
>
..snip..
> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8186-sys-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8186-sys-clock.yaml
> index 6733dafbde16..199eac2034f2 100644
> --- a/Documentation/devicetree/bindings/clock/mediatek,mt8186-sys-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8186-sys-clock.yaml
> @@ -4,7 +4,7 @@
> $id: http://devicetree.org/schemas/clock/mediatek,mt8186-sys-clock.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: MediaTek System Clock Controller for MT8186
> +title: MediaTek System Clock Controller for Mediatek SoCs
"for<space><space>MediaTek SoCs"
You should really fix the double space, or if that can be fixed while applying,
that's also fine for me.
So, with that fixed, either by you or while applying...
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 06/18] dt-bindings: clock: mediatek: regroup MT8192 dt-bindings into MT8186
From: AngeloGioacchino Del Regno @ 2026-07-09 14:03 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-6-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Regroup the MT8192 clock and system clock dt-bindings into MT8186 ones
> to ease maintainability and have common files for several currently
> supported SoC or new future ones, that have the same kind of clock
> controller design.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 07/18] dt-bindings: clock: mediatek: regroup MT8195 dt-bindings into MT8186
From: AngeloGioacchino Del Regno @ 2026-07-09 14:03 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-7-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Regroup the MT8195 clock and system clock dt-bindings into MT8186 ones
> to ease maintainability and have common files for several currently
> supported SoC or new future ones, that have the same kind of clock
> controller design.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
^ permalink raw reply
* Re: [PATCH v2 08/18] dt-bindings: clock: mediatek: Add MT8189 clocks
From: AngeloGioacchino Del Regno @ 2026-07-09 14:05 UTC (permalink / raw)
To: Louis-Alexis Eyraud, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Matthias Brugger, Chun-Jie Chen, Philipp Zabel, Edward-JW Yang,
Richard Cochran
Cc: kernel, linux-clk, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, netdev, Irving-CH Lin
In-Reply-To: <20260709-mt8189-clocks-system-base-v2-8-2926da3db6cf@collabora.com>
On 7/9/26 15:42, Louis-Alexis Eyraud wrote:
> Add dt schema and IDs for the clocks of MediaTek MT8189 SoC.
> The MT8189 clock IP provide clock control for main system
> (apmixedsys, topcksys and vlpcksys) and subsys (eg. peri, scp,
> ufs...).
>
> Also, add compatible for frequency hopping and spread spectrum clock
> functionality and reset controller header file for MT8189 UFS reset
> controller support.
>
> Co-developed-by: Irving-CH Lin <irving-ch.lin@mediatek.com>
> Signed-off-by: Irving-CH Lin <irving-ch.lin@mediatek.com>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
Both the commit description and title are misleading, as in, you're not adding
MT8189 clocks, but *both* clocks *and* resets.
Fix it please.
After which:
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> .../bindings/clock/mediatek,mt8186-clock.yaml | 15 +
> .../bindings/clock/mediatek,mt8186-fhctl.yaml | 1 +
> .../bindings/clock/mediatek,mt8186-sys-clock.yaml | 5 +
> include/dt-bindings/clock/mediatek,mt8189-clk.h | 433 +++++++++++++++++++++
> include/dt-bindings/reset/mediatek,mt8189-resets.h | 17 +
> 5 files changed, 471 insertions(+)
>
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] bridge: mcast: Fix a false positive lockdep splat
From: Ido Schimmel @ 2026-07-09 14:08 UTC (permalink / raw)
To: netdev, bridge; +Cc: davem, kuba, pabeni, edumazet, horms, razor, petrm
In-Reply-To: <20260708122820.1298718-2-idosch@nvidia.com>
On Wed, Jul 08, 2026 at 03:28:19PM +0300, Ido Schimmel wrote:
> @@ -1783,6 +1841,7 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
> u8 sflag,
> bool *need_rexmit)
> {
> + struct sk_buff_head *queue;
> bool over_lmqt = !!sflag;
> struct sk_buff *skb;
> u8 igmp_type;
> @@ -1791,7 +1850,12 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
> !br_multicast_ctx_matches_vlan_snooping(brmctx))
> return;
>
> + queue = pmctx ? &pmctx->query_queue : &brmctx->query_queue;
> +
> again_under_lmqt:
> + if (skb_queue_len_lockless(queue) >= BR_MULTICAST_QUERY_QUEUE_LEN_MAX)
> + return;
> +
> skb = br_multicast_alloc_query(brmctx, pmctx, pg, ip_dst, group,
> with_srcs, over_lmqt, sflag, &igmp_type,
> need_rexmit);
> @@ -1800,11 +1864,8 @@ static void __br_multicast_send_query(struct net_bridge_mcast *brmctx,
>
> if (pmctx) {
> skb->dev = pmctx->port->dev;
> - br_multicast_count(brmctx->br, pmctx->port, skb, igmp_type,
> - BR_MCAST_DIR_TX);
> - NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_OUT,
> - dev_net(pmctx->port->dev), NULL, skb, NULL, skb->dev,
> - br_dev_queue_push_xmit);
> + skb_queue_tail(queue, skb);
> + queue_work(system_highpri_wq, &pmctx->query_queue_work);
The issue reported by sashiko-gemini [1] is addressed in patch #2.
[1]
"This isn't a bug, but it looks like the local variable igmp_type is no
longer used in __br_multicast_send_query.
Since br_multicast_count() was moved out of this function and into the
workqueue processing, the value returned via the &igmp_type parameter
from br_multicast_alloc_query() is no longer read. Could we clean this
up and remove the local variable entirely?"
^ permalink raw reply
* [PATCH v2] docs: packaging: fix brackets
From: Manuel Ebner @ 2026-07-09 14:10 UTC (permalink / raw)
To: Vladimir Oltean, Jonathan Corbet, Shuah Khan, Jacob Keller,
Jakub Kicinski
Cc: Manuel Ebner, Andrew Lunn, Vladimir Oltean, netdev, linux-doc,
linux-kernel
Add two ')' to nested functions in code block.
Fixes: a9ad2a8dfb43 ("lib: packing: document recently added APIs")
Signed-off-by: Manuel Ebner <manuelebner@mailbox.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
[v2]
add Fixes tag.
add Reviewed-by.
[v1]
https://lore.kernel.org/all/20260709121427.391749-2-manuelebner@mailbox.org/
---
Documentation/core-api/packing.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/core-api/packing.rst b/Documentation/core-api/packing.rst
index f68f1e08fef9..cff1a262efce 100644
--- a/Documentation/core-api/packing.rst
+++ b/Documentation/core-api/packing.rst
@@ -330,7 +330,7 @@ Here is an example of how to use the fields APIs:
void unpack_your_data(const packed_buf_t *buf, struct data *unpacked)
{
- BUILD_BUG_ON(sizeof(*buf) != SIZE;
+ BUILD_BUG_ON(sizeof(*buf) != SIZE);
unpack_fields(buf, sizeof(*buf), unpacked, fields,
QUIRK_LITTLE_ENDIAN);
@@ -338,7 +338,7 @@ Here is an example of how to use the fields APIs:
void pack_your_data(const struct data *unpacked, packed_buf_t *buf)
{
- BUILD_BUG_ON(sizeof(*buf) != SIZE;
+ BUILD_BUG_ON(sizeof(*buf) != SIZE);
pack_fields(buf, sizeof(*buf), unpacked, fields,
QUIRK_LITTLE_ENDIAN);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH V3 4/4] dmaengine: xilinx_dma: Extend metadata handling for AXI DMA and MCDMA
From: Pandey, Radhey Shyam @ 2026-07-09 14:13 UTC (permalink / raw)
To: Srinivas Neeli, Vinod Koul, Radhey Shyam Pandey
Cc: Frank Li, Michal Simek, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Suraj Gupta,
Marek Vasut, Tomi Valkeinen, Alex Bereza, Folker Schwesinger,
dmaengine, netdev, linux-arm-kernel, linux-kernel, git
In-Reply-To: <20260708100652.603074-5-srinivas.neeli@amd.com>
> From: Suraj Gupta <suraj.gupta2@amd.com>
>
> xilinx_dma_get_metadata_ptr() exposed only the descriptor APP fields. Both
> AXI DMA and AXI MCDMA descriptors carry a status word with the transfer
> status, and AXI MCDMA additionally carries an AXI4-Stream sideband status
> word holding TID, TDEST and TUSER that clients may need. Extend the
> metadata handling to expose these.
>
> The returned pointer now starts at the descriptor status word for both AXI
> DMA and AXI MCDMA. As the descriptor words are contiguous, the client sees
> the status at index 0, followed for AXI MCDMA by the sideband status
> (TID/TDEST/TUSER) at index 1 and the APP fields, or for AXI DMA by the APP
> fields directly. The payload length is derived from the field sizes.
>
> This changes the get_metadata_ptr() contract for AXI DMA. The pointer now
> starts at the status word of the last (EOF) descriptor instead of the APP
> fields of the first, and the payload grows from 20 to 24 bytes. A client
> reading app[0] now reads the status word. No in-tree consumer is affected,
> as the axienet driver reads the RX frame length from result->residue
> rather than the APP fields. Reading the EOF descriptor is also correct, as
> the hardware writes the status and APP fields there.
>
> The index 0 and 1 layout described above is for the AXI MCDMA receive
> (S2MM) direction, which is where metadata is consumed. On the transmit
> (MM2S) direction the same descriptor words hold different fields.
>
> The probe logic is extended to read xlnx,axistream-connected for MCDMA, and
> xilinx_mcdma_prep_slave_sg() attaches metadata_ops when an AXI Stream
> interface is present, so MCDMA clients can use the metadata API in the same
> way as AXI DMA clients.
Nit - make the commit description precise.>
> Signed-off-by: Suraj Gupta <suraj.gupta2@amd.com>
> Co-developed-by: Srinivas Neeli <srinivas.neeli@amd.com>
> Signed-off-by: Srinivas Neeli <srinivas.neeli@amd.com>
> ---
> Changes in V3:
> - Renamed subject to include "AXI DMA and MCDMA" (was "AXI MCDMA" only).
> - Complete rewrite of commit message and implementation.
> - Metadata pointer now returns status field at index 0 instead of APP
> fields, exposing status and sideband information to clients.
> - Changed from list_first_entry to list_last_entry to return the EOF
> descriptor where hardware writes status and APP fields.
> - Added explicit handling for both AXIDMA and MCDMA types with proper
> payload length calculation.
> - Added WARN_ON_ONCE for unsupported DMA types.
> - Removed the 'chan' field from struct xilinx_dma_tx_descriptor (was
> added in V2) as it's no longer needed; channel is obtained from
> tx->chan instead.
> - Dropped V2 patches 4/5 (dt-bindings xlnx,include-stscntrl-strm) and
> 5/5 (xferred_bytes support) as the approach changed to use residue.
>
> Changes in V2:
> - Added support for MCDMA metadata handling alongside AXIDMA.
> - Added 'chan' field to struct xilinx_dma_tx_descriptor.
> ---
> drivers/dma/xilinx/xilinx_dma.c | 48 ++++++++++++++++++++++++++++-----
> 1 file changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
> index 1b5b00f08c5f..f5c4e0ca2cc4 100644
> --- a/drivers/dma/xilinx/xilinx_dma.c
> +++ b/drivers/dma/xilinx/xilinx_dma.c
> @@ -651,18 +651,48 @@ static inline void xilinx_aximcdma_buf(struct xilinx_dma_chan *chan,
> * @tx: async transaction descriptor
> * @payload_len: metadata payload length
> * @max_len: metadata max length
> - * Return: The app field pointer.
> + *
> + * The returned pointer starts at the descriptor status word for both AXI DMA
> + * and AXI MCDMA. As the descriptor words are contiguous, the client sees the
> + * status at index 0, followed for AXI MCDMA by the sideband status
> + * (TID/TDEST/TUSER) at index 1 and the APP fields from index 2, or for AXI DMA
> + * by the APP fields from index 1. These fields are populated by the hardware on
> + * the End-Of-Frame descriptor, so the pointer is taken from there.
> + *
> + * Return: Pointer to the descriptor status field.
> */
> static void *xilinx_dma_get_metadata_ptr(struct dma_async_tx_descriptor *tx,
> size_t *payload_len, size_t *max_len)
> {
> struct xilinx_dma_tx_descriptor *desc = to_dma_tx_descriptor(tx);
> - struct xilinx_axidma_tx_segment *seg;
> + struct xilinx_dma_chan *chan = to_xilinx_chan(tx->chan);
> +
> + if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIMCDMA) {
> + struct xilinx_aximcdma_tx_segment *seg =
> + list_last_entry(&desc->segments,
> + struct xilinx_aximcdma_tx_segment, node);
> +
This func is common for MCDMA s2mm and mm2s channel. Please make it
generic to address both.
> + /* [0] = status, [1] = sideband (TID/TDEST/TUSER), [2..] = app */
> + *max_len = *payload_len = sizeof(seg->hw.s2mm_status) +
> + sizeof(seg->hw.s2mm_sideband_status) +
> + sizeof(seg->hw.app);
> + return &seg->hw.s2mm_status;
> + }
> +
> + if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> + struct xilinx_axidma_tx_segment *seg =
> + list_last_entry(&desc->segments,
> + struct xilinx_axidma_tx_segment, node);
> +
> + /* [0] = status, [1..] = app */
> + *max_len = *payload_len = sizeof(seg->hw.status) +
> + sizeof(seg->hw.app);
> + return &seg->hw.status;
> + }
>
> - *max_len = *payload_len = sizeof(u32) * XILINX_DMA_NUM_APP_WORDS;
> - seg = list_first_entry(&desc->segments,
> - struct xilinx_axidma_tx_segment, node);
> - return seg->hw.app;
> + /* Only AXIDMA and MCDMA attach metadata_ops today. */
> + WARN_ON_ONCE(1);
> + return ERR_PTR(-EINVAL);
We are registering metadata ops for MCDMA and AXI DMA so when is this
error condition going to happen?
> }
>
> static struct dma_descriptor_metadata_ops xilinx_dma_metadata_ops = {
> @@ -2639,6 +2669,9 @@ xilinx_mcdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> segment->hw.control |= XILINX_MCDMA_BD_EOP;
> }
>
> + if (chan->xdev->has_axistream_connected)
> + desc->async_tx.metadata_ops = &xilinx_dma_metadata_ops;
> +
> return &desc->async_tx;
>
> error:
> @@ -3287,7 +3320,8 @@ static int xilinx_dma_probe(struct platform_device *pdev)
>
> dma_set_max_seg_size(xdev->dev, xdev->max_buffer_len);
>
> - if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
> + if (xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA ||
> + xdev->dma_config->dmatype == XDMA_TYPE_AXIMCDMA) {
> xdev->has_axistream_connected =
> of_property_read_bool(node, "xlnx,axistream-connected");
> }
^ permalink raw reply
* Re: [PATCH net-next v20 05/12] virtio_net: Query and set flow filter caps
From: Michael S. Tsirkin @ 2026-07-09 14:18 UTC (permalink / raw)
To: Shahar Shitrit
Cc: Daniel Jurgens, netdev, jasowang, pabeni, virtualization, parav,
yohadt, xuanzhuo, eperezma, jgg, kevin.tian, kuba, andrew+netdev,
edumazet
In-Reply-To: <a7f724e5-7648-4362-bfc0-ef4cd87211a8@nvidia.com>
On Thu, Jul 09, 2026 at 04:37:11PM +0300, Shahar Shitrit wrote:
>
>
> On 08/02/2026 13:51, Michael S. Tsirkin wrote:
> > On Thu, Feb 05, 2026 at 04:47:00PM -0600, Daniel Jurgens wrote:
> >> When probing a virtnet device, attempt to read the flow filter
> >> capabilities. In order to use the feature the caps must also
> >> be set. For now setting what was read is sufficient.
> >>
> >> This patch adds uapi definitions virtio_net flow filters define in
> >> version 1.4 of the VirtIO spec.
> >>
> >> Signed-off-by: Daniel Jurgens <danielj@nvidia.com>
> >> Reviewed-by: Parav Pandit <parav@nvidia.com>
> >> Reviewed-by: Shahar Shitrit <shshitrit@nvidia.com>
> >>
> >> ---
> >> v4:
> >> - Validate the length in the selector caps
> >> - Removed __free usage.
> >> - Removed for(int.
> >> v5:
> >> - Remove unneed () after MAX_SEL_LEN macro (test bot)
> >> v6:
> >> - Fix sparse warning "array of flexible structures" Jakub K/Simon H
> >> - Use new variable and validate ff_mask_size before set_cap. MST
> >> v7:
> >> - Set ff->ff_{caps, mask, actions} NULL in error path. Paolo Abeni
> >> - Return errors from virtnet_ff_init, -ENOTSUPP is not fatal. Xuan
> >>
> >> v8:
> >> - Use real_ff_mask_size when setting the selector caps. Jason Wang
> >>
> >> v9:
> >> - Set err after failed memory allocations. Simon Horman
> >>
> >> v10:
> >> - Return -EOPNOTSUPP in virnet_ff_init before allocing any memory.
> >> Jason/Paolo.
> >>
> >> v11:
> >> - Return -EINVAL if any resource limit is 0. Simon Horman
> >> - Ensure we don't overrun alloced space of ff->ff_mask by moving the
> >> real_ff_mask_size > ff_mask_size check into the loop. Simon Horman
> >>
> >> v12:
> >> - Move uapi includes to virtio_net.c vs header file. MST
> >> - Remove kernel.h header in virtio_net_ff uapi. MST
> >> - WARN_ON_ONCE in error paths validating selectors. MST
> >> - Move includes from .h to .c files. MST
> >> - Add WARN_ON_ONCE if obj_destroy fails. MST
> >> - Comment cleanup in virito_net_ff.h uapi. MST
> >> - Add 2 byte pad to the end of virtio_net_ff_cap_data.
> >> https://lore.kernel.org/virtio-comment/20251119044029-mutt-send-email-mst@kernel.org/T/#m930988a5d3db316c68546d8b61f4b94f6ebda030
> >> - Cleanup and reinit in the freeze/restore path. MST
> >>
> >> v13:
> >> - Added /* private: */ comment before reserved field. Jakub
> >> - Change ff_mask validation to break at unkonwn selector type. This
> >> will allow compatability with newer controllers if the types of
> >> selectors is expanded. MST
> >>
> >> v14:
> >> - Handle err from virtnet_ff_init in virtnet_restore_up. MST
> >>
> >> v15:
> >> - In virtnet_restore_up only call virtnet_close in err path if
> >> netif_runnig. AI
> >>
> >> v16:
> >> - Return 0 from virtnet_restore_up if virtnet_init_ff return not
> >> supported. AI
> >>
> >> v17:
> >> - During restore freeze_down on error during ff_init. AI
> >>
> >> v18:
> >> - Changed selector cap validation to verify size for each type
> >> instead of just checking they weren't bigger than max size. AI
> >> - Added __count_by attribute to flexible members in uapi. Paolo A
> >>
> >> v19:
> >> - Fixed ;; and incorrect plural in comment. AI
> >>
> >> v20:
> >> - include uapi/linux/stddef.h for __counted_by. AI
> >
> > AI has led you astray, sadly (
> will fix in v21
> >
> >
> >
> >
> >> ---
> >> drivers/net/virtio_net.c | 231 ++++++++++++++++++++++++++++-
> >> include/uapi/linux/virtio_net_ff.h | 91 ++++++++++++
> >> 2 files changed, 321 insertions(+), 1 deletion(-)
> >> create mode 100644 include/uapi/linux/virtio_net_ff.h
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index db88dcaefb20..2cfa37e2f83f 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -26,6 +26,11 @@
> >> #include <net/netdev_rx_queue.h>
> >> #include <net/netdev_queues.h>
> >> #include <net/xdp_sock_drv.h>
> >> +#include <linux/virtio_admin.h>
> >> +#include <net/ipv6.h>
> >> +#include <net/ip.h>
> >> +#include <uapi/linux/virtio_pci.h>
> >> +#include <uapi/linux/virtio_net_ff.h>
> >>
> >> static int napi_weight = NAPI_POLL_WEIGHT;
> >> module_param(napi_weight, int, 0444);
> >> @@ -281,6 +286,14 @@ static const struct virtnet_stat_desc virtnet_stats_tx_speed_desc_qstat[] = {
> >> VIRTNET_STATS_DESC_TX_QSTAT(speed, ratelimit_packets, hw_drop_ratelimits),
> >> };
> >>
> >> +struct virtnet_ff {
> >> + struct virtio_device *vdev;
> >> + bool ff_supported;
> >> + struct virtio_net_ff_cap_data *ff_caps;
> >> + struct virtio_net_ff_cap_mask_data *ff_mask;
> >> + struct virtio_net_ff_actions *ff_actions;
> >> +};
> >> +
> >> #define VIRTNET_Q_TYPE_RX 0
> >> #define VIRTNET_Q_TYPE_TX 1
> >> #define VIRTNET_Q_TYPE_CQ 2
> >> @@ -488,6 +501,7 @@ struct virtnet_info {
> >> TRAILING_OVERLAP(struct virtio_net_rss_config_trailer, rss_trailer, hash_key_data,
> >> u8 rss_hash_key_data[VIRTIO_NET_RSS_MAX_KEY_SIZE];
> >> );
> >> + struct virtnet_ff ff;
> >> };
> >> static_assert(offsetof(struct virtnet_info, rss_trailer.hash_key_data) ==
> >> offsetof(struct virtnet_info, rss_hash_key_data));
> >> @@ -526,6 +540,7 @@ static struct sk_buff *virtnet_skb_append_frag(struct sk_buff *head_skb,
> >> struct page *page, void *buf,
> >> int len, int truesize);
> >> static void virtnet_xsk_completed(struct send_queue *sq, int num);
> >> +static void remove_vq_common(struct virtnet_info *vi);
> >>
> >> enum virtnet_xmit_type {
> >> VIRTNET_XMIT_TYPE_SKB,
> >> @@ -5684,6 +5699,192 @@ static const struct netdev_stat_ops virtnet_stat_ops = {
> >> .get_base_stats = virtnet_get_base_stats,
> >> };
> >>
> >> +static size_t get_mask_size(u16 type)
> >> +{
> >> + switch (type) {
> >> + case VIRTIO_NET_FF_MASK_TYPE_ETH:
> >> + return sizeof(struct ethhdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_IPV4:
> >> + return sizeof(struct iphdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_IPV6:
> >> + return sizeof(struct ipv6hdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_TCP:
> >> + return sizeof(struct tcphdr);
> >> + case VIRTIO_NET_FF_MASK_TYPE_UDP:
> >> + return sizeof(struct udphdr);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int virtnet_ff_init(struct virtnet_ff *ff, struct virtio_device *vdev)
> >> +{
> >> + size_t ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data) +
> >> + sizeof(struct virtio_net_ff_selector) *
> >> + VIRTIO_NET_FF_MASK_TYPE_MAX;
> >> + struct virtio_admin_cmd_query_cap_id_result *cap_id_list;
> >> + struct virtio_net_ff_selector *sel;
> >> + unsigned long sel_types = 0;
> >> + size_t real_ff_mask_size;
> >> + int err;
> >> + int i;
> >> +
> >> + if (!vdev->config->admin_cmd_exec)
> >> + return -EOPNOTSUPP;
> >> +
> >> + cap_id_list = kzalloc(sizeof(*cap_id_list), GFP_KERNEL);
> >> + if (!cap_id_list)
> >> + return -ENOMEM;
> >> +
> >> + err = virtio_admin_cap_id_list_query(vdev, cap_id_list);
> >> + if (err)
> >> + goto err_cap_list;
> >> +
> >> + if (!(VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_RESOURCE_CAP) &&
> >> + VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_SELECTOR_CAP) &&
> >> + VIRTIO_CAP_IN_LIST(cap_id_list,
> >> + VIRTIO_NET_FF_ACTION_CAP))) {
> >> + err = -EOPNOTSUPP;
> >> + goto err_cap_list;
> >> + }
> >> +
> >> + ff->ff_caps = kzalloc(sizeof(*ff->ff_caps), GFP_KERNEL);
> >> + if (!ff->ff_caps) {
> >> + err = -ENOMEM;
> >> + goto err_cap_list;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_RESOURCE_CAP,
> >> + ff->ff_caps,
> >> + sizeof(*ff->ff_caps));
> >> +
> >> + if (err)
> >> + goto err_ff;
> >> +
> >> + if (!ff->ff_caps->groups_limit ||
> >> + !ff->ff_caps->classifiers_limit ||
> >> + !ff->ff_caps->rules_limit ||
> >> + !ff->ff_caps->rules_per_group_limit) {
> >> + err = -EINVAL;
> >> + goto err_ff;
> >> + }
> >> +
> >> + /* VIRTIO_NET_FF_MASK_TYPE start at 1 */
> >> + for (i = 1; i <= VIRTIO_NET_FF_MASK_TYPE_MAX; i++)
> >> + ff_mask_size += get_mask_size(i);
> >> +
> >> + ff->ff_mask = kzalloc(ff_mask_size, GFP_KERNEL);
> >> + if (!ff->ff_mask) {
> >> + err = -ENOMEM;
> >> + goto err_ff;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_SELECTOR_CAP,
> >> + ff->ff_mask,
> >> + ff_mask_size);
> >
> > So ff_actions is from device and ff_actions->count does not seem to be checked.
> >
> > If device somehow gains a larger mask down the road, can it not then overflow?
> > or malicious?
> see below
> >
> >
> >> +
> >> + if (err)
> >> + goto err_ff_mask;
> >> +
> >> + ff->ff_actions = kzalloc(sizeof(*ff->ff_actions) +
> >> + VIRTIO_NET_FF_ACTION_MAX,
> >> + GFP_KERNEL);
> >> + if (!ff->ff_actions) {
> >> + err = -ENOMEM;
> >> + goto err_ff_mask;
> >> + }
> >> +
> >> + err = virtio_admin_cap_get(vdev,
> >> + VIRTIO_NET_FF_ACTION_CAP,
> >> + ff->ff_actions,
> >> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
> >
> > So ff_actions is from device and ff_actions->count is not checked.
> >
> > If device gains a ton of actions down the road, can it not then overflow?
> > or malicious?
>
> it can't overflow, as it can write up to sizeof(*ff->ff_actions) +
> VIRTIO_NET_FF_ACTION_MAX bytes. But it is a valid concern in case of
> someone in the future iterates on count. Better to handle it now so it
> won't be forgotten. Checking that ff_actions->count doesn't exceed
> VIRTIO_NET_FF_ACTION_MAX can break backward compatibility.
These are admin commands right? They are truncated silently.
All you have to do is just check the length returned.
At least that's my vague understanding, don't have the
time to re-review now. HTH
> If this max
> value is extended in a new spec/controller then this driver would fail
> to load. Instead, I will assign ff->ff_actions->count with the minimum
> of what we got and the max.
> >
> >> +
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_RESOURCE_CAP,
> >> + ff->ff_caps,
> >> + sizeof(*ff->ff_caps));
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + real_ff_mask_size = sizeof(struct virtio_net_ff_cap_mask_data);
> >> + sel = (void *)&ff->ff_mask->selectors;
> >> +
> >> + for (i = 0; i < ff->ff_mask->count; i++) {
> >> + /* If the selector type is unknown it may indicate the spec
> >> + * has been revised to include new types of selectors
> >> + */
> >> + if (sel->type > VIRTIO_NET_FF_MASK_TYPE_MAX)
> >
> > do you want to check sel->type 0 too?
> yes, will add in v21.
> >
> >> + break;
> >
> > but count remains unchanged? should we not to reduce count here
> > so device knows what driver can drive?
>
> we should. will add it in v21.
> >
> >
> >> +
> >> + if (sel->length != get_mask_size(sel->type) ||
> >> + test_and_set_bit(sel->type, &sel_types)) {
> >> + WARN_ON_ONCE(true);
> >> + err = -EINVAL;
> >> + goto err_ff_action;
> >> + }
> >> + real_ff_mask_size += sizeof(struct virtio_net_ff_selector) + sel->length;
> >> + if (real_ff_mask_size > ff_mask_size) {
> >> + WARN_ON_ONCE(true);
> >> + err = -EINVAL;
> >> + goto err_ff_action;
> >> + }
> >> + sel = (void *)sel + sizeof(*sel) + sel->length;
> >> + }
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_SELECTOR_CAP,
> >> + ff->ff_mask,
> >> + real_ff_mask_size);
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + err = virtio_admin_cap_set(vdev,
> >> + VIRTIO_NET_FF_ACTION_CAP,
> >> + ff->ff_actions,
> >> + sizeof(*ff->ff_actions) + VIRTIO_NET_FF_ACTION_MAX);
> >> + if (err)
> >> + goto err_ff_action;
> >> +
> >> + ff->vdev = vdev;
> >> + ff->ff_supported = true;
> >> +
> >> + kfree(cap_id_list);
> >> +
> >> + return 0;
> >> +
> >> +err_ff_action:
> >> + kfree(ff->ff_actions);
> >> + ff->ff_actions = NULL;
> >> +err_ff_mask:
> >> + kfree(ff->ff_mask);
> >> + ff->ff_mask = NULL;
> >> +err_ff:
> >> + kfree(ff->ff_caps);
> >> + ff->ff_caps = NULL;
> >> +err_cap_list:
> >> + kfree(cap_id_list);
> >> +
> >> + return err;
> >> +}
> >> +
> >> +static void virtnet_ff_cleanup(struct virtnet_ff *ff)
> >> +{
> >> + if (!ff->ff_supported)
> >> + return;
> >> +
> >> + kfree(ff->ff_actions);
> >> + kfree(ff->ff_mask);
> >> + kfree(ff->ff_caps);
> >> + ff->ff_supported = false;
> >> +}
> >> +
> >> static void virtnet_freeze_down(struct virtio_device *vdev)
> >> {
> >> struct virtnet_info *vi = vdev->priv;
> >> @@ -5702,6 +5903,10 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
> >> netif_tx_lock_bh(vi->dev);
> >> netif_device_detach(vi->dev);
> >> netif_tx_unlock_bh(vi->dev);
> >> +
> >> + rtnl_lock();
> >> + virtnet_ff_cleanup(&vi->ff);
> >> + rtnl_unlock();
> >> }
> >>
> >> static int init_vqs(struct virtnet_info *vi);
> >> @@ -5727,10 +5932,23 @@ static int virtnet_restore_up(struct virtio_device *vdev)
> >> return err;
> >> }
> >>
> >> + /* Initialize flow filters. Not supported is an acceptable and common
> >> + * return code
> >> + */
> >> + rtnl_lock();
> >> + err = virtnet_ff_init(&vi->ff, vi->vdev);
> >> + if (err && err != -EOPNOTSUPP) {
> >> + rtnl_unlock();
> >> + virtnet_freeze_down(vi->vdev);
> >> + remove_vq_common(vi);
> >> + return err;
> >> + }
> >> + rtnl_unlock();
> >> +
> >> netif_tx_lock_bh(vi->dev);
> >> netif_device_attach(vi->dev);
> >> netif_tx_unlock_bh(vi->dev);
> >> - return err;
> >> + return 0;
> >> }
> >>
> >> static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
> >> @@ -7058,6 +7276,15 @@ static int virtnet_probe(struct virtio_device *vdev)
> >> }
> >> vi->guest_offloads_capable = vi->guest_offloads;
> >>
> >> + /* Initialize flow filters. Not supported is an acceptable and common
> >> + * return code
> >> + */
> >> + err = virtnet_ff_init(&vi->ff, vi->vdev);
> >> + if (err && err != -EOPNOTSUPP) {
> >> + rtnl_unlock();
> >> + goto free_unregister_netdev;
> >> + }
> >> +
> >> rtnl_unlock();
> >>
> >> err = virtnet_cpu_notif_add(vi);
> >> @@ -7073,6 +7300,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> >>
> >> free_unregister_netdev:
> >> unregister_netdev(dev);
> >> + virtnet_ff_cleanup(&vi->ff);
> >> free_failover:
> >> net_failover_destroy(vi->failover);
> >> free_vqs:
> >> @@ -7121,6 +7349,7 @@ static void virtnet_remove(struct virtio_device *vdev)
> >> virtnet_free_irq_moder(vi);
> >>
> >> unregister_netdev(vi->dev);
> >> + virtnet_ff_cleanup(&vi->ff);
> >>
> >> net_failover_destroy(vi->failover);
> >>
> >> diff --git a/include/uapi/linux/virtio_net_ff.h b/include/uapi/linux/virtio_net_ff.h
> >> new file mode 100644
> >> index 000000000000..552a6b3a8a91
> >> --- /dev/null
> >> +++ b/include/uapi/linux/virtio_net_ff.h
> >> @@ -0,0 +1,91 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
> >> + *
> >> + * Header file for virtio_net flow filters
> >> + */
> >> +#ifndef _LINUX_VIRTIO_NET_FF_H
> >> +#define _LINUX_VIRTIO_NET_FF_H
> >> +
> >> +#include <linux/types.h>
> >> +#include <uapi/linux/stddef.h>
> >> +
> >> +#define VIRTIO_NET_FF_RESOURCE_CAP 0x800
> >> +#define VIRTIO_NET_FF_SELECTOR_CAP 0x801
> >> +#define VIRTIO_NET_FF_ACTION_CAP 0x802
> >> +
> >> +/**
> >> + * struct virtio_net_ff_cap_data - Flow filter resource capability limits
> >> + * @groups_limit: maximum number of flow filter groups supported by the device
> >> + * @classifiers_limit: maximum number of classifiers supported by the device
> >> + * @rules_limit: maximum number of rules supported device-wide across all groups
> >> + * @rules_per_group_limit: maximum number of rules allowed in a single group
> >> + * @last_rule_priority: priority value associated with the lowest-priority rule
> >> + * @selectors_per_classifier_limit: maximum selectors allowed in one classifier
> >> + */
> >> +struct virtio_net_ff_cap_data {
> >> + __le32 groups_limit;
> >> + __le32 classifiers_limit;
> >> + __le32 rules_limit;
> >> + __le32 rules_per_group_limit;
> >> + __u8 last_rule_priority;
> >> + __u8 selectors_per_classifier_limit;
> >> + /* private: */
> >> + __u8 reserved[2];
> >> +};
> >> +
> >> +/**
> >> + * struct virtio_net_ff_selector - Selector mask descriptor
> >> + * @type: selector type, one of VIRTIO_NET_FF_MASK_TYPE_* constants
> >> + * @flags: selector flags, see VIRTIO_NET_FF_MASK_F_* constants
> >> + * @reserved: must be set to 0 by the driver and ignored by the device
> >> + * @length: size in bytes of @mask
> >> + * @reserved1: must be set to 0 by the driver and ignored by the device
> >> + * @mask: variable-length mask payload for @type, length given by @length
> >> + *
> >> + * A selector describes a header mask that a classifier can apply. The format
> >> + * of @mask depends on @type.
> >> + */
> >> +struct virtio_net_ff_selector {
> >> + __u8 type;
> >> + __u8 flags;
> >> + __u8 reserved[2];
> >> + __u8 length;
> >> + __u8 reserved1[3];
> >> + __u8 mask[] __counted_by(length);
> >> +};
> >> +
> >> +#define VIRTIO_NET_FF_MASK_TYPE_ETH 1
> >> +#define VIRTIO_NET_FF_MASK_TYPE_IPV4 2
> >> +#define VIRTIO_NET_FF_MASK_TYPE_IPV6 3
> >> +#define VIRTIO_NET_FF_MASK_TYPE_TCP 4
> >> +#define VIRTIO_NET_FF_MASK_TYPE_UDP 5
> >> +#define VIRTIO_NET_FF_MASK_TYPE_MAX VIRTIO_NET_FF_MASK_TYPE_UDP
> >> +
> >> +/**
> >> + * struct virtio_net_ff_cap_mask_data - Supported selector mask formats
> >> + * @count: number of entries in @selectors
> >> + * @reserved: must be set to 0 by the driver and ignored by the device
> >> + * @selectors: packed array of struct virtio_net_ff_selector.
> >> + */
> >> +struct virtio_net_ff_cap_mask_data {
> >> + __u8 count;
> >> + __u8 reserved[7];
> >> + __u8 selectors[] __counted_by(count);
> >
> > This looks wrong to me. count is # of selectors (packed entries) not
> > bytes.
> right, will remove it.
> > >
> >
> >
> >> +};
> >> +
> >> +#define VIRTIO_NET_FF_MASK_F_PARTIAL_MASK (1 << 0)
> >> +
> >> +#define VIRTIO_NET_FF_ACTION_DROP 1
> >> +#define VIRTIO_NET_FF_ACTION_RX_VQ 2
> >> +#define VIRTIO_NET_FF_ACTION_MAX VIRTIO_NET_FF_ACTION_RX_VQ
> >> +/**
> >> + * struct virtio_net_ff_actions - Supported flow actions
> >> + * @count: number of supported actions in @actions
> >> + * @reserved: must be set to 0 by the driver and ignored by the device
> >> + * @actions: array of action identifiers (VIRTIO_NET_FF_ACTION_*)
> >> + */
> >> +struct virtio_net_ff_actions {
> >> + __u8 count;
> >> + __u8 reserved[7];
> >> + __u8 actions[] __counted_by(count);
> >
> >
> > this too.
> count directly represents the number of __u8 elements in actions[], so
> the size of the flexible array is count * sizeof(__u8). Therefore, here
> it is correct.
> >
> >> +};
> >> +#endif
> >> --
> >> 2.50.1
> >
> >
^ permalink raw reply
* Re: Please backport bridge multicast exponential field encoding fix series to 6.1.y/6.6.y/6.12.y/6.18.y/7.0.y
From: Greg KH @ 2026-07-09 14:22 UTC (permalink / raw)
To: Ujjal Roy
Cc: Linux Stable, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, Nikolay Aleksandrov, Ido Schimmel,
David Ahern, Shuah Khan, Andy Roulin, Yong Wang, Petr Machata,
Ujjal Roy, bridge, netdev, linux-kernel, linux-kselftest
In-Reply-To: <CAE2MWkmcQdhGp3LTMtpgAse3AFcfKcAcpQe89+iijfP5e0w_QQ@mail.gmail.com>
On Thu, Jul 09, 2026 at 06:35:04PM +0530, Ujjal Roy wrote:
> On Thu, Jul 9, 2026 at 6:23 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Jul 09, 2026 at 06:12:40PM +0530, Ujjal Roy wrote:
> > > On Thu, Jul 9, 2026 at 4:34 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Thu, Jul 09, 2026 at 10:13:27AM +0000, Ujjal Roy wrote:
> > > > > Hi Greg,
> > > > >
> > > > > Please consider backporting the following bridge multicast fix series to 6.1.y, 6.6.y, 6.12.y, 6.18.y and 7.0.y.
> > > > >
> > > > > 726fa7da2d8c ("ipv4: igmp: get rid of IGMPV3_{QQIC,MRC} and simplify calculation")
> > > > > 12cfb4ecc471 ("ipv6: mld: rename mldv2_mrc() and add mldv2_qqi()")
> > > > > 95bfd196f0dc ("ipv4: igmp: encode multicast exponential fields")
> > > > > e51560f4220a ("ipv6: mld: encode multicast exponential fields")
> > > > > 529dbe762de0 ("selftests: net: bridge: add MRC and QQIC field encoding tests")
> > > >
> > > > Why is any of this needed in older kernels?
> > > >
> > > > And 7.0.y is long end-of-life.
> > > >
> > > > And why, if this does fix issues, was it not tagged for stable to start
> > > > with?
> > > >
> > > > thanks,
> > > >
> > > > greg k-h
> > >
> > > I already explained this in the email thread, "Please backport bridge
> > > multicast exponential field encoding fix series to stable kernels".
> >
> > Sorry, but that's not here (remember, some of us get 1000+ emails a
> > day.)
> >
> > Please explain why patches need to be backported when asking for them to
> > be backported.
> >
> > thanks,
> >
> > greg k-h
>
> Sorry for breaking the thread. I understand your point, I will
> maintain this in the future.
> How should I send the patchset that addresses the conflicts on 5.10.y
> and 7.1.y? Shall I send the conflicts patchset as a series via a
> different thread or how? I've never done this before, so I'm asking.
>
> Here is the explanation for why the patches need to be backported:
>
> History: The multicast stack currently supports decoding of IGMPv3 and
> MLDv2 exponential timer field encodings, but lacks the corresponding
> encoding logic when generating multicast query packets. As a result,
> query intervals and response codes exceeding the linear encoding range
> can be transmitted incorrectly. This can cause multicast queriers and
> listeners to interpret different timing values, resulting in protocol
> interoperability issues, membership timeouts, and premature multicast
> group expiration.
>
> Testing: The series adds the missing encoding support for both IGMPv3
> and MLDv2 and includes selftests that validate the behavior.
> I backported the series to v6.6.123.2 and verified the accompanying
> selftests. The selftests fail on the unpatched kernel and pass after
> applying the series, demonstrating both the bug and the effectiveness
> of the fix.
>
> Given that this is a protocol correctness issue affecting multicast
> query generation, please consider backporting the complete series to
> all applicable stable kernels.
>
But this really seems like a new feature being added, it's not fixing a
regression of something that previously worked, right?
WHy can't people just update to the latest kernel release to get this if
they need it for their environments?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2] mptcp: only set DATA_FIN when a mapping is present
From: Matthieu Baerts @ 2026-07-09 14:31 UTC (permalink / raw)
To: Michael Bommarito
Cc: gang.yan, Geliang Tang, Paolo Abeni, Eric Dumazet, Jakub Kicinski,
mptcp, netdev, linux-kernel, Mat Martineau
In-Reply-To: <CAJJ9bXy29+cwu+Me8WNrf=f4qc=uoXSMCrUG3NJbR-xANJxseA@mail.gmail.com>
On 09/07/2026 15:12, Michael Bommarito wrote:
> On Thu, Jul 9, 2026 at 8:27 AM Matthieu Baerts <matttbe@kernel.org> wrote:
>
>> By chance, do you have a reproducer? Maybe using Packetdrill with MPTCP
>> support? [1]
>> If not, how did you discover this issue?
>
> Yup, I'll send to you offlist. It's a custom Python/TUN/qemu setup,
> not Packetdrill, but I'm guessing it could be ported or extended for
> testing/fuzzing.
Interesting. Yes, it could be useful for various protocols.
> I discovered it with my homegrown agentic stack, which is a Rube
> Goldberg machine full of various tools (like cocci, semgrep) and
> knowledge graphs (built with libclang). The agent logs suggest that
> it came from a family of rules that surfaces suspicious struct init
> candidates, which I can share too.
Is it also based on crafted traffic? It could also be useful!
>> If I may add, please also send new version in a separate thread, not as
>> a reply, not to confuse various tools, as mentioned in the same doc.
>
> Sure, thanks to both you and Gang for the feedback. I'll send a v3
> based on your suggestions later today.
Thanks!
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply
* Re: Ethtool : PRBS feature
From: Andrew Lunn @ 2026-07-09 14:37 UTC (permalink / raw)
To: Lee Trager
Cc: Das, Shubham, Srinivasan, Vijay, Alexander Duyck,
Maxime Chevallier, netdev@vger.kernel.org, mkubecek@suse.cz,
D H, Siddaraju, Chintalapalle, Balaji, Lindberg, Magnus,
niklas.damberg@ericsson.com, Wirandi, Jonas
In-Reply-To: <46f714b7-ec6e-4b26-9ff8-b7f27f11fe33@trager.us>
On Wed, Jul 08, 2026 at 05:58:52PM -0700, Lee Trager wrote:
> On 7/7/26 2:06 AM, Das, Shubham wrote:
>
> > Thanks Andrew, Lee for the feedback.
> >
> > Lee,
> > I don't see prbs11.0, prbs11.1, prbs11.2, prbs11.3, prbs13.0, prbs13.1, prbs13.2, prbs13.3, prbs16 and prbs32 in IEEE 802.3 2022 standard.
> > Is this specific to fnic based on base PRBS pattern or it is mentioned in some other standard ?
> Those were from the fbnic spec, I'm not sure where they originate from. I
> suppose we can drop those for now but its good to keep in mind more tests
> may be added in the future.
Can you find out what they actually do. It might be something as silly
as, the .1 indicate it only runs on lane 1? So these are in fact
standard prbs11 and being able to specify the lane via another
attribute is sufficient?
Andrew
^ permalink raw reply
* Re: [PATCH v2 iproute2-next] ss: stop displaying dccp sockets
From: patchwork-bot+netdevbpf @ 2026-07-09 14:40 UTC (permalink / raw)
To: Yafang Shao; +Cc: stephen, kuniyu, netdev
In-Reply-To: <20260701025005.76589-1-laoar.shao@gmail.com>
Hello:
This patch was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:
On Wed, 1 Jul 2026 10:50:05 +0800 you wrote:
> DCCP support was retired in kernel commit 2a63dd0edf38 ("net: Retire
> DCCP socket."). However, ss still attempts to query DCCP sockets via
> netlink, which triggers repeated SELinux warnings in dmesg:
>
> SELinux: unrecognized netlink message: protocol=4 nlmsg_type=19 \
> sclass=netlink_tcpdiag_socket pid=188945 comm=ss
>
> [...]
Here is the summary with links:
- [v2,iproute2-next] ss: stop displaying dccp sockets
https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=3c0b939e88e4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH iproute2-next v2] ip: add OVPN device mode support
From: patchwork-bot+netdevbpf @ 2026-07-09 14:40 UTC (permalink / raw)
To: Marco Baffo; +Cc: netdev, dsahern, stephen, antonio
In-Reply-To: <20260709102819.48809-1-marco@mandelbit.com>
Hello:
This patch was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:
On Thu, 9 Jul 2026 12:28:19 +0200 you wrote:
> ovpn devices (OpenVPN data channel offload, kernel v6.16) can already
> be created with "ip link add ovpn0 type ovpn", but the mode is always
> left to the kernel default (p2p mode).
>
> Add ovpn link type support so that point-to-point (p2p) or multi-peer
> (mp) mode can be selected at creation time and shown by
> "ip -d link show".
>
> [...]
Here is the summary with links:
- [iproute2-next,v2] ip: add OVPN device mode support
https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=92f89d0e66b2
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Questions about resilient next-hop groups (6.18.35)
From: Froehlich, Dominik @ 2026-07-09 14:46 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hello everyone,
I am running kernel 6.18.35 and I have the following multi path route setup:
10.181.0.0/16 nhid 400
nexthop dev bond0-ip6tnl0 weight 1
nexthop dev bond0-ip6tnl1 weight 1
100.80.0.0/12 nhid 400
nexthop dev bond0-ip6tnl0 weight 1
nexthop dev bond0-ip6tnl1 weight 1
100.112.0.0/13 nhid 400
nexthop dev bond0-ip6tnl0 weight 1
nexthop dev bond0-ip6tnl1 weight 1
The links are ip6tnl devices sending traffic over an IPv6 tunnel to the other end.
I am using L4 ECMP hash policy:
~ # sysctl net.ipv4.fib_multipath_hash_policy
net.ipv4.fib_multipath_hash_policy = 1
~ # sysctl net.ipv6.fib_multipath_hash_policy
net.ipv6.fib_multipath_hash_policy = 1
Here are the next hop definitions:
id 400 group 4000,128/4001,128 type resilient buckets 1024 idle_timer 60 unbalanced_timer 0 unbalanced_time 0
id 4000 dev bond0-ip6tnl0 scope host
id 4001 dev bond0-ip6tnl1 scope host
So I have two equal weighted next hops with 512 buckets each.
Now I am running a TCP connection in a shell on a host in the 10.181.0.0/16 network over one of the next hops that continuously produces traffic to keep its bucket busy:
while true; do
sleep 0.1
date
done
Thu Jul 9 13:27:03 UTC 2026
Thu Jul 9 13:27:03 UTC 2026
Thu Jul 9 13:27:03 UTC 2026
Thu Jul 9 13:27:04 UTC 2026
(…)
This roughly sends 10 pkts/s.
Now I want to drain next hop 4000 which is where my traffic is flowing:
ip nexthop replace id 400 group 4000,1/4001,255 type resilient buckets 1024 idle_timer 60 unbalanced_timer 0
Assigning it a weight of 1 should reduce the number of buckets assigned to next hop 4000 to 4 and move idle buckets to next hop 4001.
However, what I am seeing is that the connection is immediately terminated.
When I am changing the weights more slowly it kind of works:
~ # ip nexthop replace id 400 group 4000,128/4001,128 type resilient idle_timer 60 unbalanced_timer 0
~ # ip nexthop replace id 400 group 4000,64/4001,192 type resilient idle_timer 60 unbalanced_timer 0
~ # ip nexthop replace id 400 group 4000,5/4001,250 type resilient idle_timer 60 unbalanced_timer 0
~ # ip nexthop replace id 400 group 4000,1/4001,255 type resilient idle_timer 60 unbalanced_timer 0
I can then see the buckets reduced to just 4 over time as expected:
Every 1.0s: ip nexthop bucket list nhid 4000 2026-07-09 13:44:22
id 400 index 447 idle_time 135.51 nhid 4000
id 400 index 631 idle_time 117.56 nhid 4000
id 400 index 720 idle_time 73.11 nhid 4000
id 400 index 742 idle_time 136.19 nhid 4000
My guess is that my connection is in bucket index 720 as it has the lowest idle_time reported.
My questions:
- Why is the idle_time reported so high? There should be packets flowing through that bucket every 0.1 seconds, so I was expecting the idle_time to be near zero.
- Why does my connection get cut immediately when draining rapidly but kind of stays alive when changing weights more slowly? Shouldn't it be a busy (undrainable) bucket either way?
Bucket nr 720 already has idle_time higher than 60 so I think it would be migrated on any further weight changes - which would disrupt the existing connection again.
- When / how is the idle_time of a bucket updated? The docs mention that “Every time a packet is forwarded through a bucket, this timer is updated to current jiffies value.” I found that the bucket idle_time is refreshed more frequently when fever buckets are used. But why does the number of buckets even matter here? Fewer buckets mean more flows per bucket, I get that. But if there is at least one busy connection in a bucket, shouldn't that be enough to flag it busy?
Best regards,
Dom
^ permalink raw reply
* [PATCH net v3] tipc: fix NULL deref in tipc_lxc_xmit() on node up
From: Weiming Shi @ 2026-07-09 14:47 UTC (permalink / raw)
To: Jon Maloy, Tung Quang Nguyen, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: Xiang Mei, netdev, tipc-discussion, linux-kernel, stable,
Weiming Shi
tipc_named_node_up() builds a bulk of this node's cluster-scope service
bindings for a peer that just came up and sends it with tipc_node_xmit().
Neither step tolerates an empty skb chain: named_distribute() finishes
with buf_msg(skb_peek_tail(list)) to tag the last message, and for a
same-host peer tipc_node_xmit() routes into tipc_lxc_xmit(), which reads
buf_msg(skb_peek(list)). skb_peek*() returns NULL on an empty chain, so
buf_msg(NULL) faults.
The chain is empty in two cases: when cluster_scope itself is empty, and
when named_distribute() bails out on an allocation failure without
queueing any buffer. cluster_scope is legitimately empty during the
window in tipc_net_finalize() between setting the node address, after
which peers can link up and trigger tipc_named_node_up(), and
tipc_nametbl_publish() inserting the first self-binding. A peer linking
in that window crashes the node. It is reachable by an unprivileged
user, who can gain CAP_NET_ADMIN in a private net namespace and drive
TIPC there.
Oops: general protection fault, probably for non-canonical address
KASAN: null-ptr-deref in range [0x00000000000000d8-0x00000000000000df]
RIP: 0010:tipc_lxc_xmit (net/tipc/node.c:1629 net/tipc/msg.h:202)
tipc_node_xmit (net/tipc/node.c:1718)
tipc_named_node_up (net/tipc/name_distr.c:222)
tipc_node_write_unlock (net/tipc/node.c:428)
tipc_rcv (net/tipc/node.c:2185)
tipc_l2_rcv_msg (net/tipc/bearer.c:669)
Distribute only when cluster_scope is non-empty, and send only when
named_distribute() actually produced a buffer. An empty bulk carries no
bindings, so not sending it changes nothing. tipc_node_xmit() does not
touch cluster_scope, so move it out of cluster_scope_lock.
Fixes: cad2929dc432 ("tipc: update a binding service via broadcast")
Reported-by: Xiang Mei <xmei5@asu.edu>
Suggested-by: Tung Quang Nguyen <tung.quang.nguyen@est.tech>
Assisted-by: Claude:claude-opus-4-8
Cc: stable@vger.kernel.org
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
v3:
- Guard tipc_node_xmit() with skb_queue_empty(&head) so an empty chain
from a named_distribute() allocation failure is not sent either, and
move it out of cluster_scope_lock since it does not use cluster_scope
(Tung Quang Nguyen).
v2:
- Guard in tipc_named_node_up() instead of inside named_distribute().
net/tipc/name_distr.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index ba4f4906e13b..888a87d769e7 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -218,9 +218,12 @@ void tipc_named_node_up(struct net *net, u32 dnode, u16 capabilities)
spin_unlock_bh(&tn->nametbl_lock);
read_lock_bh(&nt->cluster_scope_lock);
- named_distribute(net, &head, dnode, &nt->cluster_scope, seqno);
- tipc_node_xmit(net, &head, dnode, 0);
+ if (!list_empty(&nt->cluster_scope))
+ named_distribute(net, &head, dnode, &nt->cluster_scope, seqno);
read_unlock_bh(&nt->cluster_scope_lock);
+
+ if (!skb_queue_empty(&head))
+ tipc_node_xmit(net, &head, dnode, 0);
}
/**
--
2.43.0
^ permalink raw reply related
* [PATCH bpf-next v11 0/5] bpf: add icmp_send kfunc
From: Mahe Tardy @ 2026-07-09 14:48 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
sdf.kernel, Mahe Tardy
Hello,
This is v11 of adding the icmp_send kfunc, as suggested during
LSF/MM/BPF 2025[^1]. The goal is to allow cgroup_skb programs to
actively reject east-west traffic, similarly to what is possible to do
with netfilter reject target. Applications can receive early feedback
that something went wrong during the TCP handshake.
The first step to implement this is using ICMP control messages, with
the ICMP_DEST_UNREACH type with various code ICMP_NET_UNREACH,
ICMP_HOST_UNREACH, ICMP_PROT_UNREACH, etc. This is easier to implement
than a TCP RST reply and will already hint the client TCP stack to abort
the connection and not retry extensively.
Note that this is different than the sock_destroy kfunc, that along
calls tcp_abort and thus sends a reset, destroying the underlying
socket.
Caveats of this kfunc design are that a program can call this function N
times, thus send N ICMP unreach control messages and that the program
can return from the BPF filter with pass leading to a potential
confusing situation where the TCP connection was established while the
client received ICMP_DEST_UNREACH messages.
v2 updates:
- fix a build error from a missing function call rename;
- avoid changing return line in bpf_kfunc_init;
- return SK_DROP from the kfunc (similarly to bpf_redirect);
- check the return value in the selftest.
v3 update:
- fix an undefined reference build error.
v4 updates:
- prevent the kfunc to be called recursively and add a test (thanks to
Martin).
- do not fetch dst route when unnecessary (thanks to Martin).
- extend the test for IPv6 (thanks to Martin).
- use SK_DROP in examples and use non blocking sockets for testing
(thanks to Martin).
- test when the kfunc returns -EINVAL (thanks to Jordan).
- add the kfunc to bpf_kfunc_set_skb as suggested by Alexei.
- guard the IPv4 parts with IS_ENABLED(CONFIG_INET).
- fix a wrong initial value for client_fd (thanks to Yonghong).
- add documentation to the kfunc.
- to Jordan: I couldn't include <linux/icmp.h> because of redefines from
<network_helpers.h>.
v5 updates:
- kfunc name is now icmp_send and takes the control message type as
parameter for future potential extension (daniel)
- drop the net patches to route packet since now the kfunc is limited to
cgroup_skb and tc progs (daniel & martin)
- linearize skb headers (sashiko)
- zero SKB control block (sashiko)
- bind to port 0 instead of fixed port (sashiko)
- poll to wait for POLLERR event (sashiko)
- do not use ASSERT_EQ in CMSG_NXTHDR loop (sashiko)
- fix comment about byte order (sashiko)
- fix endianness IP address issue (sashiko)
- add forgotten cleanup_cgroup_environment (sashiko)
- let packets pass in recursion test (sashiko)
- clarify evaluation order for recursion test (sashiko)
v6 updates (all from sashiko):
- bring back the net patches to route packet since tc ingress needs it.
- rename the ip_route_reply helpers from fetch to fill.
- call pskb_network_may_pull on the cloned pkt.
- check explicitly that we received one and only one ICMP err ctrl msg.
v7 updates:
- use consume_skb on success path (stanislav)
- replace recursion protection with CPU_ARRAY by checking the nature of
the sk (daniel, offline)
- use reverse xmas tree in read_icmp_errqueue (jordan)
- use ASSERT_OK_FD instead of ASSERT_GE whenever possible (jordan)
- add a test for tc (jordan)
- better filtering from host cgroup test progs (sashiko)
v8 updates:
- mostly a resend as it's been sitting as "New" in the queue for almost
one month, fixed a few nits.
- on new bpf_icmp_send kfunc cgroup_skb test (patch 4/7):
- guard a close fd with fd >= 0 (jordan)
- use ASSERT_OK_FD instead of ASSERT_GE (jordan)
- fixed comment style (sashiko)
- on recursion test (patch 7/7):
- guard a close fd with fd >= 0 (jordan)
- fixed comments style (sashiko)
- filter bpf prog on pid and ICMP message types (sashiko)
v9 updates:
- first, there was a v8.5 that I discussed here[^2] with Emil
Tsalapatis. I tried once again to make tc work but the ai review found
something fundamentally wrong. This version removes the tc support for
now and focuses on cgroup_skb.
- use helper get_socket_local_port instead of getsockname (sashiko)
- use if_nametoindex("lo") instead of value 1 (bpf-ci)
- fix IPV6_RECVERR appearance before IPv6 patch (bpf-ci)
- precise that 0 on success mean icmp_send was called but it was just an
attempt since this function does not return anything (sashiko)
- explicitly consider ICMP_FRAG_NEEDED as invalid in bpf_icmp_send as
it would miss the next-hop MTU info. Also test it. (sashiko)
- test for max_code + 1 for invalid (sashiko)
- add review-by tags from Jordan and Emil but remove it on the main
patch as I have significantly changed it.
- check for rec_count in recursion test (sashiko)
- re-order setup_cgroup_environmment in test (sashiko)
- reset kfunc_ret on every test run (sashiko)
- check for skb route for icmp_send as the function would quietly fail
and add a test (sashiko)
v10 updates:
- guard against skbs with metadata_dst before calling icmpv6_send
(sashiko)
- add more review-by tags from Emil and Jordan.
v11 updates:
- move !skb_valid_dst(skb) check to both ipv4 and ipv6 (daniel)
[^1]: https://lwn.net/Articles/1022034/
[^2]: https://lore.kernel.org/bpf/ajvDRCw8cPqXAqQq@gmail.com/
Link to v10: https://lore.kernel.org/bpf/20260625110321.28236-1-mahe.tardy@gmail.com/
Mahe Tardy (5):
bpf: add bpf_icmp_send kfunc
selftests/bpf: add bpf_icmp_send kfunc cgroup_skb tests
selftests/bpf: add bpf_icmp_send kfunc cgroup_skb IPv6 tests
selftests/bpf: add bpf_icmp_send recursion test
selftests/bpf: add bpf_icmp_send no route test
net/core/filter.c | 90 ++++++
.../bpf/prog_tests/icmp_send_kfunc.c | 292 ++++++++++++++++++
tools/testing/selftests/bpf/progs/icmp_send.c | 123 ++++++++
3 files changed, 505 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
create mode 100644 tools/testing/selftests/bpf/progs/icmp_send.c
--
2.34.1
^ permalink raw reply
* [PATCH bpf-next v11 1/5] bpf: add bpf_icmp_send kfunc
From: Mahe Tardy @ 2026-07-09 14:48 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
sdf.kernel, Mahe Tardy
In-Reply-To: <20260709144900.245904-1-mahe.tardy@gmail.com>
This is needed in the context of Tetragon to provide improved feedback
(in contrast to just dropping packets) to east-west traffic when blocked
by policies using cgroup_skb programs.
This reuses concepts from netfilter reject target codepath with the
differences that:
* Packets are cloned since the BPF user can still let the packet pass
(SK_PASS from the cgroup_skb progs for example) and the current skb
need to stay untouched (cgroup_skb hooks only allow read-only skb
payload).
* We protect against recursion since the kfunc, by generating an ICMP
error message, could retrigger the BPF prog that invoked it.
Only ICMP_DEST_UNREACH and ICMPV6_DEST_UNREACH are currently supported.
The interface accepts a type parameter to facilitate future extension to
other ICMP control message types.
For normal cgroup_skb paths, the skb dst route should already be set.
However, bpf_prog_test_run_skb can create synthetic IPv4/IPv6 skbs
without an attached route. In that case, icmp_send returns early, and
the kfunc would otherwise report success despite no ICMP reply being
sent. This check also reject metadata dsts, which are not valid struct
rtable instances. While IPv6 would stricly require only rejecting
metadata dsts, same check is applied for API consistency.
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Reviewed-by: Jordan Rife <jordan@jrife.io>
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
net/core/filter.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index 4f5cbcac3e78..e4697036c67b 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -84,6 +84,9 @@
#include <linux/un.h>
#include <net/xdp_sock_drv.h>
#include <net/inet_dscp.h>
+#include <linux/icmpv6.h>
+#include <net/icmp.h>
+#include <net/ip6_route.h>
#include "dev.h"
@@ -12548,6 +12551,83 @@ __bpf_kfunc int bpf_xdp_pull_data(struct xdp_md *x, u32 len)
return 0;
}
+/**
+ * bpf_icmp_send - Send an ICMP control message
+ * @skb_ctx: Packet that triggered the control message
+ * @type: ICMP type (only ICMP_DEST_UNREACH/ICMPV6_DEST_UNREACH supported)
+ * @code: ICMP code (0-15 except ICMP_FRAG_NEEDED for IPv4, 0-6 for IPv6)
+ *
+ * Sends an ICMP control message in response to the packet. The original packet
+ * is cloned before sending the ICMP message, so the BPF program can still let
+ * the packet pass if desired.
+ *
+ * Currently only ICMP_DEST_UNREACH (IPv4) and ICMPV6_DEST_UNREACH (IPv6) are
+ * supported.
+ *
+ * Return: 0 on success (send attempt), negative error code on failure:
+ * -EBUSY: Recursion detected
+ * -EPROTONOSUPPORT: Non-IP protocol
+ * -EOPNOTSUPP: Unsupported ICMP type
+ * -EINVAL: Invalid code parameter
+ * -ENETUNREACH: No usable route/dst for the ICMP reply
+ * -ENOMEM: Memory allocation failed
+ */
+__bpf_kfunc int bpf_icmp_send(struct __sk_buff *skb_ctx, int type, int code)
+{
+ struct sk_buff *skb = (struct sk_buff *)skb_ctx;
+ struct sk_buff *nskb;
+ struct sock *sk;
+
+ sk = skb_to_full_sk(skb);
+ if (sk && sk->sk_kern_sock &&
+ (sk->sk_protocol == IPPROTO_ICMP || sk->sk_protocol == IPPROTO_ICMPV6))
+ return -EBUSY;
+
+ if (!skb_valid_dst(skb))
+ return -ENETUNREACH;
+
+ switch (skb->protocol) {
+#if IS_ENABLED(CONFIG_INET)
+ case htons(ETH_P_IP): {
+ if (type != ICMP_DEST_UNREACH)
+ return -EOPNOTSUPP;
+ if (code < 0 || code > NR_ICMP_UNREACH ||
+ code == ICMP_FRAG_NEEDED) /* needs a valid next-hop MTU */
+ return -EINVAL;
+
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return -ENOMEM;
+
+ memset(IPCB(nskb), 0, sizeof(*IPCB(nskb)));
+ icmp_send(nskb, type, code, 0);
+ consume_skb(nskb);
+ break;
+ }
+#endif
+#if IS_ENABLED(CONFIG_IPV6)
+ case htons(ETH_P_IPV6):
+ if (type != ICMPV6_DEST_UNREACH)
+ return -EOPNOTSUPP;
+ if (code < 0 || code > ICMPV6_REJECT_ROUTE)
+ return -EINVAL;
+
+ nskb = skb_clone(skb, GFP_ATOMIC);
+ if (!nskb)
+ return -ENOMEM;
+
+ memset(IP6CB(nskb), 0, sizeof(*IP6CB(nskb)));
+ icmpv6_send(nskb, type, code, 0);
+ consume_skb(nskb);
+ break;
+#endif
+ default:
+ return -EPROTONOSUPPORT;
+ }
+
+ return 0;
+}
+
__bpf_kfunc_end_defs();
int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,
@@ -12590,6 +12670,10 @@ BTF_KFUNCS_START(bpf_kfunc_check_set_sock_ops)
BTF_ID_FLAGS(func, bpf_sock_ops_enable_tx_tstamp)
BTF_KFUNCS_END(bpf_kfunc_check_set_sock_ops)
+BTF_KFUNCS_START(bpf_kfunc_check_set_icmp_send)
+BTF_ID_FLAGS(func, bpf_icmp_send)
+BTF_KFUNCS_END(bpf_kfunc_check_set_icmp_send)
+
static const struct btf_kfunc_id_set bpf_kfunc_set_skb = {
.owner = THIS_MODULE,
.set = &bpf_kfunc_check_set_skb,
@@ -12620,6 +12704,11 @@ static const struct btf_kfunc_id_set bpf_kfunc_set_sock_ops = {
.set = &bpf_kfunc_check_set_sock_ops,
};
+static const struct btf_kfunc_id_set bpf_kfunc_set_icmp_send = {
+ .owner = THIS_MODULE,
+ .set = &bpf_kfunc_check_set_icmp_send,
+};
+
static int __init bpf_kfunc_init(void)
{
int ret;
@@ -12641,6 +12730,7 @@ static int __init bpf_kfunc_init(void)
ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
&bpf_kfunc_set_sock_addr);
ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_kfunc_set_tcp_reqsk);
+ ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SKB, &bpf_kfunc_set_icmp_send);
return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SOCK_OPS, &bpf_kfunc_set_sock_ops);
}
late_initcall(bpf_kfunc_init);
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v11 2/5] selftests/bpf: add bpf_icmp_send kfunc cgroup_skb tests
From: Mahe Tardy @ 2026-07-09 14:48 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
sdf.kernel, Mahe Tardy
In-Reply-To: <20260709144900.245904-1-mahe.tardy@gmail.com>
This test opens a server and client, enters a new cgroup, attach a
cgroup_skb program on egress and calls the bpf_icmp_send function from
the client egress so that an ICMP unreach control message is sent back
to the client. It then fetches the message from the error queue to
confirm the correct ICMP unreach code has been sent.
Note that, for the client, we have to connect in non-blocking mode to
let the test execute faster. Otherwise, we need to wait for the TCP
three-way handshake to timeout in the kernel before reading the errno.
Also note that we don't set IP_RECVERR on the socket in
connect_to_fd_nonblock since the error will be transferred anyway in our
test because the connection is rejected at the beginning of the TCP
handshake. See in net/ipv4/tcp_ipv4.c:tcp_v4_err for more details.
Reviewed-by: Jordan Rife <jordan@jrife.io>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
.../bpf/prog_tests/icmp_send_kfunc.c | 164 ++++++++++++++++++
tools/testing/selftests/bpf/progs/icmp_send.c | 38 ++++
2 files changed, 202 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
create mode 100644 tools/testing/selftests/bpf/progs/icmp_send.c
diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
new file mode 100644
index 000000000000..b8a98c90053e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include <linux/errqueue.h>
+#include <poll.h>
+#include "icmp_send.skel.h"
+
+#define TIMEOUT_MS 1000
+
+#define ICMP_DEST_UNREACH 3
+
+#define ICMP_FRAG_NEEDED 4
+#define NR_ICMP_UNREACH 15
+
+#define KFUNC_RET_UNSET -1
+
+static int connect_to_fd_nonblock(int server_fd)
+{
+ struct sockaddr_storage addr;
+ socklen_t len = sizeof(addr);
+ int fd, err;
+
+ if (getsockname(server_fd, (struct sockaddr *)&addr, &len))
+ return -1;
+
+ fd = socket(addr.ss_family, SOCK_STREAM | SOCK_NONBLOCK, 0);
+ if (fd < 0)
+ return -1;
+
+ err = connect(fd, (struct sockaddr *)&addr, len);
+ if (err < 0 && errno != EINPROGRESS) {
+ close(fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+static void read_icmp_errqueue(int sockfd, int expected_code)
+{
+ struct sock_extended_err *sock_err;
+ char ctrl_buf[512];
+ struct msghdr msg = {
+ .msg_control = ctrl_buf,
+ .msg_controllen = sizeof(ctrl_buf),
+ };
+ struct pollfd pfd = {
+ .fd = sockfd,
+ .events = POLLERR,
+ };
+ struct cmsghdr *cm;
+ ssize_t n;
+
+ if (!ASSERT_GE(poll(&pfd, 1, TIMEOUT_MS), 1, "poll_errqueue"))
+ return;
+
+ n = recvmsg(sockfd, &msg, MSG_ERRQUEUE);
+ if (!ASSERT_GE(n, 0, "recvmsg_errqueue"))
+ return;
+
+ cm = CMSG_FIRSTHDR(&msg);
+ if (!ASSERT_NEQ(cm, NULL, "cm_firsthdr_null"))
+ return;
+
+ for (; cm; cm = CMSG_NXTHDR(&msg, cm)) {
+ if (cm->cmsg_level != IPPROTO_IP || cm->cmsg_type != IP_RECVERR)
+ continue;
+
+ sock_err = (struct sock_extended_err *)CMSG_DATA(cm);
+
+ if (!ASSERT_EQ(sock_err->ee_origin, SO_EE_ORIGIN_ICMP,
+ "sock_err_origin_icmp"))
+ return;
+ if (!ASSERT_EQ(sock_err->ee_type, ICMP_DEST_UNREACH,
+ "sock_err_type_dest_unreach"))
+ return;
+ ASSERT_EQ(sock_err->ee_code, expected_code, "sock_err_code");
+ return;
+ }
+
+ ASSERT_FAIL("no IP_RECVERR control message found");
+}
+
+static bool valid_unreach_code(int code)
+{
+ if (code < 0)
+ return false;
+
+ return code <= NR_ICMP_UNREACH && code != ICMP_FRAG_NEEDED;
+}
+
+static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
+{
+ int srv_fd = -1, client_fd = -1;
+ int port;
+
+ srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", 0, TIMEOUT_MS);
+ if (!ASSERT_OK_FD(srv_fd, "start_server"))
+ return;
+
+ port = get_socket_local_port(srv_fd);
+ if (!ASSERT_GE(port, 0, "get_socket_local_port")) {
+ close(srv_fd);
+ return;
+ }
+
+ skel->bss->server_port = ntohs(port);
+ skel->bss->unreach_code = code;
+ skel->data->kfunc_ret = KFUNC_RET_UNSET;
+
+ client_fd = connect_to_fd_nonblock(srv_fd);
+ if (!ASSERT_OK_FD(client_fd, "client_connect_nonblock")) {
+ close(srv_fd);
+ return;
+ }
+
+ if (valid_unreach_code(code))
+ read_icmp_errqueue(client_fd, code);
+
+ close(client_fd);
+ close(srv_fd);
+}
+
+void test_icmp_send_unreach_cgroup(void)
+{
+ struct icmp_send *skel;
+ int cgroup_fd = -1;
+
+ skel = icmp_send__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ goto cleanup;
+
+ cgroup_fd = test__join_cgroup("/icmp_send_unreach_cgroup");
+ if (!ASSERT_OK_FD(cgroup_fd, "join_cgroup"))
+ goto cleanup;
+
+ skel->links.egress =
+ bpf_program__attach_cgroup(skel->progs.egress, cgroup_fd);
+ if (!ASSERT_OK_PTR(skel->links.egress, "prog_attach_cgroup"))
+ goto cleanup;
+
+ for (int code = 0; code <= NR_ICMP_UNREACH; code++) {
+ if (code == ICMP_FRAG_NEEDED)
+ continue;
+
+ trigger_prog_read_icmp_errqueue(skel, code);
+ ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
+ }
+
+ /* Test invalid codes */
+ trigger_prog_read_icmp_errqueue(skel, -1);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+ trigger_prog_read_icmp_errqueue(skel, NR_ICMP_UNREACH + 1);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+ trigger_prog_read_icmp_errqueue(skel, ICMP_FRAG_NEEDED);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+cleanup:
+ icmp_send__destroy(skel);
+ if (cgroup_fd >= 0)
+ close(cgroup_fd);
+}
diff --git a/tools/testing/selftests/bpf/progs/icmp_send.c b/tools/testing/selftests/bpf/progs/icmp_send.c
new file mode 100644
index 000000000000..6d0be0a9afe1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/icmp_send.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_endian.h>
+
+/* 127.0.0.1 in host byte order */
+#define SERVER_IP 0x7F000001
+
+#define ICMP_DEST_UNREACH 3
+
+__u16 server_port = 0;
+int unreach_code = 0;
+int kfunc_ret = -1;
+
+SEC("cgroup_skb/egress")
+int egress(struct __sk_buff *skb)
+{
+ void *data = (void *)(long)skb->data;
+ void *data_end = (void *)(long)skb->data_end;
+ struct iphdr *iph;
+ struct tcphdr *tcph;
+
+ iph = data;
+ if ((void *)(iph + 1) > data_end || iph->version != 4 ||
+ iph->protocol != IPPROTO_TCP || iph->daddr != bpf_htonl(SERVER_IP))
+ return SK_PASS;
+
+ tcph = (void *)iph + iph->ihl * 4;
+ if ((void *)(tcph + 1) > data_end ||
+ tcph->dest != bpf_htons(server_port))
+ return SK_PASS;
+
+ kfunc_ret = bpf_icmp_send(skb, ICMP_DEST_UNREACH, unreach_code);
+
+ return SK_DROP;
+}
+
+char LICENSE[] SEC("license") = "Dual BSD/GPL";
--
2.34.1
^ permalink raw reply related
* [PATCH bpf-next v11 3/5] selftests/bpf: add bpf_icmp_send kfunc cgroup_skb IPv6 tests
From: Mahe Tardy @ 2026-07-09 14:48 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, john.fastabend, jordan, martin.lau,
yonghong.song, emil, netdev, edumazet, kuba, pabeni, davem, horms,
sdf.kernel, Mahe Tardy
In-Reply-To: <20260709144900.245904-1-mahe.tardy@gmail.com>
This test extends the existing cgroup_skb tests with IPv6 support.
Note that we need to set IPV6_RECVERR on the socket for IPv6 in
connect_to_fd_nonblock otherwise the error will be ignored even if we
are in the middle of the TCP handshake. See in
net/ipv6/datagram.c:ipv6_icmp_error for more details.
Reviewed-by: Jordan Rife <jordan@jrife.io>
Signed-off-by: Mahe Tardy <mahe.tardy@gmail.com>
---
.../bpf/prog_tests/icmp_send_kfunc.c | 91 +++++++++++++------
tools/testing/selftests/bpf/progs/icmp_send.c | 48 ++++++++--
2 files changed, 101 insertions(+), 38 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
index b8a98c90053e..bbb3c3d4509c 100644
--- a/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
+++ b/tools/testing/selftests/bpf/prog_tests/icmp_send_kfunc.c
@@ -8,9 +8,11 @@
#define TIMEOUT_MS 1000
#define ICMP_DEST_UNREACH 3
+#define ICMPV6_DEST_UNREACH 1
#define ICMP_FRAG_NEEDED 4
#define NR_ICMP_UNREACH 15
+#define ICMPV6_REJECT_ROUTE 6
#define KFUNC_RET_UNSET -1
@@ -18,7 +20,7 @@ static int connect_to_fd_nonblock(int server_fd)
{
struct sockaddr_storage addr;
socklen_t len = sizeof(addr);
- int fd, err;
+ int fd, err, on = 1;
if (getsockname(server_fd, (struct sockaddr *)&addr, &len))
return -1;
@@ -27,6 +29,12 @@ static int connect_to_fd_nonblock(int server_fd)
if (fd < 0)
return -1;
+ if (addr.ss_family == AF_INET6 &&
+ setsockopt(fd, IPPROTO_IPV6, IPV6_RECVERR, &on, sizeof(on)) < 0) {
+ close(fd);
+ return -1;
+ }
+
err = connect(fd, (struct sockaddr *)&addr, len);
if (err < 0 && errno != EINPROGRESS) {
close(fd);
@@ -36,8 +44,14 @@ static int connect_to_fd_nonblock(int server_fd)
return fd;
}
-static void read_icmp_errqueue(int sockfd, int expected_code)
+static void read_icmp_errqueue(int sockfd, int expected_code, int af)
{
+ int expected_ee_type = (af == AF_INET) ? ICMP_DEST_UNREACH :
+ ICMPV6_DEST_UNREACH;
+ int expected_origin = (af == AF_INET) ? SO_EE_ORIGIN_ICMP :
+ SO_EE_ORIGIN_ICMP6;
+ int expected_level = (af == AF_INET) ? IPPROTO_IP : IPPROTO_IPV6;
+ int expected_type = (af == AF_INET) ? IP_RECVERR : IPV6_RECVERR;
struct sock_extended_err *sock_err;
char ctrl_buf[512];
struct msghdr msg = {
@@ -63,38 +77,43 @@ static void read_icmp_errqueue(int sockfd, int expected_code)
return;
for (; cm; cm = CMSG_NXTHDR(&msg, cm)) {
- if (cm->cmsg_level != IPPROTO_IP || cm->cmsg_type != IP_RECVERR)
+ if (cm->cmsg_level != expected_level ||
+ cm->cmsg_type != expected_type)
continue;
sock_err = (struct sock_extended_err *)CMSG_DATA(cm);
- if (!ASSERT_EQ(sock_err->ee_origin, SO_EE_ORIGIN_ICMP,
- "sock_err_origin_icmp"))
+ if (!ASSERT_EQ(sock_err->ee_origin, expected_origin,
+ "sock_err_origin"))
return;
- if (!ASSERT_EQ(sock_err->ee_type, ICMP_DEST_UNREACH,
+ if (!ASSERT_EQ(sock_err->ee_type, expected_ee_type,
"sock_err_type_dest_unreach"))
return;
ASSERT_EQ(sock_err->ee_code, expected_code, "sock_err_code");
return;
}
- ASSERT_FAIL("no IP_RECVERR control message found");
+ ASSERT_FAIL("no IP_RECVERR/IPV6_RECVERR control message found");
}
-static bool valid_unreach_code(int code)
+static bool valid_unreach_code(int code, int af)
{
if (code < 0)
return false;
- return code <= NR_ICMP_UNREACH && code != ICMP_FRAG_NEEDED;
+ if (af == AF_INET)
+ return code <= NR_ICMP_UNREACH && code != ICMP_FRAG_NEEDED;
+
+ return code <= ICMPV6_REJECT_ROUTE;
}
-static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
+static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code,
+ int af, const char *ip)
{
int srv_fd = -1, client_fd = -1;
int port;
- srv_fd = start_server(AF_INET, SOCK_STREAM, "127.0.0.1", 0, TIMEOUT_MS);
+ srv_fd = start_server(af, SOCK_STREAM, ip, 0, TIMEOUT_MS);
if (!ASSERT_OK_FD(srv_fd, "start_server"))
return;
@@ -105,6 +124,8 @@ static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
}
skel->bss->server_port = ntohs(port);
+ skel->bss->unreach_type = (af == AF_INET) ? ICMP_DEST_UNREACH :
+ ICMPV6_DEST_UNREACH;
skel->bss->unreach_code = code;
skel->data->kfunc_ret = KFUNC_RET_UNSET;
@@ -114,13 +135,37 @@ static void trigger_prog_read_icmp_errqueue(struct icmp_send *skel, int code)
return;
}
- if (valid_unreach_code(code))
- read_icmp_errqueue(client_fd, code);
+ if (valid_unreach_code(code, af))
+ read_icmp_errqueue(client_fd, code, af);
close(client_fd);
close(srv_fd);
}
+static void run_icmp_test(struct icmp_send *skel, int af, const char *ip,
+ int max_code)
+{
+ for (int code = 0; code <= max_code; code++) {
+ if (af == AF_INET && code == ICMP_FRAG_NEEDED)
+ continue;
+
+ trigger_prog_read_icmp_errqueue(skel, code, af, ip);
+ ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
+ }
+
+ /* Test invalid codes */
+ trigger_prog_read_icmp_errqueue(skel, -1, af, ip);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+ trigger_prog_read_icmp_errqueue(skel, max_code + 1, af, ip);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+
+ if (af == AF_INET) {
+ trigger_prog_read_icmp_errqueue(skel, ICMP_FRAG_NEEDED, af, ip);
+ ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+ }
+}
+
void test_icmp_send_unreach_cgroup(void)
{
struct icmp_send *skel;
@@ -139,23 +184,11 @@ void test_icmp_send_unreach_cgroup(void)
if (!ASSERT_OK_PTR(skel->links.egress, "prog_attach_cgroup"))
goto cleanup;
- for (int code = 0; code <= NR_ICMP_UNREACH; code++) {
- if (code == ICMP_FRAG_NEEDED)
- continue;
-
- trigger_prog_read_icmp_errqueue(skel, code);
- ASSERT_EQ(skel->data->kfunc_ret, 0, "kfunc_ret");
- }
-
- /* Test invalid codes */
- trigger_prog_read_icmp_errqueue(skel, -1);
- ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+ if (test__start_subtest("ipv4"))
+ run_icmp_test(skel, AF_INET, "127.0.0.1", NR_ICMP_UNREACH);
- trigger_prog_read_icmp_errqueue(skel, NR_ICMP_UNREACH + 1);
- ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
-
- trigger_prog_read_icmp_errqueue(skel, ICMP_FRAG_NEEDED);
- ASSERT_EQ(skel->data->kfunc_ret, -EINVAL, "kfunc_ret");
+ if (test__start_subtest("ipv6"))
+ run_icmp_test(skel, AF_INET6, "::1", ICMPV6_REJECT_ROUTE);
cleanup:
icmp_send__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/icmp_send.c b/tools/testing/selftests/bpf/progs/icmp_send.c
index 6d0be0a9afe1..6e1ba539eeb0 100644
--- a/tools/testing/selftests/bpf/progs/icmp_send.c
+++ b/tools/testing/selftests/bpf/progs/icmp_send.c
@@ -5,10 +5,11 @@
/* 127.0.0.1 in host byte order */
#define SERVER_IP 0x7F000001
-
-#define ICMP_DEST_UNREACH 3
+/* ::1 in host byte order (last 32-bit word) */
+#define SERVER_IP6_LO 0x00000001
__u16 server_port = 0;
+int unreach_type = 0;
int unreach_code = 0;
int kfunc_ret = -1;
@@ -18,19 +19,48 @@ int egress(struct __sk_buff *skb)
void *data = (void *)(long)skb->data;
void *data_end = (void *)(long)skb->data_end;
struct iphdr *iph;
+ struct ipv6hdr *ip6h;
struct tcphdr *tcph;
+ __u8 version;
- iph = data;
- if ((void *)(iph + 1) > data_end || iph->version != 4 ||
- iph->protocol != IPPROTO_TCP || iph->daddr != bpf_htonl(SERVER_IP))
+ if (data + 1 > data_end)
return SK_PASS;
- tcph = (void *)iph + iph->ihl * 4;
- if ((void *)(tcph + 1) > data_end ||
- tcph->dest != bpf_htons(server_port))
+ version = (*((__u8 *)data)) >> 4;
+
+ if (version == 4) {
+ iph = data;
+ if ((void *)(iph + 1) > data_end ||
+ iph->protocol != IPPROTO_TCP ||
+ iph->daddr != bpf_htonl(SERVER_IP))
+ return SK_PASS;
+
+ tcph = (void *)iph + iph->ihl * 4;
+ if ((void *)(tcph + 1) > data_end ||
+ tcph->dest != bpf_htons(server_port))
+ return SK_PASS;
+
+ } else if (version == 6) {
+ ip6h = data;
+ if ((void *)(ip6h + 1) > data_end ||
+ ip6h->nexthdr != IPPROTO_TCP)
+ return SK_PASS;
+
+ if (ip6h->daddr.in6_u.u6_addr32[0] != 0 ||
+ ip6h->daddr.in6_u.u6_addr32[1] != 0 ||
+ ip6h->daddr.in6_u.u6_addr32[2] != 0 ||
+ ip6h->daddr.in6_u.u6_addr32[3] != bpf_htonl(SERVER_IP6_LO))
+ return SK_PASS;
+
+ tcph = (void *)(ip6h + 1);
+ if ((void *)(tcph + 1) > data_end ||
+ tcph->dest != bpf_htons(server_port))
+ return SK_PASS;
+ } else {
return SK_PASS;
+ }
- kfunc_ret = bpf_icmp_send(skb, ICMP_DEST_UNREACH, unreach_code);
+ kfunc_ret = bpf_icmp_send(skb, unreach_type, unreach_code);
return SK_DROP;
}
--
2.34.1
^ permalink raw reply related
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