diff mbox series

[v4,13/15] examples/bbdev: switch to dynamic mbuf field

Message ID 20201028102640.3191964-14-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series remove mbuf userdata |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon Oct. 28, 2020, 10:26 a.m. UTC 
  The example used the deprecated mbuf field udata64 as input mbuf pointer.
It is moved to a dynamic field in order to allow removal of udata64.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 examples/bbdev_app/main.c | 49 +++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 10 deletions(-)

Comments

Andrew Rybchenko Oct. 28, 2020, 11:51 a.m. UTC | #1 
On 10/28/20 1:26 PM, Thomas Monjalon wrote:
> The example used the deprecated mbuf field udata64 as input mbuf pointer.
> It is moved to a dynamic field in order to allow removal of udata64.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

I like the approach with inline function(s) more than approach
with macros. Not sure that get/set is needed. Approach in
David's patches looks a bit simpler to me - less duplicated
code (i.e. just one function which returns pointer of correct
type to a dynamic field and allow to get and set it).
Thomas Monjalon Oct. 28, 2020, 12:21 p.m. UTC | #2 
28/10/2020 12:51, Andrew Rybchenko:
> On 10/28/20 1:26 PM, Thomas Monjalon wrote:
> > The example used the deprecated mbuf field udata64 as input mbuf pointer.
> > It is moved to a dynamic field in order to allow removal of udata64.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> I like the approach with inline function(s) more than approach
> with macros. Not sure that get/set is needed. Approach in
> David's patches looks a bit simpler to me - less duplicated
> code (i.e. just one function which returns pointer of correct
> type to a dynamic field and allow to get and set it).

Yes I tend to agree with you.
Olivier was asking for more functions,
especially if it has a semantic meaning.
Here get/set has no real added value but I don't have strong opinion,
and I don't think it deserves a respin :)
Andrew Rybchenko Oct. 28, 2020, 12:55 p.m. UTC | #3 
On 10/28/20 3:21 PM, Thomas Monjalon wrote:
> 28/10/2020 12:51, Andrew Rybchenko:
>> On 10/28/20 1:26 PM, Thomas Monjalon wrote:
>>> The example used the deprecated mbuf field udata64 as input mbuf pointer.
>>> It is moved to a dynamic field in order to allow removal of udata64.
>>>
>>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>
>> I like the approach with inline function(s) more than approach
>> with macros. Not sure that get/set is needed. Approach in
>> David's patches looks a bit simpler to me - less duplicated
>> code (i.e. just one function which returns pointer of correct
>> type to a dynamic field and allow to get and set it).
> 
> Yes I tend to agree with you.
> Olivier was asking for more functions,
> especially if it has a semantic meaning.
> Here get/set has no real added value but I don't have strong opinion,
> and I don't think it deserves a respin :)
> 

Agreed.
diff mbox series

Patch

diff --git a/examples/bbdev_app/main.c b/examples/bbdev_app/main.c
index e512c807cd..63e8b18e44 100644
--- a/examples/bbdev_app/main.c
+++ b/examples/bbdev_app/main.c
@@ -28,6 +28,7 @@ 
 #include <rte_lcore.h>
 #include <rte_malloc.h>
 #include <rte_mbuf.h>
+#include <rte_mbuf_dyn.h>
 #include <rte_memory.h>
 #include <rte_mempool.h>
 #include <rte_log.h>
@@ -59,6 +60,22 @@ 
 	} \
 } while (0)
 
+static int input_dynfield_offset = -1;
+
+static inline struct rte_mbuf *
+get_mbuf_input(struct rte_mbuf *mbuf)
+{
+	return *RTE_MBUF_DYNFIELD(mbuf,
+			input_dynfield_offset, struct rte_mbuf **);
+}
+
+static inline void
+set_mbuf_input(struct rte_mbuf *mbuf, struct rte_mbuf *input)
+{
+	*RTE_MBUF_DYNFIELD(mbuf,
+			input_dynfield_offset, struct rte_mbuf **) = input;
+}
+
 static const struct rte_eth_conf port_conf = {
 	.rxmode = {
 		.mq_mode = ETH_MQ_RX_NONE,
@@ -294,11 +311,11 @@  pktmbuf_free_bulk(struct rte_mbuf **mbufs, unsigned int nb_to_free)
 }
 
 static inline void
-pktmbuf_userdata_free_bulk(struct rte_mbuf **mbufs, unsigned int nb_to_free)
+pktmbuf_input_free_bulk(struct rte_mbuf **mbufs, unsigned int nb_to_free)
 {
 	unsigned int i;
 	for (i = 0; i < nb_to_free; ++i) {
-		struct rte_mbuf *rx_pkt = mbufs[i]->userdata;
+		struct rte_mbuf *rx_pkt = get_mbuf_input(mbufs[i]);
 		rte_pktmbuf_free(rx_pkt);
 		rte_pktmbuf_free(mbufs[i]);
 	}
@@ -429,7 +446,7 @@  verify_data(struct rte_mbuf **mbufs, uint16_t num_pkts)
 	uint16_t i;
 	for (i = 0; i < num_pkts; ++i) {
 		struct rte_mbuf *out = mbufs[i];
-		struct rte_mbuf *in = out->userdata;
+		struct rte_mbuf *in = get_mbuf_input(out);
 
 		if (memcmp(rte_pktmbuf_mtod_offset(in, uint8_t *,
 				sizeof(struct rte_ether_hdr)),
@@ -731,7 +748,7 @@  run_encoding(struct lcore_conf *lcore_conf)
 				rte_pktmbuf_data_len(rx_pkts_burst[i]) -
 				sizeof(struct rte_ether_hdr);
 		/* save input mbuf pointer for later comparison */
-		enc_out_pkts[i]->userdata = rx_pkts_burst[i];
+		set_mbuf_input(enc_out_pkts[i], rx_pkts_burst[i]);
 
 		/* copy ethernet header */
 		rte_pktmbuf_reset(enc_out_pkts[i]);
@@ -775,7 +792,7 @@  run_encoding(struct lcore_conf *lcore_conf)
 	nb_enq = rte_bbdev_enqueue_enc_ops(bbdev_id, enc_queue_id,
 			bbdev_ops_burst, nb_rx);
 	if (unlikely(nb_enq < nb_rx)) {
-		pktmbuf_userdata_free_bulk(&enc_out_pkts[nb_enq],
+		pktmbuf_input_free_bulk(&enc_out_pkts[nb_enq],
 				nb_rx - nb_enq);
 		rte_bbdev_enc_op_free_bulk(&bbdev_ops_burst[nb_enq],
 				nb_rx - nb_enq);
@@ -805,7 +822,7 @@  run_encoding(struct lcore_conf *lcore_conf)
 	nb_sent = rte_ring_enqueue_burst(enc_to_dec_ring, (void **)enc_out_pkts,
 			nb_deq, NULL);
 	if (unlikely(nb_sent < nb_deq)) {
-		pktmbuf_userdata_free_bulk(&enc_out_pkts[nb_sent],
+		pktmbuf_input_free_bulk(&enc_out_pkts[nb_sent],
 				nb_deq - nb_sent);
 		lcore_stats->enc_to_dec_lost_packets += nb_deq - nb_sent;
 	}
@@ -842,7 +859,7 @@  run_decoding(struct lcore_conf *lcore_conf)
 
 	if (unlikely(rte_bbdev_dec_op_alloc_bulk(bbdev_op_pool, bbdev_ops_burst,
 			nb_recv) != 0)) {
-		pktmbuf_userdata_free_bulk(recv_pkts_burst, nb_recv);
+		pktmbuf_input_free_bulk(recv_pkts_burst, nb_recv);
 		lcore_stats->rx_lost_packets += nb_recv;
 		return;
 	}
@@ -871,7 +888,7 @@  run_decoding(struct lcore_conf *lcore_conf)
 	nb_enq = rte_bbdev_enqueue_dec_ops(bbdev_id, bbdev_queue_id,
 			bbdev_ops_burst, nb_recv);
 	if (unlikely(nb_enq < nb_recv)) {
-		pktmbuf_userdata_free_bulk(&recv_pkts_burst[nb_enq],
+		pktmbuf_input_free_bulk(&recv_pkts_burst[nb_enq],
 				nb_recv - nb_enq);
 		rte_bbdev_dec_op_free_bulk(&bbdev_ops_burst[nb_enq],
 				nb_recv - nb_enq);
@@ -898,12 +915,12 @@  run_decoding(struct lcore_conf *lcore_conf)
 
 	/* Free the RX mbufs after verification */
 	for (i = 0; i < nb_deq; ++i)
-		rte_pktmbuf_free(recv_pkts_burst[i]->userdata);
+		rte_pktmbuf_free(get_mbuf_input(recv_pkts_burst[i]));
 
 	/* Transmit the packets */
 	nb_tx = rte_eth_tx_burst(port_id, tx_queue_id, recv_pkts_burst, nb_deq);
 	if (unlikely(nb_tx < nb_deq)) {
-		pktmbuf_userdata_free_bulk(&recv_pkts_burst[nb_tx],
+		pktmbuf_input_free_bulk(&recv_pkts_burst[nb_tx],
 				nb_deq - nb_tx);
 		lcore_stats->tx_lost_packets += nb_deq - nb_tx;
 	}
@@ -1046,6 +1063,12 @@  main(int argc, char **argv)
 	bool stats_thread_started = false;
 	unsigned int main_lcore_id = rte_get_main_lcore();
 
+	static const struct rte_mbuf_dynfield input_dynfield_desc = {
+		.name = "example_bbdev_dynfield_input",
+		.size = sizeof(struct rte_mbuf *),
+		.align = __alignof__(struct rte_mbuf *),
+	};
+
 	rte_atomic16_init(&global_exit_flag);
 
 	sigret = signal(SIGTERM, signal_handler);
@@ -1115,6 +1138,12 @@  main(int argc, char **argv)
 	if (bbdev_mbuf_mempool == NULL)
 		rte_exit(EXIT_FAILURE, "Cannot create ethdev mbuf mempool\n");
 
+	/* register mbuf field to store input pointer */
+	input_dynfield_offset =
+		rte_mbuf_dynfield_register(&input_dynfield_desc);
+	if (input_dynfield_offset < 0)
+		rte_exit(EXIT_FAILURE, "Cannot register mbuf field\n");
+
 	/* initialize ports */
 	ret = initialize_ports(&app_params, ethdev_mbuf_mempool);