[dpdk-stable] [PATCH v5 2/2] app/testpmd: fix txonly forwording
Ivan Malov
Ivan.Malov at oktetlabs.ru
Thu Sep 23 06:25:59 CEST 2021
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 at dpdk.org
>
> Signed-off-by: Alvin Zhang <alvinx.zhang at intel.com>
> Acked-by: Xiaoyun Li <xiaoyun.li at 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, ×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
More information about the stable
mailing list