frame: change checksum function

At the end of function, we used to call ntohs() but we also call the
same function before providing the checksum to POKE_UINT16 (which will
in turn call ntohs). We remove those two first extra calls as they
aren't necessary.

Moreover, the SNMP agent uses this function and this makes it
endian-dependant. We don't want that.

The function is quite convulated and isn't a classic checksum
function. I don't remember how I came with this function but since there
is the special case for Cisco devices and I know this special case work
now, I don't want to change to another function which may breaks this
special case.

Add a NEWS entry to let people know, just in case.

diff --git a/NEWS b/NEWS
index 2840326e5b1655f14951b785d9e097e3538ae957..95772c47f0dde5ddf6cebd8f0edf0f908f9a8648 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ lldpd (0.7.15)
   * Features:
     + Optional features can be configured with "auto" to autodetect if
       they are usable. This is the default value for JSON and XML support.
+  * Change:
+    + Modify checksum function. While this should be strictly
+      equivalent, if you notice CDP packets not accepted anymore, this
+      change is the first culprit.
 
 lldpd (0.7.14)
   * Features:

diff --git a/src/daemon/frame.c b/src/daemon/frame.c
index a94fd3e4e9e2bb8a5af730b434c3f5720ca43dc3..a440764e5655a605004f4765d48eb8a7790999e8 100644 (file)
--- a/src/daemon/frame.c
+++ b/src/daemon/frame.c
@@ -17,13 +17,15 @@
 
 #include "lldpd.h"
 
+/**
+ * Compute the checksum as 16-bit word.
+ */
 u_int16_t
 frame_checksum(const u_char *cp, int len, int cisco)
 {
        unsigned int sum = 0, v = 0;
        int oddbyte = 0;
 
-       /* We compute in network byte order */
        while ((len -= 2) >= 0) {
                sum += *cp++ << 8;
                sum += *cp++;
@@ -61,6 +63,5 @@ frame_checksum(const u_char *cp, int len, int cisco)
 
        sum = (sum >> 16) + (sum & 0xffff);
        sum += sum >> 16;
-       sum = ntohs(sum);
        return (0xffff & ~sum);
 }

diff --git a/src/daemon/protocols/cdp.c b/src/daemon/protocols/cdp.c
index 4974b00eecbf7196654313801bb0510cae35dcfe..cc6f8318c062e6718e37992a386a5faeef44d0e3 100644 (file)
--- a/src/daemon/protocols/cdp.c
+++ b/src/daemon/protocols/cdp.c
@@ -226,7 +226,7 @@ cdp_send(struct lldpd *global,
        if (!(POKE_UINT16(end - pos_llc))) goto toobig;
        checksum = frame_checksum(pos_cdp, end - pos_cdp, (version != 0) ? 1 : 0);
        POKE_RESTORE(pos_checksum);
-       if (!(POKE_UINT16(ntohs(checksum)))) goto toobig;
+       if (!(POKE_UINT16(checksum))) goto toobig;
 
        if (interfaces_send_helper(global, hardware,
                (char *)packet, end - packet) == -1) {

diff --git a/src/daemon/protocols/edp.c b/src/daemon/protocols/edp.c
index 106d9f6387bc5ae84e722666f39dc487cfed924f..acbe53dd56ed0e8cb8f4b84303bce26a9a177273 100644 (file)
--- a/src/daemon/protocols/edp.c
+++ b/src/daemon/protocols/edp.c
@@ -198,7 +198,7 @@ edp_send(struct lldpd *global,
                POKE_RESTORE(pos_len_edp);
                if (!(POKE_UINT16(v))) goto toobig;
                checksum = frame_checksum(pos_edp, v, 0);
-               if (!(POKE_UINT16(ntohs(checksum)))) goto toobig;
+               if (!(POKE_UINT16(checksum))) goto toobig;
 
                if (interfaces_send_helper(global, hardware,
                        (char *)packet, end - packet) == -1) {