diff mbox

(no subject)

Message ID 20150630071813.GB5599@ad.nay.redhat.com
State New
Headers show

Commit Message

Fam Zheng June 30, 2015, 7:18 a.m. UTC
On Tue, 06/30 00:49, Scott Feldman wrote:
> Hi Fam, Stefan,
> 
> I'm running a test with rocker device using UDP sockets connections
> and I'm seeing the socket s->read_poll stay disabled if the device
> receives a packet when the device's can_receive returns false.
> Receive is stuck after that; nothing ever re-enables s->read_poll.  I
> see the first packet queued on queue->packets and that's it.   No more
> receives.  If I modify the device to lie and always return
> can_receive=true, and drop the pkt in driver receive, then things work
> fine.
> 
> I think this patch broke can_receive semantics for net/socket.c:

Yes. The semantics now is if .can_receive returns false, the NIC needs to flush
the queue explicitly when the conditions in .can_receive become true, because
net/{socket,tap,...} no longer polls .can_receive().
> 
> commit 6e99c631f116221d169ea53953d91b8aa74d297a
> Author: Fam Zheng <famz@redhat.com>
> Date:   Thu Jun 4 14:45:16 2015 +0800
> 
>     net/socket: Drop net_socket_can_send
> 
> Anything jump out?
> 
> (In the test, rocker device is enabling the netdev port once the guest
> OS driver signals to enable the port based on STP process running on
> the guest.  The initial STP state is DISABLED, so the port is isolated
> from the network.  As STP algo progresses, the port is opened up and
> the netdev is enabled for Rx traffic).
> 
> -scott
> 

Does dropping .can_receive or forcing 1 work for you? Or maybe something like
this:


---

Fam

Comments

Scott Feldman June 30, 2015, 2:22 p.m. UTC | #1
On Tue, Jun 30, 2015 at 3:18 AM, Fam Zheng <famz@redhat.com> wrote:
> On Tue, 06/30 00:49, Scott Feldman wrote:
>> Hi Fam, Stefan,
>>
>> I'm running a test with rocker device using UDP sockets connections
>> and I'm seeing the socket s->read_poll stay disabled if the device
>> receives a packet when the device's can_receive returns false.
>> Receive is stuck after that; nothing ever re-enables s->read_poll.  I
>> see the first packet queued on queue->packets and that's it.   No more
>> receives.  If I modify the device to lie and always return
>> can_receive=true, and drop the pkt in driver receive, then things work
>> fine.
>>
>> I think this patch broke can_receive semantics for net/socket.c:
>
> Yes. The semantics now is if .can_receive returns false, the NIC needs to flush
> the queue explicitly when the conditions in .can_receive become true, because
> net/{socket,tap,...} no longer polls .can_receive().

Ah, that makes sense.

>> commit 6e99c631f116221d169ea53953d91b8aa74d297a
>> Author: Fam Zheng <famz@redhat.com>
>> Date:   Thu Jun 4 14:45:16 2015 +0800
>>
>>     net/socket: Drop net_socket_can_send
>>
>> Anything jump out?
>>
>> (In the test, rocker device is enabling the netdev port once the guest
>> OS driver signals to enable the port based on STP process running on
>> the guest.  The initial STP state is DISABLED, so the port is isolated
>> from the network.  As STP algo progresses, the port is opened up and
>> the netdev is enabled for Rx traffic).
>>
>> -scott
>>
>
> Does dropping .can_receive or forcing 1 work for you? Or maybe something like
> this:

Dropping .can_receive works.  Actually, we really don't want any pkts
queued when the device port is disabled, otherwise when the port
transitions to enabled, we'll receive stale network pkts.  This would
be bad for a switch device.  So I think this is a happy outcome:
removing .can_receive prevents pkt queuing on the backend, and the
.receive handler can eat the pkt if the port is disabled.  I'll send a
rocker patch to fix this.  Thanks Fam.

-scott
diff mbox

Patch

diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index d8d934c..3209ccd 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -203,6 +203,7 @@  void fp_port_enable(FpPort *port)
     fp_port_set_link(port, true);
     port->enabled = true;
     DPRINTF("port %d enabled\n", port->index);
+    qemu_flush_queued_packets(qemu_get_queue(port->nic));
 }
 
 void fp_port_disable(FpPort *port)