From patchwork Sat Nov 11 16:56:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Stephen Hemminger X-Patchwork-Id: 134116 X-Patchwork-Delegate: ferruh.yigit@amd.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 8A7D143304; Sat, 11 Nov 2023 17:56:38 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 593364027E; Sat, 11 Nov 2023 17:56:38 +0100 (CET) Received: from mail-pf1-f179.google.com (mail-pf1-f179.google.com [209.85.210.179]) by mails.dpdk.org (Postfix) with ESMTP id 8A7C0400D7 for ; Sat, 11 Nov 2023 17:56:37 +0100 (CET) Received: by mail-pf1-f179.google.com with SMTP id d2e1a72fcca58-6b2018a11efso3175572b3a.0 for ; Sat, 11 Nov 2023 08:56:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1699721796; x=1700326596; darn=dpdk.org; h=content-transfer-encoding:mime-version:message-id:subject:cc:to :from:date:from:to:cc:subject:date:message-id:reply-to; bh=imbg628G+K5crMKGaZltGvQNpKzcmfIBuNeiltX7X6E=; b=uIPY8lO8Qn09v08zXADLwnovXYLVWzxKdxisbXHrQICNjXJSiFljw/bJ+2m5piP8py zLiRgMraGvF/WRgNojTCX04IiyWK9mtGb0eXM36FYg0nyWJN1FoZPmC2yTGPp0Vl32cX I3UlMJ0fq+z2BZpLzAJcrWwACVECj7FMFFFbRTC1hZP0UWnibAmTgTILGdpWy9+kFU/q 2q1gSlbwSQN53OBJwuxSwxIKV90np5rwwM+c5GU9MAAhmY88+5jc0D9++9qScgFfW0vT I+IBbQwPs0tgh9m3tL6oOkCgQDksWNuYu/3sa9PmVord4Tz4UdJnVNFw/5rzuOerCH9h bxDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699721796; x=1700326596; h=content-transfer-encoding:mime-version:message-id:subject:cc:to :from:date:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=imbg628G+K5crMKGaZltGvQNpKzcmfIBuNeiltX7X6E=; b=VZ4fGdtfpayCiH5Rpy9TEqvpG0LPKCsvTCcpwwVW3Zf7sOYFcYhn10R0CUuWfBQ1z1 elhhDi3czMi6wffyHCCjFfEbLxjCyAuagaMwka0S0CNuR0HkE/6uOsydZ/3P0imuXHpd kcLLgNj6jReHCks2dCf2mJPPhAAphIAoaRfYKrWRMb8oQaLm8WdhSuubf+uRXzMOCx2y CflbsdLuGrcZ/MQlX15VSvcbX3Zp5zqf18ku8tG0qBpeBGwy6tk0QCcl3JUFlWOSMEcD 3l7eWpDi2C4FjIbJ2psUsItO8HEbI4dY4BolowYr3pGnuFDgEmA/xuJpAkKLkp5XgSWg ng2A== X-Gm-Message-State: AOJu0YyIA7BaeP62I1it9arlhEpqMdgjwRxE0VjowL0V1q8zeOu8sUcW I2K1z31iRIgChNOkqcOQOPFqsTdhb9H0Vgh4YwQ= X-Google-Smtp-Source: AGHT+IH8mMEGEwDejHWGH76M+QYxJMdEP5SWdVCZVbsk9By1o2T/RVME4CJzmIYUXCSLBWp/UtCuRg== X-Received: by 2002:a05:6a21:66cb:b0:154:6480:8588 with SMTP id ze11-20020a056a2166cb00b0015464808588mr2862196pzb.0.1699721796489; Sat, 11 Nov 2023 08:56:36 -0800 (PST) Received: from hermes.local (204-195-123-141.wavecable.com. [204.195.123.141]) by smtp.gmail.com with ESMTPSA id c21-20020aa78815000000b006c107a9e8f0sm1485425pfo.128.2023.11.11.08.56.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 11 Nov 2023 08:56:36 -0800 (PST) Date: Sat, 11 Nov 2023 08:56:34 -0800 From: Stephen Hemminger To: Andrew Rybchenko Cc: dev@dpdk.org Subject: BUILD bug hidden in SFC driver. Message-ID: <20231111085634.5df512bf@hermes.local> MIME-Version: 1.0 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 While examining the use of VLA in DPDK, ran into a bug in sfc driver. If DPDK is built with -Wvla, then the RTE_BUILD_BUG_ON() macro won't work as written. Experimenting with a better more portable version of that macro as: #define RTE_BUILD_BUG_ON(e) _Static_assert(!(e), #e) revealed that the SFC driver was calling RTE_BUILD_BUG_ON with non constant expression. ../drivers/net/sfc/sfc_ef100_tx.c: In function ‘sfc_ef100_tx_pkt_descs_max’: ../lib/eal/include/rte_common.h:585:20: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare] 585 | _a < _b ? _a : _b; \ | ^ ../lib/eal/include/rte_common.h:498:46: note: in definition of macro ‘RTE_BUILD_BUG_ON’ 498 | #define RTE_BUILD_BUG_ON(e) _Static_assert(!(e), #e) | ^ ../drivers/net/sfc/sfc_ef100_tx.c:566:34: note: in expansion of macro ‘RTE_MIN’ 566 | RTE_MIN((unsigned int)EFX_MAC_PDU_MAX, | ^~~~~~~ ../lib/eal/include/rte_common.h:585:32: warning: operand of ‘?:’ changes signedness from ‘int’ to ‘unsigned int’ due to unsignedness of other operand [-Wsign-compare] 585 | _a < _b ? _a : _b; \ | ^~ ../lib/eal/include/rte_common.h:498:46: note: in definition of macro ‘RTE_BUILD_BUG_ON’ 498 | #define RTE_BUILD_BUG_ON(e) _Static_assert(!(e), #e) | ^ ../drivers/net/sfc/sfc_ef100_tx.c:566:34: note: in expansion of macro ‘RTE_MIN’ 566 | RTE_MIN((unsigned int)EFX_MAC_PDU_MAX, | ^~~~~~~ ../lib/eal/include/rte_common.h:498:44: error: expression in static assertion is not constant 498 | #define RTE_BUILD_BUG_ON(e) _Static_assert(!(e), #e) | ^~~~ ../drivers/net/sfc/sfc_ef100_tx.c:565:17: note: in expansion of macro ‘RTE_BUILD_BUG_ON’ The problem is that Gcc does not evaluate a ternary operator expression with all constants at compile time to produce a constant value! Apparently, the language standards leave this as ambiguous. If the code is expanded into two assertions as: Then a new problem arises: In file included from ../lib/mbuf/rte_mbuf.h:36, from ../drivers/net/sfc/sfc_ef100_tx.c:12: ../drivers/net/sfc/sfc_ef100_tx.c: In function ‘sfc_ef100_tx_pkt_descs_max’: ../lib/eal/include/rte_common.h:498:29: error: static assertion failed: "SFC_EF100_TX_SEND_DESC_LEN_MAX < SFC_MBUF_SEG_LEN_MAX" 498 | #define RTE_BUILD_BUG_ON(e) _Static_assert(!(e), #e) | ^~~~~~~~~~~~~~ ../drivers/net/sfc/sfc_ef100_tx.c:566:17: note: in expansion of macro ‘RTE_BUILD_BUG_ON’ 566 | RTE_BUILD_BUG_ON(SFC_EF100_TX_SEND_DESC_LEN_MAX < SFC_MBUF_SEG_LEN_MAX); | ^~~~~~~~~~~~~~~~ Building a little program to unwind the #defines reveals: SFC_EF100_TX_SEND_DESC_LEN_MAX = 16383 EFX_MAC_PDU_MAX = 9240 SFC_MBUF_SEG_LEN_MAX = 65535 I.e: RTE_BUILD_BUG_ON(16383 < RTE_MIN(9240, 65535)); Therefore the current driver should be getting build bug, but the existing macro hides it. diff --git a/drivers/net/sfc/sfc_ef100_tx.c b/drivers/net/sfc/sfc_ef100_tx.c index 1b6374775f07..25e6633d6679 100644 --- a/drivers/net/sfc/sfc_ef100_tx.c +++ b/drivers/net/sfc/sfc_ef100_tx.c @@ -562,9 +562,8 @@ sfc_ef100_tx_pkt_descs_max(const struct rte_mbuf *m) * Make sure that the first segment does not need fragmentation * (split into many Tx descriptors). */ - RTE_BUILD_BUG_ON(SFC_EF100_TX_SEND_DESC_LEN_MAX < - RTE_MIN((unsigned int)EFX_MAC_PDU_MAX, - SFC_MBUF_SEG_LEN_MAX)); + RTE_BUILD_BUG_ON(SFC_EF100_TX_SEND_DESC_LEN_MAX < EFX_MAC_PDU_MAX); + RTE_BUILD_BUG_ON(SFC_EF100_TX_SEND_DESC_LEN_MAX < SFC_MBUF_SEG_LEN_MAX); } if (m->ol_flags & sfc_dp_mport_override) {