On 2/2/2023 10:01 AM, Andrew Rybchenko wrote:
> On 1/26/23 19:19, Ferruh Yigit wrote:
>> From: Thomas Monjalon <tho...@monjalon.net>
>>
>> Some protocols (ARP, MPLS and HIGIG2) were using uint16_t and uint32_t
>> types for their 16 and 32-bit fields.
>> It was correct but not conveying the big endian nature of these fields.
>>
>> As for other protocols defined in this directory,
>> all types are explicitly marked as big endian fields.
>>
>> Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
>
> One nit below,
>
> Acked-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
>
>> diff --git a/lib/net/rte_arp.h b/lib/net/rte_arp.h
>> index 076c8ab314ee..151e6c641fc5 100644
>> --- a/lib/net/rte_arp.h
>> +++ b/lib/net/rte_arp.h
>> @@ -23,28 +23,28 @@ extern "C" {
>> */
>> struct rte_arp_ipv4 {
>> struct rte_ether_addr arp_sha; /**< sender hardware address */
>> - uint32_t arp_sip; /**< sender IP address */
>> + rte_be32_t arp_sip; /**< sender IP address */
>> struct rte_ether_addr arp_tha; /**< target hardware address */
>> - uint32_t arp_tip; /**< target IP address */
>> + rte_be32_t arp_tip; /**< target IP address */
>> } __rte_packed __rte_aligned(2);
>> /**
>> * ARP header.
>> */
>> struct rte_arp_hdr {
>> - uint16_t arp_hardware; /* format of hardware address */
>> -#define RTE_ARP_HRD_ETHER 1 /* ARP Ethernet address format */
>> + rte_be16_t arp_hardware; /** format of hardware address */
>> +#define RTE_ARP_HRD_ETHER 1 /** ARP Ethernet address format */
>
> The comment is fixed above, but it is still wrong. It should be
> /**<. So, I'd either don't touch it at all, or fix in a right
> way. Same for all fields below.
>
ack
>> - uint16_t arp_protocol; /* format of protocol address */
>> - uint8_t arp_hlen; /* length of hardware address */
>> - uint8_t arp_plen; /* length of protocol address */
>> - uint16_t arp_opcode; /* ARP opcode (command) */
>> -#define RTE_ARP_OP_REQUEST 1 /* request to resolve address */
>> -#define RTE_ARP_OP_REPLY 2 /* response to previous request */
>> -#define RTE_ARP_OP_REVREQUEST 3 /* request proto addr given
>> hardware */
>> -#define RTE_ARP_OP_REVREPLY 4 /* response giving protocol
>> address */
>> -#define RTE_ARP_OP_INVREQUEST 8 /* request to identify peer */
>> -#define RTE_ARP_OP_INVREPLY 9 /* response identifying peer */
>> + rte_be16_t arp_protocol; /** format of protocol address */
>> + uint8_t arp_hlen; /** length of hardware address */
>> + uint8_t arp_plen; /** length of protocol address */
>> + rte_be16_t arp_opcode; /** ARP opcode (command) */
>> +#define RTE_ARP_OP_REQUEST 1 /** request to resolve address */
>> +#define RTE_ARP_OP_REPLY 2 /** response to previous request */
>> +#define RTE_ARP_OP_REVREQUEST 3 /** request proto addr given
>> hardware */
>> +#define RTE_ARP_OP_REVREPLY 4 /** response giving protocol
>> address */
>> +#define RTE_ARP_OP_INVREQUEST 8 /** request to identify peer */
>> +#define RTE_ARP_OP_INVREPLY 9 /** response identifying peer */
>> struct rte_arp_ipv4 arp_data;
>> } __rte_packed __rte_aligned(2);
>
> [snip[
