dev: Fix buffer length when unserializing an eth pkt
Review Request #3705 - Created Nov. 15, 2016 and updated
| Information | |
|---|---|
| Michael LeBeane | |
| gem5 | |
| default | |
| Reviewers | |
| Default | |
Changeset 11705:008f04dcc085 --------------------------- dev: Fix buffer length when unserializing an eth pkt Changeset 11701 only serialized the useful portion of of an ethernet packets' payload. However, the device models expect each ethernet packet to contain a 16KB buffer, even if there is no data in it. This patch adds a 'bufLength' field to EthPacketData so the original size of the packet buffer can always be unserialized. Reported-by: Gabor Dozsa <Gabor.Dozsa@arm.com>
Ship It!
Ship It!
do we need a checkpoint updater, or does the call to paramIn return 0 if the parameter is not populated?
The patch fixes my full system test which failed before.
I think it needs a chekpoint upgrader though. Making bufLength optional is not enough to avoid the problem of not allocating (enough) buffer space for an ethernet packet when unserializing a legacy checkpoint.
Change Summary:
Fix unserialize to support checkpoints created before changeset 11701.
Diff: |
Revision 2 (+22 -16) |
|---|
-
src/dev/net/dist_etherlink.cc (Diff revision 2) -
No need for the fixed 16KB buffer size here. (No further data will be appended to the packet.) You might allocate space only for the existing data in the unserialize() instead (see my comments at EthPacketData::unserialize()).
-
src/dev/net/dist_iface.cc (Diff revision 2) -
The same here - no need for fixed 16KB buffer size.
-
src/dev/net/etherlink.cc (Diff revision 2) -
The same here - no need for fixed 16KB buffer size.
-
src/dev/net/etherlink.cc (Diff revision 2) -
The same here - no need for fixed 16KB buffer size.
-
src/dev/net/etherpkt.cc (Diff revision 2) -
It might make sense to check that the current bufLength value is either zero or not smaller than the new one read from the checkpoint. Otherwise, we might overflow the buffer below at arrayParamIn(...).
-
src/dev/net/etherpkt.cc (Diff revision 2) -
If we have an old checkpoint then this assertion may fire unless we always allocate the fixed size buffer before unserializing any eth packet (which would be nice to avoid). This could be fixed by using 'length' as buffer size if 'bufLength' is not present in the checkpoint.
-
src/dev/net/ns_gige.cc (Diff revision 2) -
No need to allocate the fix 16KB buffer for an rx packet.
-
src/dev/net/pktfifo.cc (Diff revision 2) -
No need for fixed 16KB size here either. PacketFifo is only used to store "finalized" eth packets (i.e. no further data will be appended to the packets).