[v3,2/5] net/sfc: fix non-constant expression in RTE_BUILD_BUG_ON()
Checks
Commit Message
The macro RTE_MIN has some hidden assignments to provide type
safety which means the statement can not be fully evaluated in
first pass of compiler. Replace RTE_MIN() with equivalent macro.
This will cause errors from checkpatch about multiple evaluations
of same expression in macro but it is ok in this case.
Fixes: 4f936666d790 ("net/sfc: support TSO for EF100 native datapath")
Cc: ivan.malov@oktetlabs.ru
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
drivers/net/sfc/sfc_ef100_tx.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Comments
On 1/16/24 21:41, Stephen Hemminger wrote:
> The macro RTE_MIN has some hidden assignments to provide type
> safety which means the statement can not be fully evaluated in
> first pass of compiler. Replace RTE_MIN() with equivalent macro.
>
> This will cause errors from checkpatch about multiple evaluations
> of same expression in macro but it is ok in this case.
>
> Fixes: 4f936666d790 ("net/sfc: support TSO for EF100 native datapath")
I'm not sure that it is really a fix.
> Cc: ivan.malov@oktetlabs.ru
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> drivers/net/sfc/sfc_ef100_tx.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/sfc/sfc_ef100_tx.c b/drivers/net/sfc/sfc_ef100_tx.c
> index 1b6374775f07..f4bcadc1e8e0 100644
> --- a/drivers/net/sfc/sfc_ef100_tx.c
> +++ b/drivers/net/sfc/sfc_ef100_tx.c
> @@ -26,6 +26,10 @@
> #include "sfc_ef100.h"
> #include "sfc_nic_dma_dp.h"
>
> +#ifndef MIN
> +/* not typesafe but is a constant */
> +#define MIN(x, y) ((x) < (y) ? (x) : (y))
> +#endif
IMHO adding it in specific driver is a wrong direction. I'm afraid it
will result in duplication of such macros in code base without clear
reason why.
May be it is better to add it with a proper name to EAL?
>
> #define sfc_ef100_tx_err(_txq, ...) \
> SFC_DP_LOG(SFC_KVARG_DATAPATH_EF100, ERR, &(_txq)->dp.dpq, __VA_ARGS__)
> @@ -563,8 +567,7 @@ sfc_ef100_tx_pkt_descs_max(const struct rte_mbuf *m)
> * (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));
> + MIN((unsigned int)EFX_MAC_PDU_MAX, SFC_MBUF_SEG_LEN_MAX));
> }
>
> if (m->ol_flags & sfc_dp_mport_override) {
@@ -26,6 +26,10 @@
#include "sfc_ef100.h"
#include "sfc_nic_dma_dp.h"
+#ifndef MIN
+/* not typesafe but is a constant */
+#define MIN(x, y) ((x) < (y) ? (x) : (y))
+#endif
#define sfc_ef100_tx_err(_txq, ...) \
SFC_DP_LOG(SFC_KVARG_DATAPATH_EF100, ERR, &(_txq)->dp.dpq, __VA_ARGS__)
@@ -563,8 +567,7 @@ sfc_ef100_tx_pkt_descs_max(const struct rte_mbuf *m)
* (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));
+ MIN((unsigned int)EFX_MAC_PDU_MAX, SFC_MBUF_SEG_LEN_MAX));
}
if (m->ol_flags & sfc_dp_mport_override) {