[6/6] app/testpmd: factorize fwd engine Tx
Checks
Commit Message
Reduce code duplication by introducing a helper that takes care of
transmitting, retrying if enabled and incrementing tx counter.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
app/test-pmd/5tswap.c | 22 +------------
app/test-pmd/csumonly.c | 23 +-------------
app/test-pmd/flowgen.c | 22 +------------
app/test-pmd/icmpecho.c | 28 ++---------------
app/test-pmd/iofwd.c | 22 +------------
app/test-pmd/macfwd.c | 21 +------------
app/test-pmd/macswap.c | 27 ++--------------
app/test-pmd/noisy_vnf.c | 67 +++++++---------------------------------
app/test-pmd/testpmd.h | 30 ++++++++++++++++++
app/test-pmd/txonly.c | 33 +++++---------------
10 files changed, 58 insertions(+), 237 deletions(-)
Comments
On 1/24/2023 4:17 PM, David Marchand wrote:
> Reduce code duplication by introducing a helper that takes care of
> transmitting, retrying if enabled and incrementing tx counter.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> app/test-pmd/5tswap.c | 22 +------------
> app/test-pmd/csumonly.c | 23 +-------------
> app/test-pmd/flowgen.c | 22 +------------
> app/test-pmd/icmpecho.c | 28 ++---------------
> app/test-pmd/iofwd.c | 22 +------------
> app/test-pmd/macfwd.c | 21 +------------
> app/test-pmd/macswap.c | 27 ++--------------
> app/test-pmd/noisy_vnf.c | 67 +++++++---------------------------------
> app/test-pmd/testpmd.h | 30 ++++++++++++++++++
> app/test-pmd/txonly.c | 33 +++++---------------
> 10 files changed, 58 insertions(+), 237 deletions(-)
>
> diff --git a/app/test-pmd/5tswap.c b/app/test-pmd/5tswap.c
> index 27da867d7f..ff8c2dcde5 100644
> --- a/app/test-pmd/5tswap.c
> +++ b/app/test-pmd/5tswap.c
> @@ -91,9 +91,6 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
> uint64_t ol_flags;
> uint16_t proto;
> uint16_t nb_rx;
> - uint16_t nb_tx;
> - uint32_t retry;
> -
> int i;
> union {
> struct rte_ether_hdr *eth;
> @@ -155,24 +152,7 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
> }
> mbuf_field_set(mb, ol_flags);
> }
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_rx)) {
> - fs->fwd_dropped += (nb_rx - nb_tx);
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> + common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
>
> return true;
> }
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index d758ae0ac6..fc85c22a77 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -847,12 +847,10 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> uint8_t gro_enable;
> #endif
> uint16_t nb_rx;
> - uint16_t nb_tx;
> uint16_t nb_prep;
> uint16_t i;
> uint64_t rx_ol_flags, tx_ol_flags;
> uint64_t tx_offloads;
> - uint32_t retry;
> uint32_t rx_bad_ip_csum;
> uint32_t rx_bad_l4_csum;
> uint32_t rx_bad_outer_l4_csum;
> @@ -1169,32 +1167,13 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> rte_pktmbuf_free_bulk(&tx_pkts_burst[nb_prep], nb_rx - nb_prep);
> }
>
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, tx_pkts_burst,
> - nb_prep);
> + common_fwd_stream_transmit(fs, tx_pkts_burst, nb_prep);
>
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_prep) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_prep && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &tx_pkts_burst[nb_tx], nb_prep - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> fs->rx_bad_ip_csum += rx_bad_ip_csum;
> fs->rx_bad_l4_csum += rx_bad_l4_csum;
> fs->rx_bad_outer_l4_csum += rx_bad_outer_l4_csum;
> fs->rx_bad_outer_ip_csum += rx_bad_outer_ip_csum;
>
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_prep)) {
> - fs->fwd_dropped += (nb_prep - nb_tx);
> - rte_pktmbuf_free_bulk(&tx_pkts_burst[nb_tx], nb_prep - nb_tx);
> - }
> -
> return true;
> }
>
> diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
> index 3705cc60c5..5a0d096309 100644
> --- a/app/test-pmd/flowgen.c
> +++ b/app/test-pmd/flowgen.c
> @@ -71,11 +71,9 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
> uint16_t vlan_tci, vlan_tci_outer;
> uint64_t ol_flags = 0;
> uint16_t nb_rx;
> - uint16_t nb_tx;
> uint16_t nb_dropped;
> uint16_t nb_pkt;
> uint16_t nb_clones = nb_pkt_flowgen_clones;
> - uint32_t retry;
> uint64_t tx_offloads;
> int next_flow = RTE_PER_LCORE(_next_flow);
>
> @@ -158,30 +156,12 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
> next_flow = 0;
> }
>
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_pkt - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> -
> - inc_tx_burst_stats(fs, nb_tx);
> - nb_dropped = nb_pkt - nb_tx;
> + nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
> if (unlikely(nb_dropped > 0)) {
> /* Back out the flow counter. */
> next_flow -= nb_dropped;
> while (next_flow < 0)
> next_flow += nb_flows_flowgen;
> -
> - fs->fwd_dropped += nb_dropped;
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
> }
>
> RTE_PER_LCORE(_next_flow) = next_flow;
> diff --git a/app/test-pmd/icmpecho.c b/app/test-pmd/icmpecho.c
> index 48f8fe0bf1..68524484e3 100644
> --- a/app/test-pmd/icmpecho.c
> +++ b/app/test-pmd/icmpecho.c
> @@ -280,10 +280,8 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
> struct rte_ipv4_hdr *ip_h;
> struct rte_icmp_hdr *icmp_h;
> struct rte_ether_addr eth_addr;
> - uint32_t retry;
> uint32_t ip_addr;
> uint16_t nb_rx;
> - uint16_t nb_tx;
> uint16_t nb_replies;
> uint16_t eth_type;
> uint16_t vlan_id;
> @@ -476,30 +474,8 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
> }
>
> /* Send back ICMP echo replies, if any. */
> - if (nb_replies > 0) {
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
> - nb_replies);
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_replies) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_replies &&
> - retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port,
> - fs->tx_queue,
> - &pkts_burst[nb_tx],
> - nb_replies - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_replies)) {
> - fs->fwd_dropped += (nb_replies - nb_tx);
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_replies - nb_tx);
> - }
> - }
> + if (nb_replies > 0)
> + common_fwd_stream_transmit(fs, pkts_burst, nb_replies);
>
> return true;
> }
> diff --git a/app/test-pmd/iofwd.c b/app/test-pmd/iofwd.c
> index 69b583cb5b..ba06fae4a6 100644
> --- a/app/test-pmd/iofwd.c
> +++ b/app/test-pmd/iofwd.c
> @@ -46,8 +46,6 @@ pkt_burst_io_forward(struct fwd_stream *fs)
> {
> struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> uint16_t nb_rx;
> - uint16_t nb_tx;
> - uint32_t retry;
>
> /*
> * Receive a burst of packets and forward them.
> @@ -56,25 +54,7 @@ pkt_burst_io_forward(struct fwd_stream *fs)
> if (unlikely(nb_rx == 0))
> return false;
>
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - pkts_burst, nb_rx);
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_rx)) {
> - fs->fwd_dropped += (nb_rx - nb_tx);
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> + common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
>
> return true;
> }
> diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c
> index a72f5ccb75..7316d73315 100644
> --- a/app/test-pmd/macfwd.c
> +++ b/app/test-pmd/macfwd.c
> @@ -48,9 +48,7 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
> struct rte_port *txp;
> struct rte_mbuf *mb;
> struct rte_ether_hdr *eth_hdr;
> - uint32_t retry;
> uint16_t nb_rx;
> - uint16_t nb_tx;
> uint16_t i;
> uint64_t ol_flags = 0;
> uint64_t tx_offloads;
> @@ -87,25 +85,8 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
> mb->vlan_tci = txp->tx_vlan_id;
> mb->vlan_tci_outer = txp->tx_vlan_id_outer;
> }
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> - }
>
> - fs->tx_packets += nb_tx;
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_rx)) {
> - fs->fwd_dropped += (nb_rx - nb_tx);
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> + common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
>
> return true;
> }
> diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
> index ab37123404..57f77003fe 100644
> --- a/app/test-pmd/macswap.c
> +++ b/app/test-pmd/macswap.c
> @@ -51,10 +51,7 @@ static bool
> pkt_burst_mac_swap(struct fwd_stream *fs)
> {
> struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
> - struct rte_port *txp;
> uint16_t nb_rx;
> - uint16_t nb_tx;
> - uint32_t retry;
>
> /*
> * Receive a burst of packets and forward them.
> @@ -63,28 +60,8 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
> if (unlikely(nb_rx == 0))
> return false;
>
> - txp = &ports[fs->tx_port];
> -
> - do_macswap(pkts_burst, nb_rx, txp);
> -
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_rx)) {
> - fs->fwd_dropped += (nb_rx - nb_tx);
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
> - }
> + do_macswap(pkts_burst, nb_rx, &ports[fs->tx_port]);
> + common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
>
> return true;
> }
> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c
> index 937d5a1d7d..3875590132 100644
> --- a/app/test-pmd/noisy_vnf.c
> +++ b/app/test-pmd/noisy_vnf.c
> @@ -93,30 +93,6 @@ sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts)
> }
> }
>
> -static uint16_t
> -do_retry(uint16_t nb_rx, uint16_t nb_tx, struct rte_mbuf **pkts,
> - struct fwd_stream *fs)
> -{
> - uint32_t retry = 0;
> -
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts[nb_tx], nb_rx - nb_tx);
> - }
> -
> - return nb_tx;
> -}
> -
> -static uint32_t
> -drop_pkts(struct rte_mbuf **pkts, uint16_t nb_rx, uint16_t nb_tx)
> -{
> - if (nb_tx < nb_rx)
> - rte_pktmbuf_free_bulk(&pkts[nb_tx], nb_rx - nb_tx);
> -
> - return nb_rx - nb_tx;
> -}
> -
> /*
> * Forwarding of packets in noisy VNF mode. Forward packets but perform
> * memory operations first as specified on cmdline.
> @@ -156,38 +132,23 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
>
> if (!ncf->do_buffering) {
> sim_memory_lookups(ncf, nb_rx);
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - pkts_burst, nb_rx);
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
> - nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs);
> - inc_tx_burst_stats(fs, nb_tx);
> - fs->tx_packets += nb_tx;
> - fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx);
> + nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
>
> return true;
> }
>
> fifo_free = rte_ring_free_count(ncf->f);
> if (fifo_free >= nb_rx) {
> - nb_enqd = rte_ring_enqueue_burst(ncf->f,
> - (void **) pkts_burst, nb_rx, NULL);
> - if (nb_enqd < nb_rx)
> - fs->fwd_dropped += drop_pkts(pkts_burst,
> - nb_rx, nb_enqd);
> - } else {
> - nb_deqd = rte_ring_dequeue_burst(ncf->f,
> - (void **) tmp_pkts, nb_rx, NULL);
> - nb_enqd = rte_ring_enqueue_burst(ncf->f,
> - (void **) pkts_burst, nb_deqd, NULL);
> - if (nb_deqd > 0) {
> - nb_tx = rte_eth_tx_burst(fs->tx_port,
> - fs->tx_queue, tmp_pkts,
> - nb_deqd);
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
> - nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
> - inc_tx_burst_stats(fs, nb_tx);
> - fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, nb_tx);
> + nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_rx, NULL);
> + if (nb_enqd < nb_rx) {
> + fs->fwd_dropped += nb_rx - nb_enqd;
> + rte_pktmbuf_free_bulk(&pkts_burst[nb_enqd], nb_rx - nb_enqd);
> }
> + } else {
> + nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **) tmp_pkts, nb_rx, NULL);
> + nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_deqd, NULL);
> + if (nb_deqd > 0)
> + nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
> }
>
> sim_memory_lookups(ncf, nb_enqd);
> @@ -204,15 +165,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
> needs_flush = delta_ms >= noisy_tx_sw_buf_flush_time &&
> noisy_tx_sw_buf_flush_time > 0 && !nb_tx;
> while (needs_flush && !rte_ring_empty(ncf->f)) {
> - unsigned int sent;
> nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **)tmp_pkts,
> MAX_PKT_BURST, NULL);
> - sent = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - tmp_pkts, nb_deqd);
> - if (unlikely(sent < nb_deqd) && fs->retry_enabled)
> - nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
> - inc_tx_burst_stats(fs, nb_tx);
> - fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, sent);
> + nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
> ncf->prev_time = rte_get_timer_cycles();
> }
>
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index e6b28b4748..71ff70f55b 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -870,6 +870,36 @@ common_fwd_stream_receive(struct fwd_stream *fs, struct rte_mbuf **burst,
> return nb_rx;
> }
>
> +/* Returns count of dropped packets. */
> +static inline uint16_t
> +common_fwd_stream_transmit(struct fwd_stream *fs, struct rte_mbuf **burst,
> + unsigned int count)
> +{
> + uint16_t nb_tx;
> + uint32_t retry;
> +
> + nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, burst, count);
> + /*
> + * Retry if necessary
> + */
> + if (unlikely(nb_tx < count) && fs->retry_enabled) {
> + retry = 0;
> + while (nb_tx < count && retry++ < burst_tx_retry_num) {
> + rte_delay_us(burst_tx_delay_time);
> + nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> + &burst[nb_tx], count - nb_tx);
> + }
> + }
> + fs->tx_packets += nb_tx;
> + inc_tx_burst_stats(fs, nb_tx);
> + if (unlikely(nb_tx < count)) {
> + fs->fwd_dropped += (count - nb_tx);
> + rte_pktmbuf_free_bulk(&burst[nb_tx], count - nb_tx);
> + }
> +
> + return count - nb_tx;
> +}
> +
> /* Prototypes */
> unsigned int parse_item_list(const char *str, const char *item_name,
> unsigned int max_items,
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
> index b80ab6f5df..7144b3d5eb 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -331,10 +331,9 @@ pkt_burst_transmit(struct fwd_stream *fs)
> struct rte_mbuf *pkt;
> struct rte_mempool *mbp;
> struct rte_ether_hdr eth_hdr;
> - uint16_t nb_tx;
> + uint16_t nb_dropped;
> uint16_t nb_pkt;
> uint16_t vlan_tci, vlan_tci_outer;
> - uint32_t retry;
> uint64_t ol_flags = 0;
> uint64_t tx_offloads;
>
> @@ -391,34 +390,18 @@ pkt_burst_transmit(struct fwd_stream *fs)
> if (nb_pkt == 0)
> return false;
>
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
> -
> - /*
> - * Retry if necessary
> - */
> - if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
> - retry = 0;
> - while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts_burst[nb_tx], nb_pkt - nb_tx);
> - }
> - }
> - fs->tx_packets += nb_tx;
> + nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
>
> if (txonly_multi_flow)
> - RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
> + RTE_PER_LCORE(_ip_var) -= nb_dropped;
>
> - inc_tx_burst_stats(fs, nb_tx);
> - if (unlikely(nb_tx < nb_pkt)) {
> + if (unlikely(nb_dropped > 0)) {
> if (verbose_level > 0 && fs->fwd_dropped == 0)
> printf("port %d tx_queue %d - drop "
> - "(nb_pkt:%u - nb_tx:%u)=%u packets\n",
> - fs->tx_port, fs->tx_queue,
> - (unsigned) nb_pkt, (unsigned) nb_tx,
> - (unsigned) (nb_pkt - nb_tx));
> - fs->fwd_dropped += (nb_pkt - nb_tx);
> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
> + "(nb_pkt:%"PRIu16" - nb_tx:%"PRIu16")="
> + "%"PRIu16" packets\n",
> + fs->tx_port, fs->tx_queue, nb_pkt,
> + nb_pkt - nb_dropped, nb_dropped);
Build error reported in this file here-
../app/test-pmd/txonly.c:404:5: error: format specifies type 'unsigned short' but the argument has type 'int' [-Werror,-Wformat]
> }
>
> return true;
On 1/24/2023 10:47 AM, David Marchand wrote:
> Reduce code duplication by introducing a helper that takes care of
> transmitting, retrying if enabled and incrementing tx counter.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
<...>
> diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c
> index 937d5a1d7d..3875590132 100644
> --- a/app/test-pmd/noisy_vnf.c
> +++ b/app/test-pmd/noisy_vnf.c
> @@ -93,30 +93,6 @@ sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts)
> }
> }
>
> -static uint16_t
> -do_retry(uint16_t nb_rx, uint16_t nb_tx, struct rte_mbuf **pkts,
> - struct fwd_stream *fs)
> -{
> - uint32_t retry = 0;
> -
> - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> - rte_delay_us(burst_tx_delay_time);
> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - &pkts[nb_tx], nb_rx - nb_tx);
> - }
> -
> - return nb_tx;
> -}
> -
> -static uint32_t
> -drop_pkts(struct rte_mbuf **pkts, uint16_t nb_rx, uint16_t nb_tx)
> -{
> - if (nb_tx < nb_rx)
> - rte_pktmbuf_free_bulk(&pkts[nb_tx], nb_rx - nb_tx);
> -
> - return nb_rx - nb_tx;
> -}
> -
> /*
> * Forwarding of packets in noisy VNF mode. Forward packets but perform
> * memory operations first as specified on cmdline.
> @@ -156,38 +132,23 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
>
> if (!ncf->do_buffering) {
> sim_memory_lookups(ncf, nb_rx);
> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - pkts_burst, nb_rx);
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
> - nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs);
> - inc_tx_burst_stats(fs, nb_tx);
> - fs->tx_packets += nb_tx;
> - fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx);
> + nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
>
'nb_tx' is not used or necessary in this context, so assignment is not
necassary.
PS:
In the latest next-net head, there is a 'goto' here instead of return,
but that is becuase of recording cycles, becuase of optimization in this
set (patch 1/6) that needs to turn back to 'return' that is what I did
while applying patch.
> kreturn true;
> }
>
> fifo_free = rte_ring_free_count(ncf->f);
> if (fifo_free >= nb_rx) {
> - nb_enqd = rte_ring_enqueue_burst(ncf->f,
> - (void **) pkts_burst, nb_rx, NULL);
> - if (nb_enqd < nb_rx)
> - fs->fwd_dropped += drop_pkts(pkts_burst,
> - nb_rx, nb_enqd);
> - } else {
> - nb_deqd = rte_ring_dequeue_burst(ncf->f,
> - (void **) tmp_pkts, nb_rx, NULL);
> - nb_enqd = rte_ring_enqueue_burst(ncf->f,
> - (void **) pkts_burst, nb_deqd, NULL);
> - if (nb_deqd > 0) {
> - nb_tx = rte_eth_tx_burst(fs->tx_port,
> - fs->tx_queue, tmp_pkts,
> - nb_deqd);
> - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
> - nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
> - inc_tx_burst_stats(fs, nb_tx);
> - fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, nb_tx);
> + nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_rx, NULL);
> + if (nb_enqd < nb_rx) {
> + fs->fwd_dropped += nb_rx - nb_enqd;
> + rte_pktmbuf_free_bulk(&pkts_burst[nb_enqd], nb_rx - nb_enqd);
Why not keep 'drop_pkts()' for this block, it is easier to read with it.
> }
> + } else {
> + nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **) tmp_pkts, nb_rx, NULL);
> + nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_deqd, NULL);
> + if (nb_deqd > 0)
> + nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
'nb_tx' assignment looks wrong,
function returns 'nb_dropped' not 'nb_tx'. 'nb_tx' used below to detect
if flush needed ('needs_flush'), so 'needs_flush' may be set wrong
becuase dropped packet is used instead number of Tx packets.
> }
>
> sim_memory_lookups(ncf, nb_enqd);
> @@ -204,15 +165,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
> needs_flush = delta_ms >= noisy_tx_sw_buf_flush_time &&
> noisy_tx_sw_buf_flush_time > 0 && !nb_tx;
> while (needs_flush && !rte_ring_empty(ncf->f)) {
> - unsigned int sent;
> nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **)tmp_pkts,
> MAX_PKT_BURST, NULL);
> - sent = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> - tmp_pkts, nb_deqd);
> - if (unlikely(sent < nb_deqd) && fs->retry_enabled)
> - nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
> - inc_tx_burst_stats(fs, nb_tx);
> - fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, sent);
> + nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
similaryly 'nb_tx' assignment can be wrong here, and 'nb_tx' used for
return value to hint if record cycle is required, using number of
dropped packets (what 'common_fwd_stream_transmit()' returns) gives
wrong result.
> ncf->prev_time = rte_get_timer_cycles();
> }
>
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index e6b28b4748..71ff70f55b 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -870,6 +870,36 @@ common_fwd_stream_receive(struct fwd_stream *fs, struct rte_mbuf **burst,
> return nb_rx;
> }
>
> +/* Returns count of dropped packets. */
> +static inline uint16_t
> +common_fwd_stream_transmit(struct fwd_stream *fs, struct rte_mbuf **burst,
> + unsigned int count)
I would use 'nb_pkts' instead of 'count' as variable name since it is
more common, but of course this is subjective.
> +{
> + uint16_t nb_tx;
> + uint32_t retry;
> +
> + nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, burst, count);
> + /*
> + * Retry if necessary
> + */
> + if (unlikely(nb_tx < count) && fs->retry_enabled) {
> + retry = 0;
> + while (nb_tx < count && retry++ < burst_tx_retry_num) {
> + rte_delay_us(burst_tx_delay_time);
> + nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> + &burst[nb_tx], count - nb_tx);
> + }
> + }
> + fs->tx_packets += nb_tx;
> + inc_tx_burst_stats(fs, nb_tx);
> + if (unlikely(nb_tx < count)) {
> + fs->fwd_dropped += (count - nb_tx);
> + rte_pktmbuf_free_bulk(&burst[nb_tx], count - nb_tx);
> + }
> +
> + return count - nb_tx;
Instead of returning number of dropped packets, what about returning
number of packets sent ('nb_tx')?
Intuitively this is what expected from a function named
'common_fwd_stream_transmit()', and even if it is more optimised to
return number of dropped packet, this may have only a little impact on
the caller code.
And even 'fs->tx_packets' updated withing the function, updating it
externally and explicitly feels me as it clarifies usage more, although
this part is up to you, I mean usage like:
fs->tx_packets += common_fwd_stream_transmit(fs, pkts, nb_pkts);
On 2/14/2023 11:03 AM, Singh, Aman Deep wrote:
>
> On 1/24/2023 4:17 PM, David Marchand wrote:
>> Reduce code duplication by introducing a helper that takes care of
>> transmitting, retrying if enabled and incrementing tx counter.
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
<...>
>> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
>> index b80ab6f5df..7144b3d5eb 100644
>> --- a/app/test-pmd/txonly.c
>> +++ b/app/test-pmd/txonly.c
>> @@ -331,10 +331,9 @@ pkt_burst_transmit(struct fwd_stream *fs)
>> struct rte_mbuf *pkt;
>> struct rte_mempool *mbp;
>> struct rte_ether_hdr eth_hdr;
>> - uint16_t nb_tx;
>> + uint16_t nb_dropped;
>> uint16_t nb_pkt;
>> uint16_t vlan_tci, vlan_tci_outer;
>> - uint32_t retry;
>> uint64_t ol_flags = 0;
>> uint64_t tx_offloads;
>> @@ -391,34 +390,18 @@ pkt_burst_transmit(struct fwd_stream *fs)
>> if (nb_pkt == 0)
>> return false;
>> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
>> nb_pkt);
>> -
>> - /*
>> - * Retry if necessary
>> - */
>> - if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
>> - retry = 0;
>> - while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
>> - rte_delay_us(burst_tx_delay_time);
>> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
>> - &pkts_burst[nb_tx], nb_pkt - nb_tx);
>> - }
>> - }
>> - fs->tx_packets += nb_tx;
>> + nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
>> if (txonly_multi_flow)
>> - RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
>> + RTE_PER_LCORE(_ip_var) -= nb_dropped;
>> - inc_tx_burst_stats(fs, nb_tx);
>> - if (unlikely(nb_tx < nb_pkt)) {
>> + if (unlikely(nb_dropped > 0)) {
>> if (verbose_level > 0 && fs->fwd_dropped == 0)
>> printf("port %d tx_queue %d - drop "
>> - "(nb_pkt:%u - nb_tx:%u)=%u packets\n",
>> - fs->tx_port, fs->tx_queue,
>> - (unsigned) nb_pkt, (unsigned) nb_tx,
>> - (unsigned) (nb_pkt - nb_tx));
>> - fs->fwd_dropped += (nb_pkt - nb_tx);
>> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
>> + "(nb_pkt:%"PRIu16" - nb_tx:%"PRIu16")="
>> + "%"PRIu16" packets\n",
>> + fs->tx_port, fs->tx_queue, nb_pkt,
>> + nb_pkt - nb_dropped, nb_dropped);
>
> Build error reported in this file here-
> ../app/test-pmd/txonly.c:404:5: error: format specifies type 'unsigned
> short' but the argument has type 'int' [-Werror,-Wformat]
>
both 'nb_pkt' & 'nb_dropped' are 'uint16_t' (unsigned short), I wonder
which argument is causing this warning?
On 2/14/2023 11:47 PM, Ferruh Yigit wrote:
> On 2/14/2023 11:03 AM, Singh, Aman Deep wrote:
>> On 1/24/2023 4:17 PM, David Marchand wrote:
>>> Reduce code duplication by introducing a helper that takes care of
>>> transmitting, retrying if enabled and incrementing tx counter.
>>>
>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> <...>
>
>>> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
>>> index b80ab6f5df..7144b3d5eb 100644
>>> --- a/app/test-pmd/txonly.c
>>> +++ b/app/test-pmd/txonly.c
>>> @@ -331,10 +331,9 @@ pkt_burst_transmit(struct fwd_stream *fs)
>>> struct rte_mbuf *pkt;
>>> struct rte_mempool *mbp;
>>> struct rte_ether_hdr eth_hdr;
>>> - uint16_t nb_tx;
>>> + uint16_t nb_dropped;
>>> uint16_t nb_pkt;
>>> uint16_t vlan_tci, vlan_tci_outer;
>>> - uint32_t retry;
>>> uint64_t ol_flags = 0;
>>> uint64_t tx_offloads;
>>> @@ -391,34 +390,18 @@ pkt_burst_transmit(struct fwd_stream *fs)
>>> if (nb_pkt == 0)
>>> return false;
>>> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
>>> nb_pkt);
>>> -
>>> - /*
>>> - * Retry if necessary
>>> - */
>>> - if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
>>> - retry = 0;
>>> - while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
>>> - rte_delay_us(burst_tx_delay_time);
>>> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
>>> - &pkts_burst[nb_tx], nb_pkt - nb_tx);
>>> - }
>>> - }
>>> - fs->tx_packets += nb_tx;
>>> + nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
>>> if (txonly_multi_flow)
>>> - RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
>>> + RTE_PER_LCORE(_ip_var) -= nb_dropped;
>>> - inc_tx_burst_stats(fs, nb_tx);
>>> - if (unlikely(nb_tx < nb_pkt)) {
>>> + if (unlikely(nb_dropped > 0)) {
>>> if (verbose_level > 0 && fs->fwd_dropped == 0)
>>> printf("port %d tx_queue %d - drop "
>>> - "(nb_pkt:%u - nb_tx:%u)=%u packets\n",
>>> - fs->tx_port, fs->tx_queue,
>>> - (unsigned) nb_pkt, (unsigned) nb_tx,
>>> - (unsigned) (nb_pkt - nb_tx));
>>> - fs->fwd_dropped += (nb_pkt - nb_tx);
>>> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
>>> + "(nb_pkt:%"PRIu16" - nb_tx:%"PRIu16")="
>>> + "%"PRIu16" packets\n",
>>> + fs->tx_port, fs->tx_queue, nb_pkt,
>>> + nb_pkt - nb_dropped, nb_dropped);
>> Build error reported in this file here-
>> ../app/test-pmd/txonly.c:404:5: error: format specifies type 'unsigned
>> short' but the argument has type 'int' [-Werror,-Wformat]
>>
> both 'nb_pkt' & 'nb_dropped' are 'uint16_t' (unsigned short), I wonder
> which argument is causing this warning?
I think, subtraction of two unsigned numbers promotes the result to int.
As 'nb_pkt > nb_dropped', we may explicitly typecast it-
'(unsigned)(nb_pkt - nb_dropped)'
>
>
On 2/16/2023 8:01 AM, Singh, Aman Deep wrote:
>
> On 2/14/2023 11:47 PM, Ferruh Yigit wrote:
>> On 2/14/2023 11:03 AM, Singh, Aman Deep wrote:
>>> On 1/24/2023 4:17 PM, David Marchand wrote:
>>>> Reduce code duplication by introducing a helper that takes care of
>>>> transmitting, retrying if enabled and incrementing tx counter.
>>>>
>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> <...>
>>
>>>> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
>>>> index b80ab6f5df..7144b3d5eb 100644
>>>> --- a/app/test-pmd/txonly.c
>>>> +++ b/app/test-pmd/txonly.c
>>>> @@ -331,10 +331,9 @@ pkt_burst_transmit(struct fwd_stream *fs)
>>>> struct rte_mbuf *pkt;
>>>> struct rte_mempool *mbp;
>>>> struct rte_ether_hdr eth_hdr;
>>>> - uint16_t nb_tx;
>>>> + uint16_t nb_dropped;
>>>> uint16_t nb_pkt;
>>>> uint16_t vlan_tci, vlan_tci_outer;
>>>> - uint32_t retry;
>>>> uint64_t ol_flags = 0;
>>>> uint64_t tx_offloads;
>>>> @@ -391,34 +390,18 @@ pkt_burst_transmit(struct fwd_stream *fs)
>>>> if (nb_pkt == 0)
>>>> return false;
>>>> - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
>>>> nb_pkt);
>>>> -
>>>> - /*
>>>> - * Retry if necessary
>>>> - */
>>>> - if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
>>>> - retry = 0;
>>>> - while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
>>>> - rte_delay_us(burst_tx_delay_time);
>>>> - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
>>>> - &pkts_burst[nb_tx], nb_pkt - nb_tx);
>>>> - }
>>>> - }
>>>> - fs->tx_packets += nb_tx;
>>>> + nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
>>>> if (txonly_multi_flow)
>>>> - RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
>>>> + RTE_PER_LCORE(_ip_var) -= nb_dropped;
>>>> - inc_tx_burst_stats(fs, nb_tx);
>>>> - if (unlikely(nb_tx < nb_pkt)) {
>>>> + if (unlikely(nb_dropped > 0)) {
>>>> if (verbose_level > 0 && fs->fwd_dropped == 0)
>>>> printf("port %d tx_queue %d - drop "
>>>> - "(nb_pkt:%u - nb_tx:%u)=%u packets\n",
>>>> - fs->tx_port, fs->tx_queue,
>>>> - (unsigned) nb_pkt, (unsigned) nb_tx,
>>>> - (unsigned) (nb_pkt - nb_tx));
>>>> - fs->fwd_dropped += (nb_pkt - nb_tx);
>>>> - rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
>>>> + "(nb_pkt:%"PRIu16" - nb_tx:%"PRIu16")="
>>>> + "%"PRIu16" packets\n",
>>>> + fs->tx_port, fs->tx_queue, nb_pkt,
>>>> + nb_pkt - nb_dropped, nb_dropped);
>>> Build error reported in this file here-
>>> ../app/test-pmd/txonly.c:404:5: error: format specifies type 'unsigned
>>> short' but the argument has type 'int' [-Werror,-Wformat]
>>>
>> both 'nb_pkt' & 'nb_dropped' are 'uint16_t' (unsigned short), I wonder
>> which argument is causing this warning?
>
> I think, subtraction of two unsigned numbers promotes the result to int.
> As 'nb_pkt > nb_dropped', we may explicitly typecast it-
> '(unsigned)(nb_pkt - nb_dropped)'
>
You are right, subtraction promoted to int.
May be better to cast to 'uint16_t', since warning is not just sign but
also type related.
On Tue, Feb 14, 2023 at 7:16 PM Ferruh Yigit <ferruh.yigit@amd.com> wrote:
>
> On 1/24/2023 10:47 AM, David Marchand wrote:
> > Reduce code duplication by introducing a helper that takes care of
> > transmitting, retrying if enabled and incrementing tx counter.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> <...>
>
> > diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c
> > index 937d5a1d7d..3875590132 100644
> > --- a/app/test-pmd/noisy_vnf.c
> > +++ b/app/test-pmd/noisy_vnf.c
> > @@ -93,30 +93,6 @@ sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts)
> > }
> > }
> >
> > -static uint16_t
> > -do_retry(uint16_t nb_rx, uint16_t nb_tx, struct rte_mbuf **pkts,
> > - struct fwd_stream *fs)
> > -{
> > - uint32_t retry = 0;
> > -
> > - while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
> > - rte_delay_us(burst_tx_delay_time);
> > - nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> > - &pkts[nb_tx], nb_rx - nb_tx);
> > - }
> > -
> > - return nb_tx;
> > -}
> > -
> > -static uint32_t
> > -drop_pkts(struct rte_mbuf **pkts, uint16_t nb_rx, uint16_t nb_tx)
> > -{
> > - if (nb_tx < nb_rx)
> > - rte_pktmbuf_free_bulk(&pkts[nb_tx], nb_rx - nb_tx);
> > -
> > - return nb_rx - nb_tx;
> > -}
> > -
> > /*
> > * Forwarding of packets in noisy VNF mode. Forward packets but perform
> > * memory operations first as specified on cmdline.
> > @@ -156,38 +132,23 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
> >
> > if (!ncf->do_buffering) {
> > sim_memory_lookups(ncf, nb_rx);
> > - nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> > - pkts_burst, nb_rx);
> > - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
> > - nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs);
> > - inc_tx_burst_stats(fs, nb_tx);
> > - fs->tx_packets += nb_tx;
> > - fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx);
> > + nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
> >
>
> 'nb_tx' is not used or necessary in this context, so assignment is not
> necassary.
>
> PS:
> In the latest next-net head, there is a 'goto' here instead of return,
> but that is becuase of recording cycles, becuase of optimization in this
> set (patch 1/6) that needs to turn back to 'return' that is what I did
> while applying patch.
This part changed a bit after rebasing and I think nb_tx is still
needed in this context.
Please have a look at v2.
>
> > kreturn true;
> > }
> >
> > fifo_free = rte_ring_free_count(ncf->f);
> > if (fifo_free >= nb_rx) {
> > - nb_enqd = rte_ring_enqueue_burst(ncf->f,
> > - (void **) pkts_burst, nb_rx, NULL);
> > - if (nb_enqd < nb_rx)
> > - fs->fwd_dropped += drop_pkts(pkts_burst,
> > - nb_rx, nb_enqd);
> > - } else {
> > - nb_deqd = rte_ring_dequeue_burst(ncf->f,
> > - (void **) tmp_pkts, nb_rx, NULL);
> > - nb_enqd = rte_ring_enqueue_burst(ncf->f,
> > - (void **) pkts_burst, nb_deqd, NULL);
> > - if (nb_deqd > 0) {
> > - nb_tx = rte_eth_tx_burst(fs->tx_port,
> > - fs->tx_queue, tmp_pkts,
> > - nb_deqd);
> > - if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
> > - nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
> > - inc_tx_burst_stats(fs, nb_tx);
> > - fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, nb_tx);
> > + nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_rx, NULL);
> > + if (nb_enqd < nb_rx) {
> > + fs->fwd_dropped += nb_rx - nb_enqd;
> > + rte_pktmbuf_free_bulk(&pkts_burst[nb_enqd], nb_rx - nb_enqd);
>
> Why not keep 'drop_pkts()' for this block, it is easier to read with it.
That would be the only case where drop_pkts() is used.
I am not convinced about readability but if you feel strongly about
it, I don't mind restoring it (in a v3, I'll post v2 unchanged on this
matter).
>
> > }
> > + } else {
> > + nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **) tmp_pkts, nb_rx, NULL);
> > + nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_deqd, NULL);
> > + if (nb_deqd > 0)
> > + nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
>
> 'nb_tx' assignment looks wrong,
> function returns 'nb_dropped' not 'nb_tx'. 'nb_tx' used below to detect
> if flush needed ('needs_flush'), so 'needs_flush' may be set wrong
> becuase dropped packet is used instead number of Tx packets.
Indeed, this is wrong, I had caught the issue when preparing v2 after
rebasing over Robin series.
I had changed common_fwd_stream_transmit() so it returns the number of
transmitted packets which is more in line with rte_eth_tx_burst().
This is easier to understand too.
>
> > }
> >
> > sim_memory_lookups(ncf, nb_enqd);
> > @@ -204,15 +165,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
> > needs_flush = delta_ms >= noisy_tx_sw_buf_flush_time &&
> > noisy_tx_sw_buf_flush_time > 0 && !nb_tx;
> > while (needs_flush && !rte_ring_empty(ncf->f)) {
> > - unsigned int sent;
> > nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **)tmp_pkts,
> > MAX_PKT_BURST, NULL);
> > - sent = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> > - tmp_pkts, nb_deqd);
> > - if (unlikely(sent < nb_deqd) && fs->retry_enabled)
> > - nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
> > - inc_tx_burst_stats(fs, nb_tx);
> > - fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, sent);
> > + nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
>
> similaryly 'nb_tx' assignment can be wrong here, and 'nb_tx' used for
> return value to hint if record cycle is required, using number of
> dropped packets (what 'common_fwd_stream_transmit()' returns) gives
> wrong result.
Yes, and there are other issues in this part which I moved to a
separate patch for v2.
>
> > ncf->prev_time = rte_get_timer_cycles();
> > }
> >
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> > index e6b28b4748..71ff70f55b 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -870,6 +870,36 @@ common_fwd_stream_receive(struct fwd_stream *fs, struct rte_mbuf **burst,
> > return nb_rx;
> > }
> >
> > +/* Returns count of dropped packets. */
> > +static inline uint16_t
> > +common_fwd_stream_transmit(struct fwd_stream *fs, struct rte_mbuf **burst,
> > + unsigned int count)
>
> I would use 'nb_pkts' instead of 'count' as variable name since it is
> more common, but of course this is subjective.
Ok for me.
I did the same renaming for the rx helper.
>
> > +{
> > + uint16_t nb_tx;
> > + uint32_t retry;
> > +
> > + nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, burst, count);
> > + /*
> > + * Retry if necessary
> > + */
> > + if (unlikely(nb_tx < count) && fs->retry_enabled) {
> > + retry = 0;
> > + while (nb_tx < count && retry++ < burst_tx_retry_num) {
> > + rte_delay_us(burst_tx_delay_time);
> > + nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
> > + &burst[nb_tx], count - nb_tx);
> > + }
> > + }
> > + fs->tx_packets += nb_tx;
> > + inc_tx_burst_stats(fs, nb_tx);
> > + if (unlikely(nb_tx < count)) {
> > + fs->fwd_dropped += (count - nb_tx);
> > + rte_pktmbuf_free_bulk(&burst[nb_tx], count - nb_tx);
> > + }
> > +
> > + return count - nb_tx;
>
> Instead of returning number of dropped packets, what about returning
> number of packets sent ('nb_tx')?
> Intuitively this is what expected from a function named
> 'common_fwd_stream_transmit()', and even if it is more optimised to
> return number of dropped packet, this may have only a little impact on
> the caller code.
Ah ah, well, I thought the same after rebasing v1.
This change is in v2.
>
>
> And even 'fs->tx_packets' updated withing the function, updating it
> externally and explicitly feels me as it clarifies usage more, although
> this part is up to you, I mean usage like:
> fs->tx_packets += common_fwd_stream_transmit(fs, pkts, nb_pkts);
We would have fs->tx_packets managed by fwd engine code, but
fwd_dropped by the common helper?
I prefer fwd engines do not have to deal with the stats at all.
We have a lot of fwd engines, and small issues are often
(re-)introduced with new fwd engines.
A centralised approach is more robust.
Thanks Ferruh.
@@ -91,9 +91,6 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
uint64_t ol_flags;
uint16_t proto;
uint16_t nb_rx;
- uint16_t nb_tx;
- uint32_t retry;
-
int i;
union {
struct rte_ether_hdr *eth;
@@ -155,24 +152,7 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
}
mbuf_field_set(mb, ol_flags);
}
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts_burst[nb_tx], nb_rx - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_rx)) {
- fs->fwd_dropped += (nb_rx - nb_tx);
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
- }
+ common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
return true;
}
@@ -847,12 +847,10 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
uint8_t gro_enable;
#endif
uint16_t nb_rx;
- uint16_t nb_tx;
uint16_t nb_prep;
uint16_t i;
uint64_t rx_ol_flags, tx_ol_flags;
uint64_t tx_offloads;
- uint32_t retry;
uint32_t rx_bad_ip_csum;
uint32_t rx_bad_l4_csum;
uint32_t rx_bad_outer_l4_csum;
@@ -1169,32 +1167,13 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
rte_pktmbuf_free_bulk(&tx_pkts_burst[nb_prep], nb_rx - nb_prep);
}
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, tx_pkts_burst,
- nb_prep);
+ common_fwd_stream_transmit(fs, tx_pkts_burst, nb_prep);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_prep) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_prep && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &tx_pkts_burst[nb_tx], nb_prep - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
fs->rx_bad_ip_csum += rx_bad_ip_csum;
fs->rx_bad_l4_csum += rx_bad_l4_csum;
fs->rx_bad_outer_l4_csum += rx_bad_outer_l4_csum;
fs->rx_bad_outer_ip_csum += rx_bad_outer_ip_csum;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_prep)) {
- fs->fwd_dropped += (nb_prep - nb_tx);
- rte_pktmbuf_free_bulk(&tx_pkts_burst[nb_tx], nb_prep - nb_tx);
- }
-
return true;
}
@@ -71,11 +71,9 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
uint16_t vlan_tci, vlan_tci_outer;
uint64_t ol_flags = 0;
uint16_t nb_rx;
- uint16_t nb_tx;
uint16_t nb_dropped;
uint16_t nb_pkt;
uint16_t nb_clones = nb_pkt_flowgen_clones;
- uint32_t retry;
uint64_t tx_offloads;
int next_flow = RTE_PER_LCORE(_next_flow);
@@ -158,30 +156,12 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
next_flow = 0;
}
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts_burst[nb_tx], nb_pkt - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
-
- inc_tx_burst_stats(fs, nb_tx);
- nb_dropped = nb_pkt - nb_tx;
+ nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
if (unlikely(nb_dropped > 0)) {
/* Back out the flow counter. */
next_flow -= nb_dropped;
while (next_flow < 0)
next_flow += nb_flows_flowgen;
-
- fs->fwd_dropped += nb_dropped;
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
}
RTE_PER_LCORE(_next_flow) = next_flow;
@@ -280,10 +280,8 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
struct rte_ipv4_hdr *ip_h;
struct rte_icmp_hdr *icmp_h;
struct rte_ether_addr eth_addr;
- uint32_t retry;
uint32_t ip_addr;
uint16_t nb_rx;
- uint16_t nb_tx;
uint16_t nb_replies;
uint16_t eth_type;
uint16_t vlan_id;
@@ -476,30 +474,8 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
}
/* Send back ICMP echo replies, if any. */
- if (nb_replies > 0) {
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst,
- nb_replies);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_replies) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_replies &&
- retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port,
- fs->tx_queue,
- &pkts_burst[nb_tx],
- nb_replies - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_replies)) {
- fs->fwd_dropped += (nb_replies - nb_tx);
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_replies - nb_tx);
- }
- }
+ if (nb_replies > 0)
+ common_fwd_stream_transmit(fs, pkts_burst, nb_replies);
return true;
}
@@ -46,8 +46,6 @@ pkt_burst_io_forward(struct fwd_stream *fs)
{
struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
uint16_t nb_rx;
- uint16_t nb_tx;
- uint32_t retry;
/*
* Receive a burst of packets and forward them.
@@ -56,25 +54,7 @@ pkt_burst_io_forward(struct fwd_stream *fs)
if (unlikely(nb_rx == 0))
return false;
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- pkts_burst, nb_rx);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts_burst[nb_tx], nb_rx - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_rx)) {
- fs->fwd_dropped += (nb_rx - nb_tx);
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
- }
+ common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
return true;
}
@@ -48,9 +48,7 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
struct rte_port *txp;
struct rte_mbuf *mb;
struct rte_ether_hdr *eth_hdr;
- uint32_t retry;
uint16_t nb_rx;
- uint16_t nb_tx;
uint16_t i;
uint64_t ol_flags = 0;
uint64_t tx_offloads;
@@ -87,25 +85,8 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
mb->vlan_tci = txp->tx_vlan_id;
mb->vlan_tci_outer = txp->tx_vlan_id_outer;
}
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts_burst[nb_tx], nb_rx - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_rx)) {
- fs->fwd_dropped += (nb_rx - nb_tx);
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
- }
+ common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
return true;
}
@@ -51,10 +51,7 @@ static bool
pkt_burst_mac_swap(struct fwd_stream *fs)
{
struct rte_mbuf *pkts_burst[MAX_PKT_BURST];
- struct rte_port *txp;
uint16_t nb_rx;
- uint16_t nb_tx;
- uint32_t retry;
/*
* Receive a burst of packets and forward them.
@@ -63,28 +60,8 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
if (unlikely(nb_rx == 0))
return false;
- txp = &ports[fs->tx_port];
-
- do_macswap(pkts_burst, nb_rx, txp);
-
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_rx) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts_burst[nb_tx], nb_rx - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_rx)) {
- fs->fwd_dropped += (nb_rx - nb_tx);
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_rx - nb_tx);
- }
+ do_macswap(pkts_burst, nb_rx, &ports[fs->tx_port]);
+ common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
return true;
}
@@ -93,30 +93,6 @@ sim_memory_lookups(struct noisy_config *ncf, uint16_t nb_pkts)
}
}
-static uint16_t
-do_retry(uint16_t nb_rx, uint16_t nb_tx, struct rte_mbuf **pkts,
- struct fwd_stream *fs)
-{
- uint32_t retry = 0;
-
- while (nb_tx < nb_rx && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts[nb_tx], nb_rx - nb_tx);
- }
-
- return nb_tx;
-}
-
-static uint32_t
-drop_pkts(struct rte_mbuf **pkts, uint16_t nb_rx, uint16_t nb_tx)
-{
- if (nb_tx < nb_rx)
- rte_pktmbuf_free_bulk(&pkts[nb_tx], nb_rx - nb_tx);
-
- return nb_rx - nb_tx;
-}
-
/*
* Forwarding of packets in noisy VNF mode. Forward packets but perform
* memory operations first as specified on cmdline.
@@ -156,38 +132,23 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
if (!ncf->do_buffering) {
sim_memory_lookups(ncf, nb_rx);
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- pkts_burst, nb_rx);
- if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
- nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs);
- inc_tx_burst_stats(fs, nb_tx);
- fs->tx_packets += nb_tx;
- fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx);
+ nb_tx = common_fwd_stream_transmit(fs, pkts_burst, nb_rx);
return true;
}
fifo_free = rte_ring_free_count(ncf->f);
if (fifo_free >= nb_rx) {
- nb_enqd = rte_ring_enqueue_burst(ncf->f,
- (void **) pkts_burst, nb_rx, NULL);
- if (nb_enqd < nb_rx)
- fs->fwd_dropped += drop_pkts(pkts_burst,
- nb_rx, nb_enqd);
- } else {
- nb_deqd = rte_ring_dequeue_burst(ncf->f,
- (void **) tmp_pkts, nb_rx, NULL);
- nb_enqd = rte_ring_enqueue_burst(ncf->f,
- (void **) pkts_burst, nb_deqd, NULL);
- if (nb_deqd > 0) {
- nb_tx = rte_eth_tx_burst(fs->tx_port,
- fs->tx_queue, tmp_pkts,
- nb_deqd);
- if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
- nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
- inc_tx_burst_stats(fs, nb_tx);
- fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, nb_tx);
+ nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_rx, NULL);
+ if (nb_enqd < nb_rx) {
+ fs->fwd_dropped += nb_rx - nb_enqd;
+ rte_pktmbuf_free_bulk(&pkts_burst[nb_enqd], nb_rx - nb_enqd);
}
+ } else {
+ nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **) tmp_pkts, nb_rx, NULL);
+ nb_enqd = rte_ring_enqueue_burst(ncf->f, (void **) pkts_burst, nb_deqd, NULL);
+ if (nb_deqd > 0)
+ nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
}
sim_memory_lookups(ncf, nb_enqd);
@@ -204,15 +165,9 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
needs_flush = delta_ms >= noisy_tx_sw_buf_flush_time &&
noisy_tx_sw_buf_flush_time > 0 && !nb_tx;
while (needs_flush && !rte_ring_empty(ncf->f)) {
- unsigned int sent;
nb_deqd = rte_ring_dequeue_burst(ncf->f, (void **)tmp_pkts,
MAX_PKT_BURST, NULL);
- sent = rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- tmp_pkts, nb_deqd);
- if (unlikely(sent < nb_deqd) && fs->retry_enabled)
- nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
- inc_tx_burst_stats(fs, nb_tx);
- fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, sent);
+ nb_tx = common_fwd_stream_transmit(fs, tmp_pkts, nb_deqd);
ncf->prev_time = rte_get_timer_cycles();
}
@@ -870,6 +870,36 @@ common_fwd_stream_receive(struct fwd_stream *fs, struct rte_mbuf **burst,
return nb_rx;
}
+/* Returns count of dropped packets. */
+static inline uint16_t
+common_fwd_stream_transmit(struct fwd_stream *fs, struct rte_mbuf **burst,
+ unsigned int count)
+{
+ uint16_t nb_tx;
+ uint32_t retry;
+
+ nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, burst, count);
+ /*
+ * Retry if necessary
+ */
+ if (unlikely(nb_tx < count) && fs->retry_enabled) {
+ retry = 0;
+ while (nb_tx < count && retry++ < burst_tx_retry_num) {
+ rte_delay_us(burst_tx_delay_time);
+ nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
+ &burst[nb_tx], count - nb_tx);
+ }
+ }
+ fs->tx_packets += nb_tx;
+ inc_tx_burst_stats(fs, nb_tx);
+ if (unlikely(nb_tx < count)) {
+ fs->fwd_dropped += (count - nb_tx);
+ rte_pktmbuf_free_bulk(&burst[nb_tx], count - nb_tx);
+ }
+
+ return count - nb_tx;
+}
+
/* Prototypes */
unsigned int parse_item_list(const char *str, const char *item_name,
unsigned int max_items,
@@ -331,10 +331,9 @@ pkt_burst_transmit(struct fwd_stream *fs)
struct rte_mbuf *pkt;
struct rte_mempool *mbp;
struct rte_ether_hdr eth_hdr;
- uint16_t nb_tx;
+ uint16_t nb_dropped;
uint16_t nb_pkt;
uint16_t vlan_tci, vlan_tci_outer;
- uint32_t retry;
uint64_t ol_flags = 0;
uint64_t tx_offloads;
@@ -391,34 +390,18 @@ pkt_burst_transmit(struct fwd_stream *fs)
if (nb_pkt == 0)
return false;
- nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_pkt);
-
- /*
- * Retry if necessary
- */
- if (unlikely(nb_tx < nb_pkt) && fs->retry_enabled) {
- retry = 0;
- while (nb_tx < nb_pkt && retry++ < burst_tx_retry_num) {
- rte_delay_us(burst_tx_delay_time);
- nb_tx += rte_eth_tx_burst(fs->tx_port, fs->tx_queue,
- &pkts_burst[nb_tx], nb_pkt - nb_tx);
- }
- }
- fs->tx_packets += nb_tx;
+ nb_dropped = common_fwd_stream_transmit(fs, pkts_burst, nb_pkt);
if (txonly_multi_flow)
- RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
+ RTE_PER_LCORE(_ip_var) -= nb_dropped;
- inc_tx_burst_stats(fs, nb_tx);
- if (unlikely(nb_tx < nb_pkt)) {
+ if (unlikely(nb_dropped > 0)) {
if (verbose_level > 0 && fs->fwd_dropped == 0)
printf("port %d tx_queue %d - drop "
- "(nb_pkt:%u - nb_tx:%u)=%u packets\n",
- fs->tx_port, fs->tx_queue,
- (unsigned) nb_pkt, (unsigned) nb_tx,
- (unsigned) (nb_pkt - nb_tx));
- fs->fwd_dropped += (nb_pkt - nb_tx);
- rte_pktmbuf_free_bulk(&pkts_burst[nb_tx], nb_pkt - nb_tx);
+ "(nb_pkt:%"PRIu16" - nb_tx:%"PRIu16")="
+ "%"PRIu16" packets\n",
+ fs->tx_port, fs->tx_queue, nb_pkt,
+ nb_pkt - nb_dropped, nb_dropped);
}
return true;