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

Reply via email to