| Message ID | 1435728305-5064-1-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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
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(-)