patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Zhang, AlvinX" <alvinx.zhang@intel.com>
To: Ivan Malov <Ivan.Malov@oktetlabs.ru>,
	"Li, Xiaoyun" <xiaoyun.li@intel.com>,
	"Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [dpdk-stable] [PATCH v5 2/2] app/testpmd: fix txonly forwording
Date: Thu, 23 Sep 2021 05:11:04 +0000
Message-ID: <BN6PR11MB3890B91C692836D6294FF1349FA39@BN6PR11MB3890.namprd11.prod.outlook.com> (raw)
In-Reply-To: <abf66d31-cb0d-fd91-eb96-d2e22b0aef40@oktetlabs.ru>

> -----Original Message-----
> From: Ivan Malov <Ivan.Malov@oktetlabs.ru>
> Sent: Thursday, September 23, 2021 12:26 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Li, Xiaoyun
> <xiaoyun.li@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH v5 2/2] app/testpmd: fix txonly forwording
> 
> Hi Alvin,
> 
> There's a typo in the commit summary: forwording -> forwarding.

I'll update it in V6.

> 
> 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.

I'll update them in V6.

> 
> > +			    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  5:11 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02  8:20 [dpdk-stable] [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
     [not found] ` <20210917013923.4004-1-alvinx.zhang@intel.com>
2021-09-17  1:39   ` [dpdk-stable] [PATCH v2 2/2] app/testpmd: fix txonly forwording Alvin Zhang
     [not found]   ` <20210918030659.12448-1-alvinx.zhang@intel.com>
2021-09-18  3:06     ` [dpdk-stable] [PATCH v3 " Alvin Zhang
2021-09-18  8:20       ` Li, Xiaoyun
2021-09-22  2:49     ` [dpdk-stable] [PATCH v4 1/2] app/testpmd: update forward engine beginning Alvin Zhang
2021-09-22  2:49       ` [dpdk-stable] [PATCH v4 2/2] app/testpmd: fix txonly forwording Alvin Zhang
2021-09-22  5:58         ` Li, Xiaoyun
2021-09-22  5:59       ` [dpdk-stable] [PATCH v4 1/2] app/testpmd: update forward engine beginning Li, Xiaoyun
2021-09-23  1:49       ` [dpdk-stable] [PATCH v5 " Alvin Zhang
2021-09-23  1:49         ` [dpdk-stable] [PATCH v5 2/2] app/testpmd: fix txonly forwording Alvin Zhang
2021-09-23  4:25           ` Ivan Malov
2021-09-23  5:11             ` Zhang, AlvinX [this message]
2021-09-23  8:01         ` [dpdk-stable] [PATCH v6 1/2] app/testpmd: update forward engine beginning Alvin Zhang
2021-09-23  8:01           ` [dpdk-stable] [PATCH v6 2/2] app/testpmd: fix txonly forwarding Alvin Zhang
2021-10-08 17:01           ` [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=BN6PR11MB3890B91C692836D6294FF1349FA39@BN6PR11MB3890.namprd11.prod.outlook.com \
    --to=alvinx.zhang@intel.com \
    --cc=Ivan.Malov@oktetlabs.ru \
    --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

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git