ip: fix ip zero checksum verification

In one's complement, there are two representations of zero: the all
zero and the all one bit values, often referred to as +0 and -0. See
RFC 1624 section 3 for more details.
This used to be taken care of in ip4_header_checksum(), but it is no
longer the case. The check ip->checksum == ip4_header_checksum (ip) is
no longer correct in the -0 case.
Always use ip4_header_checksum_is_valid() instead (which behaves
correctly since 9a79a1ab931c3b5a7ae07d6f0fcfef7c4368a2c4).

Type: fix
Fixes: e5f0050c7a5d411f96af6401797529d58825e2af

Change-Id: Iacc6b60645a834287b085aecb9e3fdb4554cf0cf
Signed-off-by: Benoît Ganne <bganne@cisco.com>

10 files changed, 20 insertions, 19 deletions

diff --git a/src/plugins/flowprobe/node.c b/src/plugins/flowprobe/node.c
index 2dd49b3f48f..a81f7a6c45b 100644
--- a/src/plugins/flowprobe/node.c
+++ b/src/plugins/flowprobe/node.c
@@ -598,7 +598,7 @@ flowprobe_export_send (vlib_main_t * vm, vlib_buffer_t * b0,
 	udp->checksum = 0xffff;
     }
 
-  ASSERT (ip->checksum == ip4_header_checksum (ip));
+  ASSERT (ip4_header_checksum_is_valid (ip));
 
   /* Find or allocate a frame */
   f = fm->context[which].frames_per_worker[my_cpu_number];
diff --git a/src/plugins/ioam/udp-ping/udp_ping_export.c b/src/plugins/ioam/udp-ping/udp_ping_export.c
index d25eb1041dd..3c632c86900 100644
--- a/src/plugins/ioam/udp-ping/udp_ping_export.c
+++ b/src/plugins/ioam/udp-ping/udp_ping_export.c
@@ -152,7 +152,7 @@ udp_ping_send_flows (flow_report_main_t * frm, flow_report_t * fr,
 		  if (udp->checksum == 0)
 		    udp->checksum = 0xffff;
 
-		  ASSERT (ip->checksum == ip4_header_checksum (ip));
+		  ASSERT (ip4_header_checksum_is_valid (ip));
 
 		  to_next[0] = bi0;
 		  f->n_vectors++;
@@ -203,7 +203,7 @@ udp_ping_send_flows (flow_report_main_t * frm, flow_report_t * fr,
       if (udp->checksum == 0)
 	udp->checksum = 0xffff;
 
-      ASSERT (ip->checksum == ip4_header_checksum (ip));
+      ASSERT (ip4_header_checksum_is_valid (ip));
 
       to_next[0] = bi0;
       f->n_vectors++;
diff --git a/src/plugins/map/ip4_map.c b/src/plugins/map/ip4_map.c
index ea63901bf89..a4889627f0b 100644
--- a/src/plugins/map/ip4_map.c
+++ b/src/plugins/map/ip4_map.c
@@ -97,7 +97,7 @@ ip4_map_decrement_ttl (ip4_header_t * ip, u8 * error)
   *error = ttl <= 0 ? IP4_ERROR_TIME_EXPIRED : *error;
 
   /* Verify checksum. */
-  ASSERT (ip->checksum == ip4_header_checksum (ip));
+  ASSERT (ip4_header_checksum_is_valid (ip));
 }
 
 static u32
diff --git a/src/plugins/nat/nat_ipfix_logging.c b/src/plugins/nat/nat_ipfix_logging.c
index 3b75260148f..42252b2eb0c 100644
--- a/src/plugins/nat/nat_ipfix_logging.c
+++ b/src/plugins/nat/nat_ipfix_logging.c
@@ -567,7 +567,7 @@ snat_ipfix_send (u32 thread_index, flow_report_main_t * frm,
 	udp->checksum = 0xffff;
     }
 
-  ASSERT (ip->checksum == ip4_header_checksum (ip));
+  ASSERT (ip4_header_checksum_is_valid (ip));
 
   vlib_put_frame_to_node (vm, ip4_lookup_node.index, f);
 }
diff --git a/src/plugins/ping/ping.c b/src/plugins/ping/ping.c
index f56f44ffb26..0ce4f9698f0 100644
--- a/src/plugins/ping/ping.c
+++ b/src/plugins/ping/ping.c
@@ -474,8 +474,8 @@ ip4_icmp_echo_request (vlib_main_t * vm,
 	  ip0->checksum = ip_csum_fold (sum0);
 	  ip1->checksum = ip_csum_fold (sum1);
 
-	  ASSERT (ip0->checksum == ip4_header_checksum (ip0));
-	  ASSERT (ip1->checksum == ip4_header_checksum (ip1));
+	  ASSERT (ip4_header_checksum_is_valid (ip0));
+	  ASSERT (ip4_header_checksum_is_valid (ip1));
 
 	  p0->flags |= VNET_BUFFER_F_LOCALLY_ORIGINATED;
 	  p1->flags |= VNET_BUFFER_F_LOCALLY_ORIGINATED;
@@ -531,7 +531,7 @@ ip4_icmp_echo_request (vlib_main_t * vm,
 
 	  ip0->checksum = ip_csum_fold (sum0);
 
-	  ASSERT (ip0->checksum == ip4_header_checksum (ip0));
+	  ASSERT (ip4_header_checksum_is_valid (ip0));
 
 	  p0->flags |= VNET_BUFFER_F_LOCALLY_ORIGINATED;
 	}
diff --git a/src/vnet/ip/ip4_format.c b/src/vnet/ip/ip4_format.c
index 786a01d396b..c6639b20716 100644
--- a/src/vnet/ip/ip4_format.c
+++ b/src/vnet/ip/ip4_format.c
@@ -150,9 +150,10 @@ format_ip4_header (u8 * s, va_list * args)
 
   /* Check and report invalid checksums. */
   {
-    u16 c = ip4_header_checksum (ip);
-    if (c != ip->checksum)
-      s = format (s, " (should be 0x%04x)", clib_net_to_host_u16 (c));
+    if (!ip4_header_checksum_is_valid (ip))
+      s =
+	format (s, " (should be 0x%04x)",
+		clib_net_to_host_u16 (ip4_header_checksum (ip)));
   }
 
   s = format (s, " dscp %U ecn %U",
diff --git a/src/vnet/ip/ip4_forward.c b/src/vnet/ip/ip4_forward.c
index 595a0a1913c..3bf305303d4 100644
--- a/src/vnet/ip/ip4_forward.c
+++ b/src/vnet/ip/ip4_forward.c
@@ -2063,7 +2063,7 @@ ip4_ttl_inc (vlib_buffer_t * b, ip4_header_t * ip)
   ttl += 1;
   ip->ttl = ttl;
 
-  ASSERT (ip->checksum == ip4_header_checksum (ip));
+  ASSERT (ip4_header_checksum_is_valid (ip));
 }
 
 /* Decrement TTL & update checksum.
@@ -2104,7 +2104,7 @@ ip4_ttl_and_checksum_check (vlib_buffer_t * b, ip4_header_t * ip, u16 * next,
     }
 
   /* Verify checksum. */
-  ASSERT ((ip->checksum == ip4_header_checksum (ip)) ||
+  ASSERT (ip4_header_checksum_is_valid (ip) ||
 	  (b->flags & VNET_BUFFER_F_OFFLOAD_IP_CKSUM));
 }
 
diff --git a/src/vnet/ip/ip4_pg.c b/src/vnet/ip/ip4_pg.c
index d89424496f0..2ccd2b45a6f 100644
--- a/src/vnet/ip/ip4_pg.c
+++ b/src/vnet/ip/ip4_pg.c
@@ -90,8 +90,8 @@ compute_length_and_or_checksum (vlib_main_t * vm,
 	  ip0->checksum = ~ip_csum_fold (sum0);
 	  ip1->checksum = ~ip_csum_fold (sum1);
 
-	  ASSERT (ip0->checksum == ip4_header_checksum (ip0));
-	  ASSERT (ip1->checksum == ip4_header_checksum (ip1));
+	  ASSERT (ip4_header_checksum_is_valid (ip0));
+	  ASSERT (ip4_header_checksum_is_valid (ip1));
 	}
     }
 
@@ -123,7 +123,7 @@ compute_length_and_or_checksum (vlib_main_t * vm,
 	  ip4_partial_header_checksum_x1 (ip0, sum0);
 	  ip0->checksum = ~ip_csum_fold (sum0);
 
-	  ASSERT (ip0->checksum == ip4_header_checksum (ip0));
+	  ASSERT (ip4_header_checksum_is_valid (ip0));
 	}
     }
 }
diff --git a/src/vnet/ipfix-export/flow_report_classify.c b/src/vnet/ipfix-export/flow_report_classify.c
index f004e9a0806..58f623db145 100644
--- a/src/vnet/ipfix-export/flow_report_classify.c
+++ b/src/vnet/ipfix-export/flow_report_classify.c
@@ -321,7 +321,7 @@ ipfix_classify_send_flows (flow_report_main_t * frm,
 			udp->checksum = 0xffff;
 		    }
 
-		  ASSERT (ip->checksum == ip4_header_checksum (ip));
+		  ASSERT (ip4_header_checksum_is_valid (ip));
 
 		  to_next[0] = bi0;
 		  f->n_vectors++;
@@ -376,7 +376,7 @@ flush:
 	    udp->checksum = 0xffff;
 	}
 
-      ASSERT (ip->checksum == ip4_header_checksum (ip));
+      ASSERT (ip4_header_checksum_is_valid (ip));
 
       to_next[0] = bi0;
       f->n_vectors++;
diff --git a/src/vnet/ipfix-export/ipfix_doc.md b/src/vnet/ipfix-export/ipfix_doc.md
index 59118f68e11..1c7aad7750d 100644
--- a/src/vnet/ipfix-export/ipfix_doc.md
+++ b/src/vnet/ipfix-export/ipfix_doc.md
@@ -259,7 +259,7 @@ This function creates the packet header for an ipfix data packet
       	udp->checksum = 0xffff;
           }
 
-        ASSERT (ip->checksum == ip4_header_checksum (ip));
+        ASSERT (ip4_header_checksum_is_valid (ip));
 
         vlib_put_frame_to_node (vm, ip4_lookup_node.index, f);
       }