[Qemu-devel] [PATCH] ne2000: Drop ne2000_can_receive

[Qemu-devel] [PATCH] ne2000: Drop ne2000_can_receive

[Fam Zheng]

ne2000_receive already checks the same conditions and drops the packet
if it's not ready, removing the .can_receive callback avoids the
necessity to add explicit flushes when the conditions turn true (which
is required by the new semantics of .can_receive since 6e99c63
"net/socket: Drop net_socket_can_send").

Plus the "return 1" if E8390_STOP is also suspicious.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/net/ne2000-isa.c |  1 -
 hw/net/ne2000.c     | 10 ----------
 hw/net/ne2000.h     |  1 -
 3 files changed, 12 deletions(-)

diff --git a/hw/net/ne2000-isa.c b/hw/net/ne2000-isa.c
index 17e7199..18b0644 100644
--- a/hw/net/ne2000-isa.c
+++ b/hw/net/ne2000-isa.c
@@ -44,7 +44,6 @@ typedef struct ISANE2000State {
 static NetClientInfo net_ne2000_isa_info = {
     .type = NET_CLIENT_OPTIONS_KIND_NIC,
     .size = sizeof(NICState),
-    .can_receive = ne2000_can_receive,
     .receive = ne2000_receive,
 };
 
diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 3492db3..53c704a 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -165,15 +165,6 @@ static int ne2000_buffer_full(NE2000State *s)
     return 0;
 }
 
-int ne2000_can_receive(NetClientState *nc)
-{
-    NE2000State *s = qemu_get_nic_opaque(nc);
-
-    if (s->cmd & E8390_STOP)
-        return 1;
-    return !ne2000_buffer_full(s);
-}
-
 #define MIN_BUF_SIZE 60
 
 ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
@@ -705,7 +696,6 @@ void ne2000_setup_io(NE2000State *s, DeviceState *dev, unsigned size)
 static NetClientInfo net_ne2000_info = {
     .type = NET_CLIENT_OPTIONS_KIND_NIC,
     .size = sizeof(NICState),
-    .can_receive = ne2000_can_receive,
     .receive = ne2000_receive,
 };
 
diff --git a/hw/net/ne2000.h b/hw/net/ne2000.h
index e500306..d022b28 100644
--- a/hw/net/ne2000.h
+++ b/hw/net/ne2000.h
@@ -34,7 +34,6 @@ typedef struct NE2000State {
 void ne2000_setup_io(NE2000State *s, DeviceState *dev, unsigned size);
 extern const VMStateDescription vmstate_ne2000;
 void ne2000_reset(NE2000State *s);
-int ne2000_can_receive(NetClientState *nc);
 ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_);
 
 #endif
-- 
2.4.3

Re: [Qemu-devel] [PATCH] ne2000: Drop ne2000_can_receive

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]

On Wed, Jul 01, 2015 at 01:25:05PM +0800, Fam Zheng wrote:
> ne2000_receive already checks the same conditions and drops the packet
> if it's not ready, removing the .can_receive callback avoids the
> necessity to add explicit flushes when the conditions turn true (which
> is required by the new semantics of .can_receive since 6e99c63
> "net/socket: Drop net_socket_can_send").
> 
> Plus the "return 1" if E8390_STOP is also suspicious.

I think that is actually correct behavior.  It says we want to discard
packets instead of queuing them if the NIC is stopped.

This patch is incomplete.  Previously the NIC never woke up after
ne2000_buffer_full() transitioned false -> true.  Now the NIC never
queues packets anymore.

ne2000_receive() needs to be changed to:

  if (s->cmd & E8390_STOP) {
      return -1;
  }
  if (ne2000_buffer_full(s)) {
      return 0;
  }

This moves the old .can_receive() behavior into .receive().

Flush calls need to be added to each of the locations where
ne2000_buffer_full() transitions from true -> false.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH] ne2000: Drop ne2000_can_receive

[Stefan Hajnoczi]

On Wed, Jul 01, 2015 at 01:25:05PM +0800, Fam Zheng wrote:
> ne2000_receive already checks the same conditions and drops the packet
> if it's not ready, removing the .can_receive callback avoids the
> necessity to add explicit flushes when the conditions turn true (which
> is required by the new semantics of .can_receive since 6e99c63
> "net/socket: Drop net_socket_can_send").
> 
> Plus the "return 1" if E8390_STOP is also suspicious.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  hw/net/ne2000-isa.c |  1 -
>  hw/net/ne2000.c     | 10 ----------
>  hw/net/ne2000.h     |  1 -
>  3 files changed, 12 deletions(-)

Sorry, this was dropped and waiting to see how .can_receive() would turn
out for QEMU 2.4.

Thanks, applied to my net tree:
https://github.com/stefanha/qemu/commits/net

Stefan