[dpdk-dev] [PATCH 0/2] Removal of RTE_MBUF_REFCNT

[dpdk-dev] [PATCH 0/2] Removal of RTE_MBUF_REFCNT

[Sergio Gonzalez Monroy]

This patch tries to remove the RTE_MBUF_REFCNT config options and dependencies
by introducing a new mbuf flag IND_ATTACHED_MBUF that would indicate when the mbuf
is an indirect attached mbuf, to differentiate between indirect mbufs and mbufs
with external memory buffers (ie. vhost zero copy).

Previous discussion:
http://dpdk.org/ml/archives/dev/2014-October/007127.html

Currently for mbufs with refcnt, we cannot free mbufs with external memory
buffers (ie. vhost zero copy), as they are recognized as indirect
attached mbufs and therefore we free the direct mbuf it points to,
resulting in an error in the case of external memory buffers.

We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
that the mbuf is an indirect attached mbuf pointing to another mbuf.
When we free an mbuf, we only free the direct mbuf if the flag is set.
Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
The flag is set during attach and clear on detach.

So in the case of vhost zero copy where we have mbufs with external
buffers, by default we just free the mbuf and it is up to the user to deal with
the external buffer.

Sergio Gonzalez Monroy (2):
  mbuf: Introduce IND_ATTACHED_MBUF flag
  Remove RTE_MBUF_REFCNT references

 app/test/test_link_bonding.c            | 15 -----------
 app/test/test_mbuf.c                    | 17 +++----------
 config/common_bsdapp                    |  1 -
 config/common_linuxapp                  |  1 -
 examples/Makefile                       |  4 +--
 examples/ip_fragmentation/Makefile      |  4 ---
 examples/ip_pipeline/Makefile           |  3 ---
 examples/ip_pipeline/main.c             |  5 ----
 examples/ipv4_multicast/Makefile        |  4 ---
 examples/vhost/main.c                   | 19 +++-----------
 lib/librte_ip_frag/Makefile             |  4 ---
 lib/librte_ip_frag/rte_ip_frag.h        |  4 ---
 lib/librte_mbuf/rte_mbuf.c              |  2 --
 lib/librte_mbuf/rte_mbuf.h              | 45 +++++++--------------------------
 lib/librte_pmd_bond/Makefile            |  4 ---
 lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
 lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
 lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 --------
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 ------
 lib/librte_port/Makefile                |  4 ---
 20 files changed, 19 insertions(+), 139 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2] mbuf: Introduce IND_ATTACHED_MBUF flag

[Sergio Gonzalez Monroy]

Currently for mbufs with refcnt, we cannot free mbufs with external memory
buffers (ie. vhost zero copy), as they are recognized as indirect
attached mbufs and therefore we free the direct mbuf it points to,
resulting in an error in the case of external memory buffers.

We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
that the mbuf is an indirect attached mbuf pointing to another mbuf.
When we free an mbuf, we only free the direct mbuf if the flag is set.
Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
The flag is set during attach and clear on detach.

So in the case of vhost zero copy where we have mbufs with external
buffers, by default we just free the mbuf and it is up to the user to deal with
the external buffer.

This patch would allow the removal of the RTE_MBUF_REFCNT config option,
setting refcnt for all mbufs permanently.

The patch also modifies the vhost example as it was using the
RTE_MBUF_INDERECT macro to detect if it was an mbuf with external buffer.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 examples/vhost/main.c      |  6 ++++--
 lib/librte_mbuf/rte_mbuf.h | 15 +++++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 3a35359..5e341d6 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -139,6 +139,8 @@
 /* Number of descriptors per cacheline. */
 #define DESC_PER_CACHELINE (RTE_CACHE_LINE_SIZE / sizeof(struct vring_desc))
 
+#define MBUF_EXT_MEM(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+
 /* mask of enabled ports */
 static uint32_t enabled_port_mask = 0;
 
@@ -1567,7 +1569,7 @@ txmbuf_clean_zcp(struct virtio_net *dev, struct vpool *vpool)
 
 	for (index = 0; index < mbuf_count; index++) {
 		mbuf = __rte_mbuf_raw_alloc(vpool->pool);
-		if (likely(RTE_MBUF_INDIRECT(mbuf)))
+		if (likely(MBUF_EXT_MEM(mbuf)))
 			pktmbuf_detach_zcp(mbuf);
 		rte_ring_sp_enqueue(vpool->ring, mbuf);
 
@@ -1630,7 +1632,7 @@ static void mbuf_destroy_zcp(struct vpool *vpool)
 	for (index = 0; index < mbuf_count; index++) {
 		mbuf = __rte_mbuf_raw_alloc(vpool->pool);
 		if (likely(mbuf != NULL)) {
-			if (likely(RTE_MBUF_INDIRECT(mbuf)))
+			if (likely(MBUF_EXT_MEM(mbuf)))
 				pktmbuf_detach_zcp(mbuf);
 			rte_ring_sp_enqueue(vpool->ring, (void *)mbuf);
 		}
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 16059c6..12e7545 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -162,6 +162,8 @@ extern "C" {
 /** Tell the NIC it's an outer IPv6 packet for tunneling packet */
 #define PKT_TX_OUTER_IPV6    (1ULL << 60)
 
+#define IND_ATTACHED_MBUF    (1ULL << 62) /**< Indirect attached mbuf */
+
 /* Use final bit of flags to indicate a control mbuf */
 #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */
 
@@ -305,13 +307,12 @@ struct rte_mbuf {
 /**
  * Returns TRUE if given mbuf is indirect, or FALSE otherwise.
  */
-#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
 
 /**
  * Returns TRUE if given mbuf is direct, or FALSE otherwise.
  */
-#define RTE_MBUF_DIRECT(mb)     (RTE_MBUF_FROM_BADDR((mb)->buf_addr) == (mb))
-
+#define RTE_MBUF_DIRECT(mb)     (!RTE_MBUF_INDIRECT(mb))
 
 /**
  * Private data in case of pktmbuf pool.
@@ -713,7 +714,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *md)
 	mi->next = NULL;
 	mi->pkt_len = mi->data_len;
 	mi->nb_segs = 1;
-	mi->ol_flags = md->ol_flags;
+	mi->ol_flags = md->ol_flags | IND_ATTACHED_MBUF;
 	mi->packet_type = md->packet_type;
 
 	__rte_mbuf_sanity_check(mi, 1);
@@ -744,6 +745,8 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
 			RTE_PKTMBUF_HEADROOM : m->buf_len;
 
 	m->data_len = 0;
+
+	m->ol_flags = 0;
 }
 
 #endif /* RTE_MBUF_REFCNT */
@@ -757,7 +760,6 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 #ifdef RTE_MBUF_REFCNT
 	if (likely (rte_mbuf_refcnt_read(m) == 1) ||
 			likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
-		struct rte_mbuf *md = RTE_MBUF_FROM_BADDR(m->buf_addr);
 
 		rte_mbuf_refcnt_set(m, 0);
 
@@ -765,7 +767,8 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 		 *  - detach mbuf
 		 *  - free attached mbuf segment
 		 */
-		if (unlikely (md != m)) {
+		if (RTE_MBUF_INDIRECT(m)) {
+			struct rte_mbuf *md = RTE_MBUF_FROM_BADDR(m->buf_addr);
 			rte_pktmbuf_detach(m);
 			if (rte_mbuf_refcnt_update(md, -1) == 0)
 				__rte_mbuf_raw_free(md);
-- 
1.9.3

[dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Sergio Gonzalez Monroy]

This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
field in the mbuf struct permanently.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 app/test/test_link_bonding.c            | 15 ---------------
 app/test/test_mbuf.c                    | 17 ++++-------------
 config/common_bsdapp                    |  1 -
 config/common_linuxapp                  |  1 -
 examples/Makefile                       |  4 ++--
 examples/ip_fragmentation/Makefile      |  4 ----
 examples/ip_pipeline/Makefile           |  3 ---
 examples/ip_pipeline/main.c             |  5 -----
 examples/ipv4_multicast/Makefile        |  4 ----
 examples/vhost/main.c                   | 13 -------------
 lib/librte_ip_frag/Makefile             |  4 ----
 lib/librte_ip_frag/rte_ip_frag.h        |  4 ----
 lib/librte_mbuf/rte_mbuf.c              |  2 --
 lib/librte_mbuf/rte_mbuf.h              | 30 ------------------------------
 lib/librte_pmd_bond/Makefile            |  4 ----
 lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
 lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
 lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 ----------
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 --------
 lib/librte_port/Makefile                |  4 ----
 20 files changed, 6 insertions(+), 131 deletions(-)

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 579ebbf..54895ab 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -708,9 +708,7 @@ test_set_bonding_mode(void)
 	int bonding_modes[] = { BONDING_MODE_ROUND_ROBIN,
 							BONDING_MODE_ACTIVE_BACKUP,
 							BONDING_MODE_BALANCE,
-#ifdef RTE_MBUF_REFCNT
 							BONDING_MODE_BROADCAST
-#endif
 							};
 
 	/* Test supported link bonding modes */
@@ -1425,7 +1423,6 @@ test_roundrobin_tx_burst(void)
 	return remove_slaves_and_stop_bonded_device();
 }
 
-#ifdef RTE_MBUF_REFCNT
 static int
 verify_mbufs_ref_count(struct rte_mbuf **mbufs, int nb_mbufs, int val)
 {
@@ -1439,8 +1436,6 @@ verify_mbufs_ref_count(struct rte_mbuf **mbufs, int nb_mbufs, int val)
 	}
 	return 0;
 }
-#endif
-
 
 static void
 free_mbufs(struct rte_mbuf **mbufs, int nb_mbufs)
@@ -1545,12 +1540,10 @@ test_roundrobin_tx_burst_slave_tx_fail(void)
 				(unsigned int)port_stats.opackets, slave_expected_tx_count);
 	}
 
-#ifdef RTE_MBUF_REFCNT
 	/* Verify that all mbufs have a ref value of zero */
 	TEST_ASSERT_SUCCESS(verify_mbufs_ref_count(&pkt_burst[tx_count],
 			TEST_RR_SLAVE_TX_FAIL_PACKETS_COUNT, 1),
 			"mbufs refcnts not as expected");
-#endif
 	free_mbufs(&pkt_burst[tx_count], TEST_RR_SLAVE_TX_FAIL_PACKETS_COUNT);
 
 	/* Clean up and remove slaves from bonded device */
@@ -3056,12 +3049,10 @@ test_balance_tx_burst_slave_tx_fail(void)
 				(unsigned int)port_stats.opackets,
 				TEST_BAL_SLAVE_TX_FAIL_BURST_SIZE_2);
 
-#ifdef RTE_MBUF_REFCNT
 	/* Verify that all mbufs have a ref value of zero */
 	TEST_ASSERT_SUCCESS(verify_mbufs_ref_count(&pkts_burst_1[tx_count_1],
 			TEST_BAL_SLAVE_TX_FAIL_PACKETS_COUNT, 1),
 			"mbufs refcnts not as expected");
-#endif
 
 	free_mbufs(&pkts_burst_1[tx_count_1],
 			TEST_BAL_SLAVE_TX_FAIL_PACKETS_COUNT);
@@ -3472,9 +3463,6 @@ test_balance_verify_slave_link_status_change_behaviour(void)
 	return remove_slaves_and_stop_bonded_device();
 }
 
-#ifdef RTE_MBUF_REFCNT
-/** Broadcast Mode Tests */
-
 static int
 test_broadcast_tx_burst(void)
 {
@@ -4001,7 +3989,6 @@ test_broadcast_verify_slave_link_status_change_behaviour(void)
 	/* Clean up and remove slaves from bonded device */
 	return remove_slaves_and_stop_bonded_device();
 }
-#endif
 
 static int
 test_reconfigure_bonded_device(void)
@@ -4592,14 +4579,12 @@ static struct unit_test_suite link_bonding_test_suite  = {
 		TEST_CASE(test_tlb_verify_mac_assignment),
 		TEST_CASE(test_tlb_verify_promiscuous_enable_disable),
 		TEST_CASE(test_tlb_verify_slave_link_status_change_failover),
-#ifdef RTE_MBUF_REFCNT
 		TEST_CASE(test_broadcast_tx_burst),
 		TEST_CASE(test_broadcast_tx_burst_slave_tx_fail),
 		TEST_CASE(test_broadcast_rx_burst),
 		TEST_CASE(test_broadcast_verify_promiscuous_enable_disable),
 		TEST_CASE(test_broadcast_verify_mac_assignment),
 		TEST_CASE(test_broadcast_verify_slave_link_status_change_behaviour),
-#endif
 		TEST_CASE(test_reconfigure_bonded_device),
 		TEST_CASE(test_close_bonded_device),
 
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index e86ba22..9de6dea 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -81,7 +81,7 @@
 
 static struct rte_mempool *pktmbuf_pool = NULL;
 
-#if defined RTE_MBUF_REFCNT  && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
 
 static struct rte_mempool *refcnt_pool = NULL;
 static struct rte_ring *refcnt_mbuf_ring = NULL;
@@ -322,9 +322,6 @@ fail:
 static int
 testclone_testupdate_testdetach(void)
 {
-#ifndef RTE_MBUF_REFCNT
-	return 0;
-#else
 	struct rte_mbuf *mc = NULL;
 	struct rte_mbuf *clone = NULL;
 
@@ -363,7 +360,6 @@ fail:
 	if (mc)
 		rte_pktmbuf_free(mc);
 	return -1;
-#endif /* RTE_MBUF_REFCNT */
 }
 #undef GOTO_FAIL
 
@@ -396,13 +392,11 @@ test_pktmbuf_pool(void)
 		printf("Error pool not empty");
 		ret = -1;
 	}
-#ifdef RTE_MBUF_REFCNT
 	extra = rte_pktmbuf_clone(m[0], pktmbuf_pool);
 	if(extra != NULL) {
 		printf("Error pool not empty");
 		ret = -1;
 	}
-#endif
 	/* free them */
 	for (i=0; i<NB_MBUF; i++) {
 		if (m[i] != NULL)
@@ -504,12 +498,11 @@ test_pktmbuf_free_segment(void)
 
 /*
  * Stress test for rte_mbuf atomic refcnt.
- * Implies that:
- * RTE_MBUF_REFCNT and RTE_MBUF_REFCNT_ATOMIC are both defined.
+ * Implies that RTE_MBUF_REFCNT_ATOMIC is defined.
  * For more efficency, recomended to run with RTE_LIBRTE_MBUF_DEBUG defined.
  */
 
-#if defined RTE_MBUF_REFCNT  && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
 
 static int
 test_refcnt_slave(__attribute__((unused)) void *arg)
@@ -614,7 +607,7 @@ test_refcnt_master(void)
 static int
 test_refcnt_mbuf(void)
 {
-#if defined RTE_MBUF_REFCNT  && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
 
 	unsigned lnum, master, slave, tref;
 
@@ -747,7 +740,6 @@ test_failing_mbuf_sanity_check(void)
 		return -1;
 	}
 
-#ifdef RTE_MBUF_REFCNT
 	badbuf = *buf;
 	badbuf.refcnt = 0;
 	if (verify_mbuf_check_panics(&badbuf)) {
@@ -761,7 +753,6 @@ test_failing_mbuf_sanity_check(void)
 		printf("Error with bad-refcnt(MAX) mbuf test\n");
 		return -1;
 	}
-#endif
 
 	return 0;
 }
diff --git a/config/common_bsdapp b/config/common_bsdapp
index 57bacb8..ad2d3cb 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -247,7 +247,6 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 #
 CONFIG_RTE_LIBRTE_MBUF=y
 CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
-CONFIG_RTE_MBUF_REFCNT=y
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
 
diff --git a/config/common_linuxapp b/config/common_linuxapp
index d428f84..5af645f 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -255,7 +255,6 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 #
 CONFIG_RTE_LIBRTE_MBUF=y
 CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
-CONFIG_RTE_MBUF_REFCNT=y
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
 
diff --git a/examples/Makefile b/examples/Makefile
index 81f1d2f..5a784d4 100644
--- a/examples/Makefile
+++ b/examples/Makefile
@@ -46,8 +46,8 @@ DIRS-y += exception_path
 DIRS-y += helloworld
 DIRS-y += ip_pipeline
 DIRS-y += ip_reassembly
-DIRS-$(CONFIG_RTE_MBUF_REFCNT) += ip_fragmentation
-DIRS-$(CONFIG_RTE_MBUF_REFCNT) += ipv4_multicast
+DIRS-$(CONFIG_RTE_IP_FRAG) += ip_fragmentation
+DIRS-y += ipv4_multicast
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += kni
 DIRS-y += l2fwd
 DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += l2fwd-ivshmem
diff --git a/examples/ip_fragmentation/Makefile b/examples/ip_fragmentation/Makefile
index ea0e11f..c321e6a 100644
--- a/examples/ip_fragmentation/Makefile
+++ b/examples/ip_fragmentation/Makefile
@@ -39,10 +39,6 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
-ifneq ($(CONFIG_RTE_MBUF_REFCNT),y)
-$(error This application requires RTE_MBUF_REFCNT to be enabled)
-endif
-
 # binary name
 APP = ip_fragmentation
 
diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
index b0a4106..e70fdc7 100644
--- a/examples/ip_pipeline/Makefile
+++ b/examples/ip_pipeline/Makefile
@@ -51,11 +51,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_tx.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_flow_classification.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_passthrough.c
-
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_ipv4_frag.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_ipv4_ras.c
-endif
 
 ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_firewall.c
diff --git a/examples/ip_pipeline/main.c b/examples/ip_pipeline/main.c
index 2c53877..7eee187 100644
--- a/examples/ip_pipeline/main.c
+++ b/examples/ip_pipeline/main.c
@@ -148,17 +148,12 @@ app_lcore_main_loop(__attribute__((unused)) void *arg)
 			rte_exit(EXIT_FAILURE, "ACL not present in build\n");
 #endif
 
-#ifdef RTE_MBUF_REFCNT
 		case APP_CORE_IPV4_FRAG:
 			app_main_loop_pipeline_ipv4_frag();
 			return 0;
 		case APP_CORE_IPV4_RAS:
 			app_main_loop_pipeline_ipv4_ras();
 			return 0;
-#else
-			rte_exit(EXIT_FAILURE,
-				"mbuf chaining not present in build\n");
-#endif
 
 		default:
 			rte_panic("%s: Invalid core type for core %u\n",
diff --git a/examples/ipv4_multicast/Makefile b/examples/ipv4_multicast/Makefile
index 604cebe..44f0a3b 100644
--- a/examples/ipv4_multicast/Makefile
+++ b/examples/ipv4_multicast/Makefile
@@ -39,10 +39,6 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
-ifneq ($(CONFIG_RTE_MBUF_REFCNT),y)
-$(error This application requires RTE_MBUF_REFCNT to be enabled)
-endif
-
 # binary name
 APP = ipv4_multicast
 
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 5e341d6..592bc28 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -726,19 +726,6 @@ us_vhost_parse_args(int argc, char **argv)
 					return -1;
 				} else
 					zero_copy = ret;
-
-				if (zero_copy) {
-#ifdef RTE_MBUF_REFCNT
-					RTE_LOG(ERR, VHOST_CONFIG, "Before running "
-					"zero copy vhost APP, please "
-					"disable RTE_MBUF_REFCNT\n"
-					"in config file and then rebuild DPDK "
-					"core lib!\n"
-					"Otherwise please disable zero copy "
-					"flag in command line!\n");
-					return -1;
-#endif
-				}
 			}
 
 			/* Specify the descriptor number on RX. */
diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index fe926f7..9d06780 100644
--- a/lib/librte_ip_frag/Makefile
+++ b/lib/librte_ip_frag/Makefile
@@ -42,12 +42,8 @@ EXPORT_MAP := rte_ipfrag_version.map
 LIBABIVER := 1
 
 #source files
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_fragmentation.c
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_fragmentation.c
-else
-$(info WARNING: Fragmentation feature is disabled because it needs MBUF_REFCNT.)
-endif
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_reassembly.c
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_reassembly.c
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ip_frag_common.c
diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
index 1083d44..f673728 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -180,7 +180,6 @@ rte_ip_frag_table_destroy( struct rte_ip_frag_tbl *tbl)
 	rte_free(tbl);
 }
 
-#ifdef RTE_MBUF_REFCNT
 /**
  * This function implements the fragmentation of IPv6 packets.
  *
@@ -209,7 +208,6 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
 		uint16_t mtu_size,
 		struct rte_mempool *pool_direct,
 		struct rte_mempool *pool_indirect);
-#endif
 
 /*
  * This function implements reassembly of fragmented IPv6 packets.
@@ -258,7 +256,6 @@ rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
 		return NULL;
 }
 
-#ifdef RTE_MBUF_REFCNT
 /**
  * IPv4 fragmentation.
  *
@@ -287,7 +284,6 @@ int32_t rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in,
 			uint16_t nb_pkts_out, uint16_t mtu_size,
 			struct rte_mempool *pool_direct,
 			struct rte_mempool *pool_indirect);
-#endif
 
 /*
  * This function implements reassembly of fragmented IPv4 packets.
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 1b14e02..470e3c2 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -146,11 +146,9 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
 	if (m->buf_addr == NULL)
 		rte_panic("bad virt addr\n");
 
-#ifdef RTE_MBUF_REFCNT
 	uint16_t cnt = rte_mbuf_refcnt_read(m);
 	if ((cnt == 0) || (cnt == UINT16_MAX))
 		rte_panic("bad ref cnt\n");
-#endif
 
 	/* nothing to check for sub-segments */
 	if (is_header == 0)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 12e7545..d168488 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -220,11 +220,8 @@ struct rte_mbuf {
 	 * config option.
 	 */
 	union {
-#ifdef RTE_MBUF_REFCNT
 		rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
 		uint16_t refcnt;              /**< Non-atomically accessed refcnt */
-#endif
-		uint16_t refcnt_reserved;     /**< Do not use this field */
 	};
 	uint8_t nb_segs;          /**< Number of segments. */
 	uint8_t port;             /**< Input port. */
@@ -354,7 +351,6 @@ if (!(exp)) {                                                        \
 
 #endif /*  RTE_LIBRTE_MBUF_DEBUG */
 
-#ifdef RTE_MBUF_REFCNT
 #ifdef RTE_MBUF_REFCNT_ATOMIC
 
 /**
@@ -436,15 +432,6 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
 		rte_prefetch0(m);               \
 } while (0)
 
-#else /* ! RTE_MBUF_REFCNT */
-
-/** Mbuf prefetch */
-#define RTE_MBUF_PREFETCH_TO_FREE(m) do { } while(0)
-
-#define rte_mbuf_refcnt_set(m,v) do { } while(0)
-
-#endif /* RTE_MBUF_REFCNT */
-
 
 /**
  * Sanity checks on an mbuf.
@@ -479,10 +466,8 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
 	if (rte_mempool_get(mp, &mb) < 0)
 		return NULL;
 	m = (struct rte_mbuf *)mb;
-#ifdef RTE_MBUF_REFCNT
 	RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
 	rte_mbuf_refcnt_set(m, 1);
-#endif /* RTE_MBUF_REFCNT */
 	return (m);
 }
 
@@ -497,9 +482,7 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
 static inline void __attribute__((always_inline))
 __rte_mbuf_raw_free(struct rte_mbuf *m)
 {
-#ifdef RTE_MBUF_REFCNT
 	RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
-#endif /* RTE_MBUF_REFCNT */
 	rte_mempool_put(m->pool, m);
 }
 
@@ -674,8 +657,6 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 	return (m);
 }
 
-#ifdef RTE_MBUF_REFCNT
-
 /**
  * Attach packet mbuf to another packet mbuf.
  * After attachment we refer the mbuf we attached as 'indirect',
@@ -749,15 +730,11 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
 	m->ol_flags = 0;
 }
 
-#endif /* RTE_MBUF_REFCNT */
-
-
 static inline struct rte_mbuf* __attribute__((always_inline))
 __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 {
 	__rte_mbuf_sanity_check(m, 0);
 
-#ifdef RTE_MBUF_REFCNT
 	if (likely (rte_mbuf_refcnt_read(m) == 1) ||
 			likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
 
@@ -773,12 +750,9 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 			if (rte_mbuf_refcnt_update(md, -1) == 0)
 				__rte_mbuf_raw_free(md);
 		}
-#endif
 		return(m);
-#ifdef RTE_MBUF_REFCNT
 	}
 	return (NULL);
-#endif
 }
 
 /**
@@ -821,8 +795,6 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
 	}
 }
 
-#ifdef RTE_MBUF_REFCNT
-
 /**
  * Creates a "clone" of the given packet mbuf.
  *
@@ -897,8 +869,6 @@ static inline void rte_pktmbuf_refcnt_update(struct rte_mbuf *m, int16_t v)
 	} while ((m = m->next) != NULL);
 }
 
-#endif /* RTE_MBUF_REFCNT */
-
 /**
  * Get the headroom in a packet mbuf.
  *
diff --git a/lib/librte_pmd_bond/Makefile b/lib/librte_pmd_bond/Makefile
index d6c81a8..e9d94e1 100644
--- a/lib/librte_pmd_bond/Makefile
+++ b/lib/librte_pmd_bond/Makefile
@@ -51,10 +51,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_pmd.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_args.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_8023ad.c
 
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),n)
-$(info WARNING: Link Bonding Broadcast mode is disabled because it needs MBUF_REFCNT.)
-endif
-
 #
 # Export include files
 #
diff --git a/lib/librte_pmd_bond/rte_eth_bond.h b/lib/librte_pmd_bond/rte_eth_bond.h
index 7177983..99568d4 100644
--- a/lib/librte_pmd_bond/rte_eth_bond.h
+++ b/lib/librte_pmd_bond/rte_eth_bond.h
@@ -71,12 +71,10 @@ extern "C" {
  * slaves using one of three available transmit policies - l2, l2+3 or l3+4.
  * See BALANCE_XMIT_POLICY macros definitions for further details on transmit
  * policies. */
-#ifdef RTE_MBUF_REFCNT
 #define BONDING_MODE_BROADCAST			(3)
 /**< Broadcast (Mode 3).
  * In this mode all transmitted packets will be transmitted on all available
  * active slaves of the bonded. */
-#endif
 #define BONDING_MODE_8023AD				(4)
 /**< 802.3AD (Mode 4).
  *
diff --git a/lib/librte_pmd_bond/rte_eth_bond_args.c b/lib/librte_pmd_bond/rte_eth_bond_args.c
index ca4de38..ae76bc6 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_args.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_args.c
@@ -170,9 +170,7 @@ bond_ethdev_parse_slave_mode_kvarg(const char *key __rte_unused,
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_ACTIVE_BACKUP:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 	case BONDING_MODE_8023AD:
 	case BONDING_MODE_ADAPTIVE_TRANSMIT_LOAD_BALANCING:
 		return 0;
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 09b0f30..b90bf48 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -681,7 +681,6 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 	return num_tx_total;
 }
 
-#ifdef RTE_MBUF_REFCNT
 static uint16_t
 bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
 		uint16_t nb_pkts)
@@ -741,7 +740,6 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
 
 	return max_nb_of_tx_pkts;
 }
-#endif
 
 void
 link_properties_set(struct rte_eth_dev *bonded_eth_dev,
@@ -839,9 +837,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 	switch (internals->mode) {
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 		for (i = 0; i < internals->slave_count; i++) {
 			if (mac_address_set(&rte_eth_devices[internals->slaves[i].port_id],
 					bonded_eth_dev->data->mac_addrs)) {
@@ -901,12 +897,10 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, int mode)
 		eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_balance;
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
 		break;
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
 		eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_broadcast;
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
 		break;
-#endif
 	case BONDING_MODE_8023AD:
 		if (bond_mode_8023ad_enable(eth_dev) != 0)
 			return -1;
@@ -1410,9 +1404,7 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)
 	/* Promiscuous mode is propagated to all slaves */
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 		for (i = 0; i < internals->slave_count; i++)
 			rte_eth_promiscuous_enable(internals->slaves[i].port_id);
 		break;
@@ -1439,9 +1431,7 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev)
 	/* Promiscuous mode is propagated to all slaves */
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 		for (i = 0; i < internals->slave_count; i++)
 			rte_eth_promiscuous_disable(internals->slaves[i].port_id);
 		break;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index b54cb19..8c95bd3 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -540,20 +540,12 @@ ixgbe_tx_free_bufs(struct igb_tx_queue *txq)
 	 */
 	txep = &((struct igb_tx_entry_v *)txq->sw_ring)[txq->tx_next_dd -
 			(n - 1)];
-#ifdef RTE_MBUF_REFCNT
 	m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
-#else
-	m = txep[0].mbuf;
-#endif
 	if (likely(m != NULL)) {
 		free[0] = m;
 		nb_free = 1;
 		for (i = 1; i < n; i++) {
-#ifdef RTE_MBUF_REFCNT
 			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
-#else
-			m = txep[i].mbuf;
-#endif
 			if (likely(m != NULL)) {
 				if (likely(m->pool == free[0]->pool))
 					free[nb_free++] = m;
diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile
index 0e38452..de960fc 100644
--- a/lib/librte_port/Makefile
+++ b/lib/librte_port/Makefile
@@ -49,9 +49,7 @@ LIBABIVER := 1
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ethdev.c
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ring.c
 ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_frag.c
-endif
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ras.c
 endif
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_sched.c
@@ -62,9 +60,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ethdev.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ring.h
 ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_frag.h
-endif
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ras.h
 endif
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_sched.h
-- 
1.9.3

Re: [dpdk-dev] [PATCH 0/2] Removal of RTE_MBUF_REFCNT

[Stephen Hemminger]

On Mon, 16 Feb 2015 16:08:31 +0000
Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> wrote:

> This patch tries to remove the RTE_MBUF_REFCNT config options and dependencies
> by introducing a new mbuf flag IND_ATTACHED_MBUF that would indicate when the mbuf
> is an indirect attached mbuf, to differentiate between indirect mbufs and mbufs
> with external memory buffers (ie. vhost zero copy).
> 
> Previous discussion:
> http://dpdk.org/ml/archives/dev/2014-October/007127.html
> 
> Currently for mbufs with refcnt, we cannot free mbufs with external memory
> buffers (ie. vhost zero copy), as they are recognized as indirect
> attached mbufs and therefore we free the direct mbuf it points to,
> resulting in an error in the case of external memory buffers.
> 
> We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
> that the mbuf is an indirect attached mbuf pointing to another mbuf.
> When we free an mbuf, we only free the direct mbuf if the flag is set.
> Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
> The flag is set during attach and clear on detach.
> 
> So in the case of vhost zero copy where we have mbufs with external
> buffers, by default we just free the mbuf and it is up to the user to deal with
> the external buffer.
> 
> Sergio Gonzalez Monroy (2):
>   mbuf: Introduce IND_ATTACHED_MBUF flag
>   Remove RTE_MBUF_REFCNT references
> 
>  app/test/test_link_bonding.c            | 15 -----------
>  app/test/test_mbuf.c                    | 17 +++----------
>  config/common_bsdapp                    |  1 -
>  config/common_linuxapp                  |  1 -
>  examples/Makefile                       |  4 +--
>  examples/ip_fragmentation/Makefile      |  4 ---
>  examples/ip_pipeline/Makefile           |  3 ---
>  examples/ip_pipeline/main.c             |  5 ----
>  examples/ipv4_multicast/Makefile        |  4 ---
>  examples/vhost/main.c                   | 19 +++-----------
>  lib/librte_ip_frag/Makefile             |  4 ---
>  lib/librte_ip_frag/rte_ip_frag.h        |  4 ---
>  lib/librte_mbuf/rte_mbuf.c              |  2 --
>  lib/librte_mbuf/rte_mbuf.h              | 45 +++++++--------------------------
>  lib/librte_pmd_bond/Makefile            |  4 ---
>  lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
>  lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
>  lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 --------
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 ------
>  lib/librte_port/Makefile                |  4 ---
>  20 files changed, 19 insertions(+), 139 deletions(-)
> 

What about supporting a clone operation instead of and in addition
to attach?  The refcnt is also useful when there are two paths for
a packet (going into mulitple rings).

Re: [dpdk-dev] [PATCH 0/2] Removal of RTE_MBUF_REFCNT

[Gonzalez Monroy, Sergio]

On 16/02/2015 20:47, Stephen Hemminger wrote:
> On Mon, 16 Feb 2015 16:08:31 +0000
> Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> wrote:
>
>> This patch tries to remove the RTE_MBUF_REFCNT config options and dependencies
>> by introducing a new mbuf flag IND_ATTACHED_MBUF that would indicate when the mbuf
>> is an indirect attached mbuf, to differentiate between indirect mbufs and mbufs
>> with external memory buffers (ie. vhost zero copy).
>>
>> Previous discussion:
>> http://dpdk.org/ml/archives/dev/2014-October/007127.html
>>
>> Currently for mbufs with refcnt, we cannot free mbufs with external memory
>> buffers (ie. vhost zero copy), as they are recognized as indirect
>> attached mbufs and therefore we free the direct mbuf it points to,
>> resulting in an error in the case of external memory buffers.
>>
>> We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
>> that the mbuf is an indirect attached mbuf pointing to another mbuf.
>> When we free an mbuf, we only free the direct mbuf if the flag is set.
>> Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
>> The flag is set during attach and clear on detach.
>>
>> So in the case of vhost zero copy where we have mbufs with external
>> buffers, by default we just free the mbuf and it is up to the user to deal with
>> the external buffer.
>>
>> Sergio Gonzalez Monroy (2):
>>    mbuf: Introduce IND_ATTACHED_MBUF flag
>>    Remove RTE_MBUF_REFCNT references
>>
>>   app/test/test_link_bonding.c            | 15 -----------
>>   app/test/test_mbuf.c                    | 17 +++----------
>>   config/common_bsdapp                    |  1 -
>>   config/common_linuxapp                  |  1 -
>>   examples/Makefile                       |  4 +--
>>   examples/ip_fragmentation/Makefile      |  4 ---
>>   examples/ip_pipeline/Makefile           |  3 ---
>>   examples/ip_pipeline/main.c             |  5 ----
>>   examples/ipv4_multicast/Makefile        |  4 ---
>>   examples/vhost/main.c                   | 19 +++-----------
>>   lib/librte_ip_frag/Makefile             |  4 ---
>>   lib/librte_ip_frag/rte_ip_frag.h        |  4 ---
>>   lib/librte_mbuf/rte_mbuf.c              |  2 --
>>   lib/librte_mbuf/rte_mbuf.h              | 45 +++++++--------------------------
>>   lib/librte_pmd_bond/Makefile            |  4 ---
>>   lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
>>   lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
>>   lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 --------
>>   lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 ------
>>   lib/librte_port/Makefile                |  4 ---
>>   20 files changed, 19 insertions(+), 139 deletions(-)
>>
> What about supporting a clone operation instead of and in addition
> to attach?  The refcnt is also useful when there are two paths for
> a packet (going into mulitple rings).
There is already an rte_pktmbuf_clone function, not sure if that is what 
you are asking.

Sergio

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ]

Hi Sergio,

On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> field in the mbuf struct permanently.
>
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>

I think removing the refcount compile option goes in the right
direction. However, activating the refcount will break the applications
that reserve a private zone in mbufs. This is due to the macros
RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
data buffer.

For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
mbuf pool could store the size of the private size like it's done
for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
or m->pool, we can retrieve the mbuf pool and this value, then
compute the buffer address.

For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
a backpointer to the mbuf is always located before the data buffer,
but it looks difficult to do.

Another idea would be to add a field in indirect mbufs that stores
the pointer to the "parent" mbuf.

Regards,
Olivier

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Bruce Richardson]

On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> Hi Sergio,
> 
> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >field in the mbuf struct permanently.
> >
> >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> 
> I think removing the refcount compile option goes in the right
> direction. However, activating the refcount will break the applications
> that reserve a private zone in mbufs. This is due to the macros
> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> data buffer.
>

While I understand how the macros make certain assumptions, how does activating
the refcnt specifically lead to the problems you describe? Could you explain
that part in a bit more detail?

Thanks,
/Bruce

> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> mbuf pool could store the size of the private size like it's done
> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> or m->pool, we can retrieve the mbuf pool and this value, then
> compute the buffer address.
> 
> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> a backpointer to the mbuf is always located before the data buffer,
> but it looks difficult to do.
> 
> Another idea would be to add a field in indirect mbufs that stores
> the pointer to the "parent" mbuf.
> 
> Regards,
> Olivier
> 

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Ananyev, Konstantin]

Hi lads,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Wednesday, February 18, 2015 9:36 AM
> To: Olivier MATZ
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> 
> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> > Hi Sergio,
> >
> > On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> > >This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> > >field in the mbuf struct permanently.
> > >
> > >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >
> > I think removing the refcount compile option goes in the right
> > direction. However, activating the refcount will break the applications
> > that reserve a private zone in mbufs. This is due to the macros
> > RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> > the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> > data buffer.
> >
> 
> While I understand how the macros make certain assumptions, how does activating
> the refcnt specifically lead to the problems you describe? Could you explain
> that part in a bit more detail?
> 
> Thanks,
> /Bruce
> 

Olivier, I also don't understand your concern here.
As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
Is that indirect mbuf or not. 
Instead we use a special falg for that purpose:

-#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
 
BTW, Sergio as I said before, I think it should be:
#define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)

Konstantin


> > For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> > mbuf pool could store the size of the private size like it's done
> > for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> > or m->pool, we can retrieve the mbuf pool and this value, then
> > compute the buffer address.
> >
> > For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> > a backpointer to the mbuf is always located before the data buffer,
> > but it looks difficult to do.
> >
> > Another idea would be to add a field in indirect mbufs that stores
> > the pointer to the "parent" mbuf.
> >
> > Regards,
> > Olivier
> >

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ]

Hi Bruce,

On 02/18/2015 10:35 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>> Hi Sergio,
>>
>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>> field in the mbuf struct permanently.
>>>
>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>
>> I think removing the refcount compile option goes in the right
>> direction. However, activating the refcount will break the applications
>> that reserve a private zone in mbufs. This is due to the macros
>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>> data buffer.
>>
>
> While I understand how the macros make certain assumptions, how does activating
> the refcnt specifically lead to the problems you describe? Could you explain
> that part in a bit more detail?

Indeed, you are right, removing the refcount option does not break
the applications if they do not use it. So there is probably no need
to fix that problem in this patch series.

However we should consider this problem as the refcount (which can
not be deactivated now) is not compatible with the private mbuf data.

By the way, once we are on this, shouldn't we consider removing the
RTE_MBUF_REFCNT_ATOMIC compile option?


Regards,
Olivier



>
> Thanks,
> /Bruce
>
>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>> mbuf pool could store the size of the private size like it's done
>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>> or m->pool, we can retrieve the mbuf pool and this value, then
>> compute the buffer address.
>>
>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>> a backpointer to the mbuf is always located before the data buffer,
>> but it looks difficult to do.
>>
>> Another idea would be to add a field in indirect mbufs that stores
>> the pointer to the "parent" mbuf.
>>
>> Regards,
>> Olivier
>>

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Bruce Richardson]

On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> Hi lads,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Wednesday, February 18, 2015 9:36 AM
> > To: Olivier MATZ
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> > 
> > On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> > > Hi Sergio,
> > >
> > > On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> > > >This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> > > >field in the mbuf struct permanently.
> > > >
> > > >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > >
> > > I think removing the refcount compile option goes in the right
> > > direction. However, activating the refcount will break the applications
> > > that reserve a private zone in mbufs. This is due to the macros
> > > RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> > > the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> > > data buffer.
> > >
> > 
> > While I understand how the macros make certain assumptions, how does activating
> > the refcnt specifically lead to the problems you describe? Could you explain
> > that part in a bit more detail?
> > 
> > Thanks,
> > /Bruce
> > 
> 
> Olivier, I also don't understand your concern here.
> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
> Is that indirect mbuf or not. 
> Instead we use a special falg for that purpose:
> 
> -#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> +#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
>  
> BTW, Sergio as I said before, I think it should be:
> #define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
> 
> Konstantin
> 
> 
> > > For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> > > mbuf pool could store the size of the private size like it's done
> > > for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> > > or m->pool, we can retrieve the mbuf pool and this value, then
> > > compute the buffer address.

Agreed, that makes sense.

> > >
> > > For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> > > a backpointer to the mbuf is always located before the data buffer,
> > > but it looks difficult to do.

On the other hand, with the proposed refcnt change Sergio proposes, we no
longer use this macro in any of the built-in mbuf handling for freeing mbufs.
Does this need to be solved at anything other than the application level?

/Bruce

> > >
> > > Another idea would be to add a field in indirect mbufs that stores
> > > the pointer to the "parent" mbuf.
> > >
> > > Regards,
> > > Olivier
> > >

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ]

On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
>> Hi lads,
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
>>> Sent: Wednesday, February 18, 2015 9:36 AM
>>> To: Olivier MATZ
>>> Cc: dev@dpdk.org
>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>>>
>>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>>>> Hi Sergio,
>>>>
>>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>>>> field in the mbuf struct permanently.
>>>>>
>>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>>>
>>>> I think removing the refcount compile option goes in the right
>>>> direction. However, activating the refcount will break the applications
>>>> that reserve a private zone in mbufs. This is due to the macros
>>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>>>> data buffer.
>>>>
>>>
>>> While I understand how the macros make certain assumptions, how does activating
>>> the refcnt specifically lead to the problems you describe? Could you explain
>>> that part in a bit more detail?
>>>
>>> Thanks,
>>> /Bruce
>>>
>>
>> Olivier, I also don't understand your concern here.
>> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
>> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
>> The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
>> Is that indirect mbuf or not.
>> Instead we use a special falg for that purpose:
>>
>> -#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
>> +#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
>>
>> BTW, Sergio as I said before, I think it should be:
>> #define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
>>
>> Konstantin
>>
>>
>>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>>>> mbuf pool could store the size of the private size like it's done
>>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>>>> or m->pool, we can retrieve the mbuf pool and this value, then
>>>> compute the buffer address.
>
> Agreed, that makes sense.
>
>>>>
>>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>>>> a backpointer to the mbuf is always located before the data buffer,
>>>> but it looks difficult to do.
>
> On the other hand, with the proposed refcnt change Sergio proposes, we no
> longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> Does this need to be solved at anything other than the application level?

It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
parent mbuf (direct) from the indirect mbuf beeing freed.



>>>>
>>>> Another idea would be to add a field in indirect mbufs that stores
>>>> the pointer to the "parent" mbuf.
>>>>
>>>> Regards,
>>>> Olivier
>>>>

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, February 18, 2015 10:15 AM
> To: Richardson, Bruce; Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> 
> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> > On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >> Hi lads,
> >>
> >>> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>> Sent: Wednesday, February 18, 2015 9:36 AM
> >>> To: Olivier MATZ
> >>> Cc: dev@dpdk.org
> >>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>
> >>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>> Hi Sergio,
> >>>>
> >>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>> field in the mbuf struct permanently.
> >>>>>
> >>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>
> >>>> I think removing the refcount compile option goes in the right
> >>>> direction. However, activating the refcount will break the applications
> >>>> that reserve a private zone in mbufs. This is due to the macros
> >>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>> data buffer.
> >>>>
> >>>
> >>> While I understand how the macros make certain assumptions, how does activating
> >>> the refcnt specifically lead to the problems you describe? Could you explain
> >>> that part in a bit more detail?
> >>>
> >>> Thanks,
> >>> /Bruce
> >>>
> >>
> >> Olivier, I also don't understand your concern here.
> >> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >> The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
> >> Is that indirect mbuf or not.
> >> Instead we use a special falg for that purpose:
> >>
> >> -#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >> +#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
> >>
> >> BTW, Sergio as I said before, I think it should be:
> >> #define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>
> >> Konstantin
> >>
> >>
> >>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>> mbuf pool could store the size of the private size like it's done
> >>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>> or m->pool, we can retrieve the mbuf pool and this value, then
> >>>> compute the buffer address.
> >
> > Agreed, that makes sense.
> >
> >>>>
> >>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>> a backpointer to the mbuf is always located before the data buffer,
> >>>> but it looks difficult to do.
> >
> > On the other hand, with the proposed refcnt change Sergio proposes, we no
> > longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> > Does this need to be solved at anything other than the application level?
> 
> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> parent mbuf (direct) from the indirect mbuf beeing freed.
> 

Yes, if the INDIRECT flag is set.
Though I still don't understand, what is the problem with these 2 macros with that patch?
Why we need to replace it with something?
What exactly you think will be broken?

Konstantin

> 
> 
> >>>>
> >>>> Another idea would be to add a field in indirect mbufs that stores
> >>>> the pointer to the "parent" mbuf.
> >>>>
> >>>> Regards,
> >>>> Olivier
> >>>>

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Bruce Richardson]

On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> >On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >>Hi lads,
> >>
> >>>-----Original Message-----
> >>>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>>Sent: Wednesday, February 18, 2015 9:36 AM
> >>>To: Olivier MATZ
> >>>Cc: dev@dpdk.org
> >>>Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>
> >>>On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>>Hi Sergio,
> >>>>
> >>>>On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>>This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>>field in the mbuf struct permanently.
> >>>>>
> >>>>>Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>
> >>>>I think removing the refcount compile option goes in the right
> >>>>direction. However, activating the refcount will break the applications
> >>>>that reserve a private zone in mbufs. This is due to the macros
> >>>>RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>>the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>>data buffer.
> >>>>
> >>>
> >>>While I understand how the macros make certain assumptions, how does activating
> >>>the refcnt specifically lead to the problems you describe? Could you explain
> >>>that part in a bit more detail?
> >>>
> >>>Thanks,
> >>>/Bruce
> >>>
> >>
> >>Olivier, I also don't understand your concern here.
> >>As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >>They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >>The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
> >>Is that indirect mbuf or not.
> >>Instead we use a special falg for that purpose:
> >>
> >>-#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >>+#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
> >>
> >>BTW, Sergio as I said before, I think it should be:
> >>#define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>
> >>Konstantin
> >>
> >>
> >>>>For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>>mbuf pool could store the size of the private size like it's done
> >>>>for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>>or m->pool, we can retrieve the mbuf pool and this value, then
> >>>>compute the buffer address.
> >
> >Agreed, that makes sense.
> >
> >>>>
> >>>>For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>>a backpointer to the mbuf is always located before the data buffer,
> >>>>but it looks difficult to do.
> >
> >On the other hand, with the proposed refcnt change Sergio proposes, we no
> >longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> >Does this need to be solved at anything other than the application level?
> 
> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> parent mbuf (direct) from the indirect mbuf beeing freed.
>
Yes, my bad.
How was this managed before, since refcnt field seems to be necessary in order
to effectively manage indirect mbufs? Is this just the case that this is something
that never worked and that needs to be solved, or is it something that was 
working that this patch will now break?

Thanks,
/Bruce

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ]

Hi,

On 02/18/2015 11:22 AM, Bruce Richardson wrote:
> On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
>> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
>>> On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
>>>> Hi lads,
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
>>>>> Sent: Wednesday, February 18, 2015 9:36 AM
>>>>> To: Olivier MATZ
>>>>> Cc: dev@dpdk.org
>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
>>>>>
>>>>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
>>>>>> Hi Sergio,
>>>>>>
>>>>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
>>>>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
>>>>>>> field in the mbuf struct permanently.
>>>>>>>
>>>>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>>>>>>
>>>>>> I think removing the refcount compile option goes in the right
>>>>>> direction. However, activating the refcount will break the applications
>>>>>> that reserve a private zone in mbufs. This is due to the macros
>>>>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
>>>>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
>>>>>> data buffer.
>>>>>>
>>>>>
>>>>> While I understand how the macros make certain assumptions, how does activating
>>>>> the refcnt specifically lead to the problems you describe? Could you explain
>>>>> that part in a bit more detail?
>>>>>
>>>>> Thanks,
>>>>> /Bruce
>>>>>
>>>>
>>>> Olivier, I also don't understand your concern here.
>>>> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
>>>> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
>>>> The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
>>>> Is that indirect mbuf or not.
>>>> Instead we use a special falg for that purpose:
>>>>
>>>> -#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
>>>> +#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
>>>>
>>>> BTW, Sergio as I said before, I think it should be:
>>>> #define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
>>>>
>>>> Konstantin
>>>>
>>>>
>>>>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
>>>>>> mbuf pool could store the size of the private size like it's done
>>>>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
>>>>>> or m->pool, we can retrieve the mbuf pool and this value, then
>>>>>> compute the buffer address.
>>>
>>> Agreed, that makes sense.
>>>
>>>>>>
>>>>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
>>>>>> a backpointer to the mbuf is always located before the data buffer,
>>>>>> but it looks difficult to do.
>>>
>>> On the other hand, with the proposed refcnt change Sergio proposes, we no
>>> longer use this macro in any of the built-in mbuf handling for freeing mbufs.
>>> Does this need to be solved at anything other than the application level?
>>
>> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
>> parent mbuf (direct) from the indirect mbuf beeing freed.
>>
> Yes, my bad.
> How was this managed before, since refcnt field seems to be necessary in order
> to effectively manage indirect mbufs? Is this just the case that this is something
> that never worked and that needs to be solved, or is it something that was
> working that this patch will now break?

This is something that never worked before: refcounts are not compatible
with reserving private data in mbufs. This patch does not change the
issue, it is still there.

Before the patch, an application that wanted to reserve a private
data could disable refcounts at compile-time.
After the patch, the solution is just to avoid using refcounts.

Regards,
Olivier

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Bruce Richardson]

On Wed, Feb 18, 2015 at 11:33:48AM +0100, Olivier MATZ wrote:
> Hi,
> 
> On 02/18/2015 11:22 AM, Bruce Richardson wrote:
> >On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
> >>On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> >>>On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >>>>Hi lads,
> >>>>
> >>>>>-----Original Message-----
> >>>>>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>>>>Sent: Wednesday, February 18, 2015 9:36 AM
> >>>>>To: Olivier MATZ
> >>>>>Cc: dev@dpdk.org
> >>>>>Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>>>
> >>>>>On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>>>>Hi Sergio,
> >>>>>>
> >>>>>>On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>>>>This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>>>>field in the mbuf struct permanently.
> >>>>>>>
> >>>>>>>Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>>>
> >>>>>>I think removing the refcount compile option goes in the right
> >>>>>>direction. However, activating the refcount will break the applications
> >>>>>>that reserve a private zone in mbufs. This is due to the macros
> >>>>>>RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>>>>the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>>>>data buffer.
> >>>>>>
> >>>>>
> >>>>>While I understand how the macros make certain assumptions, how does activating
> >>>>>the refcnt specifically lead to the problems you describe? Could you explain
> >>>>>that part in a bit more detail?
> >>>>>
> >>>>>Thanks,
> >>>>>/Bruce
> >>>>>
> >>>>
> >>>>Olivier, I also don't understand your concern here.
> >>>>As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >>>>They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >>>>The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
> >>>>Is that indirect mbuf or not.
> >>>>Instead we use a special falg for that purpose:
> >>>>
> >>>>-#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >>>>+#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>>BTW, Sergio as I said before, I think it should be:
> >>>>#define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>>Konstantin
> >>>>
> >>>>
> >>>>>>For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>>>>mbuf pool could store the size of the private size like it's done
> >>>>>>for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>>>>or m->pool, we can retrieve the mbuf pool and this value, then
> >>>>>>compute the buffer address.
> >>>
> >>>Agreed, that makes sense.
> >>>
> >>>>>>
> >>>>>>For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>>>>a backpointer to the mbuf is always located before the data buffer,
> >>>>>>but it looks difficult to do.
> >>>
> >>>On the other hand, with the proposed refcnt change Sergio proposes, we no
> >>>longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> >>>Does this need to be solved at anything other than the application level?
> >>
> >>It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> >>parent mbuf (direct) from the indirect mbuf beeing freed.
> >>
> >Yes, my bad.
> >How was this managed before, since refcnt field seems to be necessary in order
> >to effectively manage indirect mbufs? Is this just the case that this is something
> >that never worked and that needs to be solved, or is it something that was
> >working that this patch will now break?
> 
> This is something that never worked before: refcounts are not compatible
> with reserving private data in mbufs. This patch does not change the
> issue, it is still there.
> 
> Before the patch, an application that wanted to reserve a private
> data could disable refcounts at compile-time.
> After the patch, the solution is just to avoid using refcounts.
> 
> Regards,
> Olivier
> 
Thanks for clarifying.
So, you ok with this patch as a step in the right direction?

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Ananyev, Konstantin]

> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Wednesday, February 18, 2015 10:34 AM
> To: Richardson, Bruce
> Cc: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> 
> Hi,
> 
> On 02/18/2015 11:22 AM, Bruce Richardson wrote:
> > On Wed, Feb 18, 2015 at 11:14:42AM +0100, Olivier MATZ wrote:
> >> On 02/18/2015 11:00 AM, Bruce Richardson wrote:
> >>> On Wed, Feb 18, 2015 at 09:48:58AM +0000, Ananyev, Konstantin wrote:
> >>>> Hi lads,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> >>>>> Sent: Wednesday, February 18, 2015 9:36 AM
> >>>>> To: Olivier MATZ
> >>>>> Cc: dev@dpdk.org
> >>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references
> >>>>>
> >>>>> On Wed, Feb 18, 2015 at 10:16:56AM +0100, Olivier MATZ wrote:
> >>>>>> Hi Sergio,
> >>>>>>
> >>>>>> On 02/16/2015 05:08 PM, Sergio Gonzalez Monroy wrote:
> >>>>>>> This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
> >>>>>>> field in the mbuf struct permanently.
> >>>>>>>
> >>>>>>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >>>>>>
> >>>>>> I think removing the refcount compile option goes in the right
> >>>>>> direction. However, activating the refcount will break the applications
> >>>>>> that reserve a private zone in mbufs. This is due to the macros
> >>>>>> RTE_MBUF_TO_BADDR() and RTE_MBUF_FROM_BADDR() that suppose that
> >>>>>> the beginning of the mbuf is 128 bytes (sizeof mbuf) before the
> >>>>>> data buffer.
> >>>>>>
> >>>>>
> >>>>> While I understand how the macros make certain assumptions, how does activating
> >>>>> the refcnt specifically lead to the problems you describe? Could you explain
> >>>>> that part in a bit more detail?
> >>>>>
> >>>>> Thanks,
> >>>>> /Bruce
> >>>>>
> >>>>
> >>>> Olivier, I also don't understand your concern here.
> >>>> As I can see, that patch has nothing to do with RTE_MBUF_FROM_BADDR/ RTE_MBUF_FROM_BADDR macros.
> >>>> They are still there, for example rte_pktmbuf_detach() still uses it to restore mbuf's buf_addr.
> >>>> The only principal change here, is that we don't rely more  on RTE_MBUF_FROM_BADDR to determine,
> >>>> Is that indirect mbuf or not.
> >>>> Instead we use a special falg for that purpose:
> >>>>
> >>>> -#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
> >>>> +#define RTE_MBUF_INDIRECT(mb)   (mb->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>> BTW, Sergio as I said before, I think it should be:
> >>>> #define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
> >>>>
> >>>> Konstantin
> >>>>
> >>>>
> >>>>>> For RTE_MBUF_TO_BADDR(), it's relatively easy to replace it. The
> >>>>>> mbuf pool could store the size of the private size like it's done
> >>>>>> for mbp_priv->mbuf_data_room_size. Using rte_mempool_from_obj(m)
> >>>>>> or m->pool, we can retrieve the mbuf pool and this value, then
> >>>>>> compute the buffer address.
> >>>
> >>> Agreed, that makes sense.
> >>>
> >>>>>>
> >>>>>> For RTE_MBUF_FROM_BADDR(), it's more complex. We could ensure that
> >>>>>> a backpointer to the mbuf is always located before the data buffer,
> >>>>>> but it looks difficult to do.
> >>>
> >>> On the other hand, with the proposed refcnt change Sergio proposes, we no
> >>> longer use this macro in any of the built-in mbuf handling for freeing mbufs.
> >>> Does this need to be solved at anything other than the application level?
> >>
> >> It's still used in __rte_pktmbuf_prefree_seg() to retrieve the
> >> parent mbuf (direct) from the indirect mbuf beeing freed.
> >>
> > Yes, my bad.
> > How was this managed before, since refcnt field seems to be necessary in order
> > to effectively manage indirect mbufs? Is this just the case that this is something
> > that never worked and that needs to be solved, or is it something that was
> > working that this patch will now break?
> 
> This is something that never worked before: refcounts are not compatible
> with reserving private data in mbufs. This patch does not change the
> issue, it is still there.
> 
> Before the patch, an application that wanted to reserve a private
> data could disable refcounts at compile-time.
> After the patch, the solution is just to avoid using refcounts.

I'd say avoid using mbuf_attach/detach.
refcnt itself has nothing to do with that.
I finally understood what you  are talking about ...
About private data - I suppose it is a matter of another patch.
I still think it would be better to reserve private data space before mbuf, not after
(at mbuf pool initialisation time). 
Then *BADDR* macros could be unaffected.
Konstantin

> 
> Regards,
> Olivier
> 

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ]

Hi,

On 02/18/2015 11:37 AM, Bruce Richardson wrote:
>>> How was this managed before, since refcnt field seems to be necessary in order
>>> to effectively manage indirect mbufs? Is this just the case that this is something
>>> that never worked and that needs to be solved, or is it something that was
>>> working that this patch will now break?
>>
>> This is something that never worked before: refcounts are not compatible
>> with reserving private data in mbufs. This patch does not change the
>> issue, it is still there.
>>
>> Before the patch, an application that wanted to reserve a private
>> data could disable refcounts at compile-time.
>> After the patch, the solution is just to avoid using refcounts.
>>
>> Regards,
>> Olivier
>>
> Thanks for clarifying.
> So, you ok with this patch as a step in the right direction?

Yep,
Acked-by: Olivier Matz <olivier.matz@6wind.com>

Re: [dpdk-dev] [PATCH 2/2] Remove RTE_MBUF_REFCNT references

[Olivier MATZ]

Hi Konstantin,

On 02/18/2015 11:47 AM, Ananyev, Konstantin wrote:
>>> How was this managed before, since refcnt field seems to be necessary in order
>>> to effectively manage indirect mbufs? Is this just the case that this is something
>>> that never worked and that needs to be solved, or is it something that was
>>> working that this patch will now break?
>>
>> This is something that never worked before: refcounts are not compatible
>> with reserving private data in mbufs. This patch does not change the
>> issue, it is still there.
>>
>> Before the patch, an application that wanted to reserve a private
>> data could disable refcounts at compile-time.
>> After the patch, the solution is just to avoid using refcounts.
>
> I'd say avoid using mbuf_attach/detach.
> refcnt itself has nothing to do with that.
> I finally understood what you  are talking about ...
> About private data - I suppose it is a matter of another patch.
> I still think it would be better to reserve private data space before mbuf, not after
> (at mbuf pool initialisation time).
> Then *BADDR* macros could be unaffected.

Indeed that could be a good idea.


Regards,
Olivier

[dpdk-dev] [PATCH v2 0/2] Removal of RTE_MBUF_REFCNT

[Sergio Gonzalez Monroy]

This patch tries to remove the RTE_MBUF_REFCNT config options and dependencies
by introducing a new mbuf flag IND_ATTACHED_MBUF that would indicate when the mbuf
is an indirect attached mbuf, to differentiate between indirect mbufs and mbufs
with external memory buffers (ie. vhost zero copy).

Previous discussion:
http://dpdk.org/ml/archives/dev/2014-October/007127.html

Currently for mbufs with refcnt, we cannot free mbufs with external memory
buffers (ie. vhost zero copy), as they are recognized as indirect
attached mbufs and therefore we free the direct mbuf it points to,
resulting in an error in the case of external memory buffers.

We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
that the mbuf is an indirect attached mbuf pointing to another mbuf.
When we free an mbuf, we only free the direct mbuf if the flag is set.
Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
The flag is set during attach and clear on detach.

So in the case of vhost zero copy where we have mbufs with external
buffers, by default we just free the mbuf and it is up to the user to deal with
the external buffer.

v2:
 - Add missing parenthesis to RTE_MBUF_INDIRECT macro

Sergio Gonzalez Monroy (2):
  mbuf: Introduce IND_ATTACHED_MBUF flag
  Remove RTE_MBUF_REFCNT references

 app/test/test_link_bonding.c            | 15 -----------
 app/test/test_mbuf.c                    | 17 +++----------
 config/common_bsdapp                    |  1 -
 config/common_linuxapp                  |  1 -
 examples/Makefile                       |  4 +--
 examples/ip_fragmentation/Makefile      |  4 ---
 examples/ip_pipeline/Makefile           |  3 ---
 examples/ip_pipeline/main.c             |  5 ----
 examples/ipv4_multicast/Makefile        |  4 ---
 examples/vhost/main.c                   | 19 +++-----------
 lib/librte_ip_frag/Makefile             |  4 ---
 lib/librte_ip_frag/rte_ip_frag.h        |  4 ---
 lib/librte_mbuf/rte_mbuf.c              |  2 --
 lib/librte_mbuf/rte_mbuf.h              | 45 +++++++--------------------------
 lib/librte_pmd_bond/Makefile            |  4 ---
 lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
 lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
 lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 --------
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 ------
 lib/librte_port/Makefile                |  4 ---
 20 files changed, 19 insertions(+), 139 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH v2 1/2] mbuf: Introduce IND_ATTACHED_MBUF flag

[Sergio Gonzalez Monroy]

Currently for mbufs with refcnt, we cannot free mbufs with external memory
buffers (ie. vhost zero copy), as they are recognized as indirect
attached mbufs and therefore we free the direct mbuf it points to,
resulting in an error in the case of external memory buffers.

We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
that the mbuf is an indirect attached mbuf pointing to another mbuf.
When we free an mbuf, we only free the direct mbuf if the flag is set.
Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
The flag is set during attach and clear on detach.

So in the case of vhost zero copy where we have mbufs with external
buffers, by default we just free the mbuf and it is up to the user to deal with
the external buffer.

This patch would allow the removal of the RTE_MBUF_REFCNT config option,
setting refcnt for all mbufs permanently.

The patch also modifies the vhost example as it was using the
RTE_MBUF_INDERECT macro to detect if it was an mbuf with external buffer.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com> 
---
v2:
 - Add missing parenthesis to RTE_MBUF_INDIRECT macro

 examples/vhost/main.c      |  6 ++++--
 lib/librte_mbuf/rte_mbuf.h | 15 +++++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 3a35359..5e341d6 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -139,6 +139,8 @@
 /* Number of descriptors per cacheline. */
 #define DESC_PER_CACHELINE (RTE_CACHE_LINE_SIZE / sizeof(struct vring_desc))
 
+#define MBUF_EXT_MEM(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+
 /* mask of enabled ports */
 static uint32_t enabled_port_mask = 0;
 
@@ -1567,7 +1569,7 @@ txmbuf_clean_zcp(struct virtio_net *dev, struct vpool *vpool)
 
 	for (index = 0; index < mbuf_count; index++) {
 		mbuf = __rte_mbuf_raw_alloc(vpool->pool);
-		if (likely(RTE_MBUF_INDIRECT(mbuf)))
+		if (likely(MBUF_EXT_MEM(mbuf)))
 			pktmbuf_detach_zcp(mbuf);
 		rte_ring_sp_enqueue(vpool->ring, mbuf);
 
@@ -1630,7 +1632,7 @@ static void mbuf_destroy_zcp(struct vpool *vpool)
 	for (index = 0; index < mbuf_count; index++) {
 		mbuf = __rte_mbuf_raw_alloc(vpool->pool);
 		if (likely(mbuf != NULL)) {
-			if (likely(RTE_MBUF_INDIRECT(mbuf)))
+			if (likely(MBUF_EXT_MEM(mbuf)))
 				pktmbuf_detach_zcp(mbuf);
 			rte_ring_sp_enqueue(vpool->ring, (void *)mbuf);
 		}
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 16059c6..1e5aa1f 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -162,6 +162,8 @@ extern "C" {
 /** Tell the NIC it's an outer IPv6 packet for tunneling packet */
 #define PKT_TX_OUTER_IPV6    (1ULL << 60)
 
+#define IND_ATTACHED_MBUF    (1ULL << 62) /**< Indirect attached mbuf */
+
 /* Use final bit of flags to indicate a control mbuf */
 #define CTRL_MBUF_FLAG       (1ULL << 63) /**< Mbuf contains control data */
 
@@ -305,13 +307,12 @@ struct rte_mbuf {
 /**
  * Returns TRUE if given mbuf is indirect, or FALSE otherwise.
  */
-#define RTE_MBUF_INDIRECT(mb)   (RTE_MBUF_FROM_BADDR((mb)->buf_addr) != (mb))
+#define RTE_MBUF_INDIRECT(mb)   ((mb)->ol_flags & IND_ATTACHED_MBUF)
 
 /**
  * Returns TRUE if given mbuf is direct, or FALSE otherwise.
  */
-#define RTE_MBUF_DIRECT(mb)     (RTE_MBUF_FROM_BADDR((mb)->buf_addr) == (mb))
-
+#define RTE_MBUF_DIRECT(mb)     (!RTE_MBUF_INDIRECT(mb))
 
 /**
  * Private data in case of pktmbuf pool.
@@ -713,7 +714,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *md)
 	mi->next = NULL;
 	mi->pkt_len = mi->data_len;
 	mi->nb_segs = 1;
-	mi->ol_flags = md->ol_flags;
+	mi->ol_flags = md->ol_flags | IND_ATTACHED_MBUF;
 	mi->packet_type = md->packet_type;
 
 	__rte_mbuf_sanity_check(mi, 1);
@@ -744,6 +745,8 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
 			RTE_PKTMBUF_HEADROOM : m->buf_len;
 
 	m->data_len = 0;
+
+	m->ol_flags = 0;
 }
 
 #endif /* RTE_MBUF_REFCNT */
@@ -757,7 +760,6 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 #ifdef RTE_MBUF_REFCNT
 	if (likely (rte_mbuf_refcnt_read(m) == 1) ||
 			likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
-		struct rte_mbuf *md = RTE_MBUF_FROM_BADDR(m->buf_addr);
 
 		rte_mbuf_refcnt_set(m, 0);
 
@@ -765,7 +767,8 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 		 *  - detach mbuf
 		 *  - free attached mbuf segment
 		 */
-		if (unlikely (md != m)) {
+		if (RTE_MBUF_INDIRECT(m)) {
+			struct rte_mbuf *md = RTE_MBUF_FROM_BADDR(m->buf_addr);
 			rte_pktmbuf_detach(m);
 			if (rte_mbuf_refcnt_update(md, -1) == 0)
 				__rte_mbuf_raw_free(md);
-- 
1.9.3

[dpdk-dev] [PATCH v2 2/2] Remove RTE_MBUF_REFCNT references

[Sergio Gonzalez Monroy]

This patch removes all references to RTE_MBUF_REFCNT, setting the refcnt
field in the mbuf struct permanently.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Acked-by: Olivier Matz <olivier.matz@6wind.com> 
---
 app/test/test_link_bonding.c            | 15 ---------------
 app/test/test_mbuf.c                    | 17 ++++-------------
 config/common_bsdapp                    |  1 -
 config/common_linuxapp                  |  1 -
 examples/Makefile                       |  4 ++--
 examples/ip_fragmentation/Makefile      |  4 ----
 examples/ip_pipeline/Makefile           |  3 ---
 examples/ip_pipeline/main.c             |  5 -----
 examples/ipv4_multicast/Makefile        |  4 ----
 examples/vhost/main.c                   | 13 -------------
 lib/librte_ip_frag/Makefile             |  4 ----
 lib/librte_ip_frag/rte_ip_frag.h        |  4 ----
 lib/librte_mbuf/rte_mbuf.c              |  2 --
 lib/librte_mbuf/rte_mbuf.h              | 30 ------------------------------
 lib/librte_pmd_bond/Makefile            |  4 ----
 lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
 lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
 lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 ----------
 lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 --------
 lib/librte_port/Makefile                |  4 ----
 20 files changed, 6 insertions(+), 131 deletions(-)

diff --git a/app/test/test_link_bonding.c b/app/test/test_link_bonding.c
index 579ebbf..54895ab 100644
--- a/app/test/test_link_bonding.c
+++ b/app/test/test_link_bonding.c
@@ -708,9 +708,7 @@ test_set_bonding_mode(void)
 	int bonding_modes[] = { BONDING_MODE_ROUND_ROBIN,
 							BONDING_MODE_ACTIVE_BACKUP,
 							BONDING_MODE_BALANCE,
-#ifdef RTE_MBUF_REFCNT
 							BONDING_MODE_BROADCAST
-#endif
 							};
 
 	/* Test supported link bonding modes */
@@ -1425,7 +1423,6 @@ test_roundrobin_tx_burst(void)
 	return remove_slaves_and_stop_bonded_device();
 }
 
-#ifdef RTE_MBUF_REFCNT
 static int
 verify_mbufs_ref_count(struct rte_mbuf **mbufs, int nb_mbufs, int val)
 {
@@ -1439,8 +1436,6 @@ verify_mbufs_ref_count(struct rte_mbuf **mbufs, int nb_mbufs, int val)
 	}
 	return 0;
 }
-#endif
-
 
 static void
 free_mbufs(struct rte_mbuf **mbufs, int nb_mbufs)
@@ -1545,12 +1540,10 @@ test_roundrobin_tx_burst_slave_tx_fail(void)
 				(unsigned int)port_stats.opackets, slave_expected_tx_count);
 	}
 
-#ifdef RTE_MBUF_REFCNT
 	/* Verify that all mbufs have a ref value of zero */
 	TEST_ASSERT_SUCCESS(verify_mbufs_ref_count(&pkt_burst[tx_count],
 			TEST_RR_SLAVE_TX_FAIL_PACKETS_COUNT, 1),
 			"mbufs refcnts not as expected");
-#endif
 	free_mbufs(&pkt_burst[tx_count], TEST_RR_SLAVE_TX_FAIL_PACKETS_COUNT);
 
 	/* Clean up and remove slaves from bonded device */
@@ -3056,12 +3049,10 @@ test_balance_tx_burst_slave_tx_fail(void)
 				(unsigned int)port_stats.opackets,
 				TEST_BAL_SLAVE_TX_FAIL_BURST_SIZE_2);
 
-#ifdef RTE_MBUF_REFCNT
 	/* Verify that all mbufs have a ref value of zero */
 	TEST_ASSERT_SUCCESS(verify_mbufs_ref_count(&pkts_burst_1[tx_count_1],
 			TEST_BAL_SLAVE_TX_FAIL_PACKETS_COUNT, 1),
 			"mbufs refcnts not as expected");
-#endif
 
 	free_mbufs(&pkts_burst_1[tx_count_1],
 			TEST_BAL_SLAVE_TX_FAIL_PACKETS_COUNT);
@@ -3472,9 +3463,6 @@ test_balance_verify_slave_link_status_change_behaviour(void)
 	return remove_slaves_and_stop_bonded_device();
 }
 
-#ifdef RTE_MBUF_REFCNT
-/** Broadcast Mode Tests */
-
 static int
 test_broadcast_tx_burst(void)
 {
@@ -4001,7 +3989,6 @@ test_broadcast_verify_slave_link_status_change_behaviour(void)
 	/* Clean up and remove slaves from bonded device */
 	return remove_slaves_and_stop_bonded_device();
 }
-#endif
 
 static int
 test_reconfigure_bonded_device(void)
@@ -4592,14 +4579,12 @@ static struct unit_test_suite link_bonding_test_suite  = {
 		TEST_CASE(test_tlb_verify_mac_assignment),
 		TEST_CASE(test_tlb_verify_promiscuous_enable_disable),
 		TEST_CASE(test_tlb_verify_slave_link_status_change_failover),
-#ifdef RTE_MBUF_REFCNT
 		TEST_CASE(test_broadcast_tx_burst),
 		TEST_CASE(test_broadcast_tx_burst_slave_tx_fail),
 		TEST_CASE(test_broadcast_rx_burst),
 		TEST_CASE(test_broadcast_verify_promiscuous_enable_disable),
 		TEST_CASE(test_broadcast_verify_mac_assignment),
 		TEST_CASE(test_broadcast_verify_slave_link_status_change_behaviour),
-#endif
 		TEST_CASE(test_reconfigure_bonded_device),
 		TEST_CASE(test_close_bonded_device),
 
diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index e86ba22..9de6dea 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -81,7 +81,7 @@
 
 static struct rte_mempool *pktmbuf_pool = NULL;
 
-#if defined RTE_MBUF_REFCNT  && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
 
 static struct rte_mempool *refcnt_pool = NULL;
 static struct rte_ring *refcnt_mbuf_ring = NULL;
@@ -322,9 +322,6 @@ fail:
 static int
 testclone_testupdate_testdetach(void)
 {
-#ifndef RTE_MBUF_REFCNT
-	return 0;
-#else
 	struct rte_mbuf *mc = NULL;
 	struct rte_mbuf *clone = NULL;
 
@@ -363,7 +360,6 @@ fail:
 	if (mc)
 		rte_pktmbuf_free(mc);
 	return -1;
-#endif /* RTE_MBUF_REFCNT */
 }
 #undef GOTO_FAIL
 
@@ -396,13 +392,11 @@ test_pktmbuf_pool(void)
 		printf("Error pool not empty");
 		ret = -1;
 	}
-#ifdef RTE_MBUF_REFCNT
 	extra = rte_pktmbuf_clone(m[0], pktmbuf_pool);
 	if(extra != NULL) {
 		printf("Error pool not empty");
 		ret = -1;
 	}
-#endif
 	/* free them */
 	for (i=0; i<NB_MBUF; i++) {
 		if (m[i] != NULL)
@@ -504,12 +498,11 @@ test_pktmbuf_free_segment(void)
 
 /*
  * Stress test for rte_mbuf atomic refcnt.
- * Implies that:
- * RTE_MBUF_REFCNT and RTE_MBUF_REFCNT_ATOMIC are both defined.
+ * Implies that RTE_MBUF_REFCNT_ATOMIC is defined.
  * For more efficency, recomended to run with RTE_LIBRTE_MBUF_DEBUG defined.
  */
 
-#if defined RTE_MBUF_REFCNT  && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
 
 static int
 test_refcnt_slave(__attribute__((unused)) void *arg)
@@ -614,7 +607,7 @@ test_refcnt_master(void)
 static int
 test_refcnt_mbuf(void)
 {
-#if defined RTE_MBUF_REFCNT  && defined RTE_MBUF_REFCNT_ATOMIC
+#ifdef RTE_MBUF_REFCNT_ATOMIC
 
 	unsigned lnum, master, slave, tref;
 
@@ -747,7 +740,6 @@ test_failing_mbuf_sanity_check(void)
 		return -1;
 	}
 
-#ifdef RTE_MBUF_REFCNT
 	badbuf = *buf;
 	badbuf.refcnt = 0;
 	if (verify_mbuf_check_panics(&badbuf)) {
@@ -761,7 +753,6 @@ test_failing_mbuf_sanity_check(void)
 		printf("Error with bad-refcnt(MAX) mbuf test\n");
 		return -1;
 	}
-#endif
 
 	return 0;
 }
diff --git a/config/common_bsdapp b/config/common_bsdapp
index 57bacb8..ad2d3cb 100644
--- a/config/common_bsdapp
+++ b/config/common_bsdapp
@@ -247,7 +247,6 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 #
 CONFIG_RTE_LIBRTE_MBUF=y
 CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
-CONFIG_RTE_MBUF_REFCNT=y
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
 
diff --git a/config/common_linuxapp b/config/common_linuxapp
index d428f84..5af645f 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -255,7 +255,6 @@ CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 #
 CONFIG_RTE_LIBRTE_MBUF=y
 CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
-CONFIG_RTE_MBUF_REFCNT=y
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
 
diff --git a/examples/Makefile b/examples/Makefile
index 81f1d2f..5a784d4 100644
--- a/examples/Makefile
+++ b/examples/Makefile
@@ -46,8 +46,8 @@ DIRS-y += exception_path
 DIRS-y += helloworld
 DIRS-y += ip_pipeline
 DIRS-y += ip_reassembly
-DIRS-$(CONFIG_RTE_MBUF_REFCNT) += ip_fragmentation
-DIRS-$(CONFIG_RTE_MBUF_REFCNT) += ipv4_multicast
+DIRS-$(CONFIG_RTE_IP_FRAG) += ip_fragmentation
+DIRS-y += ipv4_multicast
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += kni
 DIRS-y += l2fwd
 DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += l2fwd-ivshmem
diff --git a/examples/ip_fragmentation/Makefile b/examples/ip_fragmentation/Makefile
index ea0e11f..c321e6a 100644
--- a/examples/ip_fragmentation/Makefile
+++ b/examples/ip_fragmentation/Makefile
@@ -39,10 +39,6 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
-ifneq ($(CONFIG_RTE_MBUF_REFCNT),y)
-$(error This application requires RTE_MBUF_REFCNT to be enabled)
-endif
-
 # binary name
 APP = ip_fragmentation
 
diff --git a/examples/ip_pipeline/Makefile b/examples/ip_pipeline/Makefile
index b0a4106..e70fdc7 100644
--- a/examples/ip_pipeline/Makefile
+++ b/examples/ip_pipeline/Makefile
@@ -51,11 +51,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_tx.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_flow_classification.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_routing.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_passthrough.c
-
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_ipv4_frag.c
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_ipv4_ras.c
-endif
 
 ifeq ($(CONFIG_RTE_LIBRTE_ACL),y)
 SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += pipeline_firewall.c
diff --git a/examples/ip_pipeline/main.c b/examples/ip_pipeline/main.c
index 2c53877..7eee187 100644
--- a/examples/ip_pipeline/main.c
+++ b/examples/ip_pipeline/main.c
@@ -148,17 +148,12 @@ app_lcore_main_loop(__attribute__((unused)) void *arg)
 			rte_exit(EXIT_FAILURE, "ACL not present in build\n");
 #endif
 
-#ifdef RTE_MBUF_REFCNT
 		case APP_CORE_IPV4_FRAG:
 			app_main_loop_pipeline_ipv4_frag();
 			return 0;
 		case APP_CORE_IPV4_RAS:
 			app_main_loop_pipeline_ipv4_ras();
 			return 0;
-#else
-			rte_exit(EXIT_FAILURE,
-				"mbuf chaining not present in build\n");
-#endif
 
 		default:
 			rte_panic("%s: Invalid core type for core %u\n",
diff --git a/examples/ipv4_multicast/Makefile b/examples/ipv4_multicast/Makefile
index 604cebe..44f0a3b 100644
--- a/examples/ipv4_multicast/Makefile
+++ b/examples/ipv4_multicast/Makefile
@@ -39,10 +39,6 @@ RTE_TARGET ?= x86_64-native-linuxapp-gcc
 
 include $(RTE_SDK)/mk/rte.vars.mk
 
-ifneq ($(CONFIG_RTE_MBUF_REFCNT),y)
-$(error This application requires RTE_MBUF_REFCNT to be enabled)
-endif
-
 # binary name
 APP = ipv4_multicast
 
diff --git a/examples/vhost/main.c b/examples/vhost/main.c
index 5e341d6..592bc28 100644
--- a/examples/vhost/main.c
+++ b/examples/vhost/main.c
@@ -726,19 +726,6 @@ us_vhost_parse_args(int argc, char **argv)
 					return -1;
 				} else
 					zero_copy = ret;
-
-				if (zero_copy) {
-#ifdef RTE_MBUF_REFCNT
-					RTE_LOG(ERR, VHOST_CONFIG, "Before running "
-					"zero copy vhost APP, please "
-					"disable RTE_MBUF_REFCNT\n"
-					"in config file and then rebuild DPDK "
-					"core lib!\n"
-					"Otherwise please disable zero copy "
-					"flag in command line!\n");
-					return -1;
-#endif
-				}
 			}
 
 			/* Specify the descriptor number on RX. */
diff --git a/lib/librte_ip_frag/Makefile b/lib/librte_ip_frag/Makefile
index fe926f7..9d06780 100644
--- a/lib/librte_ip_frag/Makefile
+++ b/lib/librte_ip_frag/Makefile
@@ -42,12 +42,8 @@ EXPORT_MAP := rte_ipfrag_version.map
 LIBABIVER := 1
 
 #source files
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_fragmentation.c
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_fragmentation.c
-else
-$(info WARNING: Fragmentation feature is disabled because it needs MBUF_REFCNT.)
-endif
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv4_reassembly.c
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ipv6_reassembly.c
 SRCS-$(CONFIG_RTE_LIBRTE_IP_FRAG) += rte_ip_frag_common.c
diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
index 1083d44..f673728 100644
--- a/lib/librte_ip_frag/rte_ip_frag.h
+++ b/lib/librte_ip_frag/rte_ip_frag.h
@@ -180,7 +180,6 @@ rte_ip_frag_table_destroy( struct rte_ip_frag_tbl *tbl)
 	rte_free(tbl);
 }
 
-#ifdef RTE_MBUF_REFCNT
 /**
  * This function implements the fragmentation of IPv6 packets.
  *
@@ -209,7 +208,6 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
 		uint16_t mtu_size,
 		struct rte_mempool *pool_direct,
 		struct rte_mempool *pool_indirect);
-#endif
 
 /*
  * This function implements reassembly of fragmented IPv6 packets.
@@ -258,7 +256,6 @@ rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
 		return NULL;
 }
 
-#ifdef RTE_MBUF_REFCNT
 /**
  * IPv4 fragmentation.
  *
@@ -287,7 +284,6 @@ int32_t rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in,
 			uint16_t nb_pkts_out, uint16_t mtu_size,
 			struct rte_mempool *pool_direct,
 			struct rte_mempool *pool_indirect);
-#endif
 
 /*
  * This function implements reassembly of fragmented IPv4 packets.
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 1b14e02..470e3c2 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -146,11 +146,9 @@ rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
 	if (m->buf_addr == NULL)
 		rte_panic("bad virt addr\n");
 
-#ifdef RTE_MBUF_REFCNT
 	uint16_t cnt = rte_mbuf_refcnt_read(m);
 	if ((cnt == 0) || (cnt == UINT16_MAX))
 		rte_panic("bad ref cnt\n");
-#endif
 
 	/* nothing to check for sub-segments */
 	if (is_header == 0)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 1e5aa1f..6a3acfb 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -220,11 +220,8 @@ struct rte_mbuf {
 	 * config option.
 	 */
 	union {
-#ifdef RTE_MBUF_REFCNT
 		rte_atomic16_t refcnt_atomic; /**< Atomically accessed refcnt */
 		uint16_t refcnt;              /**< Non-atomically accessed refcnt */
-#endif
-		uint16_t refcnt_reserved;     /**< Do not use this field */
 	};
 	uint8_t nb_segs;          /**< Number of segments. */
 	uint8_t port;             /**< Input port. */
@@ -354,7 +351,6 @@ if (!(exp)) {                                                        \
 
 #endif /*  RTE_LIBRTE_MBUF_DEBUG */
 
-#ifdef RTE_MBUF_REFCNT
 #ifdef RTE_MBUF_REFCNT_ATOMIC
 
 /**
@@ -436,15 +432,6 @@ rte_mbuf_refcnt_set(struct rte_mbuf *m, uint16_t new_value)
 		rte_prefetch0(m);               \
 } while (0)
 
-#else /* ! RTE_MBUF_REFCNT */
-
-/** Mbuf prefetch */
-#define RTE_MBUF_PREFETCH_TO_FREE(m) do { } while(0)
-
-#define rte_mbuf_refcnt_set(m,v) do { } while(0)
-
-#endif /* RTE_MBUF_REFCNT */
-
 
 /**
  * Sanity checks on an mbuf.
@@ -479,10 +466,8 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
 	if (rte_mempool_get(mp, &mb) < 0)
 		return NULL;
 	m = (struct rte_mbuf *)mb;
-#ifdef RTE_MBUF_REFCNT
 	RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
 	rte_mbuf_refcnt_set(m, 1);
-#endif /* RTE_MBUF_REFCNT */
 	return (m);
 }
 
@@ -497,9 +482,7 @@ static inline struct rte_mbuf *__rte_mbuf_raw_alloc(struct rte_mempool *mp)
 static inline void __attribute__((always_inline))
 __rte_mbuf_raw_free(struct rte_mbuf *m)
 {
-#ifdef RTE_MBUF_REFCNT
 	RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(m) == 0);
-#endif /* RTE_MBUF_REFCNT */
 	rte_mempool_put(m->pool, m);
 }
 
@@ -674,8 +657,6 @@ static inline struct rte_mbuf *rte_pktmbuf_alloc(struct rte_mempool *mp)
 	return (m);
 }
 
-#ifdef RTE_MBUF_REFCNT
-
 /**
  * Attach packet mbuf to another packet mbuf.
  * After attachment we refer the mbuf we attached as 'indirect',
@@ -749,15 +730,11 @@ static inline void rte_pktmbuf_detach(struct rte_mbuf *m)
 	m->ol_flags = 0;
 }
 
-#endif /* RTE_MBUF_REFCNT */
-
-
 static inline struct rte_mbuf* __attribute__((always_inline))
 __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 {
 	__rte_mbuf_sanity_check(m, 0);
 
-#ifdef RTE_MBUF_REFCNT
 	if (likely (rte_mbuf_refcnt_read(m) == 1) ||
 			likely (rte_mbuf_refcnt_update(m, -1) == 0)) {
 
@@ -773,12 +750,9 @@ __rte_pktmbuf_prefree_seg(struct rte_mbuf *m)
 			if (rte_mbuf_refcnt_update(md, -1) == 0)
 				__rte_mbuf_raw_free(md);
 		}
-#endif
 		return(m);
-#ifdef RTE_MBUF_REFCNT
 	}
 	return (NULL);
-#endif
 }
 
 /**
@@ -821,8 +795,6 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m)
 	}
 }
 
-#ifdef RTE_MBUF_REFCNT
-
 /**
  * Creates a "clone" of the given packet mbuf.
  *
@@ -897,8 +869,6 @@ static inline void rte_pktmbuf_refcnt_update(struct rte_mbuf *m, int16_t v)
 	} while ((m = m->next) != NULL);
 }
 
-#endif /* RTE_MBUF_REFCNT */
-
 /**
  * Get the headroom in a packet mbuf.
  *
diff --git a/lib/librte_pmd_bond/Makefile b/lib/librte_pmd_bond/Makefile
index d6c81a8..e9d94e1 100644
--- a/lib/librte_pmd_bond/Makefile
+++ b/lib/librte_pmd_bond/Makefile
@@ -51,10 +51,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_pmd.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_args.c
 SRCS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += rte_eth_bond_8023ad.c
 
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),n)
-$(info WARNING: Link Bonding Broadcast mode is disabled because it needs MBUF_REFCNT.)
-endif
-
 #
 # Export include files
 #
diff --git a/lib/librte_pmd_bond/rte_eth_bond.h b/lib/librte_pmd_bond/rte_eth_bond.h
index 7177983..99568d4 100644
--- a/lib/librte_pmd_bond/rte_eth_bond.h
+++ b/lib/librte_pmd_bond/rte_eth_bond.h
@@ -71,12 +71,10 @@ extern "C" {
  * slaves using one of three available transmit policies - l2, l2+3 or l3+4.
  * See BALANCE_XMIT_POLICY macros definitions for further details on transmit
  * policies. */
-#ifdef RTE_MBUF_REFCNT
 #define BONDING_MODE_BROADCAST			(3)
 /**< Broadcast (Mode 3).
  * In this mode all transmitted packets will be transmitted on all available
  * active slaves of the bonded. */
-#endif
 #define BONDING_MODE_8023AD				(4)
 /**< 802.3AD (Mode 4).
  *
diff --git a/lib/librte_pmd_bond/rte_eth_bond_args.c b/lib/librte_pmd_bond/rte_eth_bond_args.c
index ca4de38..ae76bc6 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_args.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_args.c
@@ -170,9 +170,7 @@ bond_ethdev_parse_slave_mode_kvarg(const char *key __rte_unused,
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_ACTIVE_BACKUP:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 	case BONDING_MODE_8023AD:
 	case BONDING_MODE_ADAPTIVE_TRANSMIT_LOAD_BALANCING:
 		return 0;
diff --git a/lib/librte_pmd_bond/rte_eth_bond_pmd.c b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
index 09b0f30..b90bf48 100644
--- a/lib/librte_pmd_bond/rte_eth_bond_pmd.c
+++ b/lib/librte_pmd_bond/rte_eth_bond_pmd.c
@@ -681,7 +681,6 @@ bond_ethdev_tx_burst_8023ad(void *queue, struct rte_mbuf **bufs,
 	return num_tx_total;
 }
 
-#ifdef RTE_MBUF_REFCNT
 static uint16_t
 bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
 		uint16_t nb_pkts)
@@ -741,7 +740,6 @@ bond_ethdev_tx_burst_broadcast(void *queue, struct rte_mbuf **bufs,
 
 	return max_nb_of_tx_pkts;
 }
-#endif
 
 void
 link_properties_set(struct rte_eth_dev *bonded_eth_dev,
@@ -839,9 +837,7 @@ mac_address_slaves_update(struct rte_eth_dev *bonded_eth_dev)
 	switch (internals->mode) {
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 		for (i = 0; i < internals->slave_count; i++) {
 			if (mac_address_set(&rte_eth_devices[internals->slaves[i].port_id],
 					bonded_eth_dev->data->mac_addrs)) {
@@ -901,12 +897,10 @@ bond_ethdev_mode_set(struct rte_eth_dev *eth_dev, int mode)
 		eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_balance;
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
 		break;
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
 		eth_dev->tx_pkt_burst = bond_ethdev_tx_burst_broadcast;
 		eth_dev->rx_pkt_burst = bond_ethdev_rx_burst;
 		break;
-#endif
 	case BONDING_MODE_8023AD:
 		if (bond_mode_8023ad_enable(eth_dev) != 0)
 			return -1;
@@ -1410,9 +1404,7 @@ bond_ethdev_promiscuous_enable(struct rte_eth_dev *eth_dev)
 	/* Promiscuous mode is propagated to all slaves */
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 		for (i = 0; i < internals->slave_count; i++)
 			rte_eth_promiscuous_enable(internals->slaves[i].port_id);
 		break;
@@ -1439,9 +1431,7 @@ bond_ethdev_promiscuous_disable(struct rte_eth_dev *dev)
 	/* Promiscuous mode is propagated to all slaves */
 	case BONDING_MODE_ROUND_ROBIN:
 	case BONDING_MODE_BALANCE:
-#ifdef RTE_MBUF_REFCNT
 	case BONDING_MODE_BROADCAST:
-#endif
 		for (i = 0; i < internals->slave_count; i++)
 			rte_eth_promiscuous_disable(internals->slaves[i].port_id);
 		break;
diff --git a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
index b54cb19..8c95bd3 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c
@@ -540,20 +540,12 @@ ixgbe_tx_free_bufs(struct igb_tx_queue *txq)
 	 */
 	txep = &((struct igb_tx_entry_v *)txq->sw_ring)[txq->tx_next_dd -
 			(n - 1)];
-#ifdef RTE_MBUF_REFCNT
 	m = __rte_pktmbuf_prefree_seg(txep[0].mbuf);
-#else
-	m = txep[0].mbuf;
-#endif
 	if (likely(m != NULL)) {
 		free[0] = m;
 		nb_free = 1;
 		for (i = 1; i < n; i++) {
-#ifdef RTE_MBUF_REFCNT
 			m = __rte_pktmbuf_prefree_seg(txep[i].mbuf);
-#else
-			m = txep[i].mbuf;
-#endif
 			if (likely(m != NULL)) {
 				if (likely(m->pool == free[0]->pool))
 					free[nb_free++] = m;
diff --git a/lib/librte_port/Makefile b/lib/librte_port/Makefile
index 0e38452..de960fc 100644
--- a/lib/librte_port/Makefile
+++ b/lib/librte_port/Makefile
@@ -49,9 +49,7 @@ LIBABIVER := 1
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ethdev.c
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ring.c
 ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_frag.c
-endif
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_ras.c
 endif
 SRCS-$(CONFIG_RTE_LIBRTE_PORT) += rte_port_sched.c
@@ -62,9 +60,7 @@ SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ethdev.h
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ring.h
 ifeq ($(CONFIG_RTE_LIBRTE_IP_FRAG),y)
-ifeq ($(CONFIG_RTE_MBUF_REFCNT),y)
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_frag.h
-endif
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_ras.h
 endif
 SYMLINK-$(CONFIG_RTE_LIBRTE_PORT)-include += rte_port_sched.h
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2 0/2] Removal of RTE_MBUF_REFCNT

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Sergio Gonzalez Monroy
> Sent: Wednesday, February 18, 2015 11:03 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 0/2] Removal of RTE_MBUF_REFCNT
> 
> This patch tries to remove the RTE_MBUF_REFCNT config options and dependencies
> by introducing a new mbuf flag IND_ATTACHED_MBUF that would indicate when the mbuf
> is an indirect attached mbuf, to differentiate between indirect mbufs and mbufs
> with external memory buffers (ie. vhost zero copy).
> 
> Previous discussion:
> http://dpdk.org/ml/archives/dev/2014-October/007127.html
> 
> Currently for mbufs with refcnt, we cannot free mbufs with external memory
> buffers (ie. vhost zero copy), as they are recognized as indirect
> attached mbufs and therefore we free the direct mbuf it points to,
> resulting in an error in the case of external memory buffers.
> 
> We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
> that the mbuf is an indirect attached mbuf pointing to another mbuf.
> When we free an mbuf, we only free the direct mbuf if the flag is set.
> Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
> The flag is set during attach and clear on detach.
> 
> So in the case of vhost zero copy where we have mbufs with external
> buffers, by default we just free the mbuf and it is up to the user to deal with
> the external buffer.
> 
> v2:
>  - Add missing parenthesis to RTE_MBUF_INDIRECT macro
> 
> Sergio Gonzalez Monroy (2):
>   mbuf: Introduce IND_ATTACHED_MBUF flag
>   Remove RTE_MBUF_REFCNT references
> 
>  app/test/test_link_bonding.c            | 15 -----------
>  app/test/test_mbuf.c                    | 17 +++----------
>  config/common_bsdapp                    |  1 -
>  config/common_linuxapp                  |  1 -
>  examples/Makefile                       |  4 +--
>  examples/ip_fragmentation/Makefile      |  4 ---
>  examples/ip_pipeline/Makefile           |  3 ---
>  examples/ip_pipeline/main.c             |  5 ----
>  examples/ipv4_multicast/Makefile        |  4 ---
>  examples/vhost/main.c                   | 19 +++-----------
>  lib/librte_ip_frag/Makefile             |  4 ---
>  lib/librte_ip_frag/rte_ip_frag.h        |  4 ---
>  lib/librte_mbuf/rte_mbuf.c              |  2 --
>  lib/librte_mbuf/rte_mbuf.h              | 45 +++++++--------------------------
>  lib/librte_pmd_bond/Makefile            |  4 ---
>  lib/librte_pmd_bond/rte_eth_bond.h      |  2 --
>  lib/librte_pmd_bond/rte_eth_bond_args.c |  2 --
>  lib/librte_pmd_bond/rte_eth_bond_pmd.c  | 10 --------
>  lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c   |  8 ------
>  lib/librte_port/Makefile                |  4 ---
>  20 files changed, 19 insertions(+), 139 deletions(-)
> 

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> --
> 1.9.3

Re: [dpdk-dev] [PATCH v2 0/2] Removal of RTE_MBUF_REFCNT

[Thomas Monjalon]

> > This patch tries to remove the RTE_MBUF_REFCNT config options and dependencies
> > by introducing a new mbuf flag IND_ATTACHED_MBUF that would indicate when the mbuf
> > is an indirect attached mbuf, to differentiate between indirect mbufs and mbufs
> > with external memory buffers (ie. vhost zero copy).
> > 
> > Previous discussion:
> > http://dpdk.org/ml/archives/dev/2014-October/007127.html
> > 
> > Currently for mbufs with refcnt, we cannot free mbufs with external memory
> > buffers (ie. vhost zero copy), as they are recognized as indirect
> > attached mbufs and therefore we free the direct mbuf it points to,
> > resulting in an error in the case of external memory buffers.
> > 
> > We solve the issue by introducing the IND_ATTACHED_MBUF flag, which indicates
> > that the mbuf is an indirect attached mbuf pointing to another mbuf.
> > When we free an mbuf, we only free the direct mbuf if the flag is set.
> > Freeing an mbuf with external buffer is the same as freeing a non attached mbuf.
> > The flag is set during attach and clear on detach.
> > 
> > So in the case of vhost zero copy where we have mbufs with external
> > buffers, by default we just free the mbuf and it is up to the user to deal with
> > the external buffer.
> > 
> > v2:
> >  - Add missing parenthesis to RTE_MBUF_INDIRECT macro
> > 
> > Sergio Gonzalez Monroy (2):
> >   mbuf: Introduce IND_ATTACHED_MBUF flag
> >   Remove RTE_MBUF_REFCNT references
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Applied, thanks