On Mon, Feb 2, 2026 at 2:17 PM Ilya Maximets <[email protected]> wrote: > > On 1/25/26 10:33 AM, Dmitry Mityugov wrote: > > Array impl_set_masked_funcs is defined in lib/odp-execute-avx512.c > > without static modifier but isn't used outside of this module. Recent > > versions of GCC can be instructed to deliver warnings for such cases, > > with -Wmissing-variable-declarations compiler option enabled. The > > problem in question can be reproduced with > > > > ./configure --enable-Werror CFLAGS="-Wmissing-variable-declarations" && make > > > > To fix the problem, the array should be defined as static. This problem > > can be reproduced only on 64-bit x86 machines, not on ARM/RISC-V. > > > > Signed-off-by: Dmitry Mityugov <[email protected]> > > --- > > Huge thanks to everyone who helped me fix the email with the original > > patch. Hopefully it will be accepted this time. > > > > Not sure about the address this patch should be sent to. Page > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev says to use > > [email protected] whereas this guide > > https://docs.openvswitch.org/en/latest/internals/contributing/submitting-patches/ > > recommends [email protected] > > > > lib/odp-execute-avx512.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/odp-execute-avx512.c b/lib/odp-execute-avx512.c > > index 13afe0c79..55af58737 100644 > > --- a/lib/odp-execute-avx512.c > > +++ b/lib/odp-execute-avx512.c > > @@ -108,7 +108,7 @@ BUILD_ASSERT_DECL(offsetof(struct ovs_key_ipv6, > > ipv6_tclass) + > > offsetof(struct ovs_key_ipv6, ipv6_hlimit)); > > > > /* Array of callback functions, one for each masked operation. */ > > -odp_execute_action_cb impl_set_masked_funcs[__OVS_KEY_ATTR_MAX]; > > +static odp_execute_action_cb impl_set_masked_funcs[__OVS_KEY_ATTR_MAX]; > > > > static inline void ALWAYS_INLINE > > avx512_dp_packet_resize_l2(struct dp_packet *b, int resize_by_bytes) > > Hi, Dmitry. Thanks for the the patch! > > I agree that we should fix the missing keyword in this file, but it doesn't > make much sense if we have no protection from re-introducing the issue later > in some other place. We already enable by default the -Wmissing-prototypes > and the -Wmissing-field-initializers, so I think, we should just turn all > -Wmissing-variable-declarations whenever available as well. This will help > developers and our CI to spot the problem. > > See the relevant code in configure.ac, look for OVS_ENABLE_OPTION. Note that > we'll need to change the test program in _OVS_CHECK_CC_OPTION as well, since > it's not compliant with this flag and the detection will fail otherwise. > > The flag is also supported by clang for a long time, some GCC part in the > subject is not particularly relevant. The new patch name prefix should be > 'configure:' with the name focusing on enabling the new option. > > Best regards, Ilya Maximets.
Hi Ilya, Thank you for your clarification. Now I better understand how the configuration files work. I prepared a new version of my patch and will post it shortly, under a different subject. I also added a similar option, -Wmissing-declarations, to this new patch. Currently it reveals no problems in the code but, hopefully, will protect from introducing new ones. Best regards, -- Dmitry _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
