From 5880ae9a12d26b3f221447a404334416cb60c3d2 Mon Sep 17 00:00:00 2001 From: Divakar Date: Fri, 29 Apr 2016 10:29:56 +0530 Subject: [PATCH] Dont treat all FFFF checksum packets as DIAG packets Currently, in the receiving Vrouter, if the checksum of the UDP packets is FFFF, it is treated as DIAG packets. But there can be some UDP packets whose checksum can be FFFF. These should not be treated as DIAG packets. To achieve this, DIAG packets are treated like below. Agent ensures that checksum of DIAG packets is not FFFF, but stores FFFF as checksum in UDP header. In the receiving Vrouter if UDP's checksum is seen as FFFF, checksum is validated again to verify whether that is correct checksum or not. If that is a wrong checksum, it would be marked as DIAG packet. If right checksum, it would be processed as any other regular packet. Also incase of MplsoUDP encapsulation, even if the configuration is to calculate the outer UDP checksum, if DIAG packet, the outer UDP checksum is not computed. This ensures that,on the receiving side, if checksum is validated by NIC, we will not have to compute the checksum for the inner packet once again. To ensure that there are not transmit errors, Agent calcualtes the checksum in the payload and validates it after receiving. Change-Id: Ie5f88deeea70e15aefda4b9dce49ccc48df3f117 closes-bug: #1576506 --- dp-core/vr_packet.c | 7 +-- include/vr_packet.h | 2 +- linux/vr_host_interface.c | 10 ++-- linux/vrouter_mod.c | 104 +++++++++++++++++--------------------- 4 files changed, 57 insertions(+), 66 deletions(-) diff --git a/dp-core/vr_packet.c b/dp-core/vr_packet.c index ff6c5523c..30b089e36 100644 --- a/dp-core/vr_packet.c +++ b/dp-core/vr_packet.c @@ -68,7 +68,7 @@ vr_ip6_proto_pull(struct vr_ip6 *ip6h) */ int vr_ip_transport_parse(struct vr_ip *iph, struct vr_ip6 *ip6h, - struct tcphdr **tcphp, unsigned int frag_size, + void **thp, unsigned int frag_size, void (do_tcp_mss_adj)(struct tcphdr *, unsigned short, unsigned char), unsigned int *hlenp, @@ -113,6 +113,9 @@ vr_ip_transport_parse(struct vr_ip *iph, struct vr_ip6 *ip6h, if (thdr_valid) { tcph_pull_len = pull_len; + if (thp) + *thp = (char *)iph + hlen; + if (ip_proto == VR_IP_PROTO_TCP) { pull_len += sizeof(struct vr_tcp); } else if (ip_proto == VR_IP_PROTO_UDP) { @@ -254,8 +257,6 @@ vr_ip_transport_parse(struct vr_ip *iph, struct vr_ip6 *ip6h, *th_csump = th_csum; if (tcph_pull_lenp) *tcph_pull_lenp = tcph_pull_len; - if (tcphp) - *tcphp = (struct tcphdr *)tcph; *pull_lenp = pull_len; return 0; diff --git a/include/vr_packet.h b/include/vr_packet.h index 8c2d45dfb..43faa2dbb 100644 --- a/include/vr_packet.h +++ b/include/vr_packet.h @@ -518,7 +518,7 @@ bool vr_ip_proto_pull(struct vr_ip *); bool vr_ip6_proto_pull(struct vr_ip6 *); int vr_ip_transport_parse(struct vr_ip *iph, struct vr_ip6 *ip6h, - struct tcphdr **tcphp, unsigned int frag_size, + void **thp, unsigned int frag_size, void (do_tcp_mss_adj)(struct tcphdr *, unsigned short, unsigned char), unsigned int *hlenp, unsigned short *th_csump, unsigned int *tcph_pull_lenp, unsigned int *pull_lenp); diff --git a/linux/vr_host_interface.c b/linux/vr_host_interface.c index c9a421b8f..7e75dbf21 100644 --- a/linux/vr_host_interface.c +++ b/linux/vr_host_interface.c @@ -345,7 +345,7 @@ linux_xmit(struct vr_interface *vif, struct sk_buff *skb, static int linux_xmit_segment(struct vr_interface *vif, struct sk_buff *seg, - unsigned short type) + unsigned short type, int diag) { int err = -ENOMEM; struct vr_ip *iph, *i_iph = NULL; @@ -403,7 +403,7 @@ linux_xmit_segment(struct vr_interface *vif, struct sk_buff *seg, goto exit_xmit; } - if (vr_udp_coff) { + if (vr_udp_coff && !diag) { skb_set_network_header(seg, ethlen); iph->ip_csum = 0; @@ -420,6 +420,7 @@ linux_xmit_segment(struct vr_interface *vif, struct sk_buff *seg, udph->check = ~csum_tcpudp_magic(iph->ip_saddr, iph->ip_daddr, htons(udph->len), IPPROTO_UDP, 0); + } else { /* * If we are encapsulating a L3/L2 packet in UDP, set the UDP @@ -464,7 +465,7 @@ linux_xmit_segments(struct vr_interface *vif, struct sk_buff *segs, do { nskb = segs->next; segs->next = NULL; - if ((err = linux_xmit_segment(vif, segs, type))) + if ((err = linux_xmit_segment(vif, segs, type, 0))) break; segs = nskb; } while (segs); @@ -880,7 +881,8 @@ linux_if_tx(struct vr_interface *vif, struct vr_packet *pkt) } } - linux_xmit_segment(vif, skb, pkt->vp_type); + linux_xmit_segment(vif, skb, pkt->vp_type, + (pkt->vp_flags & VP_FLAG_DIAG)); return 0; } diff --git a/linux/vrouter_mod.c b/linux/vrouter_mod.c index 8eaae3734..025dc17d5 100644 --- a/linux/vrouter_mod.c +++ b/linux/vrouter_mod.c @@ -927,18 +927,18 @@ lh_reset_skb_fields(struct vr_packet *pkt) * lh_csum_verify_fast - faster version of skb_checksum which avoids a call * to kmap_atomic/kunmap_atomic as we already have a pointer obtained * from an earlier call to kmap_atomic. This function can only be used if - * the skb has a TCP segment contained entirely in a single frag. Returns 0 + * the skb has a TCP/UDP segment contained entirely in a single frag. Returns 0 * if checksum is ok, non-zero otherwise. */ static int -lh_csum_verify_fast(struct vr_ip *iph, struct tcphdr *tcph, - unsigned int tcp_size) +lh_csum_verify_fast(struct vr_ip *iph, void *transport_hdr, unsigned + char proto, unsigned int size) { __wsum csum; csum = csum_tcpudp_nofold(iph->ip_saddr, iph->ip_daddr, - tcp_size, IPPROTO_TCP, 0); - if (csum_fold(csum_partial(tcph, tcp_size, csum))) { + size, proto, 0); + if (csum_fold(csum_partial(transport_hdr, size, csum))) { return -1; } @@ -954,7 +954,7 @@ lh_csum_verify(struct sk_buff *skb, struct vr_ip *iph) { skb->csum = csum_tcpudp_nofold(iph->ip_saddr, iph->ip_daddr, ntohs(iph->ip_len) - (iph->ip_hl * 4), - IPPROTO_TCP, 0); + iph->ip_proto, 0); if (__skb_checksum_complete(skb)) { return -1; } @@ -1054,14 +1054,14 @@ lh_pull_inner_headers_fast_udp(struct vr_packet *pkt, int unsigned char *va = NULL; skb_frag_t *frag; unsigned int frag_size, pull_len, hdr_len, skb_pull_len, tcp_size; - unsigned int tcph_pull_len = 0, hlen = 0; + unsigned int th_pull_len = 0, hlen = 0; struct vr_ip *iph = NULL; struct vr_ip6 *ip6h = NULL; struct vr_udp *udph; - struct tcphdr *tcph = NULL; int pkt_type = 0; struct vr_ip *outer_iph = NULL; unsigned short th_csum = 0; + void *th = NULL; int helper_ret, parse_ret; pkt_headlen = pkt_head_len(pkt); @@ -1108,10 +1108,10 @@ lh_pull_inner_headers_fast_udp(struct vr_packet *pkt, int } parse_ret = vr_ip_transport_parse(iph, ip6h, - &tcph, frag_size, + &th, frag_size, NULL, &hlen, &th_csum, - &tcph_pull_len, + &th_pull_len, &pull_len); if (parse_ret == PKT_RET_SLOW_PATH) { goto slow_path; @@ -1143,13 +1143,9 @@ lh_pull_inner_headers_fast_udp(struct vr_packet *pkt, int skb_pull(skb, skb_pull_len); outer_iph = (struct vr_ip *)pkt_network_header(pkt); - if (lh_csum_verify_udp(skb, outer_iph)) { - if (th_csum == VR_DIAG_CSUM) { - vr_pkt_set_diag(pkt); - } else { - goto cksum_err; - } - } + if (lh_csum_verify_udp(skb, outer_iph)) + goto cksum_err; + /* * Restore the skb back to its original state. This is required as * packets that get trapped to the agent assume that the skb is @@ -1159,17 +1155,19 @@ lh_pull_inner_headers_fast_udp(struct vr_packet *pkt, int } } else { /* - * Verify inner packet checksum if it is TCP as we only do GRO for TCP - * (and GRO requires that checksum has been verified). For all other - * protocols, we will let the guest verify the checksum. + * We require checksum to be validated for TCP for GRO purpose + * and in case of UDP for DIAG purpose. Rest all packets can + * have the checksum validated in VM */ if (!ip6h && iph && (!vr_ip_fragment(iph))) { - if (iph->ip_proto == VR_IP_PROTO_TCP) { + if (((iph->ip_proto == VR_IP_PROTO_UDP) && th_csum == VR_DIAG_CSUM) + || (iph->ip_proto == VR_IP_PROTO_TCP)) { + lh_handle_checksum_complete_skb(skb); if (skb_shinfo(skb)->nr_frags == 1) { tcp_size = ntohs(iph->ip_len) - hlen; - if (lh_csum_verify_fast(iph, tcph, tcp_size)) { + if (lh_csum_verify_fast(iph, th, iph->ip_proto, tcp_size)) { if (th_csum == VR_DIAG_CSUM) { vr_pkt_set_diag(pkt); } else { @@ -1178,10 +1176,10 @@ lh_pull_inner_headers_fast_udp(struct vr_packet *pkt, int } } else { /* - * Pull to the start of the TCP header + * Pull to the start of the transport header */ skb_pull_len = (pkt_data(pkt) - skb->data) + - pkt_headlen + tcph_pull_len; + pkt_headlen + th_pull_len; skb_pull(skb, skb_pull_len); if (lh_csum_verify(skb, iph)) { @@ -1200,18 +1198,8 @@ lh_pull_inner_headers_fast_udp(struct vr_packet *pkt, int */ skb_push(skb, skb_pull_len); } - skb->ip_summed = CHECKSUM_UNNECESSARY; } else { - /* - * If outer UDP header had checksum of 0 and the inner packet - * is not TCP, set ip_summed to indicate that checksum - * verification is required. - */ - if (th_csum == VR_DIAG_CSUM) { - vr_pkt_set_diag(pkt); - } - skb->ip_summed &= (~CHECKSUM_UNNECESSARY); } } @@ -1271,11 +1259,11 @@ lh_pull_inner_headers_fast_gre(struct vr_packet *pkt, int unsigned char *va = NULL; skb_frag_t *frag; unsigned int frag_size, pull_len, hlen = 0, tcp_size, skb_pull_len, - tcph_pull_len = 0; + th_pull_len = 0; unsigned short th_csum = 0; struct vr_ip *iph = NULL; struct vr_ip6 *ip6h = NULL; - struct tcphdr *tcph = NULL; + void *th = NULL; int pkt_type = 0, helper_ret, parse_ret; pkt_headlen = pkt_head_len(pkt); @@ -1380,10 +1368,10 @@ lh_pull_inner_headers_fast_gre(struct vr_packet *pkt, int } parse_ret = vr_ip_transport_parse(iph, ip6h, - &tcph, frag_size, + &th, frag_size, NULL, &hlen, &th_csum, - &tcph_pull_len, + &th_pull_len, &pull_len); if (parse_ret == PKT_RET_SLOW_PATH) { goto slow_path; @@ -1413,14 +1401,17 @@ lh_pull_inner_headers_fast_gre(struct vr_packet *pkt, int * GRE. If the outer header is UDP, we will always verify the checksum * of the outer packet and this covers the inner packet too. */ + if (!skb_csum_unnecessary(skb)) { if (!ip6h && iph && !vr_ip_fragment(iph)) { - if (iph->ip_proto == VR_IP_PROTO_TCP) { + if ((th_csum == VR_DIAG_CSUM && (iph->ip_proto == VR_IP_PROTO_UDP)) + || (iph->ip_proto == VR_IP_PROTO_TCP)) { + lh_handle_checksum_complete_skb(skb); if (skb_shinfo(skb)->nr_frags == 1) { tcp_size = ntohs(iph->ip_len) - hlen; - if (lh_csum_verify_fast(iph, tcph, tcp_size)) { + if (lh_csum_verify_fast(iph, th, iph->ip_proto, tcp_size)) { if (th_csum == VR_DIAG_CSUM) { vr_pkt_set_diag(pkt); } else { @@ -1429,10 +1420,10 @@ lh_pull_inner_headers_fast_gre(struct vr_packet *pkt, int } } else { /* - * Pull to the start of the TCP header + * Pull to the start of the transport header */ skb_pull_len = (pkt_data(pkt) - skb->data) + - pkt_headlen + tcph_pull_len; + pkt_headlen + th_pull_len; skb_pull(skb, skb_pull_len); if (lh_csum_verify(skb, iph)) { @@ -1451,12 +1442,10 @@ lh_pull_inner_headers_fast_gre(struct vr_packet *pkt, int } skb->ip_summed = CHECKSUM_UNNECESSARY; - } else if (th_csum == VR_DIAG_CSUM) { - vr_pkt_set_diag(pkt); + } else { + skb->ip_summed &= ~CHECKSUM_UNNECESSARY; } } - } else if (th_csum == VR_DIAG_CSUM) { - vr_pkt_set_diag(pkt); } if (pkt_type != PKT_MPLS_TUNNEL_L3 && skb->ip_summed == @@ -1568,7 +1557,7 @@ lh_pull_inner_headers(struct vr_packet *pkt, unsigned short icmp_pl_ip_proto; struct tcphdr *tcph = NULL; struct vr_icmp *icmph = NULL; - unsigned int tcpoff, skb_pull_len; + unsigned int toff, skb_pull_len; bool thdr_valid = false, mpls_pkt = true, outer_cksum_validate; uint32_t label, control_data; struct vr_eth *eth = NULL; @@ -1941,11 +1930,12 @@ lh_pull_inner_headers(struct vr_packet *pkt, } } else { if (!ip6h && !vr_ip_fragment(iph)) { - if (iph->ip_proto == VR_IP_PROTO_TCP) { + if (((th_csum == VR_DIAG_CSUM) && iph->ip_proto == VR_IP_PROTO_UDP) + || (iph->ip_proto == VR_IP_PROTO_TCP)) { lh_handle_checksum_complete_skb(skb); - tcpoff = (char *)tcph - (char *) skb->data; + toff = (char *)((char *)iph + (iph->ip_hl * 4)) - (char *) skb->data; - skb_pull(skb, tcpoff); + skb_pull(skb, toff); if (lh_csum_verify(skb, iph)) { if (th_csum == VR_DIAG_CSUM) { vr_pkt_set_diag(pkt); @@ -1954,17 +1944,15 @@ lh_pull_inner_headers(struct vr_packet *pkt, } } - skb_push(skb, tcpoff); - if (vr_to_vm_mss_adj) { - lh_adjust_tcp_mss(tcph, skb, vrouter_overlay_len, sizeof(struct vr_ip)); - } - } else if (th_csum == VR_DIAG_CSUM) { - vr_pkt_set_diag(pkt); + skb->ip_summed = CHECKSUM_UNNECESSARY; + + skb_push(skb, toff); + } + if ((iph->ip_proto == VR_IP_PROTO_TCP) && vr_to_vm_mss_adj) { + lh_adjust_tcp_mss(tcph, skb, vrouter_overlay_len, sizeof(struct vr_ip)); } } } - } else if (th_csum == VR_DIAG_CSUM) { - vr_pkt_set_diag(pkt); } }