On Tue, Jan 14, 2014 at 3:19 PM, Stefan Hajnoczi <stefa...@redhat.com> wrote:
> On Mon, Jan 13, 2014 at 11:16:37PM +1000, Peter Crosthwaite wrote:
>> On Mon, Jan 13, 2014 at 11:15 PM, Peter Crosthwaite
>> <peter.crosthwa...@xilinx.com> wrote:
>> > On Sat, Jan 11, 2014 at 8:13 PM, Beniamino Galvani <b.galv...@gmail.com>
>> > wrote:
>> >> + } else {
...
>> >> +static ssize_t aw_emac_receive(NetClientState *nc, const uint8_t *buf,
>> >> + size_t size)
>> >> +{
>> >> + AwEmacState *s = qemu_get_nic_opaque(nc);
>> >> + AwEmacFifo *fifo;
>> >> + uint32_t crc, *word;
>> >> + uint8_t *dest;
>> >> +
>> >> + if (s->num_rx >= NUM_RX_FIFOS) {
>> >
>> > Seems inconsistent that you check for fifo vacancy but you dont check
>> > for s->ctl & EMAC_CTL_RX_EN (as above in can_recieve). Then again,
>> > both conditions should be guarded by can_recieve, so I wonder whether
>> > you can just drop this.
>> >
>> > Stefan, if you are reading, can the recieve function rely on
>> > can_recieve success and drop such checks?
>
> It is safer to perform full checks in .receive() since
> qemu_net_queue_flush() calls .receive() repeatedly without
> .can_receive(). This means especially if the state can change between
> calls, you need to do a full check.
>
Ok, then original implementation is all good. No change required for
this issue. Sry for the noise.
Regards,
Peter
