diff mbox series

net: add missing endianness annotations

Message ID 1569585482-24513-1-git-send-email-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series net: add missing endianness annotations |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/iol-dpdk_compile_ovs success Compile Testing PASS
ci/iol-dpdk_compile success Compile Testing PASS
ci/iol-dpdk_compile_spdk success Compile Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

David Marchand Sept. 27, 2019, 11:58 a.m. UTC 
  OVS currently maintains a copy of those headers with the right endianness
annotations so that sparse checks can pass.

We introduced rte_beXX_t for better readibility in v17.08.
Let's make use of them, OVS then only needs to override those rte_beXX_t
types by exposing a tweaked rte_byteorder.h header.

Other existing dpdk users won't be affected since rte_beXX_t types are
mapped to uintXX_t types.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/librte_net/rte_icmp.h | 12 +++++++-----
 lib/librte_net/rte_ip.h   | 28 ++++++++++++++--------------
 lib/librte_net/rte_sctp.h | 10 ++++++----
 lib/librte_net/rte_tcp.h  | 20 +++++++++++---------
 lib/librte_net/rte_udp.h  | 10 ++++++----
 5 files changed, 44 insertions(+), 36 deletions(-)

Comments

Olivier Matz Oct. 18, 2019, 8:43 a.m. UTC | #1 
Hi David,

2lOn Fri, Sep 27, 2019 at 01:58:02PM +0200, David Marchand wrote:
> OVS currently maintains a copy of those headers with the right endianness
> annotations so that sparse checks can pass.
> 
> We introduced rte_beXX_t for better readibility in v17.08.
> Let's make use of them, OVS then only needs to override those rte_beXX_t
> types by exposing a tweaked rte_byteorder.h header.
> 
> Other existing dpdk users won't be affected since rte_beXX_t types are
> mapped to uintXX_t types.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  lib/librte_net/rte_icmp.h | 12 +++++++-----
>  lib/librte_net/rte_ip.h   | 28 ++++++++++++++--------------
>  lib/librte_net/rte_sctp.h | 10 ++++++----
>  lib/librte_net/rte_tcp.h  | 20 +++++++++++---------
>  lib/librte_net/rte_udp.h  | 10 ++++++----
>  5 files changed, 44 insertions(+), 36 deletions(-)

Is there a reason why you didn't change arp, gre, ... ?
David Marchand Oct. 18, 2019, 8:47 a.m. UTC | #2 
On Fri, Oct 18, 2019 at 10:43 AM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> Hi David,
>
> 2lOn Fri, Sep 27, 2019 at 01:58:02PM +0200, David Marchand wrote:
> > OVS currently maintains a copy of those headers with the right endianness
> > annotations so that sparse checks can pass.
> >
> > We introduced rte_beXX_t for better readibility in v17.08.
> > Let's make use of them, OVS then only needs to override those rte_beXX_t
> > types by exposing a tweaked rte_byteorder.h header.
> >
> > Other existing dpdk users won't be affected since rte_beXX_t types are
> > mapped to uintXX_t types.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  lib/librte_net/rte_icmp.h | 12 +++++++-----
> >  lib/librte_net/rte_ip.h   | 28 ++++++++++++++--------------
> >  lib/librte_net/rte_sctp.h | 10 ++++++----
> >  lib/librte_net/rte_tcp.h  | 20 +++++++++++---------
> >  lib/librte_net/rte_udp.h  | 10 ++++++----
> >  5 files changed, 44 insertions(+), 36 deletions(-)
>
> Is there a reason why you didn't change arp, gre, ... ?

Let me look at the other headers, I had focused on what OVS used.

Is this current patch ok?
Olivier Matz Oct. 18, 2019, 9:23 a.m. UTC | #3 
On Fri, Oct 18, 2019 at 10:47:37AM +0200, David Marchand wrote:
> On Fri, Oct 18, 2019 at 10:43 AM Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > Hi David,
> >
> > 2lOn Fri, Sep 27, 2019 at 01:58:02PM +0200, David Marchand wrote:
> > > OVS currently maintains a copy of those headers with the right endianness
> > > annotations so that sparse checks can pass.
> > >
> > > We introduced rte_beXX_t for better readibility in v17.08.
> > > Let's make use of them, OVS then only needs to override those rte_beXX_t
> > > types by exposing a tweaked rte_byteorder.h header.
> > >
> > > Other existing dpdk users won't be affected since rte_beXX_t types are
> > > mapped to uintXX_t types.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > >  lib/librte_net/rte_icmp.h | 12 +++++++-----
> > >  lib/librte_net/rte_ip.h   | 28 ++++++++++++++--------------
> > >  lib/librte_net/rte_sctp.h | 10 ++++++----
> > >  lib/librte_net/rte_tcp.h  | 20 +++++++++++---------
> > >  lib/librte_net/rte_udp.h  | 10 ++++++----
> > >  5 files changed, 44 insertions(+), 36 deletions(-)
> >
> > Is there a reason why you didn't change arp, gre, ... ?
> 
> Let me look at the other headers, I had focused on what OVS used.
> 
> Is this current patch ok?

Yes, it looks good to me.
Ferruh Yigit Oct. 18, 2019, 5 p.m. UTC | #4 
On 10/18/2019 10:23 AM, Olivier Matz wrote:
> On Fri, Oct 18, 2019 at 10:47:37AM +0200, David Marchand wrote:
>> On Fri, Oct 18, 2019 at 10:43 AM Olivier Matz <olivier.matz@6wind.com> wrote:
>>>
>>> Hi David,
>>>
>>> 2lOn Fri, Sep 27, 2019 at 01:58:02PM +0200, David Marchand wrote:
>>>> OVS currently maintains a copy of those headers with the right endianness
>>>> annotations so that sparse checks can pass.
>>>>
>>>> We introduced rte_beXX_t for better readibility in v17.08.
>>>> Let's make use of them, OVS then only needs to override those rte_beXX_t
>>>> types by exposing a tweaked rte_byteorder.h header.
>>>>
>>>> Other existing dpdk users won't be affected since rte_beXX_t types are
>>>> mapped to uintXX_t types.
>>>>
>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>> ---
>>>>  lib/librte_net/rte_icmp.h | 12 +++++++-----
>>>>  lib/librte_net/rte_ip.h   | 28 ++++++++++++++--------------
>>>>  lib/librte_net/rte_sctp.h | 10 ++++++----
>>>>  lib/librte_net/rte_tcp.h  | 20 +++++++++++---------
>>>>  lib/librte_net/rte_udp.h  | 10 ++++++----
>>>>  5 files changed, 44 insertions(+), 36 deletions(-)
>>>
>>> Is there a reason why you didn't change arp, gre, ... ?
>>
>> Let me look at the other headers, I had focused on what OVS used.
>>
>> Is this current patch ok?
> 
> Yes, it looks good to me.
> 

Converting to an explicit ack J
Acked-by: Olivier Matz <olivier.matz@6wind.com>
Ferruh Yigit Oct. 18, 2019, 5:08 p.m. UTC | #5 
On 10/18/2019 6:00 PM, Ferruh Yigit wrote:
> On 10/18/2019 10:23 AM, Olivier Matz wrote:
>> On Fri, Oct 18, 2019 at 10:47:37AM +0200, David Marchand wrote:
>>> On Fri, Oct 18, 2019 at 10:43 AM Olivier Matz <olivier.matz@6wind.com> wrote:
>>>>
>>>> Hi David,
>>>>
>>>> 2lOn Fri, Sep 27, 2019 at 01:58:02PM +0200, David Marchand wrote:
>>>>> OVS currently maintains a copy of those headers with the right endianness
>>>>> annotations so that sparse checks can pass.
>>>>>
>>>>> We introduced rte_beXX_t for better readibility in v17.08.
>>>>> Let's make use of them, OVS then only needs to override those rte_beXX_t
>>>>> types by exposing a tweaked rte_byteorder.h header.
>>>>>
>>>>> Other existing dpdk users won't be affected since rte_beXX_t types are
>>>>> mapped to uintXX_t types.
>>>>>
>>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>>> ---
>>>>>  lib/librte_net/rte_icmp.h | 12 +++++++-----
>>>>>  lib/librte_net/rte_ip.h   | 28 ++++++++++++++--------------
>>>>>  lib/librte_net/rte_sctp.h | 10 ++++++----
>>>>>  lib/librte_net/rte_tcp.h  | 20 +++++++++++---------
>>>>>  lib/librte_net/rte_udp.h  | 10 ++++++----
>>>>>  5 files changed, 44 insertions(+), 36 deletions(-)
>>>>
>>>> Is there a reason why you didn't change arp, gre, ... ?
>>>
>>> Let me look at the other headers, I had focused on what OVS used.
>>>
>>> Is this current patch ok?
>>
>> Yes, it looks good to me.
>>
> 
> Converting to an explicit ack J
> Acked-by: Olivier Matz <olivier.matz@6wind.com>
> 

Applied to dpdk-next-net/master, thanks.
diff mbox series

Patch

diff --git a/lib/librte_net/rte_icmp.h b/lib/librte_net/rte_icmp.h
index 3f8100a..e0aeed4 100644
--- a/lib/librte_net/rte_icmp.h
+++ b/lib/librte_net/rte_icmp.h
@@ -16,6 +16,8 @@ 
 
 #include <stdint.h>
 
+#include <rte_byteorder.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -24,11 +26,11 @@  extern "C" {
  * ICMP Header
  */
 struct rte_icmp_hdr {
-	uint8_t  icmp_type;   /* ICMP packet type. */
-	uint8_t  icmp_code;   /* ICMP packet code. */
-	uint16_t icmp_cksum;  /* ICMP packet checksum. */
-	uint16_t icmp_ident;  /* ICMP packet identifier. */
-	uint16_t icmp_seq_nb; /* ICMP packet sequence number. */
+	uint8_t  icmp_type;     /* ICMP packet type. */
+	uint8_t  icmp_code;     /* ICMP packet code. */
+	rte_be16_t icmp_cksum;  /* ICMP packet checksum. */
+	rte_be16_t icmp_ident;  /* ICMP packet identifier. */
+	rte_be16_t icmp_seq_nb; /* ICMP packet sequence number. */
 } __attribute__((__packed__));
 
 /* ICMP packet types */
diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index f82454b..731ee4f 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -33,14 +33,14 @@  extern "C" {
 struct rte_ipv4_hdr {
 	uint8_t  version_ihl;		/**< version and header length */
 	uint8_t  type_of_service;	/**< type of service */
-	uint16_t total_length;		/**< length of packet */
-	uint16_t packet_id;		/**< packet ID */
-	uint16_t fragment_offset;	/**< fragmentation offset */
+	rte_be16_t total_length;	/**< length of packet */
+	rte_be16_t packet_id;		/**< packet ID */
+	rte_be16_t fragment_offset;	/**< fragmentation offset */
 	uint8_t  time_to_live;		/**< time to live */
 	uint8_t  next_proto_id;		/**< protocol ID */
-	uint16_t hdr_checksum;		/**< header checksum */
-	uint32_t src_addr;		/**< source address */
-	uint32_t dst_addr;		/**< destination address */
+	rte_be16_t hdr_checksum;	/**< header checksum */
+	rte_be32_t src_addr;		/**< source address */
+	rte_be32_t dst_addr;		/**< destination address */
 } __attribute__((__packed__));
 
 /** Create IPv4 address */
@@ -354,12 +354,12 @@  rte_ipv4_udptcp_cksum(const struct rte_ipv4_hdr *ipv4_hdr, const void *l4_hdr)
  * IPv6 Header
  */
 struct rte_ipv6_hdr {
-	uint32_t vtc_flow;     /**< IP version, traffic class & flow label. */
-	uint16_t payload_len;  /**< IP packet length - includes sizeof(ip_header). */
-	uint8_t  proto;        /**< Protocol, next header. */
-	uint8_t  hop_limits;   /**< Hop limits. */
-	uint8_t  src_addr[16]; /**< IP address of source host. */
-	uint8_t  dst_addr[16]; /**< IP address of destination host(s). */
+	rte_be32_t vtc_flow;	/**< IP version, traffic class & flow label. */
+	rte_be16_t payload_len;	/**< IP packet length - includes header size */
+	uint8_t  proto;		/**< Protocol, next header. */
+	uint8_t  hop_limits;	/**< Hop limits. */
+	uint8_t  src_addr[16];	/**< IP address of source host. */
+	uint8_t  dst_addr[16];	/**< IP address of destination host(s). */
 } __attribute__((__packed__));
 
 /* IPv6 vtc_flow: IPv / TC / flow_label */
@@ -392,8 +392,8 @@  rte_ipv6_phdr_cksum(const struct rte_ipv6_hdr *ipv6_hdr, uint64_t ol_flags)
 {
 	uint32_t sum;
 	struct {
-		uint32_t len;   /* L4 length. */
-		uint32_t proto; /* L4 protocol - top 3 bytes must be zero */
+		rte_be32_t len;   /* L4 length. */
+		rte_be32_t proto; /* L4 protocol - top 3 bytes must be zero */
 	} psd_hdr;
 
 	psd_hdr.proto = (uint32_t)(ipv6_hdr->proto << 24);
diff --git a/lib/librte_net/rte_sctp.h b/lib/librte_net/rte_sctp.h
index b3661b0..ab38be7 100644
--- a/lib/librte_net/rte_sctp.h
+++ b/lib/librte_net/rte_sctp.h
@@ -20,14 +20,16 @@  extern "C" {
 
 #include <stdint.h>
 
+#include <rte_byteorder.h>
+
 /**
  * SCTP Header
  */
 struct rte_sctp_hdr {
-	uint16_t src_port; /**< Source port. */
-	uint16_t dst_port; /**< Destin port. */
-	uint32_t tag;      /**< Validation tag. */
-	uint32_t cksum;    /**< Checksum. */
+	rte_be16_t src_port; /**< Source port. */
+	rte_be16_t dst_port; /**< Destin port. */
+	rte_be32_t tag;      /**< Validation tag. */
+	rte_be32_t cksum;    /**< Checksum. */
 } __attribute__((__packed__));
 
 #ifdef __cplusplus
diff --git a/lib/librte_net/rte_tcp.h b/lib/librte_net/rte_tcp.h
index 7d649a2..06f623d 100644
--- a/lib/librte_net/rte_tcp.h
+++ b/lib/librte_net/rte_tcp.h
@@ -16,6 +16,8 @@ 
 
 #include <stdint.h>
 
+#include <rte_byteorder.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -24,15 +26,15 @@  extern "C" {
  * TCP Header
  */
 struct rte_tcp_hdr {
-	uint16_t src_port;  /**< TCP source port. */
-	uint16_t dst_port;  /**< TCP destination port. */
-	uint32_t sent_seq;  /**< TX data sequence number. */
-	uint32_t recv_ack;  /**< RX data acknowledgement sequence number. */
-	uint8_t  data_off;  /**< Data offset. */
-	uint8_t  tcp_flags; /**< TCP flags */
-	uint16_t rx_win;    /**< RX flow control window. */
-	uint16_t cksum;     /**< TCP checksum. */
-	uint16_t tcp_urp;   /**< TCP urgent pointer, if any. */
+	rte_be16_t src_port; /**< TCP source port. */
+	rte_be16_t dst_port; /**< TCP destination port. */
+	rte_be32_t sent_seq; /**< TX data sequence number. */
+	rte_be32_t recv_ack; /**< RX data acknowledgment sequence number. */
+	uint8_t  data_off;   /**< Data offset. */
+	uint8_t  tcp_flags;  /**< TCP flags */
+	rte_be16_t rx_win;   /**< RX flow control window. */
+	rte_be16_t cksum;    /**< TCP checksum. */
+	rte_be16_t tcp_urp;  /**< TCP urgent pointer, if any. */
 } __attribute__((__packed__));
 
 /**
diff --git a/lib/librte_net/rte_udp.h b/lib/librte_net/rte_udp.h
index 1c3437c..01c26b3 100644
--- a/lib/librte_net/rte_udp.h
+++ b/lib/librte_net/rte_udp.h
@@ -16,6 +16,8 @@ 
 
 #include <stdint.h>
 
+#include <rte_byteorder.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -24,10 +26,10 @@  extern "C" {
  * UDP Header
  */
 struct rte_udp_hdr {
-	uint16_t src_port;    /**< UDP source port. */
-	uint16_t dst_port;    /**< UDP destination port. */
-	uint16_t dgram_len;   /**< UDP datagram length */
-	uint16_t dgram_cksum; /**< UDP datagram checksum */
+	rte_be16_t src_port;    /**< UDP source port. */
+	rte_be16_t dst_port;    /**< UDP destination port. */
+	rte_be16_t dgram_len;   /**< UDP datagram length */
+	rte_be16_t dgram_cksum; /**< UDP datagram checksum */
 } __attribute__((__packed__));
 
 #ifdef __cplusplus