From: Dave Hart Date: Fri, 16 Oct 2009 21:00:05 +0000 (+0000) Subject: [Bug 1348] ntpd Windows port should wait for sendto() completion. X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ae9615c0be1df69b3b4648cefe476bd0e5b74333;p=thirdparty%2Fntp.git [Bug 1348] ntpd Windows port should wait for sendto() completion. bk: 4ad8ded5_RnMcSedP0wrZkcxddsYCQ --- diff --git a/ChangeLog b/ChangeLog index 3692ca0ed5..5c89c91d95 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ +* [Bug 1348] ntpd Windows port should wait for sendto() completion. (4.2.5p234-RC) 2009/10/16 Released by Harlan Stenn * [Bug 1339] redux, use unmodified lib/isc/win32/strerror.c and move #define strerror... to a header not used by lib/isc code. diff --git a/ntpd/ntp_io.c b/ntpd/ntp_io.c index 1f439f22a2..b4e19b59ed 100644 --- a/ntpd/ntp_io.c +++ b/ntpd/ntp_io.c @@ -2984,18 +2984,17 @@ sendpkt( #endif /* MCAST */ -#if defined(HAVE_IO_COMPLETION_PORT) - cc = io_completion_port_sendto(inter, pkt, len, dest); - if (cc != ERROR_SUCCESS) { -#else #ifdef SIM cc = simulate_server(dest, inter, pkt); #else /* SIM */ +#ifndef HAVE_IO_COMPLETION_PORT cc = sendto(inter->fd, (char *)pkt, (unsigned int)len, 0, (struct sockaddr *)dest, SOCKLEN(dest)); +#else + cc = io_completion_port_sendto(inter, pkt, len, dest); +#endif /* HAVE_IO_COMPLETION_PORT */ #endif /* SIM */ if (cc == -1) { -#endif inter->notsent++; packets_notsent++; } else { diff --git a/ports/winnt/include/transmitbuff.h b/ports/winnt/include/transmitbuff.h deleted file mode 100644 index aef37fa32a..0000000000 --- a/ports/winnt/include/transmitbuff.h +++ /dev/null @@ -1,46 +0,0 @@ -#ifndef TRANSMITBUFF_H -#define TRANSMITBUFF_H - -#include "ntp.h" -#include - -/* - * Format of a transmitbuf. These are used by the asynchronous receive - * routine to store outgoing packets and related information. - */ - -typedef struct transmitbuf transmitbuf_t; - -typedef struct transmitbuf { - ISC_LINK(transmitbuf_t) link; - - time_t ts; /* Time stamp for the request */ - - /* - * union { - * struct pkt pkt; - * struct ntp_control ctlpkt; - *} pkt; - */ - char pkt[512]; - -} transmitbuf; - - -extern void init_transmitbuff (void); - - -/* freetransmitbuf - make a single transmitbuf available for reuse - */ -extern void free_transmit_buffer (transmitbuf_t *); - -/* Get a free buffer (typically used so an async - * read can directly place data into the buffer - * - * The buffer is removed from the free list. Make sure - * you put it back with freetransmitbuf() or - */ -extern transmitbuf_t *get_free_transmit_buffer (void); - -#endif /* TRANSMITBUFF_H */ - diff --git a/ports/winnt/libntp/transmitbuff.c b/ports/winnt/libntp/transmitbuff.c deleted file mode 100644 index 8fcd302cb0..0000000000 --- a/ports/winnt/libntp/transmitbuff.c +++ /dev/null @@ -1,139 +0,0 @@ -/* - * DEAD CODE ALERT -- for whatever reason all this wonderful stuff is - * unused. The initialization was the only code - * exercised as of May 2009 when that was nipped. - */ - - -#ifdef HAVE_CONFIG_H -# include -#endif - -#include -#include "ntp_machine.h" -#include "ntp_fp.h" -#include "ntp_stdlib.h" -#include "ntp_syslog.h" - -#include -#include "transmitbuff.h" - -/* - * transmitbuf memory management - */ -#define TRANSMIT_INIT 10 /* 10 buffers initially */ -#define TRANSMIT_LOWAT 3 /* when we're down to three buffers get more */ -#define TRANSMIT_INC 5 /* get 5 more at a time */ -#define TRANSMIT_TOOMANY 40 /* this is way too many buffers */ - -/* - * Memory allocation - */ -static volatile u_long full_transmitbufs = 0; /* number of transmitbufs on fulllist */ -static volatile u_long free_transmitbufs = 0; /* number of transmitbufs on freelist */ - -ISC_LIST(transmitbuf_t) free_transmit_list; /* Currently used transmit buffers */ -ISC_LIST(transmitbuf_t) full_transmit_list; /* Currently used transmit buffers */ - -static u_long total_transmitbufs = 0; /* total transmitbufs currently in use */ -static u_long lowater_additions = 0; /* number of times we have added memory */ - -static CRITICAL_SECTION TransmitLock; -# define LOCK(lock) EnterCriticalSection(lock) -# define UNLOCK(lock) LeaveCriticalSection(lock) - -static inline void -initialise_buffer(transmitbuf *buff) -{ - memset(buff, 0, sizeof(*buff)); -} - -static void -add_buffer_to_freelist(transmitbuf *tb) -{ - ISC_LIST_APPEND(free_transmit_list, tb, link); - free_transmitbufs++; -} - -static void -create_buffers(int nbufs) -{ - transmitbuf_t *buf; - int i; - - buf = emalloc(nbufs * sizeof(*buf)); - for (i = 0; i < nbufs; i++) - { - initialise_buffer(buf); - add_buffer_to_freelist(buf); - total_transmitbufs++; - buf++; - } - - lowater_additions++; -} - -extern void -init_transmitbuff(void) -{ - /* - * Init buffer free list and stat counters - */ - ISC_LIST_INIT(full_transmit_list); - ISC_LIST_INIT(free_transmit_list); - free_transmitbufs = total_transmitbufs = 0; - full_transmitbufs = lowater_additions = 0; - create_buffers(TRANSMIT_INIT); - - InitializeCriticalSection(&TransmitLock); -} - -static void -delete_buffer_from_full_list(transmitbuf_t *tb) { - - transmitbuf_t *next = NULL; - transmitbuf_t *lbuf = ISC_LIST_HEAD(full_transmit_list); - - while (lbuf != NULL) { - next = ISC_LIST_NEXT(lbuf, link); - if (lbuf == tb) { - ISC_LIST_DEQUEUE(full_transmit_list, lbuf, link); - break; - } - else - lbuf = next; - } - full_transmitbufs--; -} - -extern void -free_transmit_buffer(transmitbuf_t *rb) -{ - LOCK(&TransmitLock); - delete_buffer_from_full_list(rb); - add_buffer_to_freelist(rb); - UNLOCK(&TransmitLock); -} - - -extern transmitbuf * -get_free_transmit_buffer(void) -{ - - transmitbuf_t * buffer = NULL; - LOCK(&TransmitLock); - if (free_transmitbufs <= 0) { - create_buffers(TRANSMIT_INC); - } - buffer = ISC_LIST_HEAD(free_transmit_list); - if (buffer != NULL) - { - ISC_LIST_DEQUEUE(free_transmit_list, buffer, link); - free_transmitbufs--; - ISC_LIST_APPEND(full_transmit_list, buffer, link); - full_transmitbufs++; - } - UNLOCK(&TransmitLock); - return (buffer); -} - diff --git a/ports/winnt/ntpd/ntp_iocompletionport.c b/ports/winnt/ntpd/ntp_iocompletionport.c index 731613eda1..c00118f9d8 100644 --- a/ports/winnt/ntpd/ntp_iocompletionport.c +++ b/ports/winnt/ntpd/ntp_iocompletionport.c @@ -45,7 +45,7 @@ typedef struct IoCompletionInfo { int request_type; union { recvbuf_t * recv_buf; - transmitbuf_t * trans_buf; + void * trans_buf; }; #ifdef DEBUG struct IoCompletionInfo *link; @@ -140,26 +140,14 @@ FreeHeap(IoCompletionInfo *lpo, char *fromfunc) #endif } -transmitbuf_t * -get_trans_buf() -{ - transmitbuf_t *tb = emalloc(sizeof(*tb)); - return (tb); -} - -void -free_trans_buf(transmitbuf_t *tb) -{ - free(tb); -} - HANDLE -get_io_event() +get_io_event(void) { return( WaitableIoEventHandle ); } + HANDLE -get_exit_event() +get_exit_event(void) { return( WaitableExitEventHandle ); } @@ -184,7 +172,6 @@ iocompletionthread(void *NotUsed) DWORD BytesTransferred = 0; ULONG_PTR Key = 0; IoCompletionInfo * lpo = NULL; - u_long time_next_ifscan_after_error = 0; UNUSED_ARG(NotUsed); @@ -212,8 +199,7 @@ iocompletionthread(void *NotUsed) &Key, (LPOVERLAPPED *) &lpo, INFINITE); - if (lpo == NULL) - { + if (lpo == NULL) { DPRINTF(2, ("Overlapped IO Thread Exiting\n")); break; /* fail */ } @@ -224,32 +210,7 @@ iocompletionthread(void *NotUsed) if (bSuccess) errstatus = 0; else - { errstatus = GetLastError(); - if (BytesTransferred == 0) - { - if (WSA_OPERATION_ABORTED == errstatus) { - DPRINTF(4, ("Transfer Operation aborted\n")); - } else if (ERROR_UNEXP_NET_ERR == errstatus) { - /* - * We get this error when trying to send an the network - * interface is gone or has lost link. Rescan interfaces - * to catch on sooner, but no more than once per minute. - * Once ntp is able to detect changes without polling - * this should be unneccessary - */ - if (time_next_ifscan_after_error < current_time) { - time_next_ifscan_after_error = current_time + 60; - timer_interfacetimeout(current_time); - } - DPRINTF(4, ("sendto unexpected network error, interface may be down\n")); - } - } - else - { - msyslog(LOG_ERR, "sendto error after %d bytes: %m", BytesTransferred); - } - } /* * Invoke the appropriate function based on @@ -267,6 +228,8 @@ iocompletionthread(void *NotUsed) OnSocketRecv(Key, lpo, BytesTransferred, errstatus); break; case SOCK_SEND: + NTP_INSIST(0); + break; case SERIAL_WRITE: OnWriteComplete(Key, lpo, BytesTransferred, errstatus); break; @@ -307,10 +270,6 @@ init_io_completion_port( } #endif -#if 0 /* transmitbuff.c unused, no need to initialize it */ - init_transmitbuff(); -#endif - /* Create the event used to signal an IO event */ WaitableIoEventHandle = CreateEvent(NULL, FALSE, FALSE, WAITABLEIOEVENTHANDLE); @@ -794,45 +753,30 @@ io_completion_port_add_socket(SOCKET fd, struct interface *inter) return 0; } + static int OnWriteComplete(ULONG_PTR i, IoCompletionInfo *lpo, DWORD Bytes, int errstatus) { - transmitbuf_t *buff; - struct interface *inter; + void *buff; UNUSED_ARG(Bytes); + UNUSED_ARG(errstatus); buff = lpo->trans_buf; - - free_trans_buf(buff); + free(buff); lpo->trans_buf = NULL; + FreeHeap(lpo, "OnWriteComplete"); - if (SOCK_SEND == lpo->request_type) { - switch (errstatus) { - case WSA_OPERATION_ABORTED: - case NO_ERROR: - break; - - default: - inter = (struct interface *)i; - packets_notsent++; - inter->notsent++; - break; - } - } - - if (errstatus == WSA_OPERATION_ABORTED) - FreeHeap(lpo, "OnWriteComplete: Socket Closed"); - else - FreeHeap(lpo, "OnWriteComplete"); return 1; } /* - * Return value is really GetLastError-style error code - * which is a DWORD but using int, which is large enough, - * decreases #ifdef forest in ntp_io.c harmlessly. + * io_completion_port_sendto() -- sendto() replacement for Windows + * + * Returns 0 after successful send. + * Returns -1 for any error, with the error code available via + * msyslog() %m, or GetLastError(). */ int io_completion_port_sendto( @@ -842,77 +786,54 @@ io_completion_port_sendto( sockaddr_u* dest ) { + static u_long time_next_ifscan_after_error; WSABUF wsabuf; - transmitbuf_t *buff; - DWORD Result = ERROR_SUCCESS; + DWORD octets_sent; + DWORD Result; int errval; int AddrLen; - IoCompletionInfo *lpo; - DWORD Flags; - - lpo = (IoCompletionInfo *) GetHeapAlloc("io_completion_port_sendto"); - if (lpo == NULL) - return ERROR_OUTOFMEMORY; + wsabuf.buf = (void *)pkt; + wsabuf.len = len; + AddrLen = SOCKLEN(dest); + octets_sent = 0; - if (len <= sizeof(buff->pkt)) { - buff = get_trans_buf(); - - if (buff == NULL) { - msyslog(LOG_ERR, "No more transmit buffers left - data discarded"); - FreeHeap(lpo, "io_completion_port_sendto"); - return ERROR_OUTOFMEMORY; - } - - - memcpy(&buff->pkt, pkt, len); - wsabuf.buf = buff->pkt; - wsabuf.len = len; - - AddrLen = SOCKLEN(dest); - lpo->request_type = SOCK_SEND; - lpo->trans_buf = buff; - Flags = 0; - - Result = WSASendTo(inter->fd, &wsabuf, 1, NULL, Flags, - &dest->sa, AddrLen, - (LPOVERLAPPED)lpo, NULL); - - if(Result == SOCKET_ERROR) - { - errval = WSAGetLastError(); - switch (errval) { - - case NO_ERROR : - case WSA_IO_PENDING : - Result = ERROR_SUCCESS; - break ; + Result = WSASendTo(inter->fd, &wsabuf, 1, &octets_sent, 0, + &dest->sa, AddrLen, NULL, NULL); + if (SOCKET_ERROR == Result) { + errval = GetLastError(); + if (ERROR_UNEXP_NET_ERR == errval) { /* - * Something bad happened + * We get this error when trying to send if the + * network interface is gone or has lost link. + * Rescan interfaces to catch on sooner, but no + * more often than once per minute. Once ntpd + * is able to detect changes without polling + * this should be unneccessary */ - default : - msyslog(LOG_ERR, - "WSASendTo(%s) error %d: %s", - stoa(dest), errval, strerror(errval)); - free_trans_buf(buff); - lpo->trans_buf = NULL; - FreeHeap(lpo, "io_completion_port_sendto"); - break; + if (time_next_ifscan_after_error < current_time) { + time_next_ifscan_after_error = current_time + 60; + timer_interfacetimeout(current_time); } - } -#ifdef DEBUG - if (debug > 3) - printf("WSASendTo - %d bytes to %s : %d\n", len, stoa(dest), Result); -#endif - return (Result); + DPRINTF(4, ("sendto unexpected network error, interface may be down\n")); + } else + msyslog(LOG_ERR, "WSASendTo(%s) error %m", + stoa(dest)); + SetLastError(errval); + return -1; } - else { -#ifdef DEBUG - if (debug) printf("Packet too large: %d Bytes\n", len); -#endif - return ERROR_INSUFFICIENT_BUFFER; + + if (len != (int)octets_sent) { + msyslog(LOG_ERR, "WSASendTo(%s) sent %u of %d octets", + stoa(dest), octets_sent, len); + SetLastError(ERROR_BAD_LENGTH); + return -1; } + + DPRINTF(4, ("sendto %s %d octets\n", stoa(dest), len)); + + return 0; } @@ -925,47 +846,29 @@ async_write( const void *data, unsigned int count) { - transmitbuf_t *buff; + void *buff; IoCompletionInfo *lpo; DWORD BytesWritten; - if (count > sizeof buff->pkt) { -#ifdef DEBUG - if (debug) { - printf("async_write: %d bytes too large, limit is %d\n", - count, sizeof buff->pkt); - exit(-1); - } -#endif - errno = ENOMEM; - return -1; - } - - buff = get_trans_buf(); - lpo = (IoCompletionInfo *) GetHeapAlloc("async_write"); - - if (! buff || ! lpo) { - if (buff) { - free_trans_buf(buff); - DPRINTF(1, ("async_write: out of memory\n")); - } else - msyslog(LOG_ERR, "No more transmit buffers left - data discarded"); - + buff = emalloc(count); + lpo = GetHeapAlloc("async_write"); + if (lpo == NULL) { + free(buff); + DPRINTF(1, ("async_write: out of memory\n")); errno = ENOMEM; return -1; } lpo->request_type = SERIAL_WRITE; lpo->trans_buf = buff; - memcpy(&buff->pkt, data, count); + memcpy(buff, data, count); - if (!WriteFile((HANDLE)_get_osfhandle(fd), buff->pkt, count, + if (!WriteFile((HANDLE)_get_osfhandle(fd), buff, count, &BytesWritten, (LPOVERLAPPED)lpo) && ERROR_IO_PENDING != GetLastError()) { msyslog(LOG_ERR, "async_write - error %m"); - free_trans_buf(buff); - lpo->trans_buf = NULL; + free(buff); FreeHeap(lpo, "async_write"); errno = EBADF; return -1; diff --git a/ports/winnt/vc6/libntp.dsp b/ports/winnt/vc6/libntp.dsp index d54810ae0b..5f58abe880 100644 --- a/ports/winnt/vc6/libntp.dsp +++ b/ports/winnt/vc6/libntp.dsp @@ -418,10 +418,6 @@ SOURCE=..\..\..\lib\isc\win32\time.c # End Source File # Begin Source File -SOURCE=..\libntp\transmitbuff.c -# End Source File -# Begin Source File - SOURCE=..\..\..\libntp\tsftomsu.c # End Source File # Begin Source File @@ -650,10 +646,6 @@ SOURCE=..\include\sys\time.h # End Source File # Begin Source File -SOURCE=..\include\transmitbuff.h -# End Source File -# Begin Source File - SOURCE=..\include\win32_io.h # End Source File # Begin Source File diff --git a/ports/winnt/vc6/ntpd.dsp b/ports/winnt/vc6/ntpd.dsp index 8d0f42b6a2..35431b5e6f 100644 --- a/ports/winnt/vc6/ntpd.dsp +++ b/ports/winnt/vc6/ntpd.dsp @@ -448,10 +448,6 @@ SOURCE=..\include\termios.h # End Source File # Begin Source File -SOURCE=..\include\transmitbuff.h -# End Source File -# Begin Source File - SOURCE=..\include\sys\wait.h # End Source File # Begin Source File diff --git a/ports/winnt/vs2003/libntp.vcproj b/ports/winnt/vs2003/libntp.vcproj index ba1dce414c..dfb71bdbe1 100644 --- a/ports/winnt/vs2003/libntp.vcproj +++ b/ports/winnt/vs2003/libntp.vcproj @@ -1682,27 +1682,6 @@ - - - - - - - - - - diff --git a/ports/winnt/vs2003/ntpd.vcproj b/ports/winnt/vs2003/ntpd.vcproj index 9e6580d9dd..7799ce91e4 100644 --- a/ports/winnt/vs2003/ntpd.vcproj +++ b/ports/winnt/vs2003/ntpd.vcproj @@ -892,9 +892,6 @@ - - diff --git a/ports/winnt/vs2005/libntp.vcproj b/ports/winnt/vs2005/libntp.vcproj index fc34adcd47..81e803add3 100644 --- a/ports/winnt/vs2005/libntp.vcproj +++ b/ports/winnt/vs2005/libntp.vcproj @@ -1800,28 +1800,6 @@ RelativePath="..\..\..\lib\isc\win32\time.c" > - - - - - - - - @@ -2251,10 +2229,6 @@ RelativePath="..\..\..\lib\isc\win32\include\isc\time.h" > - - diff --git a/ports/winnt/vs2005/ntpd.vcproj b/ports/winnt/vs2005/ntpd.vcproj index 36a5b433be..a417c1a9dd 100644 --- a/ports/winnt/vs2005/ntpd.vcproj +++ b/ports/winnt/vs2005/ntpd.vcproj @@ -997,10 +997,6 @@ RelativePath="..\include\termios.h" > - - diff --git a/ports/winnt/vs2008/libntp/libntp.vcproj b/ports/winnt/vs2008/libntp/libntp.vcproj index 35811a5185..fe493f7265 100644 --- a/ports/winnt/vs2008/libntp/libntp.vcproj +++ b/ports/winnt/vs2008/libntp/libntp.vcproj @@ -515,10 +515,6 @@ RelativePath="..\..\..\..\lib\isc\win32\time.c" > - - @@ -816,16 +812,12 @@ RelativePath="..\..\..\..\lib\isc\win32\include\isc\thread.h" > - - - -