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 <jijiang.liu at intel.com> > --- > 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