From: Amos Jeffries <squid3@treenet.co.nz>
Date: Mon, 28 Jan 2013 11:25:34 +0000 (-0700)
Subject: WCCP: Fix memory leak in mask assignment, improve debuggsing.
X-Git-Tag: SQUID_3_2_7~12
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=21cec590a21e82dc727ecca5fdef599d2d883228;p=thirdparty%2Fsquid.git

WCCP: Fix memory leak in mask assignment, improve debuggsing.

* Release temporary weight array allocated on each HERE_I_AM packet sent
  by Squid. For mask assignment these were not released properly and may
  build up to a significant size of memory over time.

* Add debug traces to send() events to report failures sending packets

* Also, on HERE_I_AM event send() failure, reduce the timeout to 2sec
  for the retry in a crude attempt to prevent router state flapping.

* Silence compiler warnings on use of connect() to disconnect a socket.
  Inconsistent OS behaviour makes the result useless in this case.

 Detected by Coverity Scan. Issues 740329, 740330, 740331, 740332,
    740333, 740441.
---

diff --git a/src/wccp.cc b/src/wccp.cc
index 013954d80c..9ae8d073e5 100644
--- a/src/wccp.cc
+++ b/src/wccp.cc
@@ -302,13 +302,18 @@ wccpHereIam(void *voidnotused)
     debugs(80, 6, "wccpHereIam: Called");
 
     wccp_here_i_am.id = last_id;
-    comm_udp_send(theWccpConnection,
-                  &wccp_here_i_am,
-                  sizeof(wccp_here_i_am),
-                  0);
+    double interval = 10.0; // TODO: make this configurable, possibly negotiate with the router.
+    errno = 0;
+    ssize_t sent = comm_udp_send(theWccpConnection, &wccp_here_i_am, sizeof(wccp_here_i_am), 0);
+
+    // if we failed to send the whole lot, try again at a shorter interval (20%)
+    if (sent != sizeof(wccp_here_i_am)) {
+        debugs(80, 2, "ERROR: failed to send WCCP HERE_I_AM packet: " << xstrerror());
+        interval = 2.0;
+    }
 
     if (!eventFind(wccpHereIam, NULL))
-        eventAdd("wccpHereIam", wccpHereIam, NULL, 10.0, 1);
+        eventAdd("wccpHereIam", wccpHereIam, NULL, interval, 1);
 }
 
 static void
diff --git a/src/wccp2.cc b/src/wccp2.cc
index 01c4bc5fbb..7992366dc8 100644
--- a/src/wccp2.cc
+++ b/src/wccp2.cc
@@ -1010,7 +1010,8 @@ wccp2ConnectionOpen(void)
 #if defined(IP_MTU_DISCOVER) && defined(IP_PMTUDISC_DONT)
     {
         int i = IP_PMTUDISC_DONT;
-        setsockopt(theWccp2Connection, SOL_IP, IP_MTU_DISCOVER, &i, sizeof i);
+        if (setsockopt(theWccp2Connection, SOL_IP, IP_MTU_DISCOVER, &i, sizeof i) < 0)
+            debugs(80, 2, "WARNING: Path MTU discovery could not be disabled on FD " << theWccp2Connection << ": " << xstrerror());
     }
 
 #endif
@@ -1049,8 +1050,9 @@ wccp2ConnectionOpen(void)
             /* Disconnect the sending socket. Note: FreeBSD returns error
              * but disconnects anyway so we have to just assume it worked
              */
-            if (wccp2_numrouters > 1)
-                connect(theWccp2Connection, (struct sockaddr *) &null, router_len);
+            if (wccp2_numrouters > 1) {
+                (void)connect(theWccp2Connection, (struct sockaddr *) &null, router_len);
+            }
         }
 
         service_list_ptr = service_list_ptr->next;
@@ -1608,10 +1610,9 @@ wccp2HereIam(void *voidnotused)
                                 &service_list_ptr->wccp_packet,
                                 service_list_ptr->wccp_packet_size);
             } else {
-                send(theWccp2Connection,
-                     &service_list_ptr->wccp_packet,
-                     service_list_ptr->wccp_packet_size,
-                     0);
+                errno = 0;
+                if (send(theWccp2Connection, &service_list_ptr->wccp_packet, service_list_ptr->wccp_packet_size, 0) < service_list_ptr->wccp_packet_size)
+                    debugs(80, 2, "ERROR: failed to send WCCPv2 HERE_I_AM packet to " << router << " : " << xstrerror());
             }
         }
 
@@ -1985,20 +1986,21 @@ wccp2AssignBuckets(void *voidnotused)
             if (ntohl(router_list_ptr->num_caches)) {
                 /* send packet */
 
+                /* FIXME INET6 : drop temp conversion */
+                Ip::Address tmp_rtr(router);
+
                 if (wccp2_numrouters > 1) {
-                    /* FIXME INET6 : drop temp conversion */
-                    Ip::Address tmp_rtr(router);
                     comm_udp_sendto(theWccp2Connection,
                                     tmp_rtr,
                                     &wccp_packet,
                                     offset);
                 } else {
-                    send(theWccp2Connection,
-                         &wccp_packet,
-                         offset,
-                         0);
+                    errno = 0;
+                    if (send(theWccp2Connection, &wccp_packet, offset, 0) < offset)
+                        debugs(80, 2, "ERROR: failed to send WCCPv2 HERE_I_AM packet to " << tmp_rtr << " : " << xstrerror());
                 }
             }
+            safe_free(weight);
         }
 
         service_list_ptr = service_list_ptr->next;