Skip to contentSkip to conversation

reduce header dependency on rte_mbuf.h

RFC PATCH Applied Bruce Richardson 3 participants 11 messages Apr 23, 2015 – May 11, 2015 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

cover reduce header dependency on rte_mbuf.h

A large number of our header files and libraries are dependent on one another, which can lead to problems with circular dependencies if trying to tie some of those libraries together, e.g. when prototyping with pktdev, or other schemes to get a common API for ethdev/rings/KNI. :-)

One small way to reduce issues when doing this is to eliminate #includes when they are not needed. While most includes in our headers are necessary, one common pattern seen is where a library just takes mbufs as part of it's API, but does not de-reference those in the header file. In cases like this, it's not necessary to include the whole mbuf header file just to allow pointers to mbuf structures - a forward declaration of "struct rte_mbuf" will do. Including the mbuf header file, also triggers inclusion of the mempool headers which causes the inclusion of the ring headers amongst others.

Therefore, I propose changing the header files for our libraries to just use the forward declaration instead of the full header inclusion where possible.

Bruce Richardson (8):
distributor: remove header inclusion of mbuf.h
ethdev: remove inclusion of rte_mbuf.h
kni: remove header inclusion of rte_mbuf.h
ip_frag: remove header inclusion of rte_mbuf.h
pipeline: remove header inclusion of rte_mbuf.h
virtio: remove header inclusion of rte_mbuf.h
table: remove header inclusion of rte_mbuf.h
vhost: remove header inclusion of rte_mbuf.h

 app/test/test_distributor.c              | 2 ++
 app/test/test_distributor_perf.c         | 3 +++
 lib/librte_distributor/rte_distributor.h | 3 +--
 lib/librte_ether/rte_ethdev.h            | 3 ++-
 lib/librte_ip_frag/rte_ip_frag.h         | 3 ++-
 lib/librte_kni/rte_kni.h                 | 2 +-
 lib/librte_pipeline/rte_pipeline.h       | 3 ++-
 lib/librte_pmd_virtio/virtqueue.h        | 3 ++-
 lib/librte_pmd_xenvirt/virtqueue.h       | 3 ++-
 lib/librte_table/rte_table.h             | 3 ++-
 lib/librte_vhost/rte_virtio_net.h        | 3 ++-
 11 files changed, 21 insertions(+), 10 deletions(-)
Signature · 1 line
2.1.0

A message that is not in the archive (sent off-list or not yet received): 1429794237-9498-1-git-send-email-bruce.richardson-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org

#2 lore raw

RFC PATCH 1/8 distributor: remove header inclusion of mbuf.h

The distributor header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 app/test/test_distributor.c              | 2 ++
 app/test/test_distributor_perf.c         | 3 +++
 lib/librte_distributor/rte_distributor.h | 3 +--
 3 files changed, 6 insertions(+), 2 deletions(-)
app/test/test_distributor.c +2 -0
index 9e8c06d..c8c0942 100644
@@ -37,6 +37,8 @@
 #include <string.h>
 #include <rte_cycles.h>
 #include <rte_errno.h>
+#include <rte_mempool.h>
+#include <rte_mbuf.h>
 #include <rte_distributor.h>
 
 #define ITER_POWER 20 /* log 2 of how many iterations we do when timing. */
app/test/test_distributor_perf.c +3 -0
index 31431bb..234a37e 100644
@@ -35,7 +35,10 @@
 
 #include <unistd.h>
 #include <string.h>
+#include <rte_mempool.h>
 #include <rte_cycles.h>
+#include <rte_common.h>
+#include <rte_mbuf.h>
 #include <rte_distributor.h>
 
 #define ITER_POWER 20 /* log 2 of how many iterations we do when timing. */
lib/librte_distributor/rte_distributor.h +1 -2
index cc1d559..7d36bc8 100644
@@ -46,11 +46,10 @@
 extern "C" {
 #endif
 
-#include <rte_mbuf.h>
-
 #define RTE_DISTRIBUTOR_NAMESIZE 32 /**< Length of name for instance */
 
 struct rte_distributor;
+struct rte_mbuf;
 
 /**
  * Function to create a new distributor instance
Signature · 1 line
2.1.0
#3 lore raw

RFC PATCH 2/8 ethdev: remove inclusion of rte_mbuf.h

In reply to a message not in the archive

The ethdev header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_ether/rte_ethdev.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/librte_ether/rte_ethdev.h +2 -1
index 4648290..16dbe00 100644
@@ -180,10 +180,11 @@ extern "C" {
 #include <rte_pci.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
-#include <rte_mbuf.h>
 #include "rte_ether.h"
 #include "rte_eth_ctrl.h"
 
+struct rte_mbuf;
+
 /**
  * A structure used to retrieve statistics for an Ethernet port.
  */
Signature · 1 line
2.1.0
#4 lore raw

RFC PATCH 3/8 kni: remove header inclusion of rte_mbuf.h

In reply to a message not in the archive

The kni header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_kni/rte_kni.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
lib/librte_kni/rte_kni.h +1 -1
index 815b8e2..98edd72 100644
@@ -47,7 +47,6 @@
  */
 
 #include <rte_pci.h>
-#include <rte_mbuf.h>
 
 #include <exec-env/rte_kni_common.h>
 
@@ -56,6 +55,7 @@ extern "C" {
 #endif
 
 struct rte_kni;
+struct rte_mbuf;
 
 /**
  * Structure which has the function pointers for KNI interface.
Signature · 1 line
2.1.0
#5 lore raw

RFC PATCH 4/8 ip_frag: remove header inclusion of rte_mbuf.h

In reply to a message not in the archive

The ip_frag header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_ip_frag/rte_ip_frag.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/librte_ip_frag/rte_ip_frag.h +2 -1
index f673728..52f44c9 100644
@@ -49,11 +49,12 @@ extern "C" {
 #include <stdio.h>
 
 #include <rte_malloc.h>
-#include <rte_mbuf.h>
 #include <rte_memory.h>
 #include <rte_ip.h>
 #include <rte_byteorder.h>
 
+struct rte_mbuf;
+
 enum {
 	IP_LAST_FRAG_IDX,    /**< index of last fragment */
 	IP_FIRST_FRAG_IDX,   /**< index of first fragment */
Signature · 1 line
2.1.0
#6 lore raw

RFC PATCH 5/8 pipeline: remove header inclusion of rte_mbuf.h

In reply to a message not in the archive

The pipeline header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_pipeline/rte_pipeline.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/librte_pipeline/rte_pipeline.h +2 -1
index fb1014a..145fc06 100644
@@ -85,10 +85,11 @@ extern "C" {
 
 #include <stdint.h>
 
-#include <rte_mbuf.h>
 #include <rte_port.h>
 #include <rte_table.h>
 
+struct rte_mbuf;
+
 /*
  * Pipeline
  *
Signature · 1 line
2.1.0
#7 lore raw

RFC PATCH 6/8 virtio: remove header inclusion of rte_mbuf.h

In reply to a message not in the archive

The virtqueue header files include the mbuf header file, but they do not need to do so as they only use pointers to the struct rte_mbuf type, and do not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_pmd_virtio/virtqueue.h  | 3 ++-
 lib/librte_pmd_xenvirt/virtqueue.h | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)
lib/librte_pmd_virtio/virtqueue.h +2 -1
index 41dda50..9d6079e 100644
@@ -37,7 +37,6 @@
 #include <stdint.h>
 
 #include <rte_atomic.h>
-#include <rte_mbuf.h>
 #include <rte_memory.h>
 #include <rte_memzone.h>
 #include <rte_mempool.h>
@@ -46,6 +45,8 @@
 #include "virtio_ring.h"
 #include "virtio_logs.h"
 
+struct rte_mbuf;
+
 /*
  * Per virtio_config.h in Linux.
  *     For virtio_pci on SMP, we don't need to order with respect to MMIO
lib/librte_pmd_xenvirt/virtqueue.h +2 -1
index 34a24fc..eff6208 100644
@@ -42,10 +42,11 @@
 #include <rte_memory.h>
 #include <rte_memzone.h>
 #include <rte_mempool.h>
-#include <rte_mbuf.h>
 
 #include "virtio_logs.h"
 
+struct rte_mbuf;
+
 /* The alignment to use between consumer and producer parts of vring. */
 #define VIRTIO_PCI_VRING_ALIGN 4096
 
Signature · 1 line
2.1.0
#8 lore raw

RFC PATCH 7/8 table: remove header inclusion of rte_mbuf.h

In reply to a message not in the archive

The rte_table header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_table/rte_table.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/librte_table/rte_table.h +2 -1
index d57bc33..6e51fe6 100644
@@ -55,9 +55,10 @@ extern "C" {
  ***/
 
 #include <stdint.h>
-#include <rte_mbuf.h>
 #include <rte_port.h>
 
+struct rte_mbuf;
+
 /**
  * Lookup table create
  *
Signature · 1 line
2.1.0
#9 lore raw

RFC PATCH 8/8 vhost: remove header inclusion of rte_mbuf.h

In reply to a message not in the archive

The virtio_net header file includes the mbuf header file, but it does not need to do so as it only uses pointers to the struct rte_mbuf type, and does not use any of the mbuf internals, nor any of the mbuf functions or macros. Therefore the inclusion is unnecessary, and can be replaced by a forward declaration of the mbuf type.

---
 lib/librte_vhost/rte_virtio_net.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
lib/librte_vhost/rte_virtio_net.h +2 -1
index b79bd0b..5d38185 100644
@@ -48,7 +48,8 @@
 
 #include <rte_memory.h>
 #include <rte_mempool.h>
-#include <rte_mbuf.h>
+
+struct rte_mbuf;
 
 #define VHOST_MEMORY_MAX_NREGIONS 8
 
Signature · 1 line
2.1.0
Acked-by
#10 lore raw
In reply to a message not in the archive

Hi Bruce,

Quoted 14 lines · Bruce Richardson

A large number of our header files and libraries are dependent on one another, which can lead to problems with circular dependencies if trying to tie some of those libraries together, e.g. when prototyping with pktdev, or other schemes to get a common API for ethdev/rings/KNI. :-)

One small way to reduce issues when doing this is to eliminate #includes when they are not needed. While most includes in our headers are necessary, one common pattern seen is where a library just takes mbufs as part of it's API, but does not de-reference those in the header file. In cases like this, it's not necessary to include the whole mbuf header file just to allow pointers to mbuf structures - a forward declaration of "struct rte_mbuf" will do. Including the mbuf header file, also triggers inclusion of the mempool headers which causes the inclusion of the ring headers amongst others.

Quoted 2 lines

Therefore, I propose changing the header files for our libraries to just use the forward declaration instead of the full header inclusion where possible.

Series

#11 lore raw
Quoted 14 lines
Quoted 14 lines

A large number of our header files and libraries are dependent on one another, which can lead to problems with circular dependencies if trying to tie some of those libraries together, e.g. when prototyping with pktdev, or other schemes to get a common API for ethdev/rings/KNI. :-)

One small way to reduce issues when doing this is to eliminate #includes when they are not needed. While most includes in our headers are necessary, one common pattern seen is where a library just takes mbufs as part of it's API, but does not de-reference those in the header file. In cases like this, it's not necessary to include the whole mbuf header file just to allow pointers to mbuf structures - a forward declaration of "struct rte_mbuf" will do. Including the mbuf header file, also triggers inclusion of the mempool headers which causes the inclusion of the ring headers amongst others.

Quoted 5 lines
Quoted 2 lines

Therefore, I propose changing the header files for our libraries to just use the forward declaration instead of the full header inclusion where possible.

Series

Applied, thanks