----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3493/#review8429 -----------------------------------------------------------
Thank you for the patch! It looks good, I have a few minor comments only. src/dev/net/etherpkt.cc (line 47) <http://reviews.gem5.org/r/3493/#comment7304> Could you use 'dataLength' instead of 'bufferLength' here to serialize only the useful data? src/dev/net/etherpkt.cc (line 55) <http://reviews.gem5.org/r/3493/#comment7305> If you serialize the useful data only then you could use dataLength here, too. In that case, adding an assertion to make sure dataLength <= bufferLength would be good, too. src/dev/net/etherpkt.cc (line 57) <http://reviews.gem5.org/r/3493/#comment7306> There is an implicit assumption that the EthPacketData object is "empty" (i.e. data==nullptr) at this point. It may be good to add an assertion to check that exlicitly. - Gabor Dozsa On May 31, 2016, 6:09 p.m., Michael LeBeane wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3493/ > ----------------------------------------------------------- > > (Updated May 31, 2016, 6:09 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11479:7d5bb81e87de > --------------------------- > dev: Redefine 'length' in EthPacketData > Currently, all the network devices create a 16K buffer for the 'data' field > in EthPacketData, and use 'length' to keep track of the size of the packet > in the buffer. This patch introduces 'bufferLength,' 'dataLength,' and > 'simLength' parameters to EthPacketData. The size of the 'data' buffer is > explicitly stored in 'bufferLength', which allows the packet to > serialize/unserialize without passing in the buffer size. 'dataLength' stores > the amount of space taken up by the packet in the 'data' buffer. Finally, > 'simLength' is used to hold the effective length of the packet used for all > timing calulations in the simulator. > > > Diffs > ----- > > src/dev/net/pktfifo.hh 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/pktfifo.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/sinic.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/tcp_iface.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/dist_etherlink.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/dist_iface.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/dist_packet.hh 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/etherbus.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/etherdump.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/etherlink.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/etherpkt.hh 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/etherpkt.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/etherswitch.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/ethertap.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/i8254xGBe.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > src/dev/net/ns_gige.cc 54cf9a388a9ddafb4d76c7daba6fb07e26635bc8 > > Diff: http://reviews.gem5.org/r/3493/diff/ > > > Testing > ------- > > > Thanks, > > Michael LeBeane > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
