----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3705/#review9157 -----------------------------------------------------------
src/dev/net/dist_etherlink.cc (line 237) <http://reviews.gem5.org/r/3705/#comment7871> 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 (line 537) <http://reviews.gem5.org/r/3705/#comment7872> The same here - no need for fixed 16KB buffer size. src/dev/net/etherlink.cc (line 238) <http://reviews.gem5.org/r/3705/#comment7873> The same here - no need for fixed 16KB buffer size. src/dev/net/etherlink.cc (line 254) <http://reviews.gem5.org/r/3705/#comment7874> The same here - no need for fixed 16KB buffer size. src/dev/net/etherpkt.cc (line 54) <http://reviews.gem5.org/r/3705/#comment7879> 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 (line 55) <http://reviews.gem5.org/r/3705/#comment7875> 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 (line 2367) <http://reviews.gem5.org/r/3705/#comment7876> No need to allocate the fix 16KB buffer for an rx packet. src/dev/net/pktfifo.cc (line 80) <http://reviews.gem5.org/r/3705/#comment7877> 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). - Gabor Dozsa On Nov. 21, 2016, 6:57 p.m., Michael LeBeane wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3705/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2016, 6:57 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > 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.do...@arm.com> > > > Diffs > ----- > > src/dev/net/dist_etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/dist_iface.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/pktfifo.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 > > Diff: http://reviews.gem5.org/r/3705/diff/ > > > Testing > ------- > > > Thanks, > > Michael LeBeane > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
