-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/3230/#review7842
-----------------------------------------------------------



src/dev/Ethernet.py (line 88)
<http://reviews.gem5.org/r/3230/#comment6783>

    Does 1Gbps as the default make sense? I think that is a little dated, 
particularly in the case of the systems multi/pd gem5 is meant to model. 10Gbps 
would probably be a more reasonable default.



src/dev/etherswitch.hh (line 57)
<http://reviews.gem5.org/r/3230/#comment6778>

    Don't make this virtual, in fact don't make any of EtherSwitch's methods 
virtual. It should probably be a final class since any other switch wouldn't be 
a type of EtherSwitch.
    
    If we ever did decide to have some other sort of switch, e.g., InfiniBand, 
we'd need to develop a base Switch class, and derive EtherSwitch, 
InfiniBandSwitch etc. from that.



src/dev/etherswitch.hh (line 78)
<http://reviews.gem5.org/r/3230/#comment6784>

    const, also make sure const-correctness is observed throughout.



src/dev/etherswitch.hh (line 137)
<http://reviews.gem5.org/r/3230/#comment6785>

    reference



src/dev/etherswitch.hh (line 139)
<http://reviews.gem5.org/r/3230/#comment6786>

    return packet != nullptr; better expresses what you want here



src/dev/etherswitch.hh (line 173)
<http://reviews.gem5.org/r/3230/#comment6787>

    Names won't change. Make them const.



src/dev/etherswitch.cc (line 73)
<http://reviews.gem5.org/r/3230/#comment6771>

    Use nullptr throughout


- Tony Gutierrez


On Nov. 19, 2015, 12:19 p.m., Mohammad Alian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3230/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 12:19 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> -------
> 
> This patch adds an ethernet switch model which is based on
> http://reviews.gem5.org/r/2305/
> while fixing it's issues.
> Although we provide this patch as part of distributed gem5 extenstion,
> it can be used for any other configurations.
> 
> 
> Diffs
> -----
> 
>   src/dev/Ethernet.py 2375b33bddc6 
>   src/dev/SConscript 2375b33bddc6 
>   src/dev/etherswitch.hh PRE-CREATION 
>   src/dev/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

Reply via email to