[dpdk-dev] [PATCH] mbuf:rearrange mbuf to be more mbuf chain friendly

[dpdk-dev] [PATCH] mbuf:rearrange mbuf to be more mbuf chain friendly

[Keith Wiles]

Move the next pointer to the first cacheline of the rte_mbuf structure
and move the offload values to the second cacheline to give better
performance to applications using chained mbufs.

Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
is set to No.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 config/common_base                                 |  2 +
 .../linuxapp/eal/include/exec-env/rte_kni_common.h |  8 +++
 lib/librte_mbuf/rte_mbuf.h                         | 67 +++++++++++++++-------
 3 files changed, 56 insertions(+), 21 deletions(-)

diff --git a/config/common_base b/config/common_base
index 3a04fba..bdde2e7 100644
--- a/config/common_base
+++ b/config/common_base
@@ -402,6 +402,8 @@ CONFIG_RTE_LIBRTE_MBUF=y
 CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
+# Set to y if needing to be mbuf chain friendly.
+CONFIG_RTE_MBUF_CHAIN_FRIENDLY=n
 
 #
 # Compile librte_timer
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 2acdfd9..44d65cd 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -120,11 +120,19 @@ struct rte_kni_mbuf {
 	char pad2[4];
 	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len. */
 	uint16_t data_len;      /**< Amount of data in segment buffer. */
+#ifdef RTE_MBUF_CHAIN_FRIENDLY
+	char pad3[8];
+	void *next;
 
 	/* fields on second cache line */
+	char pad4[16] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
+	void *pool;
+#else
+	/* fields on second cache line */
 	char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
 	void *pool;
 	void *next;
+#endif
 };
 
 /*
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 8798c41..d02ca28 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -758,6 +758,28 @@ typedef uint8_t  MARKER8[0];  /**< generic marker with 1B alignment */
 typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
                                * with a single assignment */
 
+typedef union {
+	uint32_t rss;     /**< RSS hash result if RSS enabled */
+	struct {
+		union {
+			struct {
+				uint16_t hash;
+				uint16_t id;
+			};
+			uint32_t lo;
+			/**< Second 4 flexible bytes */
+		};
+		uint32_t hi;
+		/**< First 4 flexible bytes or FD ID, dependent on
+			PKT_RX_FDIR_* flag in ol_flags. */
+	} fdir;           /**< Filter identifier if FDIR enabled */
+	struct {
+		uint32_t lo;
+		uint32_t hi;
+	} sched;          /**< Hierarchical scheduler */
+	uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
+} rss_hash_t;
+
 /**
  * The generic rte_mbuf, containing a packet mbuf.
  */
@@ -817,28 +839,31 @@ struct rte_mbuf {
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
 	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
 	uint16_t vlan_tci;
+#ifdef RTE_MBUF_CHAIN_FRIENDLY
+	/*
+	 * Move offload into the second cache line and next in the first.
+	 * Better performance for applications using chained mbufs to have
+	 * the next pointer in the first cache line.
+	 * If you change this structure, you must change the user-mode
+	 * version in rte_kni_common.h to match the new layout.
+	 */
+	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
+	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
+	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
+
+	/* second cache line - fields only used in slow path or on TX */
+	MARKER cacheline1 __rte_cache_min_aligned;
+
+	rss_hash_t hash;      /**< hash information */
 
 	union {
-		uint32_t rss;     /**< RSS hash result if RSS enabled */
-		struct {
-			union {
-				struct {
-					uint16_t hash;
-					uint16_t id;
-				};
-				uint32_t lo;
-				/**< Second 4 flexible bytes */
-			};
-			uint32_t hi;
-			/**< First 4 flexible bytes or FD ID, dependent on
-			     PKT_RX_FDIR_* flag in ol_flags. */
-		} fdir;           /**< Filter identifier if FDIR enabled */
-		struct {
-			uint32_t lo;
-			uint32_t hi;
-		} sched;          /**< Hierarchical scheduler */
-		uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
-	} hash;                   /**< hash information */
+		void *userdata;   /**< Can be used for external metadata */
+		uint64_t udata64; /**< Allow 8-byte userdata on 32-bit */
+	};
+
+	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
+#else
+	rss_hash_t hash;      /**< hash information */
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
@@ -855,7 +880,7 @@ struct rte_mbuf {
 
 	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
 	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
-
+#endif
 	/* fields to support TX offloads */
 	union {
 		uint64_t tx_offload;       /**< combined for easy fetch */
-- 
2.1.4

Re: [dpdk-dev] [PATCH] mbuf:rearrange mbuf to be more mbuf chain friendly

[Wiles, Keith]

On 6/25/16, 10:29 AM, "dev on behalf of Keith Wiles" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:

>Move the next pointer to the first cacheline of the rte_mbuf structure
>and move the offload values to the second cacheline to give better
>performance to applications using chained mbufs.
>
>Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
>is set to No.
>
>Signed-off-by: Keith Wiles <keith.wiles@intel.com>

nak thought I had based these on the current master ☹

[dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly

[Keith Wiles]

Move the next pointer to the first cacheline of the rte_mbuf structure
and move the offload values to the second cacheline to give better
performance to applications using chained mbufs.

Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
is set to No.

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 config/common_base                                 |  2 +
 .../linuxapp/eal/include/exec-env/rte_kni_common.h |  8 +++
 lib/librte_mbuf/rte_mbuf.h                         | 67 +++++++++++++++-------
 3 files changed, 56 insertions(+), 21 deletions(-)

diff --git a/config/common_base b/config/common_base
index 379a791..f7c624e 100644
--- a/config/common_base
+++ b/config/common_base
@@ -405,6 +405,8 @@ CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
 CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="ring_mp_mc"
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
+# Set to y if needing to be mbuf chain friendly.
+CONFIG_RTE_MBUF_CHAIN_FRIENDLY=n
 
 #
 # Compile librte_timer
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index 2acdfd9..44d65cd 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -120,11 +120,19 @@ struct rte_kni_mbuf {
 	char pad2[4];
 	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len. */
 	uint16_t data_len;      /**< Amount of data in segment buffer. */
+#ifdef RTE_MBUF_CHAIN_FRIENDLY
+	char pad3[8];
+	void *next;
 
 	/* fields on second cache line */
+	char pad4[16] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
+	void *pool;
+#else
+	/* fields on second cache line */
 	char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
 	void *pool;
 	void *next;
+#endif
 };
 
 /*
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 15e3a10..6e6ba0e 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -765,6 +765,28 @@ typedef uint8_t  MARKER8[0];  /**< generic marker with 1B alignment */
 typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
                                * with a single assignment */
 
+typedef union {
+	uint32_t rss;     /**< RSS hash result if RSS enabled */
+	struct {
+		union {
+			struct {
+				uint16_t hash;
+				uint16_t id;
+			};
+			uint32_t lo;
+			/**< Second 4 flexible bytes */
+		};
+		uint32_t hi;
+		/**< First 4 flexible bytes or FD ID, dependent on
+			PKT_RX_FDIR_* flag in ol_flags. */
+	} fdir;           /**< Filter identifier if FDIR enabled */
+	struct {
+		uint32_t lo;
+		uint32_t hi;
+	} sched;          /**< Hierarchical scheduler */
+	uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
+} rss_hash_t;
+
 /**
  * The generic rte_mbuf, containing a packet mbuf.
  */
@@ -824,28 +846,31 @@ struct rte_mbuf {
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
 	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
 	uint16_t vlan_tci;
+#ifdef RTE_MBUF_CHAIN_FRIENDLY
+	/*
+	 * Move offload into the second cache line and next in the first.
+	 * Better performance for applications using chained mbufs to have
+	 * the next pointer in the first cache line.
+	 * If you change this structure, you must change the user-mode
+	 * version in rte_kni_common.h to match the new layout.
+	 */
+	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
+	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
+	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
+
+	/* second cache line - fields only used in slow path or on TX */
+	MARKER cacheline1 __rte_cache_min_aligned;
+
+	rss_hash_t hash;      /**< hash information */
 
 	union {
-		uint32_t rss;     /**< RSS hash result if RSS enabled */
-		struct {
-			union {
-				struct {
-					uint16_t hash;
-					uint16_t id;
-				};
-				uint32_t lo;
-				/**< Second 4 flexible bytes */
-			};
-			uint32_t hi;
-			/**< First 4 flexible bytes or FD ID, dependent on
-			     PKT_RX_FDIR_* flag in ol_flags. */
-		} fdir;           /**< Filter identifier if FDIR enabled */
-		struct {
-			uint32_t lo;
-			uint32_t hi;
-		} sched;          /**< Hierarchical scheduler */
-		uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
-	} hash;                   /**< hash information */
+		void *userdata;   /**< Can be used for external metadata */
+		uint64_t udata64; /**< Allow 8-byte userdata on 32-bit */
+	};
+
+	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
+#else
+	rss_hash_t hash;      /**< hash information */
 
 	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
 
@@ -862,7 +887,7 @@ struct rte_mbuf {
 
 	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
 	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
-
+#endif
 	/* fields to support TX offloads */
 	union {
 		uint64_t tx_offload;       /**< combined for easy fetch */
-- 
2.8.0.GIT

Re: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
> Sent: Saturday, June 25, 2016 4:56 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly
> 
> Move the next pointer to the first cacheline of the rte_mbuf structure
> and move the offload values to the second cacheline to give better
> performance to applications using chained mbufs.
> 
> Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
> is set to No.

First, it would make ixgbe and i40e vector RX functions to work incorrectly.
Second, I don't think we can afford to allow people swap mbuf fields in the way they like.
Otherwise we'll end-up with totally unmaintainable code pretty soon.
So NACK.  

Konstantin

> 
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  config/common_base                                 |  2 +
>  .../linuxapp/eal/include/exec-env/rte_kni_common.h |  8 +++
>  lib/librte_mbuf/rte_mbuf.h                         | 67 +++++++++++++++-------
>  3 files changed, 56 insertions(+), 21 deletions(-)
> 
> diff --git a/config/common_base b/config/common_base
> index 379a791..f7c624e 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -405,6 +405,8 @@ CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
>  CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="ring_mp_mc"
>  CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
>  CONFIG_RTE_PKTMBUF_HEADROOM=128
> +# Set to y if needing to be mbuf chain friendly.
> +CONFIG_RTE_MBUF_CHAIN_FRIENDLY=n
> 
>  #
>  # Compile librte_timer
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-
> env/rte_kni_common.h
> index 2acdfd9..44d65cd 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
> @@ -120,11 +120,19 @@ struct rte_kni_mbuf {
>  	char pad2[4];
>  	uint32_t pkt_len;       /**< Total pkt len: sum of all segment data_len. */
>  	uint16_t data_len;      /**< Amount of data in segment buffer. */
> +#ifdef RTE_MBUF_CHAIN_FRIENDLY
> +	char pad3[8];
> +	void *next;
> 
>  	/* fields on second cache line */
> +	char pad4[16] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
> +	void *pool;
> +#else
> +	/* fields on second cache line */
>  	char pad3[8] __attribute__((__aligned__(RTE_CACHE_LINE_MIN_SIZE)));
>  	void *pool;
>  	void *next;
> +#endif
>  };
> 
>  /*
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 15e3a10..6e6ba0e 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -765,6 +765,28 @@ typedef uint8_t  MARKER8[0];  /**< generic marker with 1B alignment */
>  typedef uint64_t MARKER64[0]; /**< marker that allows us to overwrite 8 bytes
>                                 * with a single assignment */
> 
> +typedef union {
> +	uint32_t rss;     /**< RSS hash result if RSS enabled */
> +	struct {
> +		union {
> +			struct {
> +				uint16_t hash;
> +				uint16_t id;
> +			};
> +			uint32_t lo;
> +			/**< Second 4 flexible bytes */
> +		};
> +		uint32_t hi;
> +		/**< First 4 flexible bytes or FD ID, dependent on
> +			PKT_RX_FDIR_* flag in ol_flags. */
> +	} fdir;           /**< Filter identifier if FDIR enabled */
> +	struct {
> +		uint32_t lo;
> +		uint32_t hi;
> +	} sched;          /**< Hierarchical scheduler */
> +	uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
> +} rss_hash_t;
> +
>  /**
>   * The generic rte_mbuf, containing a packet mbuf.
>   */
> @@ -824,28 +846,31 @@ struct rte_mbuf {
>  	uint16_t data_len;        /**< Amount of data in segment buffer. */
>  	/** VLAN TCI (CPU order), valid if PKT_RX_VLAN_STRIPPED is set. */
>  	uint16_t vlan_tci;
> +#ifdef RTE_MBUF_CHAIN_FRIENDLY
> +	/*
> +	 * Move offload into the second cache line and next in the first.
> +	 * Better performance for applications using chained mbufs to have
> +	 * the next pointer in the first cache line.
> +	 * If you change this structure, you must change the user-mode
> +	 * version in rte_kni_common.h to match the new layout.
> +	 */
> +	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
> +	uint16_t vlan_tci_outer;  /**< Outer VLAN Tag Control Identifier (CPU order) */
> +	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
> +
> +	/* second cache line - fields only used in slow path or on TX */
> +	MARKER cacheline1 __rte_cache_min_aligned;
> +
> +	rss_hash_t hash;      /**< hash information */
> 
>  	union {
> -		uint32_t rss;     /**< RSS hash result if RSS enabled */
> -		struct {
> -			union {
> -				struct {
> -					uint16_t hash;
> -					uint16_t id;
> -				};
> -				uint32_t lo;
> -				/**< Second 4 flexible bytes */
> -			};
> -			uint32_t hi;
> -			/**< First 4 flexible bytes or FD ID, dependent on
> -			     PKT_RX_FDIR_* flag in ol_flags. */
> -		} fdir;           /**< Filter identifier if FDIR enabled */
> -		struct {
> -			uint32_t lo;
> -			uint32_t hi;
> -		} sched;          /**< Hierarchical scheduler */
> -		uint32_t usr;	  /**< User defined tags. See rte_distributor_process() */
> -	} hash;                   /**< hash information */
> +		void *userdata;   /**< Can be used for external metadata */
> +		uint64_t udata64; /**< Allow 8-byte userdata on 32-bit */
> +	};
> +
> +	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
> +#else
> +	rss_hash_t hash;      /**< hash information */
> 
>  	uint32_t seqn; /**< Sequence number. See also rte_reorder_insert() */
> 
> @@ -862,7 +887,7 @@ struct rte_mbuf {
> 
>  	struct rte_mempool *pool; /**< Pool from which mbuf was allocated. */
>  	struct rte_mbuf *next;    /**< Next segment of scattered packet. */
> -
> +#endif
>  	/* fields to support TX offloads */
>  	union {
>  		uint64_t tx_offload;       /**< combined for easy fetch */
> --
> 2.8.0.GIT

Re: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly

[Olivier Matz]

On 06/27/2016 10:21 AM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
>> Sent: Saturday, June 25, 2016 4:56 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly
>>
>> Move the next pointer to the first cacheline of the rte_mbuf structure
>> and move the offload values to the second cacheline to give better
>> performance to applications using chained mbufs.
>>
>> Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
>> is set to No.
> 
> First, it would make ixgbe and i40e vector RX functions to work incorrectly.
> Second, I don't think we can afford to allow people swap mbuf fields in the way they like.
> Otherwise we'll end-up with totally unmaintainable code pretty soon.
> So NACK.  

+1

Re: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly

[Thomas Monjalon]

2016-06-27 10:27, Olivier Matz:
> On 06/27/2016 10:21 AM, Ananyev, Konstantin wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
> >> Move the next pointer to the first cacheline of the rte_mbuf structure
> >> and move the offload values to the second cacheline to give better
> >> performance to applications using chained mbufs.
> >>
> >> Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
> >> is set to No.
> > 
> > First, it would make ixgbe and i40e vector RX functions to work incorrectly.
> > Second, I don't think we can afford to allow people swap mbuf fields in the way they like.
> > Otherwise we'll end-up with totally unmaintainable code pretty soon.
> > So NACK.  
> 
> +1

To be more precise, the arrangement of fields in rte_mbuf is open
to debate and changes.
There is a recent discussion here:
	http://dpdk.org/ml/archives/dev/2016-May/039483.html

I think we must try to improve few things in mbuf during the 16.11 cycle.
But it must not be allowed to have a build option to adapt this structure
or any other API. There is only one DPDK API for a given version.

Re: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly

[Wiles, Keith]

On 6/27/16, 4:05 AM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:

>2016-06-27 10:27, Olivier Matz:
>> On 06/27/2016 10:21 AM, Ananyev, Konstantin wrote:
>> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
>> >> Move the next pointer to the first cacheline of the rte_mbuf structure
>> >> and move the offload values to the second cacheline to give better
>> >> performance to applications using chained mbufs.
>> >>
>> >> Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
>> >> is set to No.
>> > 
>> > First, it would make ixgbe and i40e vector RX functions to work incorrectly.
>> > Second, I don't think we can afford to allow people swap mbuf fields in the way they like.
>> > Otherwise we'll end-up with totally unmaintainable code pretty soon.
>> > So NACK.  
>> 
>> +1
>
>To be more precise, the arrangement of fields in rte_mbuf is open
>to debate and changes.
>There is a recent discussion here:
>	http://dpdk.org/ml/archives/dev/2016-May/039483.html
>
>I think we must try to improve few things in mbuf during the 16.11 cycle.
>But it must not be allowed to have a build option to adapt this structure
>or any other API. There is only one DPDK API for a given version.

I just received a private email thread on this one and it appears it is not a big of a problem as was stated before. ☹ So yes we can reject this one.

Someone rejected these in patchwork already, which I expected I would be the one to reject the patches. Is this not the case? I understand if the patch just hangs round, but I would have expected after the list rejected the patch I would be the one to reject the patches. I try to keep up with my patches and rejecting a patch before I have a chance to do so seems a bit harsh to me.

>

Re: [dpdk-dev] [PATCH v2] mbuf:rearrange mbuf to be more mbuf chain friendly

[Thomas Monjalon]

2016-06-27 13:06, Wiles, Keith:
> 
> On 6/27/16, 4:05 AM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
> 
> >2016-06-27 10:27, Olivier Matz:
> >> On 06/27/2016 10:21 AM, Ananyev, Konstantin wrote:
> >> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Keith Wiles
> >> >> Move the next pointer to the first cacheline of the rte_mbuf structure
> >> >> and move the offload values to the second cacheline to give better
> >> >> performance to applications using chained mbufs.
> >> >>
> >> >> Enabled by a configuration option CONFIG_RTE_MBUF_CHAIN_FRIENDLY default
> >> >> is set to No.
> >> > 
> >> > First, it would make ixgbe and i40e vector RX functions to work incorrectly.
> >> > Second, I don't think we can afford to allow people swap mbuf fields in the way they like.
> >> > Otherwise we'll end-up with totally unmaintainable code pretty soon.
> >> > So NACK.  
> >> 
> >> +1
> >
> >To be more precise, the arrangement of fields in rte_mbuf is open
> >to debate and changes.
> >There is a recent discussion here:
> >	http://dpdk.org/ml/archives/dev/2016-May/039483.html
> >
> >I think we must try to improve few things in mbuf during the 16.11 cycle.
> >But it must not be allowed to have a build option to adapt this structure
> >or any other API. There is only one DPDK API for a given version.
> 
> I just received a private email thread on this one and it appears it is not a big of a problem as was stated before. ☹ So yes we can reject this one.
> 
> Someone rejected these in patchwork already, which I expected I would be the one to reject the patches. Is this not the case? I understand if the patch just hangs round, but I would have expected after the list rejected the patch I would be the one to reject the patches. I try to keep up with my patches and rejecting a patch before I have a chance to do so seems a bit harsh to me.

Yes it's me, sorry I've been quick when I've seen the first 2 comments.