Hi Morten,
Morten Brørup, Oct 06, 2024 at 11:02:
Personally, I would prefer following the convention of rte_ether functions to
return boolean (as int)...
static inline int
rte_is_<zero/unicast/multicast/broadcast/universal/local_admin/valid_assigned>_ether_addr(const
struct rte_ether_addr *ea)
Sorry, I haven't followed your recommendation in v3, but I have a good
reason :)
I find that functions following that naming scheme are impossible to
find since they all start with the same "rte_is_" prefix regardless of
the DPDK module in which they are defined. It it is particularly
annoying when using code completion with clangd or similar tools.
rte_is_power_of_2 <rte_bitops.h>
rte_is_aligned <rte_common.h>
rte_is_same_ether_addr <rte_ether.h>
rte_is_zero_ether_addr <rte_ether.h>
rte_is_unicast_ether_addr <rte_ether.h>
rte_is_multicast_ether_addr <rte_ether.h>
rte_is_broadcast_ether_addr <rte_ether.h>
rte_is_universal_ether_addr <rte_ether.h>
rte_is_local_admin_ether_addr <rte_ether.h>
rte_is_valid_assigned_ether_addr <rte_ether.h>
The ethernet address functions are even more extreme as they end all
with the same suffix:
rte_<action>_<namespace>_<object>
All other public API seems to follow the inverse logic:
rte_<namespace>_<object>_<action>
It does not follow the natural English language but more of an inverse
polish notation. However, it feels more user friendly and better
discoverable.
rte_ipv6_addr_eq
rte_ipv6_addr_eq_prefix
rte_ipv6_addr_is_linklocal
rte_ipv6_addr_is_loopback
rte_ipv6_addr_is_mcast
rte_ipv6_addr_is_sitelocal
rte_ipv6_addr_is_unspec
rte_ipv6_addr_is_v4compat
rte_ipv6_addr_is_v4mapped
rte_ipv6_addr_mask
rte_ipv6_llocal_from_ethernet
rte_ipv6_mask_depth
rte_ipv6_mc_scope
rte_ipv6_solnode_from_addr
I hope I'm making sense :)
static inline int rte_is_ipv6_version(const struct rte_ipv6_hdr *ip)
{
return ip->vtc_flow & RTE_IPV6_VTC_FLOW_VERSION_MASK ==
RTE_IPV6_VTC_FLOW_VERSION;
}
Or, at your preference, an optimized variant:
static inline int rte_is_version_ipv6(const struct rte_ipv6_hdr *ip)
{
return *(const unsigned char *)ip & 0xf0 == 0x60;
}
The first nibble is also used for version in IPv4, so an IPv4 variant would
look similar:
static inline int rte_is_version_ipv4(const struct rte_ip_hdr *ip)
{
return *(const unsigned char *)ip & 0xf0 == 0x40;
}
I had missed this in ipv4. I could rework it for v4 if there are other
remarks.
Thanks for the review!
