On 10/8/25 8:43 AM, David Marchand wrote: > On Tue, 7 Oct 2025 at 22:04, Ilya Maximets <[email protected]> wrote: >> >> With NDEBUG, all assertions are just ignoring the result of the >> condition and this is causing build warnings when the condition >> is a result of a function marked with OVS_WARN_UNUSED_RESULT: >> >> lib/dp-packet.c: In function 'dp_packet_ol_send_prepare': >> lib/dp-packet.c:580:28: >> error: ignoring return value of 'dp_packet_l4_proto_sctp' declared >> with attribute 'warn_unused_result' [-Werror=unused-result] >> 580 | ovs_assert(dp_packet_l4_proto_sctp(p)); >> ./include/openvswitch/util.h:58:40: >> note: in definition of macro 'ovs_assert' >> 58 | #define ovs_assert(CONDITION) ((void) (CONDITION)) >> | ^~~~~~~~~ >> lib/dp-packet.c:621:24: >> error: ignoring return value of 'dp_packet_inner_l4_proto_sctp' >> declared with attribute 'warn_unused_result' [-Werror=unused-result] >> 621 | ovs_assert(dp_packet_inner_l4_proto_sctp(p)); >> ./include/openvswitch/util.h:58:40: >> note: in definition of macro 'ovs_assert' >> 58 | #define ovs_assert(CONDITION) ((void) (CONDITION)) >> | ^~~~~~~~~ >> lib/dp-packet.c:634:20: >> error: ignoring return value of 'dp_packet_l4_proto_udp' declared >> with attribute 'warn_unused_result' [-Werror=unused-result] >> 634 | ovs_assert(dp_packet_l4_proto_udp(p)); >> ./include/openvswitch/util.h:58:40: >> note: in definition of macro 'ovs_assert' >> 58 | #define ovs_assert(CONDITION) ((void) (CONDITION)) >> | ^~~~~~~~~ >> cc1: all warnings being treated as errors >> >> Let's export the ignore() function as ovs_ignore() and use it for >> the ovs_assert in case of NDEBUG build. This will silence the >> warnings. >> >> Most files already import "util.h", so there is no need to add >> the public "openvswitch/util.h" as well. >> >> Fixes: 3daf04a4c56d ("dp-packet: Rework IP checksum offloads.") > > Nit: the previous commit 67abd51540d0 ("dp-packet: Rework tunnel > offloads.") had the same issue. > > +static inline bool OVS_WARN_UNUSED_RESULT > +dp_packet_tunnel_gre(const struct dp_packet *b) > ... > + ovs_assert(dp_packet_tunnel_gre(pkt));
Yep. Good point. > > >> Signed-off-by: Ilya Maximets <[email protected]> > > As a followup, can we add a job in GHA to catch such build issue (and > maybe run the tests with NDEBUG too)? I'm not sure if this worth spending CI time on, CI is a little bloated already and it seems like most users are not building with NDEBUG. But we can consider it. > > Otherwise this lgtm. > Reviewed-by: David Marchand <[email protected]> Thanks, David and Eelco! I added one more Fixes tag and applied the change. Backported down to 3.5. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
