From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by smtp.lore.kernel.org (Postfix) with ESMTP id 85241E7DF13 for ; Mon, 2 Feb 2026 17:39:17 +0000 (UTC) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CBB074027D; Mon, 2 Feb 2026 18:39:16 +0100 (CET) Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) by mails.dpdk.org (Postfix) with ESMTP id CEDF34026D for ; Mon, 2 Feb 2026 18:39:15 +0100 (CET) Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-48068ed1eccso43526645e9.2 for ; Mon, 02 Feb 2026 09:39:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1770053955; x=1770658755; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=erGSmHwhu6tJyFiEfkxEAiLw9zJ91NEqhHA+2rwq0L0=; b=YDPzP/tNL74OF3lTJom5xSg31i055i3XPtKVwvjm8iWadD9jlngampm2AXfO5QcWKw dSywSlsp1Br4fNtabqDtXcPfwLVLt0YAAfqFUTG2dLPE+tStyuc34B4MgC2Kme1261gR dDIrf/dkA8w449PHvZ18ErRywypUmgN301IhJWIevUHlD1Q4ERxnGmXsKWxGyAIRJIp+ 64ItnA4HLTWwzV/zNlhoaAqN63wz8TAFas5QnGwAEe20XvYWcXh5ve8dxGkiddmWZ+Hw g30kUyPLae+T1d5cbJdCPiu4F8G1mBW5XEDh9KLy6yRhSIPAGWT+mtfslEa0Wr9fVFW1 a1kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770053955; x=1770658755; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=erGSmHwhu6tJyFiEfkxEAiLw9zJ91NEqhHA+2rwq0L0=; b=eEdpijy3+ngIeQeL0zLXomDyxVf6SJw93WvWGUICIFhcK/m9eEjJrdApJw3fYrE3kH /lYFAW4HaVKUhJY+Jr3AWRmKrgiWQIQ4NCEuY+yP39t3DIed9or79wS2c3LLagCwhf8P anu74KuKfKgaOL6gwFJMV/aNuQSYuJFOZJr4WtCZs1206kdClA3XAn7W+yG7l0bZ8z8s lVluLlseTqaFOa/5J+fUpfrpcZsv1ctmI3EsqoImGGseLG2nk8yYE9Pu/BkSsak9YxYX wbU6hSISqjpp9S92n9ZlsT2tgFlLxd5IrxTxTRbeVcGv3CctMZM3TuU8L+5APjQ7JKSJ eUkA== X-Gm-Message-State: AOJu0YxbHlhEI1V2uAKgsk8ojftJdRtbbhDcy3cxvYI+GPUkeJfempim +4CVhSnNj+WKrvQZAwMKHMwq4bHMtqjppvnSyZPeZO0d+ChftNeIHOKVS04XWAK3DryzpTkzLIx 1SOF4 X-Gm-Gg: AZuq6aKyAZ8oJ3e7BP+rn1QoVnSKqKGAJmrZuidxlWxlN3/yg4rQF0ANBfaQeLNYxyW oKOuGu5V0hQmntIjX1aDS9Fka/UQqv4RAvO2edcVJEjH+1CRYR5qFUn8yCZoKOGtNAlO183+drI 0mEGblBjv1tXa0mbeGpgV3xFTtGsxHG7u5UbclMYVlmethUC/+lo+3OWzO13mqVmWkbvPJS4V3D Ezc8apJ6j5lq+wNjNNG2QIqsWru3eBysiuBhX3lDEEXI7x9ZBpS3LkRC5w7cdP2g3378hnbwcZT 8Yri3FDvetVUtsfBaALfdunMthRxjuWJmoBGF7Q+xx86s0x+OBHl+oO2sD9zE5r0pDsyIiFM+61 JWIgV6kXjoWjahX6beu1Py6ZoNr03Y4dsajRH9DnSPFEQG1C9RpxbyKj8PNG82xO77VpOb82pxT IMN+7AO0uYPHdqtHtLny0/qnDYkjSAVMoQ0Wybzb340nyQjkuHw/CcWwItuuoA X-Received: by 2002:a05:600c:64c9:b0:480:6999:27ec with SMTP id 5b1f17b1804b1-482db45ca9dmr159367425e9.13.1770053955285; Mon, 02 Feb 2026 09:39:15 -0800 (PST) Received: from phoenix.lan (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48305129321sm3766255e9.4.2026.02.02.09.39.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Feb 2026 09:39:14 -0800 (PST) From: Stephen Hemminger To: dev@dpdk.org Cc: Stephen Hemminger Subject: [REVIEW] net/ice: add L2TPv2 RSS hash support Date: Mon, 2 Feb 2026 09:39:11 -0800 Message-ID: <20260202173911.19518-1-stephen@networkplumber.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260202094244.447966-7-shaiq.wani@intel.com> References: <20260202094244.447966-7-shaiq.wani@intel.com> Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org AI-generated review of bundle-1719-L2TPV2.mbox Reviewed using Claude (claude-opus-4-5-20251101) on 2026-02-02 This is an automated review. Please verify all suggestions. --- ## Review of DPDK Patch Series: L2TPv2 Support for net/ice ### Overall Summary This is a 6-patch series adding L2TPv2 and PPP protocol support for Flow Director (FDIR) filtering in the Intel ice driver. The patches are generally well-structured, but there are several issues that need attention. --- ## Patch 1/6: net/ice: add L2TPv2 and PPP data structures ### Errors None. ### Warnings 1. **Subject line could be more specific**: The subject mentions "L2TPv2 and PPP data structures" but the patch adds PPPoE structure (`ice_fdir_pppoe`) as well as PPP structure. Consider clarifying or keeping just PPP if PPPoE isn't used. ### Info - The data structures are cleanly defined and follow existing patterns in the file. --- ## Patch 2/6: net/ice: add L2TPv2 tunnel type definition ### Errors None. ### Warnings None. ### Info - Clean addition of the tunnel type enum and pattern structure fields. --- ## Patch 3/6: net/ice: add L2TPv2 protocol and field definitions ### Errors None. ### Warnings 1. **Renaming existing macros may break compatibility**: This patch renames `ICE_PROT_MAC` to `ICE_PROT_MAC_OUTER`, `ICE_PROT_IPV4` to `ICE_PROT_IPV4_OUTER`, etc. If these macros are used anywhere outside this file, this could break compilation. Verify no external dependencies exist. 2. **Extra blank line at end of file**: There's a double blank line before the closing of the file (line after `ICE_INSET_TUN_IPV6_DST` definition). ### Info - Good organization of inner/outer protocol definitions for tunnel support. --- ## Patch 4/6: net/ice: add L2TPv2 flow patterns and FDIR support ### Errors 1. **Inconsistent indentation in IPV6 handling block** (around lines 2213-2245): The code block for `RTE_FLOW_ITEM_TYPE_IPV6` has incorrect indentation. After the `p_v6` assignment, the `if (!(ipv6_spec && ipv6_mask))` block and subsequent code uses inconsistent indentation (mix of tabs and spaces not aligned properly). The closing sections starting from `if (!(ipv6_spec && ipv6_mask))` through `p_v6->hlim = ipv6_spec->hdr.hop_limits;` should be indented one level further to align with the case block. 2. **Multiple trailing blank lines at end of function** (line 2806): There are two extra blank lines before the closing of `ice_fdir_parse_pattern()`. Should be at most one. 3. **Missing variable declaration in block scope** (line 2590): `struct ice_fdir_l2tpv2 l2tpv2_be;` is declared in the middle of the switch case after other statements. In C89/C90 style (which DPDK sometimes follows for compatibility), declarations should be at the beginning of a block. This should be moved or the declaration should be in its own block. ### Warnings 1. **Inconsistent alignment in pattern table** (lines 160-176): The new L2TPv2 pattern entries have inconsistent tab alignment compared to the existing GTPU entries. While not breaking, it reduces readability. 2. **Long lines exceeding 100 characters**: Several lines in the L2TPv2 parsing section exceed 100 characters, for example: - Line 2148: `if (tunnel_type && !is_outer && tunnel_type == ICE_FDIR_TUNNEL_TYPE_L2TPV2)` - Multiple similar conditional lines in TCP/UDP handling 3. **Potential uninitialized variable**: `flags_version` is initialized to 0 on line 1899, but is used in flow type determination even when `l2tpv2_spec` might be NULL. The logic appears safe due to the `tunnel_type` check, but the flow is not immediately clear. 4. **Inconsistent spacing in conditionals**: Line 2809 has different indentation style: ```c if (!input_set || filter->input_set_o & ~(item->input_set_mask_o | ICE_INSET_ETHERTYPE) || ``` vs the original code alignment. ### Info - The patch is large and implements comprehensive L2TPv2 FDIR support. Consider splitting into smaller patches if possible (e.g., pattern definitions separate from parsing logic). --- ## Patch 5/6: net/ice: add L2TPv2 hardware packet generation ### Errors None. ### Warnings None. ### Info - Clean addition of case statements for the new L2TPv2 packet types. --- ## Patch 6/6: net/ice: add L2TPv2 RSS hash support ### Errors None. ### Warnings 1. **Missing documentation update**: This patch adds new RSS types (`ICE_RSS_TYPE_IPV4_L2TPV2`, `ICE_RSS_TYPE_IPV6_L2TPV2`) but there's no corresponding documentation update for the new feature. Consider adding release notes for the L2TPv2 RSS support. ### Info - The RSS hash configuration follows the existing pattern for other protocols. --- ## General Issues Across All Patches ### Warnings 1. **Missing release notes**: This feature series adds significant new functionality (L2TPv2 FDIR and RSS support) but no patch updates `doc/guides/rel_notes/`. A release notes entry should be added documenting the new L2TPv2 support. 2. **No test updates**: There are no corresponding test additions in `app/test` for the new L2TPv2 functionality. ### Info - The series is well-organized with logical progression of changes. - Each patch compiles independently based on the structure (data types → enums → definitions → implementation → hardware support → RSS). - The Tested-by tags are present on all patches, indicating the code has been tested.