[v2,3/4] app/testpmd: fix packet header in txonly mode

Message ID 20200820014204.25035-4-huwei013@chinasoftinc.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series minor fixes for testpmd |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Wei Hu (Xavier) Aug. 20, 2020, 1:42 a.m. UTC
  From: Chengchang Tang <tangchengchang@huawei.com>

In txonly forward mode, the packet header is fixed by the initial
setting, including the packet length and checksum. So when the packets
varies, this may cause a packet header error. Currently, there are two
methods in txonly mode to randomly change the packets.
1. Set txsplit random and txpkts (x[,y]*), the number of segments
   each packets will be a random value between 1 and total number of
   segments determined by txpkts settings.
   The step as follows:
     a) ./testpmd -w xxx -l xx -n 4 -- -i --rxd xxxx --txd xxxx
     b) set fwd txonly
     c) set txsplit rand
     d) set txpkts 2048,2048,2048,2048
     e) start
The nb_segs of the packets sent by testpmd will be 1~4. The real packet
length will be 2048, 4096, 6144 and 8192. But in fact the packet length
in ip header and udp header will be fixed by 8178 and 8158.

2. Set txonly-multi-flow. the ip address will be varied to generate
   multiple flow.
   The step as follows:
     a) ./testpmd -w xxx -l xx -n 4 -- -i --txonly-multi-flow
     b) set fwd txonly
     c) start
The ip address of each pkts will change randomly, but since the header
is fixed, the checksum may be a error value.

Therefore, this patch adds a function to update the packet length and
check sum in the pkts header when the txsplit mode is set to rand or
multi-flow is set.

Fixes: 82010ef55e7c ("app/testpmd: make txonly mode generate multiple flows")
Fixes: 79bec05b32b7 ("app/testpmd: add ability to split outgoing packets")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
v1 -> v2: fix TYPO_SPELLING warning in the commit log.
---
 app/test-pmd/txonly.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
  

Comments

Ferruh Yigit Sept. 14, 2020, 4:23 p.m. UTC | #1
On 8/20/2020 2:42 AM, Wei Hu (Xavier) wrote:
> From: Chengchang Tang <tangchengchang@huawei.com>
> 
> In txonly forward mode, the packet header is fixed by the initial
> setting, including the packet length and checksum. So when the packets
> varies, this may cause a packet header error. Currently, there are two
> methods in txonly mode to randomly change the packets.
> 1. Set txsplit random and txpkts (x[,y]*), the number of segments
>    each packets will be a random value between 1 and total number of
>    segments determined by txpkts settings.
>    The step as follows:
>      a) ./testpmd -w xxx -l xx -n 4 -- -i --rxd xxxx --txd xxxx
>      b) set fwd txonly
>      c) set txsplit rand
>      d) set txpkts 2048,2048,2048,2048
>      e) start
> The nb_segs of the packets sent by testpmd will be 1~4. The real packet
> length will be 2048, 4096, 6144 and 8192. But in fact the packet length
> in ip header and udp header will be fixed by 8178 and 8158.

Although I confirm the patch fixes the ip & udp header packet size for
"txsplit=rand" config,
I am always getting actual packet size wrong, independent from
'txsplit', and it is always first segment size. Am I doing something wrong?


And not related to this patch but why setting 'txpkts' requires "--rxd
xxxx --txd xxxx" options explicitly set? If you are already there can
you also remove this restriction?

> 
> 2. Set txonly-multi-flow. the ip address will be varied to generate
>    multiple flow.
>    The step as follows:
>      a) ./testpmd -w xxx -l xx -n 4 -- -i --txonly-multi-flow
>      b) set fwd txonly
>      c) start
> The ip address of each pkts will change randomly, but since the header
> is fixed, the checksum may be a error value.

+1, I confirm fixing the checksum error.

> 
> Therefore, this patch adds a function to update the packet length and
> check sum in the pkts header when the txsplit mode is set to rand or
> multi-flow is set.
> 
> Fixes: 82010ef55e7c ("app/testpmd: make txonly mode generate multiple flows")
> Fixes: 79bec05b32b7 ("app/testpmd: add ability to split outgoing packets")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>

<...>
  
Chengchang Tang Sept. 17, 2020, 7:10 a.m. UTC | #2
On 2020/9/15 0:23, Ferruh Yigit wrote:
> On 8/20/2020 2:42 AM, Wei Hu (Xavier) wrote:
>> From: Chengchang Tang <tangchengchang@huawei.com>
>>
>> In txonly forward mode, the packet header is fixed by the initial
>> setting, including the packet length and checksum. So when the packets
>> varies, this may cause a packet header error. Currently, there are two
>> methods in txonly mode to randomly change the packets.
>> 1. Set txsplit random and txpkts (x[,y]*), the number of segments
>>    each packets will be a random value between 1 and total number of
>>    segments determined by txpkts settings.
>>    The step as follows:
>>      a) ./testpmd -w xxx -l xx -n 4 -- -i --rxd xxxx --txd xxxx
>>      b) set fwd txonly
>>      c) set txsplit rand
>>      d) set txpkts 2048,2048,2048,2048
>>      e) start
>> The nb_segs of the packets sent by testpmd will be 1~4. The real packet
>> length will be 2048, 4096, 6144 and 8192. But in fact the packet length
>> in ip header and udp header will be fixed by 8178 and 8158.
> 
> Although I confirm the patch fixes the ip & udp header packet size for
> "txsplit=rand" config,
> I am always getting actual packet size wrong, independent from
> 'txsplit', and it is always first segment size. Am I doing something wrong?

Yes, I miss it. If the txsplit is not set and the txpkts is set, the txonly
fwd engine will only send single segment packets, and there will be a payload
error for the data_len will be refreshed by the sum of the segment size.

May be the data_len should be the first segment size if the txsplit is not set?
I will fix it in the next version.
> 
> And not related to this patch but why setting 'txpkts' requires "--rxd
> xxxx --txd xxxx" options explicitly set? If you are already there can
> you also remove this restriction?

OK, I will fix it in next version.
> <...> 
> .
>
  
Ferruh Yigit Sept. 17, 2020, 11:16 a.m. UTC | #3
On 9/17/2020 8:10 AM, Chengchang Tang wrote:
> 
> 
> On 2020/9/15 0:23, Ferruh Yigit wrote:
>> On 8/20/2020 2:42 AM, Wei Hu (Xavier) wrote:
>>> From: Chengchang Tang <tangchengchang@huawei.com>
>>>
>>> In txonly forward mode, the packet header is fixed by the initial
>>> setting, including the packet length and checksum. So when the packets
>>> varies, this may cause a packet header error. Currently, there are two
>>> methods in txonly mode to randomly change the packets.
>>> 1. Set txsplit random and txpkts (x[,y]*), the number of segments
>>>     each packets will be a random value between 1 and total number of
>>>     segments determined by txpkts settings.
>>>     The step as follows:
>>>       a) ./testpmd -w xxx -l xx -n 4 -- -i --rxd xxxx --txd xxxx
>>>       b) set fwd txonly
>>>       c) set txsplit rand
>>>       d) set txpkts 2048,2048,2048,2048
>>>       e) start
>>> The nb_segs of the packets sent by testpmd will be 1~4. The real packet
>>> length will be 2048, 4096, 6144 and 8192. But in fact the packet length
>>> in ip header and udp header will be fixed by 8178 and 8158.
>>
>> Although I confirm the patch fixes the ip & udp header packet size for
>> "txsplit=rand" config,
>> I am always getting actual packet size wrong, independent from
>> 'txsplit', and it is always first segment size. Am I doing something wrong?
> 
> Yes, I miss it. If the txsplit is not set and the txpkts is set, the txonly
> fwd engine will only send single segment packets, and there will be a payload
> error for the data_len will be refreshed by the sum of the segment size.
> 

I am getting the size error when 'txsplit' is 'on' or 'rand'. In that 
case packet header are correct (after your patch) but wireshark 
complains the size of the packet on wire is different than the values in 
headers. And the size of the actual packet is reported by wireshark as 
the size of the first segment.

> May be the data_len should be the first segment size if the txsplit is not set?
> I will fix it in the next version.
>>
>> And not related to this patch but why setting 'txpkts' requires "--rxd
>> xxxx --txd xxxx" options explicitly set? If you are already there can
>> you also remove this restriction?
> 
> OK, I will fix it in next version.

Thanks, appreciated.
  
Chengchang Tang Sept. 17, 2020, 11:48 a.m. UTC | #4
On 2020/9/17 19:16, Ferruh Yigit wrote:
> On 9/17/2020 8:10 AM, Chengchang Tang wrote:
>>
>>
>> On 2020/9/15 0:23, Ferruh Yigit wrote:
>>> On 8/20/2020 2:42 AM, Wei Hu (Xavier) wrote:
>>>> From: Chengchang Tang <tangchengchang@huawei.com>
>>>>
>>>> In txonly forward mode, the packet header is fixed by the initial
>>>> setting, including the packet length and checksum. So when the packets
>>>> varies, this may cause a packet header error. Currently, there are two
>>>> methods in txonly mode to randomly change the packets.
>>>> 1. Set txsplit random and txpkts (x[,y]*), the number of segments
>>>>     each packets will be a random value between 1 and total number of
>>>>     segments determined by txpkts settings.
>>>>     The step as follows:
>>>>       a) ./testpmd -w xxx -l xx -n 4 -- -i --rxd xxxx --txd xxxx
>>>>       b) set fwd txonly
>>>>       c) set txsplit rand
>>>>       d) set txpkts 2048,2048,2048,2048
>>>>       e) start
>>>> The nb_segs of the packets sent by testpmd will be 1~4. The real packet
>>>> length will be 2048, 4096, 6144 and 8192. But in fact the packet length
>>>> in ip header and udp header will be fixed by 8178 and 8158.
>>>
>>> Although I confirm the patch fixes the ip & udp header packet size for
>>> "txsplit=rand" config,
>>> I am always getting actual packet size wrong, independent from
>>> 'txsplit', and it is always first segment size. Am I doing something wrong?
>>
>> Yes, I miss it. If the txsplit is not set and the txpkts is set, the txonly
>> fwd engine will only send single segment packets, and there will be a payload
>> error for the data_len will be refreshed by the sum of the segment size.
>>
> 
> I am getting the size error when 'txsplit' is 'on' or 'rand'. In that case packet header are correct (after your patch) but wireshark complains the size of the packet on wire is different than the values in headers. And the size of the actual packet is reported by wireshark as the size of the first segment.

Maybe it should be configured with multi_segs offload. If the 'txsplit' is 'on' or 'rand',
the txonly engine will send a multi segments packets. I tested it on hns3 PMD, which
support a multi_segs by default configuration. So, I miss to add these information to
the commit log. For some PMDs, the multi_segs is not support as default.

>> May be the data_len should be the first segment size if the txsplit is not set?
>> I will fix it in the next version.
>>>
<...>
> 
> 
> .
>
  

Patch

diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index 3bae367ee..ad3b0a765 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -156,6 +156,34 @@  setup_pkt_udp_ip_headers(struct rte_ipv4_hdr *ip_hdr,
 	ip_hdr->hdr_checksum = (uint16_t) ip_cksum;
 }
 
+static inline void
+update_pkt_header(struct rte_mbuf *pkt, uint32_t total_pkt_len)
+{
+	struct rte_ipv4_hdr *ip_hdr;
+	struct rte_udp_hdr *udp_hdr;
+	uint16_t pkt_data_len;
+	uint16_t pkt_len;
+
+	pkt_data_len = (uint16_t) (total_pkt_len - (
+					sizeof(struct rte_ether_hdr) +
+					sizeof(struct rte_ipv4_hdr) +
+					sizeof(struct rte_udp_hdr)));
+	/* updata udp pkt length */
+	udp_hdr = rte_pktmbuf_mtod_offset(pkt, struct rte_udp_hdr *,
+				sizeof(struct rte_ether_hdr) +
+				sizeof(struct rte_ipv4_hdr));
+	pkt_len = (uint16_t) (pkt_data_len + sizeof(struct rte_udp_hdr));
+	udp_hdr->dgram_len = RTE_CPU_TO_BE_16(pkt_len);
+
+	/* updata ip pkt length and csum */
+	ip_hdr = rte_pktmbuf_mtod_offset(pkt, struct rte_ipv4_hdr *,
+				sizeof(struct rte_ether_hdr));
+	ip_hdr->hdr_checksum = 0;
+	pkt_len = (uint16_t) (pkt_len + sizeof(struct rte_ipv4_hdr));
+	ip_hdr->total_length = RTE_CPU_TO_BE_16(pkt_len);
+	ip_hdr->hdr_checksum = rte_ipv4_cksum(ip_hdr);
+}
+
 static inline bool
 pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 		struct rte_ether_hdr *eth_hdr, const uint16_t vlan_tci,
@@ -223,6 +251,10 @@  pkt_burst_prepare(struct rte_mbuf *pkt, struct rte_mempool *mbp,
 	copy_buf_to_pkt(&pkt_udp_hdr, sizeof(pkt_udp_hdr), pkt,
 			sizeof(struct rte_ether_hdr) +
 			sizeof(struct rte_ipv4_hdr));
+
+	if (unlikely(tx_pkt_split == TX_PKT_SPLIT_RND) || txonly_multi_flow)
+		update_pkt_header(pkt, pkt_len);
+
 	if (unlikely(timestamp_enable)) {
 		uint64_t skew = RTE_PER_LCORE(timestamp_qskew);
 		struct {