-
Notifications
You must be signed in to change notification settings - Fork 544
Regression in packet synchronization with pcap_sendqueue_transmit #580
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
It was Did I missed something ? |
Hi @kayoub5. The code does not look wrong to us, but we will add extra parenthesis just to be on the safe side. But the source of your problem may be something different. Can you provide more details on the symptoms you are seeing? Are you using Npcap Version 1.60? We will be happy to take a look. |
The problem we are facing is the accuracy of the function, Npcap 1.10 allowed an inter packet duration as low as 10-20us depending on the host. npcap 1.60 can not handle any sub second gap. the only related change I was able to see in the changelog was the sync change in 1.20, so I assumed that it is responsible. |
ok, that makes a bit more sense. We did make some changes to try to avoid monopolizing the kernel for the duration of the packet send operation, so we'll start looking for issues there. |
Driver's BIOCSENDPACKETSSYNC handles inter-packet delays for as many packets as can be sent in 1s, then returns. Packet.dll must do a user-mode delay before sending the next packet. Previously, we did a very busy wait; this change uses millisecond-resolution Sleep instead. If delay is less than 1ms, we do not wait, trusting the system call overhead to make the delay close enough. Also changed calculations to use LONGLONG (64-bit int) data types and to ensure we do not overflow nor lose precision. Similar changes will be committed to the driver's sync code.
Similar to earlier change to PacketSendPackets, this commit changes calculation of inter-packet delay within the driver for BIOCSENDPACKETSSYNC. We calculate the microsecond difference first, considering that tv_usec can hold over an hour of time, then convert to clock ticks in order to see how much time remains. If over 1s, control is returned to user mode. Otherwise, and appropriate NDIS delay function with microsecond resolution is called, eliminating the need to run our own busy wait or use NdisWaitEvent (millisecond resolution).
Hi @kayoub5 (and anyone else who has experienced this). Are you able to test with the new Npcap Version 1.70. We made some changes in this area (see the commits referenced above and the changelog and so are you able to retest and let us know how it goes? We are leaving this issue open for now but will close if we don't hear back from anyone that the issue persists. |
Hi @fyodor, We are still experiencing performance degradation, previous versions of Npcap were able to allow down to 20µs inter frame gap using pcap_sendqueue_transmit with Npcap we can't even get an accurate 1ms inter frame gap. |
I've just pushed a change to move the call to In the meantime, @kayoub5 can you provide a DiagReport output and/or a detailed description of the system where this timing discrepancy is occurring? What means are you using to verify the timing? Is there a test we can use to get the same result, so we can confirm any fixes have addressed the issue? |
Hi @fyodor , we have done a comparison between WinPcap 4.1.3 and Npcap 1.72 using 2 machines with no installed antivirus and having the below characteristic:
In these tests, the packets were sent with a delay of 65µs : traces have been recorded in sender / receiver side.
Do you have an explanation for this behavior in Npcap ? |
@dmiller-nmap FYI, me and @welbouri are from the same team. |
It has been a while, but we're still interested in this. Even though we're talking about sub-millisecond transmission delays, it may affect some use cases. So we are leaving it open and interested in hearing whether anyone is having this problem with Npcap 1.80. |
Change introduced in 10d4de9#diff-fbde706694dbead2c023e9f5ba579b5d724a4a35c8831f03df023c9f0ee183c8R2620-R2623 are causing the microsecond part of the timestamp to be ignored.
(TimeFreq.QuadPart) / 1000000
will return zero if TimeFreq is less than 1000000 and inaccurate number if TimeFreq is not multiple of 1000000 (integer division), causing the whole microsecond part to be miscalculated.As the original code indicates, the division should be the last operation, otherwise the delta time precision will be messed up.
This causes the entire pcap_sendqueue_transmit to miscalculate the delta time and breaking it completely in sync mode.
The text was updated successfully, but these errors were encountered: