From 6c79d99af6fde7c2c2f9bed9c880061227ba4002 Mon Sep 17 00:00:00 2001 From: Dmytro Podgornyi Date: Thu, 2 Nov 2023 00:08:31 +0200 Subject: [PATCH 1/5] issue: Fix inline function redefinition with clang sock-redirect implements read(), recv(), etc with intention to override them. However, some versions of glibc provide "fortified" calls. This is implemented by making regular symbols as inline wrappers on top of __read_chk() or similar. gcc allows to redefine such inline wrappers with specific attributes, but clang fails with an error. As a workaround for clang, rename glibc wrappers on the preprocessing stage. Signed-off-by: Dmytro Podgornyi --- src/vma/Makefile.am | 1 + src/vma/sock/sock-redirect-internal.h | 79 +++++++++++++++++++++++++++ src/vma/sock/sock-redirect.cpp | 2 + 3 files changed, 82 insertions(+) create mode 100644 src/vma/sock/sock-redirect-internal.h diff --git a/src/vma/Makefile.am b/src/vma/Makefile.am index d2963700b..0330cb1f0 100644 --- a/src/vma/Makefile.am +++ b/src/vma/Makefile.am @@ -275,6 +275,7 @@ libvma_la_SOURCES := \ sock/sockinfo_tcp.h \ sock/sockinfo_udp.h \ sock/sock-redirect.h \ + sock/sock-redirect-internal.h \ \ util/chunk_list.h \ util/hash_map.h \ diff --git a/src/vma/sock/sock-redirect-internal.h b/src/vma/sock/sock-redirect-internal.h new file mode 100644 index 000000000..2a710da4c --- /dev/null +++ b/src/vma/sock/sock-redirect-internal.h @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2001-2023 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * + * This software is available to you under a choice of one of two + * licenses. You may choose to be licensed under the terms of the GNU + * General Public License (GPL) Version 2, available from the file + * COPYING in the main directory of this source tree, or the + * BSD license below: + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above + * copyright notice, this list of conditions and the following + * disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +#ifndef SOCK_REDIRECT_INTERNAL_H +#define SOCK_REDIRECT_INTERNAL_H + +#include "config.h" + +/* + * Workaround for clang compilation error with fortified wrapper redefinition. + */ +#ifdef __clang__ +#ifdef HAVE___READ_CHK +#define read read_unused +#endif +#ifdef HAVE___RECV_CHK +#define recv recv_unused +#endif +#ifdef HAVE___RECVFROM_CHK +#define recvfrom recvfrom_unused +#endif +#ifdef HAVE___POLL_CHK +#define poll poll_unused +#endif +#ifdef HAVE___PPOLL_CHK +#define ppoll ppoll_unused +#endif +#endif /* __clang__ */ +#include +#include +#include +#ifdef __clang__ +#ifdef HAVE___READ_CHK +#undef read +#endif +#ifdef HAVE___RECV_CHK +#undef recv +#endif +#ifdef HAVE___RECVFROM_CHK +#undef recvfrom +#endif +#ifdef HAVE___POLL_CHK +#undef poll +#endif +#ifdef HAVE___PPOLL_CHK +#undef ppoll +#endif +#endif /* __clang__ */ + +#endif /* SOCK_REDIRECT_INTERNAL_H */ diff --git a/src/vma/sock/sock-redirect.cpp b/src/vma/sock/sock-redirect.cpp index 6002e1560..b5d5de1ad 100644 --- a/src/vma/sock/sock-redirect.cpp +++ b/src/vma/sock/sock-redirect.cpp @@ -31,6 +31,8 @@ */ +// sock-redirect-internal.h must be first +#include "sock-redirect-internal.h" #include "sock-redirect.h" #include From a997e637c93974715231933b95791716dd4f9897 Mon Sep 17 00:00:00 2001 From: Dmytro Podgornyi Date: Thu, 2 Nov 2023 00:17:48 +0200 Subject: [PATCH 2/5] issue: Use system gettid() with glibc-2.30+ This fixes compilation error on a system with glibc-2.30 or higher. Signed-off-by: Dmytro Podgornyi --- configure.ac | 3 +++ src/vlogger/vlogger.cpp | 2 ++ src/vlogger/vlogger.h | 2 ++ 3 files changed, 7 insertions(+) diff --git a/configure.ac b/configure.ac index 9a19c3292..b58c7f9a3 100644 --- a/configure.ac +++ b/configure.ac @@ -315,6 +315,9 @@ show_section_title "Check for functions, types and structures" # AC_CHECK_FUNCS(__read_chk __recv_chk __recvfrom_chk __poll_chk __ppoll_chk) +# Check for gettid(). The wrapper appeared in glibc-2.30 +AC_CHECK_FUNCS(gettid) + AC_MSG_CHECKING([for SOF_TIMESTAMPING_SOFTWARE support]) AC_LINK_IFELSE([AC_LANG_PROGRAM([[ #include diff --git a/src/vlogger/vlogger.cpp b/src/vlogger/vlogger.cpp index b9ba1d2da..a4e6ebbd9 100644 --- a/src/vlogger/vlogger.cpp +++ b/src/vlogger/vlogger.cpp @@ -142,10 +142,12 @@ namespace log_level } } +#ifndef HAVE_GETTID pid_t gettid(void) { return syscall(__NR_gettid); } +#endif #if _BullseyeCoverage #pragma BullseyeCoverage off diff --git a/src/vlogger/vlogger.h b/src/vlogger/vlogger.h index 1492eb1ae..e1340bfeb 100644 --- a/src/vlogger/vlogger.h +++ b/src/vlogger/vlogger.h @@ -259,7 +259,9 @@ extern vma_log_cb_t g_vlogger_cb; #define vlog_func_all_enter() vlog_printf(VLOG_FINER,"ENTER %s\n", __PRETTY_FUNCTION__); #define vlog_func_all_exit() vlog_printf(VLOG_FINER,"EXIT %s\n",__PRETTY_FUNCTION__); +#ifndef HAVE_GETTID pid_t gettid(void); // Check vlogger.cpp for implementation +#endif void printf_backtrace(void); From 0d40106a9ef97e965d8a29cc922b27b058bb9ed3 Mon Sep 17 00:00:00 2001 From: Dmytro Podgornyi Date: Thu, 2 Nov 2023 00:20:15 +0200 Subject: [PATCH 3/5] issue: Avoid gcc extension in a struct definition Clang doesn't allow variable sized type not at the end of a struct claiming this is a GNU extension. Make a union instead of adding an array after the variable sized field. This fixes vmad compilation with clang. Signed-off-by: Dmytro Podgornyi --- tools/daemon/tc.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tools/daemon/tc.c b/tools/daemon/tc.c index 80e3124b7..fa0aa651e 100644 --- a/tools/daemon/tc.c +++ b/tools/daemon/tc.c @@ -284,8 +284,10 @@ int tc_add_filter_link(tc_t tc, int ifindex, int prio, int ht, int id, uint32_t uint32_t opt_ht = HANDLE_SET(ht, 0, 0); struct rtattr *opts = NULL; struct { - struct tc_u32_sel sel; - struct tc_u32_key keys[5]; + union { + struct tc_u32_sel sel; + uint8_t pad[sizeof(struct tc_u32_sel) + sizeof(struct tc_u32_key) * 5U]; + }; } opt_sel; tc_req(tc, ifindex, RTM_NEWTFILTER, @@ -357,8 +359,10 @@ int tc_add_filter_tap2dev(tc_t tc, int ifindex, int prio, int id, uint32_t ip, i uint32_t opt_ht = HANDLE_SET(0x800, 0, 0); struct rtattr *opts = NULL; struct { - struct tc_u32_sel sel; - struct tc_u32_key keys[5]; + union { + struct tc_u32_sel sel; + uint8_t pad[sizeof(struct tc_u32_sel) + sizeof(struct tc_u32_key) * 5U]; + }; } opt_sel; tc_req(tc, ifindex, RTM_NEWTFILTER, @@ -479,8 +483,10 @@ int tc_add_filter_dev2tap(tc_t tc, int ifindex, int prio, int ht, int bkt, int i uint32_t opt_ht = HANDLE_SET(ht, bkt, 0); struct rtattr *opts = NULL; struct { - struct tc_u32_sel sel; - struct tc_u32_key keys[10]; + union { + struct tc_u32_sel sel; + uint8_t pad[sizeof(struct tc_u32_sel) + sizeof(struct tc_u32_key) * 10U]; + }; } opt_sel; tc_req(tc, ifindex, RTM_NEWTFILTER, From eff15d0750cbc9ae8ab0ca333b5aa63cdbf93124 Mon Sep 17 00:00:00 2001 From: Dmytro Podgornyi Date: Thu, 2 Nov 2023 00:24:31 +0200 Subject: [PATCH 4/5] issue: Fix unused-but-set-variable warnings with clang Signed-off-by: Dmytro Podgornyi --- src/state_machine/sm.cpp | 1 + src/vma/dev/qp_mgr.cpp | 1 + src/vma/dev/ring_simple.cpp | 1 + src/vma/dev/ring_tap.cpp | 4 +--- src/vma/event/event_handler_manager.cpp | 1 + src/vma/ib/base/verbs_extra.h | 2 +- src/vma/iomux/io_mux_call.cpp | 1 + src/vma/sock/sockinfo_udp.cpp | 1 + 8 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/state_machine/sm.cpp b/src/state_machine/sm.cpp index 4c353c2e0..5e11bdf5a 100644 --- a/src/state_machine/sm.cpp +++ b/src/state_machine/sm.cpp @@ -194,6 +194,7 @@ BULLSEYE_EXCLUDE_BLOCK_END } sm_logdbg("SM full table processing done. Allocated memory size of %d bytes", sm_table_entries_size); + (void)sm_table_entries_size; // Suppress --enable-opt-log=high warning return 0; } diff --git a/src/vma/dev/qp_mgr.cpp b/src/vma/dev/qp_mgr.cpp index 316bb0a67..c36b428c5 100644 --- a/src/vma/dev/qp_mgr.cpp +++ b/src/vma/dev/qp_mgr.cpp @@ -432,6 +432,7 @@ void qp_mgr::release_rx_buffers() } m_last_posted_rx_wr_id = 0; // Clear the posted WR_ID flag, we just clear the entire RQ qp_logdbg("draining completed with a total of %d wce's on rx cq_mgr", total_ret); + NOT_IN_USE(total_ret); // Suppress --enable-opt-log=high warning } void qp_mgr::release_tx_buffers() diff --git a/src/vma/dev/ring_simple.cpp b/src/vma/dev/ring_simple.cpp index 50df91a5b..b0435fb53 100644 --- a/src/vma/dev/ring_simple.cpp +++ b/src/vma/dev/ring_simple.cpp @@ -876,6 +876,7 @@ int ring_simple::put_tx_buffers(mem_buf_desc_t* buff_list) buff_list = next; } ring_logfunc("buf_list: %p count: %d freed: %d\n", buff_list, count, freed); + NOT_IN_USE(freed); // Suppress unused-but-set-variable warning return_to_global_pool(); diff --git a/src/vma/dev/ring_tap.cpp b/src/vma/dev/ring_tap.cpp index a9416c61e..02d675698 100644 --- a/src/vma/dev/ring_tap.cpp +++ b/src/vma/dev/ring_tap.cpp @@ -515,8 +515,6 @@ void ring_tap::mem_buf_desc_return_single_to_owner_tx(mem_buf_desc_t* p_mem_buf_ { auto_unlocker lock(m_lock_ring_tx); - int count = 0; - if (likely(p_mem_buf_desc)) { //potential race, ref is protected here by ring_tx lock, and in dst_entry_tcp & sockinfo_tcp by tcp lock if (likely(p_mem_buf_desc->lwip_pbuf.pbuf.ref)) @@ -528,7 +526,6 @@ void ring_tap::mem_buf_desc_return_single_to_owner_tx(mem_buf_desc_t* p_mem_buf_ p_mem_buf_desc->p_next_desc = NULL; free_lwip_pbuf(&p_mem_buf_desc->lwip_pbuf); m_tx_pool.push_back(p_mem_buf_desc); - count++; } } @@ -568,6 +565,7 @@ int ring_tap::mem_buf_tx_release(mem_buf_desc_t* buff_list, bool b_accounting, b buff_list = next; } ring_logfunc("buf_list: %p count: %d freed: %d\n", buff_list, count, freed); + NOT_IN_USE(freed); // Suppress unused-but-set-variable warning return_to_global_pool(); diff --git a/src/vma/event/event_handler_manager.cpp b/src/vma/event/event_handler_manager.cpp index a32a71ea2..09b2b8979 100644 --- a/src/vma/event/event_handler_manager.cpp +++ b/src/vma/event/event_handler_manager.cpp @@ -502,6 +502,7 @@ void event_handler_manager::priv_prepare_ibverbs_async_event_queue(event_handler cnt++; } evh_logdbg("Emptied %d Events", cnt); + NOT_IN_USE(cnt); // Suppress --enable-opt-log=high warning } void event_handler_manager::priv_register_ibverbs_events(ibverbs_reg_info_t& info) diff --git a/src/vma/ib/base/verbs_extra.h b/src/vma/ib/base/verbs_extra.h index 3075e7201..5251b1c1b 100644 --- a/src/vma/ib/base/verbs_extra.h +++ b/src/vma/ib/base/verbs_extra.h @@ -300,7 +300,7 @@ typedef struct { #ifdef DEFINED_IBV_QP_SUPPORT_BURST #define vma_ibv_init_burst_attr(qp_attr, rate_limit) { qp_attr.max_burst_sz = rate_limit.max_burst_sz; qp_attr.typical_pkt_sz = rate_limit.typical_pkt_sz; } typedef struct ibv_qp_rate_limit_attr vma_ibv_rate_limit_attr; -#define vma_ibv_modify_qp_rate_limit(qp, attr, mask) ibv_modify_qp_rate_limit(qp, attr) +#define vma_ibv_modify_qp_rate_limit(qp, attr, mask) ({ NOT_IN_USE(mask); ibv_modify_qp_rate_limit(qp, attr); }) #define vma_ibv_init_qps_attr(qp_attr) { NOT_IN_USE(qp_attr); } #else typedef vma_ibv_qp_attr vma_ibv_rate_limit_attr; diff --git a/src/vma/iomux/io_mux_call.cpp b/src/vma/iomux/io_mux_call.cpp index 618e1518b..d031ae559 100644 --- a/src/vma/iomux/io_mux_call.cpp +++ b/src/vma/iomux/io_mux_call.cpp @@ -395,6 +395,7 @@ void io_mux_call::polling_loops() } __if_dbg("2nd scenario exit (loop %d, elapsed %d)", poll_counter, m_elapsed.tv_usec); + NOT_IN_USE(poll_counter); // Suppress unused-but-set-variable warning } void io_mux_call::blocking_loops() diff --git a/src/vma/sock/sockinfo_udp.cpp b/src/vma/sock/sockinfo_udp.cpp index e6edfc6ec..9e496244b 100644 --- a/src/vma/sock/sockinfo_udp.cpp +++ b/src/vma/sock/sockinfo_udp.cpp @@ -1520,6 +1520,7 @@ int sockinfo_udp::rx_request_notification(uint64_t poll_sn) m_rx_ring_map_lock.unlock(); si_udp_logfunc("armed or busy %d ring(s) and %d ring are pending processing", ring_armed_count, ring_ready_count); + NOT_IN_USE(ring_armed_count); // Suppress unused-but-set-variable warning return ring_ready_count; } From 077eab6bd602220ac5b52c7250a0c41c5724acfd Mon Sep 17 00:00:00 2001 From: Dmytro Podgornyi Date: Thu, 2 Nov 2023 00:54:48 +0200 Subject: [PATCH 5/5] issue: Fix clang analyzer warnings Suppress "dead nested assignment" warnings. Signed-off-by: Dmytro Podgornyi --- src/vma/dev/dm_mgr.cpp | 1 + src/vma/dev/ring_bond.cpp | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vma/dev/dm_mgr.cpp b/src/vma/dev/dm_mgr.cpp index 098a8a945..bcd851a79 100644 --- a/src/vma/dev/dm_mgr.cpp +++ b/src/vma/dev/dm_mgr.cpp @@ -246,6 +246,7 @@ bool dm_mgr::copy_data(struct mlx5_wqe_data_seg* seg, uint8_t* src, uint32_t len dev_mem_oob: dm_logfunc("Send OOB! Buffer[%p] length[%d] length_aligned_8[%d] continuous_left[%zu] head[%zu] used[%zu]", buff, length, length_aligned_8, continuous_left, m_head, m_used); + NOT_IN_USE(continuous_left); // Suppress dead assignment warning m_p_ring_stat->simple.n_tx_dev_mem_oob++; diff --git a/src/vma/dev/ring_bond.cpp b/src/vma/dev/ring_bond.cpp index 75af0283c..43769d739 100644 --- a/src/vma/dev/ring_bond.cpp +++ b/src/vma/dev/ring_bond.cpp @@ -919,9 +919,8 @@ void ring_bond_eth::slave_create(int if_index) void ring_bond_ib::slave_create(int if_index) { ring_slave *cur_slave; - ring_simple *cur_simple; - cur_slave = cur_simple = new ring_ib(if_index, this); + cur_slave = new ring_ib(if_index, this); if (cur_slave == NULL) { ring_logpanic("Error creating bond ring: memory allocation error"); }