Re: [Qemu-devel] [PATCH arm-devs v1 10/13] net/cadence_gem: Fix small packet FCS stripping

[Peter Maydell]

On 2 December 2013 07:14, Peter Crosthwaite
<address@hidden> 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 <address@hidden>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
>  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