[{"id":3688557,"web_url":"http://patchwork.ozlabs.org/comment/3688557/","msgid":"<20260508154159.1250696-2-horms@kernel.org>","list_archive_url":null,"date":"2026-05-08T15:42:00","subject":"Re: [Intel-wired-lan] [PATCH iwl-next v5 5/5] ice: add support for\n transmitting unreadable frags","submitter":{"id":82748,"url":"http://patchwork.ozlabs.org/api/people/82748/","name":"Simon Horman","email":"horms@kernel.org"},"content":"From: 'Simon Horman' \n\nThis is an AI-generated review of your patch. The human sending this\nemail has considered the AI review valid, or at least plausible.\nFull review at: https://sashiko.dev\n---\nice: add support for transmitting unreadable frags\n\nThis commit advertises netmem Tx support in the ice driver. It modifies\nthe transmit path to conditionally set the buffer type to ICE_TX_BUF_FRAG\nonly when the fragment is not a net_iov, allowing the driver to skip DMA\nunmapping operations for unreadable fragments.\n\n> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c\n> index a88f9e3c00771..0e61b38e53a5e 100644\n> --- a/drivers/net/ethernet/intel/ice/ice_main.c\n> +++ b/drivers/net/ethernet/intel/ice/ice_main.c\n> @@ -3453,6 +3453,7 @@ static void ice_set_ops(struct ice_vsi *vsi)\n> \n> \tnetdev->netdev_ops = &ice_netdev_ops;\n> \tnetdev->queue_mgmt_ops = &ice_queue_mgmt_ops;\n> +\tnetdev->netmem_tx = true;\n> \tnetdev->udp_tunnel_nic_info = &pf->hw.udp_tunnel_nic;\n\nBy setting netmem_tx to true here, the driver accepts packets containing\nnet_iov fragments. If a net_iov packet exceeds ICE_MAX_BUF_TXD fragments, it\nappears it will fall back to linearization in the transmit path.\n\nSince __skb_linearize() uses CPU memory accessors that expect a valid struct\npage, will this cause an invalid memory access when skb_frag_page() returns a\nnet_iov pointer?\n\nShould the driver explicitly drop highly fragmented net_iov packets instead of\nattempting to linearize them?\n\n[ ... ]\n> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c\n> index 43b4670760273..1d97e2cc2adee 100644\n> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c\n> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c\n> @@ -113,11 +113,17 @@ ice_prgm_fdir_fltr(struct ice_vsi *vsi, struct ice_fltr_desc *fdir_desc,\n> static void\n> ice_unmap_and_free_tx_buf(struct ice_tx_ring *ring, struct ice_tx_buf *tx_buf)\n> {\n> -\tif (tx_buf->type != ICE_TX_BUF_XDP_TX && dma_unmap_len(tx_buf, len))\n> +\tswitch (tx_buf->type) {\n> +\tcase ICE_TX_BUF_DUMMY:\n> +\tcase ICE_TX_BUF_FRAG:\n> +\tcase ICE_TX_BUF_SKB:\n> +\tcase ICE_TX_BUF_XDP_XMIT:\n> \t\tdma_unmap_page(ring->dev,\n> \t\t\t dma_unmap_addr(tx_buf, dma),\n> \t\t\t dma_unmap_len(tx_buf, len),\n> \t\t\t DMA_TO_DEVICE);\n> +\t\tbreak;\n> +\t}\n\nDoes removing the dma_unmap_len() check alter the safety of the DMA error\nrecovery path?\n\nIf a mapping fails in ice_tx_map(), execution jumps to dma_error and calls\nice_unmap_and_free_tx_buf(). Since the failure happens before\ndma_unmap_len_set() updates the descriptor, it seems this might unconditionally\nunmap using stale descriptor state. \n\nCould this result in 0-length unmaps or erroneously unmapping persistent\nnet_iov memory?\n\n[ ... ]\n> @@ -338,12 +344,14 @@ static bool ice_clean_tx_irq(struct ice_tx_ring *tx_ring, int napi_budget)\n> \t\t\t}\n> \n> \t\t\t/* unmap any remaining paged data */\n> -\t\t\tif (dma_unmap_len(tx_buf, len)) {\n> +\t\t\tif (tx_buf->type != ICE_TX_BUF_EMPTY) {\n> \t\t\t\tdma_unmap_page(tx_ring->dev,\n> \t\t\t\t\t dma_unmap_addr(tx_buf, dma),\n> \t\t\t\t\t dma_unmap_len(tx_buf, len),\n> \t\t\t\t\t DMA_TO_DEVICE);\n> +\n> \t\t\t\tdma_unmap_len_set(tx_buf, len, 0);\n> +\t\t\t\ttx_buf->type = ICE_TX_BUF_EMPTY;\n> \t\t\t}\n> \t\t}\n\nWhen mapping a net_iov fragment in ice_tx_map(), the descriptor is populated\nwith the length and DMA address, but tx_buf->type remains ICE_TX_BUF_EMPTY.\n\nBecause dma_unmap_len_set(tx_buf, len, 0) is now placed inside the\ntx_buf->type != ICE_TX_BUF_EMPTY check, does this skip clearing the len and\ndma fields for net_iov descriptors when they are returned to the free pool?\n\nIf these descriptor fields are leaked, could they be incorrectly unmapped later\nif the descriptor is reused and a subsequent mapping fails?","headers":{"Return-Path":"","X-Original-To":["incoming@patchwork.ozlabs.org","intel-wired-lan@lists.osuosl.org"],"Delivered-To":["patchwork-incoming@legolas.ozlabs.org","intel-wired-lan@lists.osuosl.org"],"Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=osuosl.org header.i=@osuosl.org header.a=rsa-sha256\n header.s=default header.b=kC7Zi5oI;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=osuosl.org\n (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org;\n envelope-from=intel-wired-lan-bounces@osuosl.org;\n receiver=patchwork.ozlabs.org)"],"Received":["from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4gBtfY2Km0z1yJq\n\tfor ; Sat, 09 May 2026 01:42:29 +1000 (AEST)","from localhost (localhost [127.0.0.1])\n\tby smtp1.osuosl.org (Postfix) with ESMTP id EDF0C8443D;\n\tFri, 8 May 2026 15:42:27 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id Q2GVH_99Eu7U; Fri, 8 May 2026 15:42:27 +0000 (UTC)","from lists1.osuosl.org (lists1.osuosl.org [140.211.166.142])\n\tby smtp1.osuosl.org (Postfix) with ESMTP id 3A19C84437;\n\tFri, 8 May 2026 15:42:27 +0000 (UTC)","from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138])\n by lists1.osuosl.org (Postfix) with ESMTP id 19D93358\n for ; Fri, 8 May 2026 15:42:26 +0000 (UTC)","from localhost (localhost [127.0.0.1])\n by smtp1.osuosl.org (Postfix) with ESMTP id 0B17B84435\n for ; Fri, 8 May 2026 15:42:26 +0000 (UTC)","from smtp1.osuosl.org ([127.0.0.1])\n by localhost (smtp1.osuosl.org [127.0.0.1]) (amavis, port 10024) with ESMTP\n id jBVSxYk_qt0N for ;\n Fri, 8 May 2026 15:42:25 +0000 (UTC)","from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254])\n by smtp1.osuosl.org (Postfix) with ESMTPS id 1EDBB83F9A\n for ; Fri, 8 May 2026 15:42:24 +0000 (UTC)","from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58])\n by tor.source.kernel.org (Postfix) with ESMTP id 145F3600AE;\n Fri, 8 May 2026 15:42:24 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id D1D37C2BCC9;\n Fri, 8 May 2026 15:42:20 +0000 (UTC)"],"X-Virus-Scanned":["amavis at osuosl.org","amavis at osuosl.org"],"X-Comment":"SPF check N/A for local connections - client-ip=140.211.166.142;\n helo=lists1.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org;\n receiver= ","DKIM-Filter":["OpenDKIM Filter v2.11.0 smtp1.osuosl.org 3A19C84437","OpenDKIM Filter v2.11.0 smtp1.osuosl.org 1EDBB83F9A"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=osuosl.org;\n\ts=default; t=1778254947;\n\tbh=EAVUYgE7LocLLpiISnuzYat2WqQIK0QUTqYjNVqaTtc=;\n\th=From:To:Cc:Date:In-Reply-To:References:Subject:List-Id:\n\t List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe:\n\t From;\n\tb=kC7Zi5oIOxmwf2D3ZVXCSt9Hecz+oYy/L4ExhHp+4RwPDi0dFjcEyJ30wvtHNOQLt\n\t UX8By4jvBsjws24eMG1xm6NjNt3QHHfauunpvCYlRspYoOTNzrAP5aR7/YwT9n+aOi\n\t 4WkY34CQUBOIjwl4I0HLdm2ACK+X9UGgxo7qZya4hx3V66bej30th9KRp9rRMVksLa\n\t iu1o1woXNN0BrkcwT1OYSru/metHoNG5saHh+MsEFV6z+t41yP4n0txAZ3XPi7/F+5\n\t XTPjuO/usFVmZa8CmTNcgVoeXaqWNkxQ/5mQ393Noq4vW51Lm0fnzROwo+8tYGNLkc\n\t +7ndvqtB5hSug==","Received-SPF":"Pass (mailfrom) identity=mailfrom; client-ip=172.105.4.254;\n helo=tor.source.kernel.org; envelope-from=horms@kernel.org;\n receiver=","DMARC-Filter":"OpenDMARC Filter v1.4.2 smtp1.osuosl.org 1EDBB83F9A","From":"Simon Horman ","To":"aleksander.lobakin@intel.com","Cc":"'Simon Horman' , intel-wired-lan@lists.osuosl.org,\n anthony.l.nguyen@intel.com, przemyslaw.kitszel@intel.com,\n andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,\n kuba@kernel.org, pabeni@redhat.com, kohei@enjuk.jp,\n jacob.e.keller@intel.com, aleksandr.loktionov@intel.com,\n nxne.cnse.osdt.itp.upstreaming@intel.com, netdev@vger.kernel.org,\n linux-kernel@vger.kernel.org","Date":"Fri, 8 May 2026 16:42:00 +0100","Message-ID":"<20260508154159.1250696-2-horms@kernel.org>","X-Mailer":"git-send-email 2.54.0","In-Reply-To":"<20260505152923.1040589-6-aleksander.lobakin@intel.com>","References":"<20260505152923.1040589-6-aleksander.lobakin@intel.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-Mailman-Original-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n d=kernel.org; s=k20201202; t=1778254943;\n bh=hpyMjuuhAA498YiMJD/7ERK/AlvHaCx8MmWRt7EyM+0=;\n h=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n b=r3pbmeuPmFgZURnUdoN1bDWlA7HIgEWcv02J2eJLqk59Bm/CpTuOteR+/WLaQyYkZ\n 17zYddFItB9pnhfy3LpCjzdgyYta+tHjzqTx1TyFYoWKuT7Ir83XJFQPbDlIv1t6oa\n 2eQOMaQ6LoV/Ik8zcEv6cdOs+vUR42Kg07xoyB1lDG4PNTfndVJMoIf9nw5pUtWK/4\n XBzZFOB1b+ZxjAP2QVXRxPXPqrxPo2zYCjJ9BZZT4XI6UCWixIZBSDrv//YCB78eJK\n ZyCGQFP/ZSbQhFkFVUYQL02kYrpaHabc+xf3rNsCKutfqooxKuX8/KqAS6xrX4OB8C\n YcmVPpMaTP1ow==","X-Mailman-Original-Authentication-Results":["smtp1.osuosl.org;\n dmarc=pass (p=quarantine dis=none)\n header.from=kernel.org","smtp1.osuosl.org;\n dkim=pass (2048-bit key,\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=r3pbmeuP"],"Subject":"Re: [Intel-wired-lan] [PATCH iwl-next v5 5/5] ice: add support for\n transmitting unreadable frags","X-BeenThere":"intel-wired-lan@osuosl.org","X-Mailman-Version":"2.1.30","Precedence":"list","List-Id":"Intel Wired Ethernet Linux Kernel Driver Development\n ","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"intel-wired-lan-bounces@osuosl.org","Sender":"\"Intel-wired-lan\" "}}]