Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-24 Thread Gabor Dozsa 

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

Ship it!


Ship it! Thanks!

- Gabor Dozsa


On Nov. 23, 2016, 5:36 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3705/
> ---
> 
> (Updated Nov. 23, 2016, 5:36 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:008f04dcc085
> ---
> dev: Fix buffer length when unserializing an eth pkt
> Changeset 11701 only serialized the useful portion of of an ethernet packets'
> payload.  However, the device models expect each ethernet packet to contain
> a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
> field to EthPacketData so the original size of the packet buffer can always be
> unserialized.
> 
> Reported-by: Gabor Dozsa 
> 
> 
> Diffs
> -
> 
>   src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-23 Thread Michael LeBeane 

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

(Updated Nov. 23, 2016, 5:36 p.m.)


Review request for Default.


Repository: gem5


Description
---

Changeset 11705:008f04dcc085
---
dev: Fix buffer length when unserializing an eth pkt
Changeset 11701 only serialized the useful portion of of an ethernet packets'
payload.  However, the device models expect each ethernet packet to contain
a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
field to EthPacketData so the original size of the packet buffer can always be
unserialized.

Reported-by: Gabor Dozsa 


Diffs (updated)
-

  src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 

Diff: http://reviews.gem5.org/r/3705/diff/


Testing
---


Thanks,

Michael LeBeane

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-23 Thread Gabor Dozsa 

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



src/dev/net/dist_etherlink.cc (line 237)


No need for the fixed 16KB buffer size here. (No further data will be 
appended to the packet.) You might allocate space only for the existing data in 
the unserialize() instead (see my comments at EthPacketData::unserialize()).



src/dev/net/dist_iface.cc (line 537)


The same here - no need for fixed 16KB buffer size.



src/dev/net/etherlink.cc (line 238)


The same here - no need for fixed 16KB buffer size.



src/dev/net/etherlink.cc (line 254)


The same here - no need for fixed 16KB buffer size.



src/dev/net/etherpkt.cc (line 54)


It might make sense to check that the current bufLength value is either 
zero or not smaller than the new one read from the checkpoint. Otherwise, we 
might overflow the buffer below at arrayParamIn(...).



src/dev/net/etherpkt.cc (line 55)


If we have an old checkpoint then this assertion may fire unless we always 
allocate the fixed size buffer before unserializing any eth packet (which would 
be nice to avoid). This could be fixed by using 'length' as buffer size if 
'bufLength' is not present in the checkpoint.



src/dev/net/ns_gige.cc (line 2367)


No need to allocate the fix 16KB buffer for an rx packet.



src/dev/net/pktfifo.cc (line 80)


No need for fixed 16KB size here either. PacketFifo is only used to store 
"finalized" eth packets (i.e. no further data will be appended to the packets).


- Gabor Dozsa


On Nov. 21, 2016, 6:57 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3705/
> ---
> 
> (Updated Nov. 21, 2016, 6:57 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:008f04dcc085
> ---
> dev: Fix buffer length when unserializing an eth pkt
> Changeset 11701 only serialized the useful portion of of an ethernet packets'
> payload.  However, the device models expect each ethernet packet to contain
> a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
> field to EthPacketData so the original size of the packet buffer can always be
> unserialized.
> 
> Reported-by: Gabor Dozsa 
> 
> 
> Diffs
> -
> 
>   src/dev/net/dist_etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/dist_iface.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/pktfifo.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-21 Thread Michael LeBeane 

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

(Updated Nov. 21, 2016, 6:57 p.m.)


Review request for Default.


Changes
---

Fix unserialize to support checkpoints created before changeset 11701.


Repository: gem5


Description
---

Changeset 11705:008f04dcc085
---
dev: Fix buffer length when unserializing an eth pkt
Changeset 11701 only serialized the useful portion of of an ethernet packets'
payload.  However, the device models expect each ethernet packet to contain
a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
field to EthPacketData so the original size of the packet buffer can always be
unserialized.

Reported-by: Gabor Dozsa 


Diffs (updated)
-

  src/dev/net/dist_etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/dist_iface.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/etherlink.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/i8254xGBe.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/ns_gige.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/pktfifo.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/sinic.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 

Diff: http://reviews.gem5.org/r/3705/diff/


Testing
---


Thanks,

Michael LeBeane

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-17 Thread Gabor Dozsa 

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


The patch fixes my full system test which failed before. 

I think it needs a chekpoint upgrader though. Making bufLength optional is not 
enough to avoid the problem of not allocating (enough) buffer space for an 
ethernet packet when unserializing a legacy checkpoint.

- Gabor Dozsa


On Nov. 15, 2016, 9:22 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3705/
> ---
> 
> (Updated Nov. 15, 2016, 9:22 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:008f04dcc085
> ---
> dev: Fix buffer length when unserializing an eth pkt
> Changeset 11701 only serialized the useful portion of of an ethernet packets'
> payload.  However, the device models expect each ethernet packet to contain
> a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
> field to EthPacketData so the original size of the packet buffer can always be
> unserialized.
> 
> Reported-by: Gabor Dozsa 
> 
> 
> Diffs
> -
> 
>   src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-16 Thread Andreas Hansson 

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


do we need a checkpoint updater, or does the call to paramIn return 0 if the 
parameter is not populated?

- Andreas Hansson


On Nov. 15, 2016, 9:22 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3705/
> ---
> 
> (Updated Nov. 15, 2016, 9:22 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:008f04dcc085
> ---
> dev: Fix buffer length when unserializing an eth pkt
> Changeset 11701 only serialized the useful portion of of an ethernet packets'
> payload.  However, the device models expect each ethernet packet to contain
> a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
> field to EthPacketData so the original size of the packet buffer can always be
> unserialized.
> 
> Reported-by: Gabor Dozsa 
> 
> 
> Diffs
> -
> 
>   src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-16 Thread Brandon Potter 

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

Ship it!


Ship It!

- Brandon Potter


On Nov. 15, 2016, 9:22 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3705/
> ---
> 
> (Updated Nov. 15, 2016, 9:22 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:008f04dcc085
> ---
> dev: Fix buffer length when unserializing an eth pkt
> Changeset 11701 only serialized the useful portion of of an ethernet packets'
> payload.  However, the device models expect each ethernet packet to contain
> a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
> field to EthPacketData so the original size of the packet buffer can always be
> unserialized.
> 
> Reported-by: Gabor Dozsa 
> 
> 
> Diffs
> -
> 
>   src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

Re: [gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-16 Thread Tony Gutierrez 

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

Ship it!


Ship It!

- Tony Gutierrez


On Nov. 15, 2016, 1:22 p.m., Michael LeBeane wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviews.gem5.org/r/3705/
> ---
> 
> (Updated Nov. 15, 2016, 1:22 p.m.)
> 
> 
> Review request for Default.
> 
> 
> Repository: gem5
> 
> 
> Description
> ---
> 
> Changeset 11705:008f04dcc085
> ---
> dev: Fix buffer length when unserializing an eth pkt
> Changeset 11701 only serialized the useful portion of of an ethernet packets'
> payload.  However, the device models expect each ethernet packet to contain
> a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
> field to EthPacketData so the original size of the packet buffer can always be
> unserialized.
> 
> Reported-by: Gabor Dozsa 
> 
> 
> Diffs
> -
> 
>   src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
>   src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
> 
> Diff: http://reviews.gem5.org/r/3705/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael LeBeane
> 
>

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev

[gem5-dev] Review Request 3705: dev: Fix buffer length when unserializing an eth pkt

2016-11-15 Thread Michael LeBeane 

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

Review request for Default.


Repository: gem5


Description
---

Changeset 11705:008f04dcc085
---
dev: Fix buffer length when unserializing an eth pkt
Changeset 11701 only serialized the useful portion of of an ethernet packets'
payload.  However, the device models expect each ethernet packet to contain
a 16KB buffer, even if there is no data in it.  This patch adds a 'bufLength'
field to EthPacketData so the original size of the packet buffer can always be
unserialized.

Reported-by: Gabor Dozsa 


Diffs
-

  src/dev/net/etherpkt.hh c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 
  src/dev/net/etherpkt.cc c38fcdaa5fe508dbb18cc084e758ad0ce8e2e2f4 

Diff: http://reviews.gem5.org/r/3705/diff/


Testing
---


Thanks,

Michael LeBeane

___
gem5-dev mailing list
gem5-dev@gem5.org
http://m5sim.org/mailman/listinfo/gem5-dev