frr: fix bundled build on AlmaLinux 9

[hcaldicott]

The bundled FRR dplane plugin does not compile on AlmaLinux 9: the
glibc shipped with el9 has no strlcpy()/strlcat(), and FRR's own
fallback declarations stay hidden because the plugin sources were built
without HAVE_CONFIG_H, so every use fails with an implicit function
declaration error. Define HAVE_CONFIG_H in the plugin compile args and
include lib/zebra.h first in each plugin source so FRR's configuration
and compat layer is loaded before any other header.
zebra_dplane_grout.c also gains an explicit linux/rtnetlink.h
include for the RT_TABLE_* definitions it relies on.

The bundled FRR build also invoked make -j with no job limit, which
starts one compiler per translation unit and OOMs memory-constrained
builders such as RPM build chroots and small VMs. The job count now
comes from the JOBS environment variable, defaulting to 1; set e.g.
JOBS=$(nproc) to keep the previous parallelism on machines that can
afford it.

Found while packaging grout for AlmaLinux 9 as part of the EVPN
multihoming work (#698).

Testing

• Without the fix (vanilla main on AlmaLinux 9, -Dfrr=enabled): the
  plugin fails to compile with implicit declarations of
  strlcpy/strlcat.
• With the fix: the full build including dplane_grout.so succeeds in
  the same environment (AlmaLinux 9 container, arm64), and the unit
  suites still pass.

Related: #698

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[hcaldicott]

A little more context below.

[hcaldicott]

make -j with no number means "spawn a job for every ready target simultaneously, with no cap."

For a codebase FRR's size that can mean hundreds of concurrent compiler processes, each holding maybe 100–300MB. A 64GB workstation absorbs that, but I noticed massive issues when running this in docker/open build server.

Arguably this can be ommitted, and I am open to feedback, but I wonder if allowing some control here is useful?

[rjarry]

I have no objections to this.

[hcaldicott]

zebra_dplane_grout.c uses RT_TABLE_* constants and had been getting them transitively; with the include order changed it needs to say so explicitly.

[rjarry]

frr/if_grout.c:4:1: error: code should be clang-formatted [-Wclang-format-violations]
#include <lib/zebra.h>
^

You'll need to add // clang-format off/on comments. There are other places where you can take examples.

[hcaldicott]

Oops - I've opened a can of worms with the CI here.

Defining HAVE_CONFIG_H exposed a latent Grout bug. The replay-marker code used the compile-time RT_TABLE_MAIN constant, which becomes the correct Linux value 254 once FRR’s configuration is loaded. However, Grout remaps rt_table_main_id and the default VRF to table 0. The marker was consequently injected into table 254 while the poller looked in table 0, so all the FRR restart/resync tests timed out.

That part should be fixed in Grout regardless: marker injection, lookup and cleanup should consistently use the default VRF’s effective table ID rather than RT_TABLE_MAIN?

There is also a separate system-FRR packaging issue. Installed lib/zebra.h is used without HAVE_CONFIG_H, so it defines its fallback RT_TABLE_MAIN macro as 0; subsequently including linux/rtnetlink.h then fails when the kernel header declares the corresponding enum. The new include ordering also needs clang-format guards.

Given that config.h is an internal generated FRR header and is not reliably available to external plugin builds, I think the safer fix for the original AlmaLinux issue is:

• fix the latent replay-marker table bug separately;
• use a small Grout compatibility header containing only the missing strlcpy()/strlcat() declarations;
• retain the bounded make -j${JOBS:-1} change;
• drop HAVE_CONFIG_H and the new lib/zebra.h includes from this patch.

The compatibility header only supplies declarations; the implementation still comes from glibc where available or libfrr on older glibc.

Thoughts?

[rjarry]

Given that config.h is an internal generated FRR header and is not reliably available to external plugin builds, I think the safer fix for the original AlmaLinux issue is:

• fix the latent replay-marker table bug separately;
• use a small Grout compatibility header containing only the missing strlcpy()/strlcat() declarations;
• retain the bounded make -j${JOBS:-1} change;
• drop HAVE_CONFIG_H and the new lib/zebra.h includes from this patch.

The compatibility header only supplies declarations; the implementation still comes from glibc where available or libfrr on older glibc.

Thoughts?

I guess we should do this yes. But that raises a concern about how FRR headers are shipped. If RT_TABLE_MAIN can take different values depending on some HAVE_CONFIG_H compile define, that is concerning...

Maybe something could be fixed in FRR upstream?