diff mbox

[dpdk-dev,01/15] eventdev: remove unneeded dependencies

Message ID 1484581255-148720-2-git-send-email-harry.van.haaren@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation fail apply patch file failure

Commit Message

Van Haaren, Harry Jan. 16, 2017, 3:40 p.m. UTC 
  From: Bruce Richardson <bruce.richardson@intel.com>

Since eventdev uses event structures rather than working directly on
mbufs, there is no actual dependencies on the mbuf library. The
inclusion of an mbuf pointer element inside the event itself does not
require the inclusion of the mbuf header file. Similarly the pci
header is not needed, but following their removal, rte_memory.h is
needed for the definition of the __rte_cache_aligned macro.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
---
 lib/librte_eventdev/Makefile       | 1 -
 lib/librte_eventdev/rte_eventdev.h | 5 +++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Jerin Jacob Jan. 17, 2017, 9:11 a.m. UTC | #1 
On Mon, Jan 16, 2017 at 03:40:41PM +0000, Harry van Haaren wrote:
> From: Bruce Richardson <bruce.richardson@intel.com>
> 
> Since eventdev uses event structures rather than working directly on
> mbufs, there is no actual dependencies on the mbuf library. The
> inclusion of an mbuf pointer element inside the event itself does not
> require the inclusion of the mbuf header file. Similarly the pci
> header is not needed, but following their removal, rte_memory.h is
> needed for the definition of the __rte_cache_aligned macro.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>
> ---
>  lib/librte_eventdev/Makefile       | 1 -
>  lib/librte_eventdev/rte_eventdev.h | 5 +++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
> index dac0663..396e5ec 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -52,6 +52,5 @@ EXPORT_MAP := rte_eventdev_version.map
>  
>  # library dependencies
>  DEPDIRS-y += lib/librte_eal
> -DEPDIRS-y += lib/librte_mbuf
>  
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index e1bd05f..c2f9310 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -244,8 +244,9 @@ extern "C" {
>  #endif
>  
>  #include <rte_common.h>
> -#include <rte_pci.h>
> -#include <rte_mbuf.h>
> +#include <rte_memory.h>
> +
> +struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mbuf.h */

This "struct rte_mbuf" reference is not present in dpdk-next-eventdev tree.
Are you planning to rebase to dpdk-next-eventdev?

>  
>  /* Event device capability bitmap flags */
>  #define RTE_EVENT_DEV_CAP_QUEUE_QOS           (1ULL << 0)
> -- 
> 2.7.4
>
Van Haaren, Harry Jan. 17, 2017, 9:59 a.m. UTC | #2 
> From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> >
> >  #include <rte_common.h>
> > -#include <rte_pci.h>
> > -#include <rte_mbuf.h>
> > +#include <rte_memory.h>
> > +
> > +struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mbuf.h */
> 
> This "struct rte_mbuf" reference is not present in dpdk-next-eventdev tree.
> Are you planning to rebase to dpdk-next-eventdev?


The idea was to remove the include of the header file, as we never dereference the mbuf pointer, and hence we shouldn't include a header we don't require.

The struct rte_mbuf here is just a forward declaration for the actual rte_mbuf. This allows the rte_event to contain a struct rte_mbuf* without the compiler complaining that it doesn't understand the type.


The current patches apply to dpdk-next-eventdev HEAD, I don't think I understand what you're asking about rebasing.
Jerin Jacob Jan. 17, 2017, 10:38 a.m. UTC | #3 
On Tue, Jan 17, 2017 at 09:59:59AM +0000, Van Haaren, Harry wrote:
> > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com]
> > >
> > >  #include <rte_common.h>
> > > -#include <rte_pci.h>
> > > -#include <rte_mbuf.h>
> > > +#include <rte_memory.h>
> > > +
> > > +struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mbuf.h */
> > 
> > This "struct rte_mbuf" reference is not present in dpdk-next-eventdev tree.
> > Are you planning to rebase to dpdk-next-eventdev?
> 
> 
> The idea was to remove the include of the header file, as we never dereference the mbuf pointer, and hence we shouldn't include a header we don't require.
> 
> The struct rte_mbuf here is just a forward declaration for the actual rte_mbuf. This allows the rte_event to contain a struct rte_mbuf* without the compiler complaining that it doesn't understand the type.
> 
> 
> The current patches apply to dpdk-next-eventdev HEAD, I don't think I understand what you're asking about rebasing.

Thanks for the clarification. It is clear now.
I got confused with following comment in the cover-letter.

This implementation is based on the previous software eventdev
RFC patchset[1], updated to integrate with the latest rte_eventdev.h
API.
[1] http://dpdk.org/ml/archives/dev/2016-November/050285.html
Jerin Jacob Jan. 21, 2017, 5:34 p.m. UTC | #4 
On Mon, Jan 16, 2017 at 03:40:41PM +0000, Harry van Haaren wrote:
> From: Bruce Richardson <bruce.richardson@intel.com>
> 
> Since eventdev uses event structures rather than working directly on
> mbufs, there is no actual dependencies on the mbuf library. The
> inclusion of an mbuf pointer element inside the event itself does not
> require the inclusion of the mbuf header file. Similarly the pci
> header is not needed, but following their removal, rte_memory.h is
> needed for the definition of the __rte_cache_aligned macro.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

> ---
>  lib/librte_eventdev/Makefile       | 1 -
>  lib/librte_eventdev/rte_eventdev.h | 5 +++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
> index dac0663..396e5ec 100644
> --- a/lib/librte_eventdev/Makefile
> +++ b/lib/librte_eventdev/Makefile
> @@ -52,6 +52,5 @@ EXPORT_MAP := rte_eventdev_version.map
>  
>  # library dependencies
>  DEPDIRS-y += lib/librte_eal
> -DEPDIRS-y += lib/librte_mbuf
>  
>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
> index e1bd05f..c2f9310 100644
> --- a/lib/librte_eventdev/rte_eventdev.h
> +++ b/lib/librte_eventdev/rte_eventdev.h
> @@ -244,8 +244,9 @@ extern "C" {
>  #endif
>  
>  #include <rte_common.h>
> -#include <rte_pci.h>
> -#include <rte_mbuf.h>
> +#include <rte_memory.h>
> +
> +struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mbuf.h */
>  
>  /* Event device capability bitmap flags */
>  #define RTE_EVENT_DEV_CAP_QUEUE_QOS           (1ULL << 0)
> -- 
> 2.7.4
>
diff mbox

Patch

diff --git a/lib/librte_eventdev/Makefile b/lib/librte_eventdev/Makefile
index dac0663..396e5ec 100644
--- a/lib/librte_eventdev/Makefile
+++ b/lib/librte_eventdev/Makefile
@@ -52,6 +52,5 @@  EXPORT_MAP := rte_eventdev_version.map
 
 # library dependencies
 DEPDIRS-y += lib/librte_eal
-DEPDIRS-y += lib/librte_mbuf
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_eventdev/rte_eventdev.h b/lib/librte_eventdev/rte_eventdev.h
index e1bd05f..c2f9310 100644
--- a/lib/librte_eventdev/rte_eventdev.h
+++ b/lib/librte_eventdev/rte_eventdev.h
@@ -244,8 +244,9 @@  extern "C" {
 #endif
 
 #include <rte_common.h>
-#include <rte_pci.h>
-#include <rte_mbuf.h>
+#include <rte_memory.h>
+
+struct rte_mbuf; /* we just use mbuf pointers; no need to include rte_mbuf.h */
 
 /* Event device capability bitmap flags */
 #define RTE_EVENT_DEV_CAP_QUEUE_QOS           (1ULL << 0)