Re: [PATCH] i40e: fix ipv6 TSO issue for tx function

[Zhe Tao <zhe.tao@intel.com>]

On Tue, Mar 22, 2016 at 09:38:55PM +0800, Ananyev, Konstantin wrote:
> Hi, 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhe Tao
> > Sent: Tuesday, March 22, 2016 1:14 PM
> > To: dev@dpdk.org
> > Cc: Tao, Zhe
> > Subject: [dpdk-dev] [PATCH] i40e: fix ipv6 TSO issue for tx function
> > 
> > Issue:
> > when using the following CLI in testpmd to enable ipv6 TSO feature
> > =============
> > set verbose 1
> > csum set ip hw 0
> > csum set udp hw 0
> > csum set tcp hw 0
> > csum set sctp hw 0
> > csum set outer-ip hw 0
> > csum parse_tunnel on 0
> > tso set 800 0
> > set fwd csum
> > 
> > start
> > =============
> > 
> > We will not get we want, the ipv6 packets sent out from IXIA can be received by
> > i40e, but cannot forward to another port.
> > The root cause is when HW doing the TSO offload for packets, it not only depends
> > on the context descriptor to define the MSS and TSO payload size, it also
> > need to know whether this packets is ipv4 or ipv6, ipv4 need the header csum,
> > but ipv6 doesn't need the csum. We need to use the i40e_txd_enable_checksum to
> > set the ipv6 type flag into the data descriptor when the packets are for
> > ipv6 TSO.
> > 
> > Fixes: e3f0151f (i40e: enable Tx checksum only for offloaded packets)
> > 
> > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > ---
> >  drivers/net/i40e/i40e_rxtx.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > index 1488f2f..ffd6dba 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -1545,6 +1545,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> >  	uint16_t slen;
> >  	uint64_t buf_dma_addr;
> >  	union i40e_tx_offload tx_offload = {0};
> > +	bool enable_checksum = 0;
> > 
> >  	txq = tx_queue;
> >  	sw_ring = txq->sw_ring;
> > @@ -1620,7 +1621,13 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> > 
> >  		/* Enable checksum offloading */
> >  		cd_tunneling_params = 0;
> > -		if (ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK) {
> > +		/* Check whether need to do checksum or not */
> > +		if ((ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK) ||
> > +		    ((ol_flags & PKT_TX_IPV6) && (ol_flags & PKT_TX_TCP_SEG))) {
> > +			enable_checksum = 1;
> > +		}
> > +
> > +		if (enable_checksum) {
> >  			i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
> >  				tx_offload, &cd_tunneling_params);
> >  		}
> 
> 
> Wonder can't we just include PKT_TX_TCP_SEG into I40E_TX_CKSUM_OFFLOAD_MASK,
> and keep i40e_xmit_pkts() unchanged?
> Konstantin
agreed with you, checked the code again, the logic for ipv4 TSO also has some problem,
so should add PKT_TX_TCP_SEG flag for both ipv4&ipv6 TSO to the offload mask.
 
>