osmocom
/
wireshark
14
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

From Benjamin Tse via bug 2200: 

I've created a new bug rather than reopening 1181 as the scope is constrained
somewhat more.

Basically, when capturing from a named pipe the wireshark display lags by one
packet. This is especially frustrating when the packets arrive at low rates.

tshark is fine. But the packet count in dumpcap also lags by one.

Looking at the code, the problem appears to be in cap_pipe_select(). It
attempts to use WaitForSingleObject() on the named pipe but AFAICT this never
blocks.

I've attached a diff for some code that fixes the issue for me. The semantics
of overlapped IO in Win32 is quite different from the select/read model - hence
the other changes!

I've tested this fix on WinXP, 2k server and 2003 server. I've also checked
that my changes compile on a Freespire box that I have lying around.


From me:

Adapt the changes for dumpcap, which is where the affected code now lives.

svn path=/trunk/; revision=28452
This commit is contained in:
Gerald Combs 2009-05-22 19:52:30 +00:00
parent 35b474b83f
commit f7f2a08def
2 changed files with 150 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
AUTHORS
View File

@ -2851,6 +2851,10 @@ Nick Lewis <nick.lewis@atltelecom.com> {
Show timestamp problems in RTP player
}
Benjamin Tse <benjamin_tse@agilent.com> {
Overlapped pipe I/O for dumpcap under Windows
}
and by:
Pavel Roskin <proski [AT] gnu.org>

171
dumpcap.c
View File

@ -232,6 +232,12 @@ static const char please_report[] =
*/
static loop_data global_ld;
/*
* Event used for overlapped IO reads from named pipes.
*/
#ifdef _WIN32
static HANDLE cap_pipe_read_event = 0;
#endif
/*
* Timeout, in milliseconds, for reads from the stream of captured packets.
@ -694,9 +700,9 @@ cap_pipe_adjust_header(gboolean byte_swapped, struct pcap_hdr *hdr, struct pcapr
* Returns the same values as select. If an error is returned,
* the string cap_pipe_err_str should be used instead of errno.
*/
#ifndef _WIN32
static int
cap_pipe_select(int pipe_fd) {
#ifndef _WIN32
fd_set rfds;
struct timeval timeout, *pto;
int sel_ret;
@ -715,14 +721,23 @@ cap_pipe_select(int pipe_fd) {
cap_pipe_err_str = strerror(errno);
return sel_ret;
}
#else
#else /* _WIN32 */
static int
cap_pipe_select(int pipe_fd, guint32* pre_read_word) {
/* XXX - Should we just use file handles exclusively under Windows?
* Otherwise we have to convert between file handles and file descriptors
* here and when we open a named pipe.
*/
HANDLE hPipe = (HANDLE) _get_osfhandle(pipe_fd);
wchar_t *err_str;
DWORD wait_ret;
BOOL read_file_result;
OVERLAPPED overlap;
guint32 read_word = 0;
DWORD num_bytes_read = 0;
DWORD return_val = 1;
if (hPipe == INVALID_HANDLE_VALUE) {
cap_pipe_err_str = "Could not open standard input";
@ -731,26 +746,72 @@ cap_pipe_select(int pipe_fd) {
cap_pipe_err_str = "Unknown error";
wait_ret = WaitForSingleObject(hPipe, CAP_READ_TIMEOUT);
switch (wait_ret) {
/* XXX - This probably isn't correct */
case WAIT_ABANDONED:
errno = EINTR;
return -1;
case WAIT_OBJECT_0:
return 1;
case WAIT_TIMEOUT:
return 0;
case WAIT_FAILED:
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
NULL, GetLastError(), 0, (LPTSTR) &err_str, 0, NULL);
cap_pipe_err_str = utf_16to8(err_str);
LocalFree(err_str);
return -1;
overlap.Offset = 0;
overlap.OffsetHigh = 0;
overlap.hEvent = cap_pipe_read_event;
read_file_result = ReadFile(hPipe, (LPVOID) &read_word, 4, &num_bytes_read, &overlap);
if (!read_file_result) {
DWORD lastErr = GetLastError();
switch (lastErr) {
case ERROR_HANDLE_EOF:
return_val = 0;
break;
case ERROR_IO_PENDING:
{
DWORD waitResult = WaitForSingleObject(overlap.hEvent, CAP_READ_TIMEOUT);
switch (waitResult) {
case WAIT_ABANDONED:
errno = EINTR;
return_val = -1;
break;
case WAIT_OBJECT_0:
{
DWORD numBytes = 0;
BOOL ovRes = GetOverlappedResult(hPipe, &overlap, &numBytes, FALSE);
if (ovRes) {
*pre_read_word = read_word;
return_val = 1;
} else {
return_val = 0;
}
break;
}
case WAIT_TIMEOUT:
{
BOOL cancelRes = CancelIo(hPipe);
if (!cancelRes) {
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
NULL, GetLastError(), 0, (LPTSTR) &err_str, 0, NULL);
LocalFree(err_str);
}
return_val = 0;
break;
}
case WAIT_FAILED:
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
NULL, GetLastError(), 0, (LPTSTR) &err_str, 0, NULL);
cap_pipe_err_str = utf_16to8(err_str);
LocalFree(err_str);
return_val = -1;
break;
default:
g_assert_not_reached();
return_val = -1;
break;
}
break;
}
default:
g_assert_not_reached();
return -1;
return_val = -1;
break;
}
} else {
*pre_read_word = read_word;
return_val = 1;
}
return return_val;
}
#endif
@ -767,10 +828,9 @@ cap_pipe_open_live(char *pipename, struct pcap_hdr *hdr, loop_data *ld,
#ifndef _WIN32
struct stat pipe_stat;
#else
#if 1
char *pncopy, *pos;
guint32 pre_read_word;
wchar_t *err_str;
#endif
HANDLE hPipe = NULL;
#endif
int sel_ret;
@ -855,7 +915,7 @@ cap_pipe_open_live(char *pipename, struct pcap_hdr *hdr, loop_data *ld,
/* Wait for the pipe to appear */
while (1) {
hPipe = CreateFile(utf_8to16(pipename), GENERIC_READ, 0, NULL,
OPEN_EXISTING, 0, NULL);
OPEN_EXISTING, FILE_FLAG_OVERLAPPED, NULL);
if (hPipe != INVALID_HANDLE_VALUE)
break;
@ -893,6 +953,9 @@ cap_pipe_open_live(char *pipename, struct pcap_hdr *hdr, loop_data *ld,
ld->cap_pipe_err = PIPERR;
return -1;
}
cap_pipe_read_event = CreateEvent(NULL, FALSE, FALSE, NULL);
#endif /* _WIN32 */
}
@ -901,13 +964,25 @@ cap_pipe_open_live(char *pipename, struct pcap_hdr *hdr, loop_data *ld,
/* read the pcap header */
bytes_read = 0;
while (bytes_read < sizeof magic) {
#ifndef _WIN32
sel_ret = cap_pipe_select(fd);
#else
sel_ret = cap_pipe_select(fd, &pre_read_word);
#endif
if (sel_ret < 0) {
g_snprintf(errmsg, errmsgl,
"Unexpected error from select: %s", strerror(errno));
goto error;
} else if (sel_ret > 0) {
b = read(fd, ((char *)&magic)+bytes_read, sizeof magic-bytes_read);
#ifdef _WIN32
g_assert(bytes_read == 0);
bytes_read += sizeof pre_read_word;
magic = pre_read_word;
#endif
if (bytes_read < sizeof magic)
b = read(fd, ((char *)&magic)+bytes_read, sizeof magic-bytes_read);
else
continue;
if (b <= 0) {
if (b == 0)
g_snprintf(errmsg, errmsgl, "End of file on pipe during open");
@ -956,12 +1031,20 @@ cap_pipe_open_live(char *pipename, struct pcap_hdr *hdr, loop_data *ld,
/* Read the rest of the header */
bytes_read = 0;
while (bytes_read < sizeof(struct pcap_hdr)) {
#ifndef _WIN32
sel_ret = cap_pipe_select(fd);
#else
sel_ret = cap_pipe_select(fd, &pre_read_word);
#endif
if (sel_ret < 0) {
g_snprintf(errmsg, errmsgl,
"Unexpected error from select: %s", strerror(errno));
goto error;
} else if (sel_ret > 0) {
#ifdef _WIN32
*(((guint32*)hdr) + bytes_read) = pre_read_word;
bytes_read += sizeof pre_read_word;
#endif
b = read(fd, ((char *)hdr)+bytes_read,
sizeof(struct pcap_hdr) - bytes_read);
if (b <= 0) {
@ -1005,8 +1088,12 @@ error:
/* We read one record from the pipe, take care of byte order in the record
* header, write the record to the capture file, and update capture statistics. */
#ifndef _WIN32
static int
cap_pipe_dispatch(loop_data *ld, guchar *data, char *errmsg, int errmsgl)
#else
cap_pipe_dispatch(loop_data *ld, guchar *data, char *errmsg, int errmsgl, guint32 pre_read_word)
#endif
{
struct pcap_pkthdr phdr;
int b;
@ -1028,6 +1115,10 @@ cap_pipe_dispatch(loop_data *ld, guchar *data, char *errmsg, int errmsgl)
/* Fall through */
case STATE_READ_REC_HDR:
#ifdef _WIN32
*(((guint32*) &ld->cap_pipe_rechdr) + ld->cap_pipe_bytes_read) = pre_read_word;
ld->cap_pipe_bytes_read += sizeof pre_read_word;
#endif
b = read(ld->cap_pipe_fd, ((char *)&ld->cap_pipe_rechdr)+ld->cap_pipe_bytes_read,
ld->cap_pipe_bytes_to_read - ld->cap_pipe_bytes_read);
if (b <= 0) {
@ -1048,6 +1139,10 @@ cap_pipe_dispatch(loop_data *ld, guchar *data, char *errmsg, int errmsgl)
/* Fall through */
case STATE_READ_DATA:
#ifdef _WIN32
*(((guint32*) (data+ld->cap_pipe_bytes_read) + ld->cap_pipe_bytes_read)) = pre_read_word;
ld->cap_pipe_bytes_read += sizeof pre_read_word;
#endif
b = read(ld->cap_pipe_fd, data+ld->cap_pipe_bytes_read,
ld->cap_pipe_rechdr.hdr.incl_len - ld->cap_pipe_bytes_read);
if (b <= 0) {
@ -1399,6 +1494,11 @@ static void capture_loop_close_input(loop_data *ld) {
ld->go = FALSE;
#ifdef _WIN32
if (ld->from_cap_pipe == TRUE && cap_pipe_read_event != 0) {
CloseHandle(cap_pipe_read_event);
cap_pipe_read_event = 0;
}
/* Shut down windows sockets */
WSACleanup();
#endif
@ -1565,6 +1665,9 @@ capture_loop_dispatch(capture_options *capture_opts _U_, loop_data *ld,
int sel_ret;
gint packet_count_before;
guchar pcap_data[WTAP_MAX_PACKET_SIZE];
#ifdef _WIN32
guint32 pre_read_word;
#endif
packet_count_before = ld->packet_count;
if (ld->from_cap_pipe) {
@ -1572,9 +1675,19 @@ capture_loop_dispatch(capture_options *capture_opts _U_, loop_data *ld,
#ifdef LOG_CAPTURE_VERBOSE
g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "capture_loop_dispatch: from capture pipe");
#endif
#ifndef _WIN32
sel_ret = cap_pipe_select(ld->cap_pipe_fd);
#else
sel_ret = cap_pipe_select(ld->cap_pipe_fd, &pre_read_word);
#endif
if (sel_ret <= 0) {
inpkts = 0;
#ifdef _WIN32
if (sel_ret < 0 && GetLastError() == ERROR_BROKEN_PIPE) {
/* The pipe was closed - don't output an error */
ld->go = FALSE;
} else
#endif
if (sel_ret < 0 && errno != EINTR) {
g_snprintf(errmsg, errmsg_len,
"Unexpected error from select: %s", strerror(errno));
@ -1585,7 +1698,11 @@ capture_loop_dispatch(capture_options *capture_opts _U_, loop_data *ld,
/*
* "select()" says we can read from the pipe without blocking
*/
#ifndef _WIN32
inpkts = cap_pipe_dispatch(ld, pcap_data, errmsg, errmsg_len);
#else
inpkts = cap_pipe_dispatch(ld, pcap_data, errmsg, errmsg_len, pre_read_word);
#endif
if (inpkts < 0) {
ld->go = FALSE;
}
@ -1612,7 +1729,11 @@ capture_loop_dispatch(capture_options *capture_opts _U_, loop_data *ld,
g_log(LOG_DOMAIN_CAPTURE_CHILD, G_LOG_LEVEL_DEBUG, "capture_loop_dispatch: from pcap_dispatch with select");
#endif
if (ld->pcap_fd != -1) {
#ifndef _WIN32
sel_ret = cap_pipe_select(ld->pcap_fd);
#else
sel_ret = cap_pipe_select(ld->pcap_fd, &pre_read_word);
#endif
if (sel_ret > 0) {
/*
* "select()" says we can read from it without blocking; go for