DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ivan Malov <Ivan.Malov@oktetlabs.ru>
To: Alvin Zhang <alvinx.zhang@intel.com>,
	xiaoyun.li@intel.com, konstantin.ananyev@intel.com
Cc: dev@dpdk.org, stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix txonly forwording
Date: Thu, 23 Sep 2021 07:25:59 +0300	[thread overview]
Message-ID: <abf66d31-cb0d-fd91-eb96-d2e22b0aef40@oktetlabs.ru> (raw)
In-Reply-To: <20210923014951.12696-2-alvinx.zhang@intel.com>

Hi Alvin,

There's a typo in the commit summary: forwording -> forwarding.

On 23/09/2021 04:49, Alvin Zhang wrote:
> When random number of Tx segments is enabled, because the actual
> number of segments may be only one, the first segment of the Tx
> packets must accommodate a complete being sending Eth/IP/UDP packet.
> 
> Besides, if multiple flow is enabled, the forwarding will update
> the IP and UDP header, these headers shouldn't cross segments.
> This also requires the first Tx segment can accommodate a complete
> Eth/IP/UDP packet.
> 
> In addition, if time stamp is enabled, the forwarding needs more
> Tx segment space for time stamp information.
> 
> This patch adds checks in beginning of forward engine to make sure
> all above conditions are met.
> 
> Bugzilla ID: 797
> Fixes: 79bec05b32b7 ("app/testpmd: add ability to split outgoing packets")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> ---
> 
> v5: fixes a compilation issue
> ---
>   app/test-pmd/txonly.c | 67 ++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 55 insertions(+), 12 deletions(-)
> 
> diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
> index 386a4ff..e7b1b42 100644
> --- a/app/test-pmd/txonly.c
> +++ b/app/test-pmd/txonly.c
> @@ -40,6 +40,13 @@
>   
>   #include "testpmd.h"
>   
> +struct tx_timestamp {
> +	rte_be32_t signature;
> +	rte_be16_t pkt_idx;
> +	rte_be16_t queue_idx;
> +	rte_be64_t ts;
> +};
> +
>   /* use RFC863 Discard Protocol */
>   uint16_t tx_udp_src_port = 9;
>   uint16_t tx_udp_dst_port = 9;
> @@ -257,12 +264,7 @@
>   
>   	if (unlikely(timestamp_enable)) {
>   		uint64_t skew = RTE_PER_LCORE(timestamp_qskew);
> -		struct {
> -			rte_be32_t signature;
> -			rte_be16_t pkt_idx;
> -			rte_be16_t queue_idx;
> -			rte_be64_t ts;
> -		} timestamp_mark;
> +		struct tx_timestamp timestamp_mark;
>   
>   		if (unlikely(timestamp_init_req !=
>   				RTE_PER_LCORE(timestamp_idone))) {
> @@ -438,13 +440,23 @@
>   static int
>   tx_only_begin(portid_t pi)
>   {
> -	uint16_t pkt_data_len;
> +	uint16_t pkt_hdr_len, pkt_data_len;
>   	int dynf;
>   
> -	pkt_data_len = (uint16_t) (tx_pkt_length - (
> -					sizeof(struct rte_ether_hdr) +
> -					sizeof(struct rte_ipv4_hdr) +
> -					sizeof(struct rte_udp_hdr)));
> +	pkt_hdr_len = (uint16_t)(sizeof(struct rte_ether_hdr) +
> +				 sizeof(struct rte_ipv4_hdr) +
> +				 sizeof(struct rte_udp_hdr));
> +	pkt_data_len = tx_pkt_length - pkt_hdr_len;
> +
> +	if ((tx_pkt_split == TX_PKT_SPLIT_RND || txonly_multi_flow) &&
> +	    tx_pkt_seg_lengths[0] < pkt_hdr_len) {
> +		TESTPMD_LOG(ERR,
> +			    "Random segment number or multiple flow enabled,"
> +			    " but tx_pkt_seg_lengths[0] %u < %u (needed)\n",

This should probably be on a single line:

TESTPMD_LOG(ERR, "Random segment number or multiple flow enabled, but 
tx_pkt_seg_lengths[0] %u < %u (needed)\n",

because this way it's more search-friendly. Style checks should be OK 
with that.

> +			    tx_pkt_seg_lengths[0], pkt_hdr_len);
> +		return -EINVAL;
> +	}
> +
>   	setup_pkt_udp_ip_headers(&pkt_ip_hdr, &pkt_udp_hdr, pkt_data_len);
>   
>   	timestamp_enable = false;
> @@ -463,8 +475,39 @@
>   			   timestamp_mask &&
>   			   timestamp_off >= 0 &&
>   			   !rte_eth_read_clock(pi, &timestamp_initial[pi]);
> -	if (timestamp_enable)
> +
> +	if (timestamp_enable) {
> +		pkt_hdr_len += sizeof(struct tx_timestamp);
> +
> +		if (tx_pkt_split == TX_PKT_SPLIT_RND) {
> +			if (tx_pkt_seg_lengths[0] < pkt_hdr_len) {
> +				TESTPMD_LOG(ERR,
> +					    "Time stamp and random segment number enabled,"
> +					    " but tx_pkt_seg_lengths[0] %u < %u (needed)\n",

Likewise.

> +					    tx_pkt_seg_lengths[0], pkt_hdr_len);
> +				return -EINVAL;
> +			}
> +		} else {
> +			uint16_t total = 0;
> +			uint8_t i;
> +
> +			for (i = 0; i < tx_pkt_nb_segs; i++) {
> +				total += tx_pkt_seg_lengths[i];
> +				if (total >= pkt_hdr_len)
> +					break;
> +			}
> +
> +			if (total < pkt_hdr_len) {
> +				TESTPMD_LOG(ERR,
> +					    "No enough Tx segment space for time stamp info."
> +					    " total %u < %u (needed)\n",

Likewise.

> +					    total, pkt_hdr_len);
> +				return -EINVAL;
> +			}
> +		}
>   		timestamp_init_req++;
> +	}
> +
>   	/* Make sure all settings are visible on forwarding cores.*/
>   	rte_wmb();
>   	return 0;
> 

-- 
Ivan M

  reply	other threads:[~2021-09-23  4:26 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02  8:20 [dpdk-dev] [PATCH] app/testpmd: fix random number of Tx segments Alvin Zhang
2021-09-06  8:58 ` Li, Xiaoyun
2021-09-06 10:03   ` Zhang, AlvinX
2021-09-06 10:54     ` Li, Xiaoyun
2021-09-07  2:25       ` Zhang, AlvinX
2021-09-07  8:05         ` Li, Xiaoyun
2021-09-17  1:39 ` [dpdk-dev] [PATCH v2 1/2] app/testpmd: update forward engine beginning Alvin Zhang
2021-09-17  1:39   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix txonly forwording Alvin Zhang
2021-09-18  3:06   ` [dpdk-dev] [PATCH v3 1/2] app/testpmd: update forward engine beginning Alvin Zhang
2021-09-18  3:06     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix txonly forwording Alvin Zhang
2021-09-18  8:20       ` Li, Xiaoyun
2021-09-18  8:31     ` [dpdk-dev] [PATCH v3 1/2] app/testpmd: update forward engine beginning Li, Xiaoyun
2021-09-18  8:50       ` Zhang, AlvinX
2021-09-22  2:49     ` [dpdk-dev] [PATCH v4 " Alvin Zhang
2021-09-22  2:49       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix txonly forwording Alvin Zhang
2021-09-22  5:58         ` Li, Xiaoyun
2021-09-22  5:59       ` [dpdk-dev] [PATCH v4 1/2] app/testpmd: update forward engine beginning Li, Xiaoyun
2021-09-23  1:49       ` [dpdk-dev] [PATCH v5 " Alvin Zhang
2021-09-23  1:49         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix txonly forwording Alvin Zhang
2021-09-23  4:25           ` Ivan Malov [this message]
2021-09-23  5:11             ` Zhang, AlvinX
2021-09-23  8:01         ` [dpdk-dev] [PATCH v6 1/2] app/testpmd: update forward engine beginning Alvin Zhang
2021-09-23  8:01           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: fix txonly forwarding Alvin Zhang
2021-10-08 17:01           ` [dpdk-dev] [dpdk-stable] [PATCH v6 1/2] app/testpmd: update forward engine beginning Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=abf66d31-cb0d-fd91-eb96-d2e22b0aef40@oktetlabs.ru \
    --to=ivan.malov@oktetlabs.ru \
    --cc=alvinx.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=stable@dpdk.org \
    --cc=xiaoyun.li@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).