Re: [Qemu-devel] [PATCH] rocker: don't queue receive pkts when port is disabled

[Scott Feldman]

On Tue, Jun 30, 2015 at 6:47 PM, Fam Zheng <address@hidden> wrote:
> On Tue, 06/30 07:41, address@hidden wrote:
>> From: Scott Feldman <address@hidden>
>>
>> Commit 6e99c63 ("net/socket: Drop net_socket_can_send") changed the
>> semantics around .can_receive for sockets to now require the device to
>> flush queued pkts when transitioning to a .can_receive=true state.  Rocker
>> device was not flushing the queue on .can_receive=true transition, so the
>> receiver was stuck.
>>
>> But, turns out we really don't want any queuing at all on the port when the
>> port is disabled, otherwise when the port transitions to enabled, we'd
>> receive and forward stale pkts that really should have been dropped.  So,
>> let's remove .can_receive so avoid queuing and drop the pkt in .receive if
>> the port is disabled.
>>
>> Signed-off-by: Scott Feldman <address@hidden>
>> ---
>>  hw/net/rocker/rocker_fp.c |   18 ++++++++++--------
>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
>> index d8d934c..ac53c13 100644
>> --- a/hw/net/rocker/rocker_fp.c
>> +++ b/hw/net/rocker/rocker_fp.c
>> @@ -125,18 +125,21 @@ int fp_port_eg(FpPort *port, const struct iovec *iov, 
>> int iovcnt)
>>      return ROCKER_OK;
>>  }
>>
>> -static int fp_port_can_receive(NetClientState *nc)
>> -{
>> -    FpPort *port = qemu_get_nic_opaque(nc);
>> -
>> -    return port->enabled;
>> -}
>> -
>>  static ssize_t fp_port_receive_iov(NetClientState *nc, const struct iovec 
>> *iov,
>>                                     int iovcnt)
>>  {
>>      FpPort *port = qemu_get_nic_opaque(nc);
>>
>> +    /* If the port is disabled, we want to drop this pkt
>> +     * now rather than queing it for later.  We don't want
>> +     * any stale pkts getting into the device when the port
>> +     * transitions to enabled.
>> +     */
>> +
>> +    if (!port->enabled) {
>> +        return iov_size(iov, iovcnt);
>
> Other devices return -1 when dropping packets, for example see
> e1000_receive_iov or virtio_net_receive. Maybe return -1 here too?

I posted v2 with the change.  Perhaps we should have:

#define QEMU_SEND_PACKET_DROPPED -1

Or something like that...

-scott