From: Stephen Hemminger <stephen@networkplumber.org>
To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Cc: dev@dpdk.org
Subject: BUILD bug hidden in SFC driver.
Date: Sat, 11 Nov 2023 08:56:34 -0800 [thread overview]
Message-ID: <20231111085634.5df512bf@hermes.local> (raw)
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:
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) {
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.
next reply other threads:[~2023-11-11 16:56 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-11 16:56 Stephen Hemminger [this message]
2023-11-11 17:29 ` BUILD bug hidden in SFC driver Stephen Hemminger
2023-11-13 12:04 ` Ivan Malov
2023-11-13 16:30 ` Stephen Hemminger
2023-11-13 16:28 ` Tyler Retzlaff
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231111085634.5df512bf@hermes.local \
--to=stephen@networkplumber.org \
--cc=andrew.rybchenko@oktetlabs.ru \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.