Morten Brørup, Nov 15, 2024 at 14:52:
Robin, you've totally won me over on this endian discussion. :-)
Especially the IPv6 comparison make it clear why IPv4 should also be
network byte order.
API/ABI stability is a pain... we're stuck with host endian IPv4
addresses; e.g. for the RTE_IPV4() macro, which I now agree produces
the wrong endian value (on little endian CPUs).
At least for 24.11 it is too late. But maybe we could make it right for
the next LTS?
Vladimir, could we at least consider adding a real network order mode
for the rib and fib libraries? So that we can have consistent APIs
between IPv4 and IPv6?
And/or rename RTE_FIB_F_NETWORK_ORDER to
RTE_FIB_F_NETWORK_ORDER_LOOKUP or similar. This is important if real
network order mode is added (now or later)!
Maybe we could revert that patch and defer a complete change of the
rib/fib APIs to only expose network order addresses? It would be an ABI
breakage but if properly announced in advance, it should be possible.
Thinking about it some more. Having a flag for such a drastic change in
behaviour does not seem right.
On that same topic, I wonder if it would make sense to change the API
parameters to use an opaque rte_ipv4_addr_t type instead of a native
uint32_t to avoid any confusion.
It could be considered an IPv4 address type (like the IPv6 address
type) (which should be in network endian), which it is not, so I don't
like this idea.
What the API really should offer is a choice (or a union) of uint32_t
and rte_be32_t, but that's not possible, so also using uint32_t for
big endian values seems like a viable compromise.
Another alternative, using void* for the IPv4 address array, seems
overkill to me, since compilers don't warn about mixing uint32_t with
rte_be32_t values (like mixing signed and unsigned emits warnings).
If what I proposed above is possible, then all these APIs could be using
rte_be32_t values (or even better, an rte_ipv4_addr_t alias for
consistency with IPv6). That would make everything much simpler.
Thoughts?
