| Message ID | c514492739a098b0cd94fc370071b741dd1a055d.1385967754.git.peter.crosthwaite@xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 2 December 2013 07:14, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > The minimum packet size is 64, however this is before FCS stripping > occurs. So when FCS stripping the minimum packet size is 60. Fix. > > Reported-by: Deepika Dhamija <deepika@xilinx.com> > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > hw/net/cadence_gem.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c > index eb0fa95..babd39d 100644 > --- a/hw/net/cadence_gem.c > +++ b/hw/net/cadence_gem.c > @@ -674,6 +674,14 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) > rxbuf_offset = (s->regs[GEM_NWCFG] & GEM_NWCFG_BUFF_OFST_M) >> > GEM_NWCFG_BUFF_OFST_S; > > + /* Pad to minimum length. Assume FCS field is stripped, logic > + * below will increment it to the real minimum of 64 when > + * not FCS stripping > + */ > + if (size < 60) { > + size = 60; > + } > + > /* The configure size of each receive buffer. Determines how many > * buffers needed to hold this packet. > */ > @@ -707,11 +715,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) > size += 4; > } > > - /* Pad to minimum length */ > - if (size < 64) { > - size = 64; > - } > - This change moves the padding of size from below the point where we initialize bytes_to_copy to above it, so now bytes_to_copy will get the padded value rather than the unpadded value. If this is deliberate it should probably be spelled out somewhere. (See also comments on earlier patch.) thanks -- PMM
On Mon, Dec 2, 2013 at 10:26 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 2 December 2013 07:14, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> The minimum packet size is 64, however this is before FCS stripping >> occurs. So when FCS stripping the minimum packet size is 60. Fix. >> >> Reported-by: Deepika Dhamija <deepika@xilinx.com> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> >> hw/net/cadence_gem.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c >> index eb0fa95..babd39d 100644 >> --- a/hw/net/cadence_gem.c >> +++ b/hw/net/cadence_gem.c >> @@ -674,6 +674,14 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) >> rxbuf_offset = (s->regs[GEM_NWCFG] & GEM_NWCFG_BUFF_OFST_M) >> >> GEM_NWCFG_BUFF_OFST_S; >> >> + /* Pad to minimum length. Assume FCS field is stripped, logic >> + * below will increment it to the real minimum of 64 when >> + * not FCS stripping >> + */ >> + if (size < 60) { >> + size = 60; >> + } >> + >> /* The configure size of each receive buffer. Determines how many >> * buffers needed to hold this packet. >> */ >> @@ -707,11 +715,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) >> size += 4; >> } >> >> - /* Pad to minimum length */ >> - if (size < 64) { >> - size = 64; >> - } >> - > > This change moves the padding of size from below the point where > we initialize bytes_to_copy to above it, so now bytes_to_copy will > get the padded value rather than the unpadded value. If this is deliberate > it should probably be spelled out somewhere. (See also comments on > earlier patch.) > So I cant see a good reason for that change. Reverted - just moved the added hunk to below the bytes_to_copy =. Stress tests and linux tests still pass. Regards, Peter > thanks > -- PMM >
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c index eb0fa95..babd39d 100644 --- a/hw/net/cadence_gem.c +++ b/hw/net/cadence_gem.c @@ -674,6 +674,14 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) rxbuf_offset = (s->regs[GEM_NWCFG] & GEM_NWCFG_BUFF_OFST_M) >> GEM_NWCFG_BUFF_OFST_S; + /* Pad to minimum length. Assume FCS field is stripped, logic + * below will increment it to the real minimum of 64 when + * not FCS stripping + */ + if (size < 60) { + size = 60; + } + /* The configure size of each receive buffer. Determines how many * buffers needed to hold this packet. */ @@ -707,11 +715,6 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size) size += 4; } - /* Pad to minimum length */ - if (size < 64) { - size = 64; - } - DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size); while (bytes_to_copy) {
The minimum packet size is 64, however this is before FCS stripping occurs. So when FCS stripping the minimum packet size is 60. Fix. Reported-by: Deepika Dhamija <deepika@xilinx.com> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- hw/net/cadence_gem.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)