Ah, I didn't notice the '&' there before. The only changes I had to make were to broadcast and to the constructors since I extract the MAC from the packet as an array of uint8_t.
I will probably put the switch model and some changes to EthAddr on the reviewboard next week then. Anthony Gutierrez http://web.eecs.umich.edu/~atgutier On Fri, May 23, 2014 at 11:05 AM, Steve Reinhardt via gem5-dev < gem5-dev@gem5.org> wrote: > Well that's a very different question ;-). > > The bytes() and addr() methods look fine to me; they're just using > '&data[0]' as reverse shorthand for 'data'. > > The pointer assignments you show look like bugs. I expect that Nate was > thinking in terms of structs and not arrays and expecting the whole address > to be copied over. > > The unicast/multicast/broadcast methods do get used in ns_gige and sinic, > though in the latter case all the places they are used are ifdef'd out. > They're not quite correct either, but sort of close. broadcast() is the > only one that needs to check all the bytes; as far as I can tell, > multicast() should be '(data[0] & 0x01) && !broadcast()' and unicast() > should be '!(data[0] & 0x01)'. Or I suppose you could define multicast() > as '!unicast() && !broadcast()' for minimal redundancy. > > Other than that though, it looks reasonable to me, > > Steve > > > > On Fri, May 23, 2014 at 3:49 AM, Anthony Gutierrez via gem5-dev < > gem5-dev@gem5.org> wrote: > > > I'll clarify the question. I built a simple Ethernet switch model and > > wanted to use EthAddr to store MAC address in the switch table, which > would > > be a std::map<uint64_t, EthInt*>. Looking at the code it seemed EthAddr > was > > only used in the NIC, and in that case it only used the ctor that takes > in > > a string, as well as the uint64_t cast operator, which allowed the NICs > to > > cast from a string to uint64_t to set their macAddr. > > > > However the other ctors take in either an array or reference to an > eth_addr > > struct and do initialization like: > > > > *data = *ea; > > or > > *data = *ea.data > > > > Which is equivalent to: data[0] = ea[0] or data[0] = ea.data[0]. All the > > class methods - with the exception of the overloaded uint64_t cast > operator > > - only utilize data[0] as well: > > > > const uint8_t *bytes() const { return &data[0]; } > > uint8_t *bytes() { return &data[0]; } > > > > const uint8_t *addr() const { return &data[0]; } > > bool unicast() const { return data[0] == 0x00; } > > bool multicast() const { return data[0] == 0x01; } > > bool broadcast() const { return data[0] == 0xff; } > > > > I had to modify this class to make it more robust, and I was wondering if > > this class was never completed or if this is the intended usage. > > > > > > Anthony Gutierrez > > http://web.eecs.umich.edu/~atgutier > > > > > > On Fri, May 23, 2014 at 2:11 AM, Steve Reinhardt via gem5-dev < > > gem5-dev@gem5.org> wrote: > > > > > It's an ethernet address (for binding to ethernet NICs). Note that it > > > (confusingly) corresponds to the python param class called > > 'EthernetAddr', > > > which is why grepping just for 'EthAddr' makes it look less used than > it > > > is. > > > > > > Maybe I'm not looking at the same code you are, but I don't see what > > you're > > > saying about only operating on data[0]. > > > > > > Steve > > > > > > > > > > > > On Thu, May 22, 2014 at 11:28 AM, Anthony Gutierrez via gem5-dev < > > > gem5-dev@gem5.org> wrote: > > > > > > > Hello, > > > > > > > > Can someone tell me what EthAddr was supposed to be used for? > grepping > > > > shows that it isn't really used for anything? I'm curious as to why > all > > > the > > > > constructors and methods only operate on data[0]. Is there any reason > > > all 6 > > > > bytes are not used? > > > > > > > > Thanks, > > > > Anthony Gutierrez > > > > http://web.eecs.umich.edu/~atgutier > > > > _______________________________________________ > > > > gem5-dev mailing list > > > > gem5-dev@gem5.org > > > > http://m5sim.org/mailman/listinfo/gem5-dev > > > > > > > _______________________________________________ > > > gem5-dev mailing list > > > gem5-dev@gem5.org > > > http://m5sim.org/mailman/listinfo/gem5-dev > > > > > _______________________________________________ > > gem5-dev mailing list > > gem5-dev@gem5.org > > http://m5sim.org/mailman/listinfo/gem5-dev > > > _______________________________________________ > gem5-dev mailing list > gem5-dev@gem5.org > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list gem5-dev@gem5.org http://m5sim.org/mailman/listinfo/gem5-dev
