----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3230/#review7955 -----------------------------------------------------------
Nice, thanks!! I really appreciate the effort to revamp this. It sure seems a lot simpler to me. I agree, it would be good to hear from others (particularly Tony) before committing though. Meanwhile just a couple little things below... src/dev/net/etherswitch.cc (line 90) <http://reviews.gem5.org/r/3230/#comment6926> I still don't understand why these memcpy calls are needed... it looks like the EthAddr constructor makes a copy too, so I'd think you could just call e.g. Net::EthAddr destMacAddr(packet->data); and if that doesn't work, we should probably tweak the EthAddr constructor so it does... src/dev/net/etherswitch.cc (line 131) <http://reviews.gem5.org/r/3230/#comment6927> would be good to encapsulate this delay calculation in a helper function, esp. if we're going to use it twice. src/dev/net/etherswitch.cc (line 156) <http://reviews.gem5.org/r/3230/#comment6928> do we end up charging the delay twice, since we already accounted for it in enqueue()? The delay for the outgoing link should be calculated by the link itself, right? src/dev/net/etherswitch.cc (line 206) <http://reviews.gem5.org/r/3230/#comment6929> could use std::make_pair to simplify this a bit - Steve Reinhardt On Jan. 24, 2016, 12:35 p.m., Mohammad Alian wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3230/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2016, 12:35 p.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11292:9ed37b34da3a > --------------------------- > dist,dev: add an ethernet switch model > > > Diffs > ----- > > src/dev/net/Ethernet.py 9d2364203316 > src/dev/net/SConscript 9d2364203316 > src/dev/net/etherswitch.hh PRE-CREATION > src/dev/net/etherswitch.cc PRE-CREATION > > Diff: http://reviews.gem5.org/r/3230/diff/ > > > Testing > ------- > > several testing done with different benchmarks and different switch sizes > > > Thanks, > > Mohammad Alian > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
