Memory issue during packet release about fastclick HOT 14 CLOSED

I suspect large frames being involved in this memory issue.
I will follow up with more information soon; I just posted the stack trace here for convenience, in order not to forget the issue.

from fastclick.

If extra anno makes the packet bigger than the DPDK buffer size, this may be a problem. But it is still a bug...

from fastclick.

Frame with len 2420 bytes causes the crash in this trace.
This confirms our fear about large frames which exceed the DPDK buffer size.

from fastclick.

Do you use your force length option? What is the type of the packet out of FromDump? Still a mmaped buffer, an "expensively pushed" buffer that will have a non-pool buffer?
I think that buffer is released as a pool buffer while it is actually a buffer allocated in place because it is too big to fit in the stand 2K pool.

from fastclick.

Interestingly enough, if I don't use FORCE_LEN (i.e., FORCE_LEN=-1), I uncovered another bug.
Because the packets in my raw IP trace do not have MAC header, line 814 in packet.cc causes a crash:

set_mac_header(p->mac_header() ? data() + p->mac_header_offset() : 0);

because p=0

See the stack trace below:

click: ../include/click/packet.hh:2118: void Packet::set_mac_header(const unsigned char*): Assertion `p >= buffer() && p <= end_buffer()' failed.

0 0x00007ffff5ed0428 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:54
1 0x00007ffff5ed202a in __GI_abort () at abort.c:89
2 0x00007ffff5ec8bd7 in _assert_fail_base (fmt=, assertion=assertion@entry=0xb921b8 "p >= buffer() && p <= end_buffer()", file=file@entry=0xb46afd "../include/click/packet.hh", line=line@entry=2118,
function=function@entry=0xbc5720 <ZZN6Packet14set_mac_headerEPKhE19__PRETTY_FUNCTION> "void Packet::set_mac_header(const unsigned char*)") at assert.c:92
3 0x00007ffff5ec8c82 in _GI___assert_fail (assertion=assertion@entry=0xb921b8 "p >= buffer() && p <= end_buffer()", file=file@entry=0xb46afd "../include/click/packet.hh", line=line@entry=2118,
function=function@entry=0xbc5720 <ZZN6Packet14set_mac_headerEPKhE19__PRETTY_FUNCTION> "void Packet::set_mac_header(const unsigned char*)") at assert.c:101
4 0x000000000066a84a in Packet::set_mac_header (p=, this=0x15fc100) at ../include/click/packet.hh:2118
5 Packet::copy (this=this@entry=0x15fc100, p=p@entry=0x15fc050, headroom=) at ../lib/packet.cc:814
6 0x0000000000602953 in EnsureDPDKBuffer::smaction (p=0x15fc050, this=0x15eaa80) at ../elements/userlevel/ensuredpdkbuffer.cc:83
7 EnsureDPDKBuffer::simple_action (this=0x15eaa80, p=0x15fc050) at ../elements/userlevel/ensuredpdkbuffer.cc:106
8 0x0000000000680c55 in Element::push (this=0x15eaa80, port=0, p=) at ../lib/element.cc:3061
9 0x0000000000606af8 in Element::Port::push (p=, this=) at ../include/click/element.hh:714
10 FromDump::run_task (this=0x15e9fc0) at ../elements/userlevel/fromdump.cc:610
11 0x00000000006b1199 in Task::fire (this=0x15ea190) at ../include/click/task.hh:584
12 RouterThread::run_tasks (ntasks=127, this=0x156a8f0) at ../lib/routerthread.cc:392
13 RouterThread::driver (this=0x156a8f0) at ../lib/routerthread.cc:613
14 0x000000000049129a in main (argc=, argv=) at click.cc:809

I have a fix for this already

from fastclick.

With FORCE_LEN=0 (which crafts the packets with their real length), the trace is injected without errors if I intentionally kill packets longer than 2048 bytes.

from fastclick.

Interestingly enough, if I don't use FORCE_LEN (i.e., FORCE_LEN=-1), I uncovered another bug.
Because the packets in my raw IP trace do not have MAC header, line 814 in packet.cc causes a crash:

set_mac_header(p->mac_header() ? data() + p->mac_header_offset() : 0);

This is known and is related to the way click is build, you need a MarkMACHeader after the FromDump. It's the same for IP elements that do not have a CheckIPHeader or MarkIPHeader before them. You need to parse the layer you're working on. I don't think a check should be added in the code. Maybe something static (do I have an element that sets the annotation before me?) but not in the fast path.

from fastclick.

max_buffer_length is not really true. Normally when you call make() for a packet too long for the buffer pool, it will do a malloc and will not use the buffer pool. This is not the problem...

from fastclick.

Yes, I am digging into it now. The copy operation fails and the packet is getting killed, but it seems that no reference exists. Will be back soon..

from fastclick.

WHere are we on this?

from fastclick.

no progress on this, will be back after the submission..

from fastclick.

WritablePacket::make might not succeed. There needs to be a bailout prior to q->copy.

from fastclick.

Indeed, this check is mandatory both in EnsureDPDKBuffer and EnsureNetmapBuffer smactions.
However, the problem in my case does not occur due to null pointer q returned by make().
The failure occurs in q->copy(), which in turn results in p->kill() (in the else branch).
This kill is performed on the wrong object in my opinion (q->kill() should have been called).

from fastclick.

See a fix proposed in PR #161

from fastclick.