> From: Robin Jarry [mailto:rja...@redhat.com] > Sent: Friday, 15 November 2024 14.02 > > Morten Brørup, Nov 14, 2024 at 15:35: > >> RTE_IPV4 is only useful to define addresses in unit tests. > > > > There are plenty of special IP addresses and subnets, where a > shortcut > > macro makes the address easier readable in the code. > > OK, let me reformulate. I didn't mean to say that RTE_IPV4 is useless. > But it will always generate addresses in *host order*. Which means they > cannot be used in IPv4 headers without passing them through htonl(). > This is weird in my opinion.
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). > > >> Why would control plane use a different representation of addresses > >> compared to data plane? > > > > Excellent question. > > Old habit? Growing up using big endian CPUs, we have come to think of > > IPv4 addresses as 32 bit numbers, so we keep treating them as such. > > With this old way of thinking, the only reason to use network endian > > in the fast path with little endian CPUs is for performance reasons > > (to save the byte swap) - if not, we would still prefer using host > > endian in the fast path too. > > I understand the implementation reasons why you would prefer working > with host order integers. But the APIs that deal with IPv4 addresses > should not reflect implementation details. They were probably designed based on the same way of thinking I was used to (until you convinced me I was wrong). > > >> Also for consistency with IPv6, I really think > >> that *all* addresses should be dealt in their network form. > > > > Food for thought! > > 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)! > > 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). > > Thanks!
