Re: rte_fib network order bug

["Robin Jarry" <rjarry@redhat.com>]

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?