-----邮件原件-----
发件人: Mike Pattrick <m...@redhat.com>
发送时间: 2023年9月19日 23:58
收件人: Dexia Li <dexia...@jaguarmicro.com>
抄送: David Marchand <david.march...@redhat.com>; ovs-dev@openvswitch.org;
i.maxim...@ovn.org; Eelco Chaudron <echau...@redhat.com>; Qingmin Liu
<qingmin....@jaguarmicro.com>; Joey Xing <joey.x...@jaguarmicro.com>
主题: Re: [ovs-dev] [PATCH] netdev-dpdk: fix tso bug
On Tue, Sep 19, 2023 at 7:51 AM Dexia Li <dexia...@jaguarmicro.com> wrote:
>
> Hi, David
>
> Sorry, I am missing something too...
>
> Firstly, RTE_MBUF_F_TX_TCP_SEG flag in mbuf->ol_flags implies
> RTE_MBUF_F_TX_TCP_CKSUM in last letter.
> Some drivers such as ice can use RTE_MBUF_F_TX_TCP_SEG with
> RTE_MBUF_F_TX_TCP_CKSUM but Some drivers such as Iavf will result in
> driver hang when using this two flags meanwhile. Drivers like ice Can segment
> tcp packets when only RTE_MBUF_F_TX_TCP_SEG flag is set.
>
> Secondly, testpmd csum engine support example, we can see as follows
> RTE_MBUF_F_TX_TCP_CKSUM is unset When RTE_MBUF_F_TX_TCP_SEG flag is set.
> Should ovs follow this example? I suggest so.
> if (tso_segsz)
> ol_flags |= RTE_MBUF_F_TX_TCP_SEG;
> else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
> ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
> }
>
> Thirdly, I use ovs 2.17 and dpdk 21.11 for test. In dpdk 21.11,
> iavf_build_data_desc_cmd_offset_fields func may set
> RTE_MBUF_F_TX_TCP_SEG and RTE_MBUF_F_TX_TCP_CKSUM. Code as follows
>
> if (m->ol_flags & RTE_MBUF_F_TX_TCP_SEG) {
> command |= IAVF_TX_DESC_CMD_L4T_EOFT_TCP;
> offset |= (m->l4_len >> 2) <<
> IAVF_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
> }
>
> /* Enable L4 checksum offloads */
> switch (m->ol_flags & RTE_MBUF_F_TX_L4_MASK) {
> case RTE_MBUF_F_TX_TCP_CKSUM:
> command |= IAVF_TX_DESC_CMD_L4T_EOFT_TCP;
> offset |= (sizeof(struct rte_tcp_hdr) >> 2) <<
> IAVF_TX_DESC_LENGTH_L4_FC_LEN_SHIFT;
> break;
>
> and dpdk 22.11 only set RTE_MBUF_F_TX_TCP_SEG truly.
This conflicts with the Mbuf lib programming guide, which includes an example
with both flags set:
https://doc.dpdk.org/guides/prog_guide/mbuf_lib.html
Other locations in the DPDK documentation suggest that both flags can or should
be set, that they're redundant, or one implies the other. I can't find anywhere
that suggests they are exclusive.
Thanks,
Mike
I don't mean they're exclusive. Only setting RTE_MBUF_F_TX_TCP_SEG flag
provides better compatibility I think.
However, we can also think dpdk 21.11 iavf driver has the bug when setting
RTE_MBUF_F_TX_TCP_SEG and
RTE_MBUF_F_TX_TCP_CKSUM flags meanwhile. This has been corrected in dpdk 22.11.
Ok, I accept your point of view.
Thanks
Dexia
>
> I think, upper application like ovs should provide compatibility for various
> drivers.
> Only setting RTE_MBUF_F_TX_TCP_SEG can support compatibility for various
> drivers tso fuction.
>
>
>
> -----邮件原件-----
> 发件人: Dexia Li
> 发送时间: 2023年9月18日 17:47
> 收件人: David Marchand <david.march...@redhat.com>
> 抄送: ovs-dev@openvswitch.org; i.maxim...@ovn.org; Mike Pattrick
> <m...@redhat.com>; Eelco Chaudron <echau...@redhat.com>
> 主题: 答复: [ovs-dev] [PATCH] netdev-dpdk: fix tso bug
>
> Hi，david
>
> In dpdk 22.11 rte_mbuf_core.h file, there are notes as follows:
>
> /**
> * TCP segmentation offload. To enable this offload feature for a
> * packet to be transmitted on hardware supporting TSO:
> * - set the RTE_MBUF_F_TX_TCP_SEG flag in mbuf->ol_flags (this flag implies
> * RTE_MBUF_F_TX_TCP_CKSUM)
> * - set the flag RTE_MBUF_F_TX_IPV4 or RTE_MBUF_F_TX_IPV6
> * - if it's IPv4, set the RTE_MBUF_F_TX_IP_CKSUM flag
> * - fill the mbuf offload information: l2_len, l3_len, l4_len,
> tso_segsz */
>
> And also, in testpmd example csum engine file csumonly.c, we can see
> func process_inner_cksums partial code as follows
>
> if (tso_segsz)
> ol_flags |= RTE_MBUF_F_TX_TCP_SEG;
> else if (tx_offloads & RTE_ETH_TX_OFFLOAD_TCP_CKSUM) {
> ol_flags |= RTE_MBUF_F_TX_TCP_CKSUM;
> } else {
> if (info->is_tunnel)
> l4_off = info->outer_l2_len +
> info->outer_l3_len +
> info->l2_len + info->l3_len;
> else
> l4_off = info->l2_len + info->l3_len;
> tcp_hdr->cksum = 0;
> tcp_hdr->cksum =
> get_udptcp_checksum(m, l3_hdr, l4_off,
> info->ethertype);
> }
>
> We can see when RTE_MBUF_F_TX_TCP_SEG is set, and
> RTE_ETH_TX_OFFLOAD_TCP_CKSUM is not set.
>
> Best regards
>
> dexia
>
>
> -----邮件原件-----
> 发件人: David Marchand <david.march...@redhat.com>
> 发送时间: 2023年9月14日 19:23
> 收件人: Dexia Li <dexia...@jaguarmicro.com>
> 抄送: ovs-dev@openvswitch.org; i.maxim...@ovn.org; Mike Pattrick
> <m...@redhat.com>; Eelco Chaudron <echau...@redhat.com>
> 主题: Re: [ovs-dev] [PATCH] netdev-dpdk: fix tso bug
>
> Hello,
>
> On Wed, Sep 13, 2023 at 8:55 AM Dexia Li via dev <ovs-dev@openvswitch.org>
> wrote:
> >
> > when userspace tso enabled, mbuf RTE_MBUF_F_TX_TCP_SEG and
> > RTE_MBUF_F_TX_TCP_CKSUM
> >
> > flag bits all positioned will result in driver hang using intel E810
> > vf
> >
> > driver iavf. As refered to dpdk csum example, RTE_MBUF_F_TX_TCP_SEG
> >
> > should only be positioned when tso is open.
> >
> > Signed-off-by: Dexia Li <dexia...@jaguarmicro.com>
>
> Sorry, I am missing something...
>
> The mbuf API mentions that it is implied that RTE_MBUF_F_TX_TCP_CKSUM is set
> along RTE_MBUF_F_TX_TCP_SEG.
> Meaning that an application may request both RTE_MBUF_F_TX_TCP_SEG or
> RTE_MBUF_F_TX_TCP_SEG | RTE_MBUF_F_TX_TCP_CKSUM.
> And iirc the combination of both RTE_MBUF_F_TX_TCP_CKSUM and
> RTE_MBUF_F_TX_TCP_SEG is accepted in some dpdk drivers.
>
>
> Looking at the 22.11 iavf driver, the scalar tx handler looks at tso requests
> first, prepares the descriptor and stops at this.
> It won't look at tcp checksum flags.
> https://git.dpdk.org/dpdk-stable/tree/drivers/net/iavf/iavf_rxtx.c?h=v
> 22.11.3#n2623
> https://git.dpdk.org/dpdk-stable/tree/drivers/net/iavf/iavf_rxtx.c?h=v
> 22.11.3#n2637
>
> As far as the vector tx handlers are concerned, I think they are not usable
> with TSO requests, and I think the iavf_tx_vec_queue_default() check prevents
> such requests from the application.
> Can you perhaps double check which tx handler is in use (gdb is the only
> option to retrieve this information on a running application using dpdk
> 22.11)?
>
>
> --
> David Marchand
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
