From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by dpdk.org (Postfix) with ESMTP id C543F1518 for ; Thu, 27 Nov 2014 11:23:38 +0100 (CET) Received: by mail-wg0-f42.google.com with SMTP id z12so6089000wgg.29 for ; Thu, 27 Nov 2014 02:23:38 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :subject:references:in-reply-to:content-type :content-transfer-encoding; bh=3AbkESRjNgRfhYtwDlxijWzOZxeINdwLmZ3lpXefiQI=; b=HjzWlio7YV2RWYXydNyUDuNYauTnLFNgl6J8kBvCv5cZwgtIm0YsG0Z3/YLdpEAAWY b6ZXXAP61UAKlhV/ydAYJb+p9WJ6xj31uBkFjT6uN9+TbZLOyGfMFlYNshnDXt+YIBw2 dJx9PjustkL8xDMQPAGviYFgX27iIO9m9gj12mI4CD7oHq0AaglOZjqz+SxSxd2Vj2cz ZfO3qIr4Hgd/q1CCMy8TOc9ICL2BASXRyVSFL50vOBPEJFDzTfm5XCWb4APPEOjRvw8k QA48H5OHiDtMLMXgAsLXaMv6GNzByfcDVPi4WzvFAHdg3VNs2FPwg1GJmQlNUsxAJ+LA SYSg== X-Gm-Message-State: ALoCoQlyW80LOjMO6HI1SEIzIliZ682TfYJasWlaANwon7D0pfJn/fTtwOx4nz0eBxBGUTmBeuNO X-Received: by 10.180.74.68 with SMTP id r4mr48616063wiv.33.1417083817467; Thu, 27 Nov 2014 02:23:37 -0800 (PST) Received: from [10.16.0.195] (guy78-3-82-239-227-177.fbx.proxad.net. [82.239.227.177]) by mx.google.com with ESMTPSA id e7sm10052856wjx.31.2014.11.27.02.23.36 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 27 Nov 2014 02:23:36 -0800 (PST) Message-ID: <5476FBA8.90706@6wind.com> Date: Thu, 27 Nov 2014 11:23:36 +0100 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: Jijiang Liu , dev@dpdk.org References: <1417076319-629-1-git-send-email-jijiang.liu@intel.com> <1417076319-629-4-git-send-email-jijiang.liu@intel.com> In-Reply-To: <1417076319-629-4-git-send-email-jijiang.liu@intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 3/3] testpmd:rework csum forward engine X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Nov 2014 10:23:38 -0000 Hi Jijiang, On 11/27/2014 09:18 AM, Jijiang Liu wrote: > The changes include: > 1. use the new introduced ol_flags and fields in csumonly.c file; > 2. fix an issue of outer UDP checksum check; > 3. fix an issue that is if the TESTPMD_TX_OFFLOAD_IP_CKSUM is not set, and the "ol_flags |= PKT_TX_IPV4" should be done in the process_inner_cksums(); > > Signed-off-by: Jijiang Liu > --- > app/test-pmd/csumonly.c | 55 +++++++++++++++++++++++++--------------------- > 1 files changed, 30 insertions(+), 25 deletions(-) > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c > index d8c080a..0727510 100644 > --- a/app/test-pmd/csumonly.c > +++ b/app/test-pmd/csumonly.c > @@ -189,11 +189,12 @@ process_inner_cksums(void *l3_hdr, uint16_t ethertype, uint16_t l3_len, > } else { > if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_IP_CKSUM) > ol_flags |= PKT_TX_IP_CKSUM; > - else > + else { > ipv4_hdr->hdr_checksum = > rte_ipv4_cksum(ipv4_hdr); > + ol_flags |= PKT_TX_IPV4; > + } > } > - ol_flags |= PKT_TX_IPV4; Same remark than in the cover letter. I think we currently don't agree on the meaning of PKT_TX_IPV4 and PKT_TX_IPV6. > udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + outer_l3_len); > /* do not recalculate udp cksum if it was 0 */ > if (udp_hdr->dgram_cksum != 0) { > udp_hdr->dgram_cksum = 0; > - if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) == 0) { > - if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) > - udp_hdr->dgram_cksum = > - rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr); > - else > - udp_hdr->dgram_cksum = > - rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr); > - } > + if (outer_ethertype == _htons(ETHER_TYPE_IPv4)) > + udp_hdr->dgram_cksum = > + rte_ipv4_udptcp_cksum(ipv4_hdr, udp_hdr); > + else > + udp_hdr->dgram_cksum = > + rte_ipv6_udptcp_cksum(ipv6_hdr, udp_hdr); So, I understand that today no dpdk driver support the offload of outer udp checksum, and that it has to be done in software? > @@ -383,10 +385,6 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > if (((m->ol_flags & PKT_RX_TUNNEL_IPV4_HDR) || > (m->ol_flags & PKT_RX_TUNNEL_IPV6_HDR))) > tunnel = 1; > - /* else check udp destination port, 4789 is the default > - * vxlan port (rfc7348) */ > - else if (udp_hdr->dst_port == _htons(4789)) > - tunnel = 1; > > if (tunnel == 1) { > outer_ethertype = ethertype; > @@ -394,6 +392,11 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > outer_l3_len = l3_len; > outer_l3_hdr = l3_hdr; > > + /* check udp destination port, 4789 is the default > + * vxlan port (rfc7348) */ > + if (udp_hdr->dst_port == _htons(4789)) > + l4_tun_len = ETHER_VXLAN_HLEN; > + Why moving this? It won't work anymore with drivers != i40e. > eth_hdr = (struct ether_hdr *)((char *)udp_hdr + > sizeof(struct udp_hdr) + > sizeof(struct vxlan_hdr)); > @@ -432,10 +435,11 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > > if (tunnel == 1) { > if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) { > - m->l2_len = outer_l2_len; > - m->l3_len = outer_l3_len; > - m->inner_l2_len = l2_len; > - m->inner_l3_len = l3_len; > + m->outer_l2_len = outer_l2_len; > + m->outer_l3_len = outer_l3_len; > + m->l2_len = l2_len; > + m->l3_len = l3_len; > + m->l4_tun_len = l4_tun_len; > } > else { > /* if we don't do vxlan cksum in hw, > @@ -470,7 +474,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > { PKT_TX_UDP_CKSUM, PKT_TX_L4_MASK }, > { PKT_TX_TCP_CKSUM, PKT_TX_L4_MASK }, > { PKT_TX_SCTP_CKSUM, PKT_TX_L4_MASK }, > - { PKT_TX_VXLAN_CKSUM, PKT_TX_VXLAN_CKSUM }, > + { PKT_TX_UDP_TUNNEL_PKT, PKT_TX_UDP_TUNNEL_PKT }, > { PKT_TX_TCP_SEG, PKT_TX_TCP_SEG }, > }; > unsigned j; > @@ -498,8 +502,9 @@ pkt_burst_checksum_forward(struct fwd_stream *fs) > m->l2_len, m->l3_len, m->l4_len); > if ((tunnel == 1) && > (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)) > - printf("tx: m->inner_l2_len=%d m->inner_l3_len=%d\n", > - m->inner_l2_len, m->inner_l3_len); > + printf("tx: m->outer_l2_len=%d m->outer_l3_len=%d\n" > + "m->l4_tun_len=%d", m->outer_l2_len, > + m->outer_l3_len, m->l4_tun_len); > if (tso_segsz != 0) > printf("tx: m->tso_segsz=%d\n", m->tso_segsz); > printf("tx: flags="); > One more comment: I think the help of csum forward engine in testpmd command lines + all the comments in csumonly.c should be checked. For example, I can see: " The VxLAN flag concerns the outer IP and UDP layer (if packet is recognized as a vxlan packet)". If it concerns the IP header only, the comment should be modified. Regards, Olivier