| Message ID | 20251009092228.382349-3-i.maximets@ovn.org |
|---|---|
| State | Accepted |
| Commit | f83d2e4d53ad06469d0847cb039b14ea476535b3 |
| Headers | show |
| Series | Build fixes for OVS on old distributions. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 09/10/2025 10:21, Ilya Maximets wrote: > Current code assumes that if the rtnetlink.h exists, then it must > have definitions for RTA_VIA and 'struct rtvia'. This is causing > build failures on older systems: > > lib/netlink.c: In function 'min_attr_len': > lib/netlink.c:832:38: > error: invalid application of 'sizeof' to incomplete type 'struct rtvia' > case NL_A_RTA_VIA: return sizeof(struct rtvia) + sizeof(struct in_addr); > ^ > > The code structure overall is not great, we should not include Linux > specific headers in the common netlink.c. The only reason for the > inclusion though is the structure size. We can just put a number > instead, as it is already done for all other types. It should never > change, as long as kernel uAPI is stable. > > Structure and the RTA_VIA should be defined in the Linux-specific > route-table.c, in case rtnetlink.h doesn't have a definition for them. > > Fixes: 9d9a99d157f2 ("route-table: Support parsing RTA_VIA attribute.") > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2025- > September/053800.html > Reported-by: Brendan Doyle <brendan.doyle@oracle.com> > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > acinclude.m4 | 7 +++++++ > lib/netlink.c | 14 ++------------ > lib/route-table.c | 9 +++++++++ Acked-by: Kevin Traynor <ktraynor@redhat.com>
On 9 Oct 2025, at 11:21, Ilya Maximets wrote: > Current code assumes that if the rtnetlink.h exists, then it must > have definitions for RTA_VIA and 'struct rtvia'. This is causing > build failures on older systems: > > lib/netlink.c: In function 'min_attr_len': > lib/netlink.c:832:38: > error: invalid application of 'sizeof' to incomplete type 'struct rtvia' > case NL_A_RTA_VIA: return sizeof(struct rtvia) + sizeof(struct in_addr); > ^ > > The code structure overall is not great, we should not include Linux > specific headers in the common netlink.c. The only reason for the > inclusion though is the structure size. We can just put a number > instead, as it is already done for all other types. It should never > change, as long as kernel uAPI is stable. > > Structure and the RTA_VIA should be defined in the Linux-specific > route-table.c, in case rtnetlink.h doesn't have a definition for them. > > Fixes: 9d9a99d157f2 ("route-table: Support parsing RTA_VIA attribute.") > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2025-September/053800.html > Reported-by: Brendan Doyle <brendan.doyle@oracle.com> > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Change looks good to me! Acked-by: Eelco Chaudron <echaudro@redhat.com>
diff --git a/acinclude.m4 b/acinclude.m4 index 8658bcfcb..369e37eae 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -155,6 +155,13 @@ AC_DEFUN([OVS_CHECK_LINUX_NETLINK], [ ])], [AC_DEFINE([HAVE_NLA_BITFIELD32], [1], [Define to 1 if struct nla_bitfield32 is available.])]) + + AC_COMPILE_IFELSE([ + AC_LANG_PROGRAM([#include <linux/rtnetlink.h>], [ + struct rtvia x = { 0 }; + ])], + [AC_DEFINE([HAVE_RTA_VIA], [1], + [Define to 1 if struct rtvia is available.])]) ]) dnl OVS_CHECK_LINUX_TC diff --git a/lib/netlink.c b/lib/netlink.c index 446a0679e..fbfc20e7c 100644 --- a/lib/netlink.c +++ b/lib/netlink.c @@ -29,16 +29,6 @@ #include "openvswitch/vlog.h" #include "util.h" -#ifdef HAVE_NETLINK -#include <linux/rtnetlink.h> -#else -/* RTA_VIA */ -struct rtvia { - sa_family_t rtvia_family; - uint8_t rtvia_addr[]; -}; -#endif - VLOG_DEFINE_THIS_MODULE(netlink); /* A single (bad) Netlink message can in theory dump out many, many log @@ -829,7 +819,7 @@ min_attr_len(enum nl_attr_type type) case NL_A_IPV6: return 16; case NL_A_NESTED: return 0; case NL_A_LL_ADDR: return 6; /* ETH_ALEN */ - case NL_A_RTA_VIA: return sizeof(struct rtvia) + sizeof(struct in_addr); + case NL_A_RTA_VIA: return 2 + 4; /* struct rtvia + struct in_addr. */ case N_NL_ATTR_TYPES: default: OVS_NOT_REACHED(); } } @@ -851,7 +841,7 @@ max_attr_len(enum nl_attr_type type) case NL_A_IPV6: return 16; case NL_A_NESTED: return SIZE_MAX; case NL_A_LL_ADDR: return 20; /* INFINIBAND_ALEN */ - case NL_A_RTA_VIA: return sizeof(struct rtvia) + sizeof(struct in6_addr); + case NL_A_RTA_VIA: return 2 + 16; /* struct rtvia + struct in6_addr. */ case N_NL_ATTR_TYPES: default: OVS_NOT_REACHED(); } } diff --git a/lib/route-table.c b/lib/route-table.c index 2bbb51c08..ca87ff7db 100644 --- a/lib/route-table.c +++ b/lib/route-table.c @@ -44,6 +44,15 @@ * old headers. (We can't test for it with #ifdef because it's an enum.) */ #define RTA_MARK 16 +/* Linux 4.1 added RTA_VIA. */ +#ifndef HAVE_RTA_VIA +#define RTA_VIA 18 +struct rtvia { + sa_family_t rtvia_family; + uint8_t rtvia_addr[]; +}; +#endif + VLOG_DEFINE_THIS_MODULE(route_table); COVERAGE_DEFINE(route_table_dump);
Current code assumes that if the rtnetlink.h exists, then it must have definitions for RTA_VIA and 'struct rtvia'. This is causing build failures on older systems: lib/netlink.c: In function 'min_attr_len': lib/netlink.c:832:38: error: invalid application of 'sizeof' to incomplete type 'struct rtvia' case NL_A_RTA_VIA: return sizeof(struct rtvia) + sizeof(struct in_addr); ^ The code structure overall is not great, we should not include Linux specific headers in the common netlink.c. The only reason for the inclusion though is the structure size. We can just put a number instead, as it is already done for all other types. It should never change, as long as kernel uAPI is stable. Structure and the RTA_VIA should be defined in the Linux-specific route-table.c, in case rtnetlink.h doesn't have a definition for them. Fixes: 9d9a99d157f2 ("route-table: Support parsing RTA_VIA attribute.") Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2025-September/053800.html Reported-by: Brendan Doyle <brendan.doyle@oracle.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- acinclude.m4 | 7 +++++++ lib/netlink.c | 14 ++------------ lib/route-table.c | 9 +++++++++ 3 files changed, 18 insertions(+), 12 deletions(-)