From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B818FA0C46; Thu, 23 Sep 2021 06:26:05 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7F8CA410D7; Thu, 23 Sep 2021 06:26:05 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 5AECC40E64; Thu, 23 Sep 2021 06:26:04 +0200 (CEST) Received: from [100.65.5.102] (unknown [5.144.122.215]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 7ED1E7F530; Thu, 23 Sep 2021 07:26:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 7ED1E7F530 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1632371163; bh=8UVeH9v8lbrGV0rtzdzVk5iXnHH0cNJkriojxGeBy3c=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=gVYd7F3FEvxguXYb3QYyTHTTUXRNQ2e68oVfbCTk9Z0oJYDtAmHEoqMJGHzskoXPy QxL0MWPJn66ufEektDwWY+Gsik+OV6U2J4e2/G2QGTv5rEGc+Zbjva1pBn8ONoS+og uQKZ6mEpu4UQI1crXHIstKF2DfDFVD1mAZJ2ABCM= To: Alvin Zhang , xiaoyun.li@intel.com, konstantin.ananyev@intel.com Cc: dev@dpdk.org, stable@dpdk.org References: <20210922024952.11848-1-alvinx.zhang@intel.com> <20210923014951.12696-1-alvinx.zhang@intel.com> <20210923014951.12696-2-alvinx.zhang@intel.com> From: Ivan Malov Message-ID: Date: Thu, 23 Sep 2021 07:25:59 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <20210923014951.12696-2-alvinx.zhang@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix txonly forwording X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > Acked-by: Xiaoyun Li > --- > > 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, ×tamp_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