----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3465/#review8298 -----------------------------------------------------------
Hi Mohammad, nice catch, thanks for fixing this! src/dev/net/etherswitch.hh (line 111) <http://reviews.gem5.org/r/3465/#comment7112> nullptr ? src/dev/net/etherswitch.hh (line 122) <http://reviews.gem5.org/r/3465/#comment7134> I would suggest to use a std::set instead of std::list for the fifo. You can define a compare object that uses the recvTick and the sourceId as the primary and secondary key, respectively. The fifo.push() method could then be much simpler. (See my futher commnents there.) src/dev/net/etherswitch.hh (line 123) <http://reviews.gem5.org/r/3465/#comment7113> 'const' modifier? src/dev/net/etherswitch.hh (line 147) <http://reviews.gem5.org/r/3465/#comment7137> The preferred way for serializing child objects is using 'serializeSection' from the parent. PortFifo should derive from 'Serializable' and then you can drop the extra 'base' argument here and for 'unserialize()', too. src/dev/net/etherswitch.cc (line 77) <http://reviews.gem5.org/r/3465/#comment7135> If the fifo were a std::set with a custom comparator (using recvTick as primary and sourceId as secondary key) then you could simply call 'fifo.insert(entry)'. All entries would be kept in the correct order by the container. If the size of fifo is too big after the insert then you could simply erase the last element (i.e. at fifo.rbegin()) to drop a single packet (instead of dropping potentially many packets with the current code). Does that make sense? src/dev/net/etherswitch.cc (line 120) <http://reviews.gem5.org/r/3465/#comment7136> You can use the fifo.front() accessor instead of creating a temporary copy of the entry. src/dev/net/etherswitch.cc (line 129) <http://reviews.gem5.org/r/3465/#comment7139> This could be simplified as 'for (auto i : fifo)' - Gabor Dozsa On May 6, 2016, 6:40 p.m., Mohammad Alian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3465/ > ----------------------------------------------------------- > > (Updated May 6, 2016, 6:40 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11472:91d189fd406a > --------------------------- > dist, dev: Fixed the packet ordering in etherswitch > This patch fixes the order that packets gets pushed into the output fifo > of etherswitch. If two packets arrive at the same tick to the etherswitch, > we sort and push them based on their source port id. > In dist-gem5 simulations, if there is no ordering inforced while two > packets arrive at the same tick, it can lead to non-deterministic simulations > > > Diffs > ----- > > src/dev/net/etherswitch.hh 954d3014f7f0 > src/dev/net/etherswitch.cc 954d3014f7f0 > > Diff: http://reviews.gem5.org/r/3465/diff/ > > > Testing > ------- > > > Thanks, > > Mohammad Alian > > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
