Bug #315
closedPF_RING single runmode and PF_RING off-by-one fix.
Description
On a fully loaded frame PF_RING recv code suffers from an off-by-one error. PF_RING does bounds checking on the buffer it is copying into and truncates to length. PF_RING single thread runmode i.e. all processing contained in a single thread seems to perform significantly better on networks where there is an even distribution of traffic, and worse on networks where there are one or two top talkers.
Files
Updated by Victor Julien over 12 years ago
Why does this fix work? GET_PKT_DIRECT_MAX_SIZE(p) should have the same value as default_packet_size, right?
Updated by Will Metcalf over 12 years ago
GET_PKT_DIRECT_MAX_SIZE() is defined as..
#define GET_PKT_DIRECT_MAX_SIZE(p) default_packet_size - 1
PF_RING does bounds checking when copying into a user specified buffer. If the dest buffer is larger than the segment the packet is truncated. So we only copy 1513 bytes of a fully loaded 1514 byte frame. This check appears to be broken though right? Why not?
#define GET_PKT_DIRECT_MAX_SIZE(p) default_packet_size
Updated by Victor Julien over 12 years ago
- Assignee changed from Victor Julien to Eric Leblond
Eric can you review the logic of that macro?
Updated by Eric Leblond over 12 years ago
- File 0001-Fix-macro-about-default-packet-size.patch 0001-Fix-macro-about-default-packet-size.patch added
I think I wanted to avoid any off by one in the other way... It is a bad idea. The attached patch revert to a good default.
Updated by Victor Julien over 12 years ago
- Status changed from New to Assigned
- Assignee changed from Eric Leblond to Will Metcalf
- % Done changed from 0 to 70
Applied and pushed, thanks Eric.
Will, can you see if this helps your PF_RING code?
Updated by Victor Julien over 12 years ago
- Status changed from Assigned to Closed
- % Done changed from 70 to 100
This is fixed in the current master iirc. Please reopen if the issue isn't fixed.