* [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs
@ 2015-07-15 10:19 Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 01/12] xgmac: Drop packets with eth_can_rx is false Fam Zheng
` (13 more replies)
0 siblings, 14 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
v2: Don't use BH in patches 6 and 12. [Jason]
Add Stefan's reviewed-by in all others.
Drop trace point in patch 8 and move function body upward. [Michael]
Since a90a742 "tap: Drop tap_can_send", all nics that returns false from
.can_receive() are required to explicitly flush the incoming queue when the
status of it is changing back to true, otherwise the backend will sop
processing more rx packets.
The purpose of this callback is to tell the peer backend (tap, socket, etc)
"hold on until guest consumes old data because my buffer is not ready". More
often than not NICs also do this when driver deactivated the card or disabled
rx, causing the packets being unnessarily queued, where they should actualy be
dropped.
This series adds such missing qemu_flush_queued_packets calls for all NICs, and
drops such unnecessary conditions in .can_receive(), so that NICs now:
- return false from .can_receive when guest buffers are busy.
- call qemu_flush_queued_packets when buffers are available again.
- return -1 from .receive when rx is not enabled.
e1000, ne2000, rocker and vmxnet3 are not included because they're fixed by
other patches on the list and applied to Stefan's tree.
virtio-net is covered by another series:
https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07377.html ^U
http://patchwork.ozlabs.org/patch/495379/ (the subject should have v2, my bad.)
All other NICs are okay, as they already do the flush on the state transition
points.
Please review.
Fam Zheng (12):
xgmac: Drop packets with eth_can_rx is false.
pcnet: Drop pcnet_can_receive
eepro100: Drop nic_can_receive
usbnet: Drop usbnet_can_receive
etsec: Move etsec_can_receive into etsec_receive
etsec: Flush queue when rx buffer is consumed
mcf_fec: Drop mcf_fec_can_receive
milkymist-minimac2: Flush queued packets when link comes up
mipsnet: Flush queued packets when receiving is enabled
stellaris_enet: Flush queued packets when read done
dp8393x: Flush packets when link comes up
axienet: Flush queued packets when rx is done
hw/net/dp8393x.c | 8 ++++++++
hw/net/eepro100.c | 11 -----------
hw/net/fsl_etsec/etsec.c | 20 ++++++++++----------
hw/net/fsl_etsec/etsec.h | 4 +++-
hw/net/fsl_etsec/rings.c | 17 +++++++++++------
hw/net/lance.c | 1 -
hw/net/mcf_fec.c | 9 +--------
hw/net/milkymist-minimac2.c | 33 ++++++++++++++++-----------------
hw/net/mipsnet.c | 9 +++++++--
hw/net/pcnet-pci.c | 1 -
hw/net/pcnet.c | 9 ---------
hw/net/pcnet.h | 1 -
hw/net/stellaris_enet.c | 14 +++++---------
hw/net/xgmac.c | 8 ++++----
hw/net/xilinx_axienet.c | 17 +++++++++++++----
hw/usb/dev-network.c | 20 ++++----------------
trace-events | 1 -
17 files changed, 82 insertions(+), 101 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 01/12] xgmac: Drop packets with eth_can_rx is false.
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 02/12] pcnet: Drop pcnet_can_receive Fam Zheng
` (12 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/xgmac.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/xgmac.c b/hw/net/xgmac.c
index b068f3a..15fb681 100644
--- a/hw/net/xgmac.c
+++ b/hw/net/xgmac.c
@@ -312,10 +312,8 @@ static const MemoryRegionOps enet_mem_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static int eth_can_rx(NetClientState *nc)
+static int eth_can_rx(XgmacState *s)
{
- XgmacState *s = qemu_get_nic_opaque(nc);
-
/* RX enabled? */
return s->regs[DMA_CONTROL] & DMA_CONTROL_SR;
}
@@ -329,6 +327,9 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
struct desc bd;
ssize_t ret;
+ if (!eth_can_rx(s)) {
+ return -1;
+ }
unicast = ~buf[0] & 0x1;
broadcast = memcmp(buf, sa_bcast, 6) == 0;
multicast = !unicast && !broadcast;
@@ -371,7 +372,6 @@ out:
static NetClientInfo net_xgmac_enet_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = eth_can_rx,
.receive = eth_rx,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 02/12] pcnet: Drop pcnet_can_receive
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 01/12] xgmac: Drop packets with eth_can_rx is false Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 03/12] eepro100: Drop nic_can_receive Fam Zheng
` (11 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
pcnet_receive already checks the conditions and drop packets if false.
Due to the new semantics since 6e99c63 ("net/socket: Drop
net_socket_can_send"), having .can_receive returning 0 requires us to
explicitly flush the queued packets when the conditions are becoming
true, but queuing the packets when guest driver is not ready doesn't
make much sense.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/lance.c | 1 -
hw/net/pcnet-pci.c | 1 -
hw/net/pcnet.c | 9 ---------
hw/net/pcnet.h | 1 -
4 files changed, 12 deletions(-)
diff --git a/hw/net/lance.c b/hw/net/lance.c
index 4baa016..780b39d 100644
--- a/hw/net/lance.c
+++ b/hw/net/lance.c
@@ -94,7 +94,6 @@ static const MemoryRegionOps lance_mem_ops = {
static NetClientInfo net_lance_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = pcnet_can_receive,
.receive = pcnet_receive,
.link_status_changed = pcnet_set_link_status,
};
diff --git a/hw/net/pcnet-pci.c b/hw/net/pcnet-pci.c
index 8305d1b..b4d60b8 100644
--- a/hw/net/pcnet-pci.c
+++ b/hw/net/pcnet-pci.c
@@ -273,7 +273,6 @@ static void pci_pcnet_uninit(PCIDevice *dev)
static NetClientInfo net_pci_pcnet_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = pcnet_can_receive,
.receive = pcnet_receive,
.link_status_changed = pcnet_set_link_status,
};
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 68b9981..3437376 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -995,15 +995,6 @@ static int pcnet_tdte_poll(PCNetState *s)
return !!(CSR_CXST(s) & 0x8000);
}
-int pcnet_can_receive(NetClientState *nc)
-{
- PCNetState *s = qemu_get_nic_opaque(nc);
- if (CSR_STOP(s) || CSR_SPND(s))
- return 0;
-
- return sizeof(s->buffer)-16;
-}
-
#define MIN_BUF_SIZE 60
ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
diff --git a/hw/net/pcnet.h b/hw/net/pcnet.h
index 79c4c84..dec8de8 100644
--- a/hw/net/pcnet.h
+++ b/hw/net/pcnet.h
@@ -60,7 +60,6 @@ uint32_t pcnet_ioport_readw(void *opaque, uint32_t addr);
void pcnet_ioport_writel(void *opaque, uint32_t addr, uint32_t val);
uint32_t pcnet_ioport_readl(void *opaque, uint32_t addr);
uint32_t pcnet_bcr_readw(PCNetState *s, uint32_t rap);
-int pcnet_can_receive(NetClientState *nc);
ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_);
void pcnet_set_link_status(NetClientState *nc);
void pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info);
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 03/12] eepro100: Drop nic_can_receive
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 01/12] xgmac: Drop packets with eth_can_rx is false Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 02/12] pcnet: Drop pcnet_can_receive Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 04/12] usbnet: Drop usbnet_can_receive Fam Zheng
` (10 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
nic_receive already checks the conditions and drop packets if false.
Due to the new semantics since 6e99c63 ("net/socket: Drop
net_socket_can_send"), having .can_receive returning 0 requires us to
explicitly flush the queued packets when the conditions are becoming
true, but queuing the packets when guest driver is not ready doesn't
make much sense.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/eepro100.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index c374c1a..60333b7 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1617,16 +1617,6 @@ static const MemoryRegionOps eepro100_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static int nic_can_receive(NetClientState *nc)
-{
- EEPRO100State *s = qemu_get_nic_opaque(nc);
- TRACE(RXTX, logout("%p\n", s));
- return get_ru_state(s) == ru_ready;
-#if 0
- return !eepro100_buffer_full(s);
-#endif
-}
-
static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
{
/* TODO:
@@ -1844,7 +1834,6 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
static NetClientInfo net_eepro100_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = nic_can_receive,
.receive = nic_receive,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 04/12] usbnet: Drop usbnet_can_receive
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (2 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 03/12] eepro100: Drop nic_can_receive Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 05/12] etsec: Move etsec_can_receive into etsec_receive Fam Zheng
` (9 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
usbnet_receive already drops packet if rndis_state is not
RNDIS_DATA_INITIALIZED, and queues packet if in buffer is not available.
The only difference is s->dev.config but that is similar to rndis_state.
Drop usbnet_can_receive and move these checks to usbnet_receive, so that
we don't need to explicitly flush the queue when s->dev.config changes
value.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/usb/dev-network.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 5eeb4c6..7800cee 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1268,6 +1268,10 @@ static ssize_t usbnet_receive(NetClientState *nc, const uint8_t *buf, size_t siz
uint8_t *in_buf = s->in_buf;
size_t total_size = size;
+ if (!s->dev.config) {
+ return -1;
+ }
+
if (is_rndis(s)) {
if (s->rndis_state != RNDIS_DATA_INITIALIZED) {
return -1;
@@ -1309,21 +1313,6 @@ static ssize_t usbnet_receive(NetClientState *nc, const uint8_t *buf, size_t siz
return size;
}
-static int usbnet_can_receive(NetClientState *nc)
-{
- USBNetState *s = qemu_get_nic_opaque(nc);
-
- if (!s->dev.config) {
- return 0;
- }
-
- if (is_rndis(s) && s->rndis_state != RNDIS_DATA_INITIALIZED) {
- return 1;
- }
-
- return !s->in_len;
-}
-
static void usbnet_cleanup(NetClientState *nc)
{
USBNetState *s = qemu_get_nic_opaque(nc);
@@ -1343,7 +1332,6 @@ static void usb_net_handle_destroy(USBDevice *dev)
static NetClientInfo net_usbnet_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = usbnet_can_receive,
.receive = usbnet_receive,
.cleanup = usbnet_cleanup,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 05/12] etsec: Move etsec_can_receive into etsec_receive
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (3 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 04/12] usbnet: Drop usbnet_can_receive Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 06/12] etsec: Flush queue when rx buffer is consumed Fam Zheng
` (8 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
When etsec_reset returns 0, peer would queue the packet as if
.can_receive returns false. Drop etsec_can_receive and let etsec_receive
carry the semantics.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/fsl_etsec/etsec.c | 11 +----------
hw/net/fsl_etsec/etsec.h | 2 +-
hw/net/fsl_etsec/rings.c | 14 ++++++++------
3 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index c57365f..f5170ae 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -338,13 +338,6 @@ static void etsec_reset(DeviceState *d)
MII_SR_100X_FD_CAPS | MII_SR_100T4_CAPS;
}
-static int etsec_can_receive(NetClientState *nc)
-{
- eTSEC *etsec = qemu_get_nic_opaque(nc);
-
- return etsec->rx_buffer_len == 0;
-}
-
static ssize_t etsec_receive(NetClientState *nc,
const uint8_t *buf,
size_t size)
@@ -355,8 +348,7 @@ static ssize_t etsec_receive(NetClientState *nc,
fprintf(stderr, "%s receive size:%d\n", etsec->nic->nc.name, size);
qemu_hexdump(buf, stderr, "", size);
#endif
- etsec_rx_ring_write(etsec, buf, size);
- return size;
+ return etsec_rx_ring_write(etsec, buf, size);
}
@@ -370,7 +362,6 @@ static void etsec_set_link_status(NetClientState *nc)
static NetClientInfo net_etsec_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = etsec_can_receive,
.receive = etsec_receive,
.link_status_changed = etsec_set_link_status,
};
diff --git a/hw/net/fsl_etsec/etsec.h b/hw/net/fsl_etsec/etsec.h
index 78d2c57..fc41773 100644
--- a/hw/net/fsl_etsec/etsec.h
+++ b/hw/net/fsl_etsec/etsec.h
@@ -162,7 +162,7 @@ DeviceState *etsec_create(hwaddr base,
void etsec_walk_tx_ring(eTSEC *etsec, int ring_nbr);
void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr);
-void etsec_rx_ring_write(eTSEC *etsec, const uint8_t *buf, size_t size);
+ssize_t etsec_rx_ring_write(eTSEC *etsec, const uint8_t *buf, size_t size);
void etsec_write_miim(eTSEC *etsec,
eTSEC_Register *reg,
diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index d4a494f..a11280b 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -481,40 +481,42 @@ static void rx_init_frame(eTSEC *etsec, const uint8_t *buf, size_t size)
etsec->rx_buffer_len, etsec->rx_padding);
}
-void etsec_rx_ring_write(eTSEC *etsec, const uint8_t *buf, size_t size)
+ssize_t etsec_rx_ring_write(eTSEC *etsec, const uint8_t *buf, size_t size)
{
int ring_nbr = 0; /* Always use ring0 (no filer) */
if (etsec->rx_buffer_len != 0) {
RING_DEBUG("%s: We can't receive now,"
" a buffer is already in the pipe\n", __func__);
- return;
+ return 0;
}
if (etsec->regs[RSTAT].value & 1 << (23 - ring_nbr)) {
RING_DEBUG("%s: The ring is halted\n", __func__);
- return;
+ return -1;
}
if (etsec->regs[DMACTRL].value & DMACTRL_GRS) {
RING_DEBUG("%s: Graceful receive stop\n", __func__);
- return;
+ return -1;
}
if (!(etsec->regs[MACCFG1].value & MACCFG1_RX_EN)) {
RING_DEBUG("%s: MAC Receive not enabled\n", __func__);
- return;
+ return -1;
}
if ((etsec->regs[RCTRL].value & RCTRL_RSF) && (size < 60)) {
/* CRC is not in the packet yet, so short frame is below 60 bytes */
RING_DEBUG("%s: Drop short frame\n", __func__);
- return;
+ return -1;
}
rx_init_frame(etsec, buf, size);
etsec_walk_rx_ring(etsec, ring_nbr);
+
+ return size;
}
void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr)
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 06/12] etsec: Flush queue when rx buffer is consumed
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (4 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 05/12] etsec: Move etsec_can_receive into etsec_receive Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 07/12] mcf_fec: Drop mcf_fec_can_receive Fam Zheng
` (7 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
The BH will be scheduled when etsec->rx_buffer_len is becoming 0, which
is the condition of queuing.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/net/fsl_etsec/etsec.c | 11 ++++++++++-
hw/net/fsl_etsec/etsec.h | 2 ++
hw/net/fsl_etsec/rings.c | 3 +++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index f5170ae..0f5cf44 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -342,13 +342,22 @@ static ssize_t etsec_receive(NetClientState *nc,
const uint8_t *buf,
size_t size)
{
+ ssize_t ret;
eTSEC *etsec = qemu_get_nic_opaque(nc);
#if defined(HEX_DUMP)
fprintf(stderr, "%s receive size:%d\n", etsec->nic->nc.name, size);
qemu_hexdump(buf, stderr, "", size);
#endif
- return etsec_rx_ring_write(etsec, buf, size);
+ /* Flush is unnecessary as are already in receiving path */
+ etsec->need_flush = false;
+ ret = etsec_rx_ring_write(etsec, buf, size);
+ if (ret == 0) {
+ /* The packet will be queued, let's flush it when buffer is avilable
+ * again. */
+ etsec->need_flush = true;
+ }
+ return ret;
}
diff --git a/hw/net/fsl_etsec/etsec.h b/hw/net/fsl_etsec/etsec.h
index fc41773..e7dc0a4 100644
--- a/hw/net/fsl_etsec/etsec.h
+++ b/hw/net/fsl_etsec/etsec.h
@@ -144,6 +144,8 @@ typedef struct eTSEC {
QEMUBH *bh;
struct ptimer_state *ptimer;
+ /* Whether we should flush the rx queue when buffer becomes available. */
+ bool need_flush;
} eTSEC;
#define TYPE_ETSEC_COMMON "eTSEC"
diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index a11280b..68e7b6d 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -646,6 +646,9 @@ void etsec_walk_rx_ring(eTSEC *etsec, int ring_nbr)
} else {
etsec->rx_buffer_len = 0;
etsec->rx_buffer = NULL;
+ if (etsec->need_flush) {
+ qemu_flush_queued_packets(qemu_get_queue(etsec->nic));
+ }
}
RING_DEBUG("eTSEC End of ring_write: remaining_data:%zu\n", remaining_data);
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 07/12] mcf_fec: Drop mcf_fec_can_receive
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (5 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 06/12] etsec: Flush queue when rx buffer is consumed Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 08/12] milkymist-minimac2: Flush queued packets when link comes up Fam Zheng
` (6 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
The semantics of .can_receive requires us to flush the queue explicitly
when s->rx_enabled becomes true after it returns 0, but the packet being
queued is not meaningful since the guest hasn't activated the card.
Let's just drop the packet in this case.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/mcf_fec.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c
index 0255612..0b17c1f 100644
--- a/hw/net/mcf_fec.c
+++ b/hw/net/mcf_fec.c
@@ -351,12 +351,6 @@ static void mcf_fec_write(void *opaque, hwaddr addr,
mcf_fec_update(s);
}
-static int mcf_fec_can_receive(NetClientState *nc)
-{
- mcf_fec_state *s = qemu_get_nic_opaque(nc);
- return s->rx_enabled;
-}
-
static ssize_t mcf_fec_receive(NetClientState *nc, const uint8_t *buf, size_t size)
{
mcf_fec_state *s = qemu_get_nic_opaque(nc);
@@ -370,7 +364,7 @@ static ssize_t mcf_fec_receive(NetClientState *nc, const uint8_t *buf, size_t si
DPRINTF("do_rx len %d\n", size);
if (!s->rx_enabled) {
- fprintf(stderr, "mcf_fec_receive: Unexpected packet\n");
+ return -1;
}
/* 4 bytes for the CRC. */
size += 4;
@@ -442,7 +436,6 @@ static const MemoryRegionOps mcf_fec_ops = {
static NetClientInfo net_mcf_fec_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = mcf_fec_can_receive,
.receive = mcf_fec_receive,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 08/12] milkymist-minimac2: Flush queued packets when link comes up
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (6 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 07/12] mcf_fec: Drop mcf_fec_can_receive Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 09/12] mipsnet: Flush queued packets when receiving is enabled Fam Zheng
` (5 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
Drop .can_receive and move the semantics into minimac2_rx, by returning
0.
That is once minimac2_rx returns 0, incoming packets will be queued
until the queue is explicitly flushed. We do this when s->regs[R_STATE0]
or s->regs[R_STATE1] is changed in minimac2_write.
Also drop the unused trace point.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/net/milkymist-minimac2.c | 33 ++++++++++++++++-----------------
trace-events | 1 -
2 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/hw/net/milkymist-minimac2.c b/hw/net/milkymist-minimac2.c
index f06afaa..5d1cf08 100644
--- a/hw/net/milkymist-minimac2.c
+++ b/hw/net/milkymist-minimac2.c
@@ -303,8 +303,7 @@ static ssize_t minimac2_rx(NetClientState *nc, const uint8_t *buf, size_t size)
r_state = R_STATE1;
rx_buf = s->rx1_buf;
} else {
- trace_milkymist_minimac2_drop_rx_frame(buf);
- return size;
+ return 0;
}
/* assemble frame */
@@ -354,6 +353,18 @@ minimac2_read(void *opaque, hwaddr addr, unsigned size)
return r;
}
+static int minimac2_can_rx(MilkymistMinimac2State *s)
+{
+ if (s->regs[R_STATE0] == STATE_LOADED) {
+ return 1;
+ }
+ if (s->regs[R_STATE1] == STATE_LOADED) {
+ return 1;
+ }
+
+ return 0;
+}
+
static void
minimac2_write(void *opaque, hwaddr addr, uint64_t value,
unsigned size)
@@ -387,6 +398,9 @@ minimac2_write(void *opaque, hwaddr addr, uint64_t value,
case R_STATE1:
s->regs[addr] = value;
update_rx_interrupt(s);
+ if (minimac2_can_rx(s)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
break;
case R_SETUP:
case R_COUNT0:
@@ -411,20 +425,6 @@ static const MemoryRegionOps minimac2_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static int minimac2_can_rx(NetClientState *nc)
-{
- MilkymistMinimac2State *s = qemu_get_nic_opaque(nc);
-
- if (s->regs[R_STATE0] == STATE_LOADED) {
- return 1;
- }
- if (s->regs[R_STATE1] == STATE_LOADED) {
- return 1;
- }
-
- return 0;
-}
-
static void milkymist_minimac2_reset(DeviceState *d)
{
MilkymistMinimac2State *s = MILKYMIST_MINIMAC2(d);
@@ -445,7 +445,6 @@ static void milkymist_minimac2_reset(DeviceState *d)
static NetClientInfo net_milkymist_minimac2_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = minimac2_can_rx,
.receive = minimac2_rx,
};
diff --git a/trace-events b/trace-events
index 2d395c5..94bf3bb 100644
--- a/trace-events
+++ b/trace-events
@@ -828,7 +828,6 @@ milkymist_minimac2_mdio_write(uint8_t phy_addr, uint8_t addr, uint16_t value) "p
milkymist_minimac2_mdio_read(uint8_t phy_addr, uint8_t addr, uint16_t value) "phy_addr %02x addr %02x value %04x"
milkymist_minimac2_tx_frame(uint32_t length) "length %u"
milkymist_minimac2_rx_frame(const void *buf, uint32_t length) "buf %p length %u"
-milkymist_minimac2_drop_rx_frame(const void *buf) "buf %p"
milkymist_minimac2_rx_transfer(const void *buf, uint32_t length) "buf %p length %d"
milkymist_minimac2_raise_irq_rx(void) "Raise IRQ RX"
milkymist_minimac2_lower_irq_rx(void) "Lower IRQ RX"
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 09/12] mipsnet: Flush queued packets when receiving is enabled
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (7 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 08/12] milkymist-minimac2: Flush queued packets when link comes up Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 10/12] stellaris_enet: Flush queued packets when read done Fam Zheng
` (4 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
Drop .can_receive and move the semantics to mipsnet_receive, by
returning 0.
After 0 is returned, we must flush the queue explicitly to restart it:
Call qemu_flush_queued_packets when s->busy or s->rx_count is being
updated.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/mipsnet.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c
index c813e0c..f261011 100644
--- a/hw/net/mipsnet.c
+++ b/hw/net/mipsnet.c
@@ -80,7 +80,7 @@ static ssize_t mipsnet_receive(NetClientState *nc, const uint8_t *buf, size_t si
trace_mipsnet_receive(size);
if (!mipsnet_can_receive(nc))
- return -1;
+ return 0;
s->busy = 1;
@@ -134,6 +134,9 @@ static uint64_t mipsnet_ioport_read(void *opaque, hwaddr addr,
if (s->rx_count) {
s->rx_count--;
ret = s->rx_buffer[s->rx_read++];
+ if (mipsnet_can_receive(s->nic->ncs)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
}
break;
/* Reads as zero. */
@@ -170,6 +173,9 @@ static void mipsnet_ioport_write(void *opaque, hwaddr addr,
}
s->busy = !!s->intctl;
mipsnet_update_irq(s);
+ if (mipsnet_can_receive(s->nic->ncs)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
break;
case MIPSNET_TX_DATA_BUFFER:
s->tx_buffer[s->tx_written++] = val;
@@ -214,7 +220,6 @@ static const VMStateDescription vmstate_mipsnet = {
static NetClientInfo net_mipsnet_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = mipsnet_can_receive,
.receive = mipsnet_receive,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 10/12] stellaris_enet: Flush queued packets when read done
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (8 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 09/12] mipsnet: Flush queued packets when receiving is enabled Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 11/12] dp8393x: Flush packets when link comes up Fam Zheng
` (3 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
If s->np reaches 31, the queue will be disabled by peer when it sees
stellaris_enet_can_receive() returns false, until we explicitly flushes
it which notifies the peer. Do this when guest is done reading all
existing data.
Move the semantics to stellaris_enet_receive, by returning 0 when the
buffer is full, so that new packets will be queued. In
stellaris_enet_read, flush and restart the queue when guest has done
reading.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/stellaris_enet.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/hw/net/stellaris_enet.c b/hw/net/stellaris_enet.c
index 278a654..21a4773 100644
--- a/hw/net/stellaris_enet.c
+++ b/hw/net/stellaris_enet.c
@@ -228,8 +228,7 @@ static ssize_t stellaris_enet_receive(NetClientState *nc, const uint8_t *buf, si
if ((s->rctl & SE_RCTL_RXEN) == 0)
return -1;
if (s->np >= 31) {
- DPRINTF("Packet dropped\n");
- return -1;
+ return 0;
}
DPRINTF("Received packet len=%zu\n", size);
@@ -260,13 +259,8 @@ static ssize_t stellaris_enet_receive(NetClientState *nc, const uint8_t *buf, si
return size;
}
-static int stellaris_enet_can_receive(NetClientState *nc)
+static int stellaris_enet_can_receive(stellaris_enet_state *s)
{
- stellaris_enet_state *s = qemu_get_nic_opaque(nc);
-
- if ((s->rctl & SE_RCTL_RXEN) == 0)
- return 1;
-
return (s->np < 31);
}
@@ -307,6 +301,9 @@ static uint64_t stellaris_enet_read(void *opaque, hwaddr offset,
s->next_packet = 0;
s->np--;
DPRINTF("RX done np=%d\n", s->np);
+ if (!s->np && stellaris_enet_can_receive(s)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
}
return val;
}
@@ -454,7 +451,6 @@ static void stellaris_enet_reset(stellaris_enet_state *s)
static NetClientInfo net_stellaris_enet_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = stellaris_enet_can_receive,
.receive = stellaris_enet_receive,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 11/12] dp8393x: Flush packets when link comes up
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (9 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 10/12] stellaris_enet: Flush queued packets when read done Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done Fam Zheng
` (2 subsequent siblings)
13 siblings, 0 replies; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
.can_receive callback changes semantics that once return 0, backend will
try sending again until explicitly flushed, change the device to meet
that.
dp8393x_can_receive checks SONIC_CR_RXEN bit in SONIC_CR register and
SONIC_ISR_RBE bit in SONIC_ISR register, try flushing the queue when
either bit is being updated.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/net/dp8393x.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index cd889bc..451ff72 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -327,9 +327,14 @@ static void dp8393x_do_stop_timer(dp8393xState *s)
dp8393x_update_wt_regs(s);
}
+static int dp8393x_can_receive(NetClientState *nc);
+
static void dp8393x_do_receiver_enable(dp8393xState *s)
{
s->regs[SONIC_CR] &= ~SONIC_CR_RXDIS;
+ if (dp8393x_can_receive(s->nic->ncs)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
}
static void dp8393x_do_receiver_disable(dp8393xState *s)
@@ -569,6 +574,9 @@ static void dp8393x_write(void *opaque, hwaddr addr, uint64_t data,
dp8393x_do_read_rra(s);
}
dp8393x_update_irq(s);
+ if (dp8393x_can_receive(s->nic->ncs)) {
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
break;
/* Ignore least significant bit */
case SONIC_RSA:
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (10 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 11/12] dp8393x: Flush packets when link comes up Fam Zheng
@ 2015-07-15 10:19 ` Fam Zheng
2015-07-16 2:58 ` Jason Wang
2015-07-16 5:38 ` [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Jason Wang
2015-07-20 17:09 ` Stefan Hajnoczi
13 siblings, 1 reply; 18+ messages in thread
From: Fam Zheng @ 2015-07-15 10:19 UTC (permalink / raw
To: qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
eth_can_rx checks s->rxsize and returns false if it is non-zero. Because
of the .can_receive semantics change, this will make the incoming queue
disabled by peer, until it is explicitly flushed. So we should flush it
when s->rxsize is becoming zero.
Squash eth_can_rx semantics into etx_rx and drop .can_receive()
callback, also add flush when rx buffer becomes available again after a
packet gets queued.
The other conditions, "!axienet_rx_resetting(s) &&
axienet_rx_enabled(s)" are OK because enet_write already calls
qemu_flush_queued_packets when the register bits are changed.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
hw/net/xilinx_axienet.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 9205770..d63c423 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -401,6 +401,9 @@ struct XilinxAXIEnet {
uint8_t rxapp[CONTROL_PAYLOAD_SIZE];
uint32_t rxappsize;
+
+ /* Whether axienet_eth_rx_notify should flush incoming queue. */
+ bool need_flush;
};
static void axienet_rx_reset(XilinxAXIEnet *s)
@@ -658,10 +661,8 @@ static const MemoryRegionOps enet_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static int eth_can_rx(NetClientState *nc)
+static int eth_can_rx(XilinxAXIEnet *s)
{
- XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
-
/* RX enabled? */
return !s->rxsize && !axienet_rx_resetting(s) && axienet_rx_enabled(s);
}
@@ -701,6 +702,10 @@ static void axienet_eth_rx_notify(void *opaque)
s->rxpos += ret;
if (!s->rxsize) {
s->regs[R_IS] |= IS_RX_COMPLETE;
+ if (s->need_flush) {
+ s->need_flush = false;
+ qemu_flush_queued_packets(qemu_get_queue(s->nic));
+ }
}
}
enet_update_irq(s);
@@ -721,6 +726,11 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
DENET(qemu_log("%s: %zd bytes\n", __func__, size));
+ if (!eth_can_rx(s)) {
+ s->need_flush = true;
+ return 0;
+ }
+
unicast = ~buf[0] & 0x1;
broadcast = memcmp(buf, sa_bcast, 6) == 0;
multicast = !unicast && !broadcast;
@@ -925,7 +935,6 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size)
static NetClientInfo net_xilinx_enet_info = {
.type = NET_CLIENT_OPTIONS_KIND_NIC,
.size = sizeof(NICState),
- .can_receive = eth_can_rx,
.receive = eth_rx,
};
--
2.4.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done Fam Zheng
@ 2015-07-16 2:58 ` Jason Wang
2015-07-16 3:32 ` Fam Zheng
0 siblings, 1 reply; 18+ messages in thread
From: Jason Wang @ 2015-07-16 2:58 UTC (permalink / raw
To: Fam Zheng, qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, Michael Walle,
Gerd Hoffmann, stefanha, Edgar E. Iglesias
On 07/15/2015 06:19 PM, Fam Zheng wrote:
> eth_can_rx checks s->rxsize and returns false if it is non-zero. Because
> of the .can_receive semantics change, this will make the incoming queue
> disabled by peer, until it is explicitly flushed. So we should flush it
> when s->rxsize is becoming zero.
>
> Squash eth_can_rx semantics into etx_rx and drop .can_receive()
> callback, also add flush when rx buffer becomes available again after a
> packet gets queued.
>
> The other conditions, "!axienet_rx_resetting(s) &&
> axienet_rx_enabled(s)" are OK because enet_write already calls
> qemu_flush_queued_packets when the register bits are changed.
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> hw/net/xilinx_axienet.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> index 9205770..d63c423 100644
> --- a/hw/net/xilinx_axienet.c
> +++ b/hw/net/xilinx_axienet.c
> @@ -401,6 +401,9 @@ struct XilinxAXIEnet {
>
> uint8_t rxapp[CONTROL_PAYLOAD_SIZE];
> uint32_t rxappsize;
> +
> + /* Whether axienet_eth_rx_notify should flush incoming queue. */
> + bool need_flush;
> };
>
> static void axienet_rx_reset(XilinxAXIEnet *s)
> @@ -658,10 +661,8 @@ static const MemoryRegionOps enet_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> -static int eth_can_rx(NetClientState *nc)
> +static int eth_can_rx(XilinxAXIEnet *s)
> {
> - XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
> -
> /* RX enabled? */
> return !s->rxsize && !axienet_rx_resetting(s) && axienet_rx_enabled(s);
> }
> @@ -701,6 +702,10 @@ static void axienet_eth_rx_notify(void *opaque)
> s->rxpos += ret;
> if (!s->rxsize) {
> s->regs[R_IS] |= IS_RX_COMPLETE;
> + if (s->need_flush) {
> + s->need_flush = false;
> + qemu_flush_queued_packets(qemu_get_queue(s->nic));
> + }
> }
> }
> enet_update_irq(s);
> @@ -721,6 +726,11 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>
> DENET(qemu_log("%s: %zd bytes\n", __func__, size));
>
> + if (!eth_can_rx(s)) {
> + s->need_flush = true;
> + return 0;
> + }
> +
axienet_eth_rx_notify() was only called by eth_rx(). So when
s->need_flush is true, we won't ever reach axienet_eth_rx_notify()?
> unicast = ~buf[0] & 0x1;
> broadcast = memcmp(buf, sa_bcast, 6) == 0;
> multicast = !unicast && !broadcast;
> @@ -925,7 +935,6 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size)
> static NetClientInfo net_xilinx_enet_info = {
> .type = NET_CLIENT_OPTIONS_KIND_NIC,
> .size = sizeof(NICState),
> - .can_receive = eth_can_rx,
> .receive = eth_rx,
> };
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done
2015-07-16 2:58 ` Jason Wang
@ 2015-07-16 3:32 ` Fam Zheng
2015-07-16 5:38 ` Jason Wang
0 siblings, 1 reply; 18+ messages in thread
From: Fam Zheng @ 2015-07-16 3:32 UTC (permalink / raw
To: Jason Wang
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, qemu-devel,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
On Thu, 07/16 10:58, Jason Wang wrote:
>
>
> On 07/15/2015 06:19 PM, Fam Zheng wrote:
> > eth_can_rx checks s->rxsize and returns false if it is non-zero. Because
> > of the .can_receive semantics change, this will make the incoming queue
> > disabled by peer, until it is explicitly flushed. So we should flush it
> > when s->rxsize is becoming zero.
> >
> > Squash eth_can_rx semantics into etx_rx and drop .can_receive()
> > callback, also add flush when rx buffer becomes available again after a
> > packet gets queued.
> >
> > The other conditions, "!axienet_rx_resetting(s) &&
> > axienet_rx_enabled(s)" are OK because enet_write already calls
> > qemu_flush_queued_packets when the register bits are changed.
> >
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > ---
> > hw/net/xilinx_axienet.c | 17 +++++++++++++----
> > 1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
> > index 9205770..d63c423 100644
> > --- a/hw/net/xilinx_axienet.c
> > +++ b/hw/net/xilinx_axienet.c
> > @@ -401,6 +401,9 @@ struct XilinxAXIEnet {
> >
> > uint8_t rxapp[CONTROL_PAYLOAD_SIZE];
> > uint32_t rxappsize;
> > +
> > + /* Whether axienet_eth_rx_notify should flush incoming queue. */
> > + bool need_flush;
> > };
> >
> > static void axienet_rx_reset(XilinxAXIEnet *s)
> > @@ -658,10 +661,8 @@ static const MemoryRegionOps enet_ops = {
> > .endianness = DEVICE_LITTLE_ENDIAN,
> > };
> >
> > -static int eth_can_rx(NetClientState *nc)
> > +static int eth_can_rx(XilinxAXIEnet *s)
> > {
> > - XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
> > -
> > /* RX enabled? */
> > return !s->rxsize && !axienet_rx_resetting(s) && axienet_rx_enabled(s);
> > }
> > @@ -701,6 +702,10 @@ static void axienet_eth_rx_notify(void *opaque)
> > s->rxpos += ret;
> > if (!s->rxsize) {
> > s->regs[R_IS] |= IS_RX_COMPLETE;
> > + if (s->need_flush) {
> > + s->need_flush = false;
> > + qemu_flush_queued_packets(qemu_get_queue(s->nic));
> > + }
> > }
> > }
> > enet_update_irq(s);
> > @@ -721,6 +726,11 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
> >
> > DENET(qemu_log("%s: %zd bytes\n", __func__, size));
> >
> > + if (!eth_can_rx(s)) {
> > + s->need_flush = true;
> > + return 0;
> > + }
> > +
>
> axienet_eth_rx_notify() was only called by eth_rx(). So when
> s->need_flush is true, we won't ever reach axienet_eth_rx_notify()?
We will.
If we are here it measn a previous call to axienet_eth_rx_notify hasn't drained
the buffer:
static void axienet_eth_rx_notify(void *opaque)
{
...
while (s->rxsize && stream_can_push(s->tx_data_dev,
axienet_eth_rx_notify, s)) {
size_t ret = stream_push(s->tx_data_dev, (void *)s->rxmem + s->rxpos,
s->rxsize);
s->rxsize -= ret;
s->rxpos += ret;
if (!s->rxsize) {
s->regs[R_IS] |= IS_RX_COMPLETE;
}
}
...
}
axienet_eth_rx_notify is passed to stream_can_push so it will be reached again
once s->tx_data_dev can receive more data:
typedef struct StreamSlaveClass {
InterfaceClass parent;
/**
* can push - determine if a stream slave is capable of accepting at least
* one byte of data. Returns false if cannot accept. If not implemented, the
* slave is assumed to always be capable of receiving.
* @notify: Optional callback that the slave will call when the slave is
* capable of receiving again. Only called if false is returned.
* @notify_opaque: opaque data to pass to notify call.
*/
bool (*can_push)(StreamSlave *obj, StreamCanPushNotifyFn notify,
void *notify_opaque);
...
Am I missing anything?
Fam
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done
2015-07-16 3:32 ` Fam Zheng
@ 2015-07-16 5:38 ` Jason Wang
0 siblings, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-07-16 5:38 UTC (permalink / raw
To: Fam Zheng
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, qemu-devel,
Michael Walle, Gerd Hoffmann, stefanha, Edgar E. Iglesias
On 07/16/2015 11:32 AM, Fam Zheng wrote:
> On Thu, 07/16 10:58, Jason Wang wrote:
>>
>> On 07/15/2015 06:19 PM, Fam Zheng wrote:
>>> eth_can_rx checks s->rxsize and returns false if it is non-zero. Because
>>> of the .can_receive semantics change, this will make the incoming queue
>>> disabled by peer, until it is explicitly flushed. So we should flush it
>>> when s->rxsize is becoming zero.
>>>
>>> Squash eth_can_rx semantics into etx_rx and drop .can_receive()
>>> callback, also add flush when rx buffer becomes available again after a
>>> packet gets queued.
>>>
>>> The other conditions, "!axienet_rx_resetting(s) &&
>>> axienet_rx_enabled(s)" are OK because enet_write already calls
>>> qemu_flush_queued_packets when the register bits are changed.
>>>
>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>> ---
>>> hw/net/xilinx_axienet.c | 17 +++++++++++++----
>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
>>> index 9205770..d63c423 100644
>>> --- a/hw/net/xilinx_axienet.c
>>> +++ b/hw/net/xilinx_axienet.c
>>> @@ -401,6 +401,9 @@ struct XilinxAXIEnet {
>>>
>>> uint8_t rxapp[CONTROL_PAYLOAD_SIZE];
>>> uint32_t rxappsize;
>>> +
>>> + /* Whether axienet_eth_rx_notify should flush incoming queue. */
>>> + bool need_flush;
>>> };
>>>
>>> static void axienet_rx_reset(XilinxAXIEnet *s)
>>> @@ -658,10 +661,8 @@ static const MemoryRegionOps enet_ops = {
>>> .endianness = DEVICE_LITTLE_ENDIAN,
>>> };
>>>
>>> -static int eth_can_rx(NetClientState *nc)
>>> +static int eth_can_rx(XilinxAXIEnet *s)
>>> {
>>> - XilinxAXIEnet *s = qemu_get_nic_opaque(nc);
>>> -
>>> /* RX enabled? */
>>> return !s->rxsize && !axienet_rx_resetting(s) && axienet_rx_enabled(s);
>>> }
>>> @@ -701,6 +702,10 @@ static void axienet_eth_rx_notify(void *opaque)
>>> s->rxpos += ret;
>>> if (!s->rxsize) {
>>> s->regs[R_IS] |= IS_RX_COMPLETE;
>>> + if (s->need_flush) {
>>> + s->need_flush = false;
>>> + qemu_flush_queued_packets(qemu_get_queue(s->nic));
>>> + }
>>> }
>>> }
>>> enet_update_irq(s);
>>> @@ -721,6 +726,11 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>>>
>>> DENET(qemu_log("%s: %zd bytes\n", __func__, size));
>>>
>>> + if (!eth_can_rx(s)) {
>>> + s->need_flush = true;
>>> + return 0;
>>> + }
>>> +
>> axienet_eth_rx_notify() was only called by eth_rx(). So when
>> s->need_flush is true, we won't ever reach axienet_eth_rx_notify()?
> We will.
>
> If we are here it measn a previous call to axienet_eth_rx_notify hasn't drained
> the buffer:
>
> static void axienet_eth_rx_notify(void *opaque)
> {
> ...
>
> while (s->rxsize && stream_can_push(s->tx_data_dev,
> axienet_eth_rx_notify, s)) {
> size_t ret = stream_push(s->tx_data_dev, (void *)s->rxmem + s->rxpos,
> s->rxsize);
> s->rxsize -= ret;
> s->rxpos += ret;
> if (!s->rxsize) {
> s->regs[R_IS] |= IS_RX_COMPLETE;
> }
> }
> ...
>
> }
>
> axienet_eth_rx_notify is passed to stream_can_push so it will be reached again
> once s->tx_data_dev can receive more data:
>
> typedef struct StreamSlaveClass {
> InterfaceClass parent;
> /**
> * can push - determine if a stream slave is capable of accepting at least
> * one byte of data. Returns false if cannot accept. If not implemented, the
> * slave is assumed to always be capable of receiving.
> * @notify: Optional callback that the slave will call when the slave is
> * capable of receiving again. Only called if false is returned.
> * @notify_opaque: opaque data to pass to notify call.
> */
> bool (*can_push)(StreamSlave *obj, StreamCanPushNotifyFn notify,
> void *notify_opaque);
> ...
>
> Am I missing anything?
>
> Fam
Probably not. I misses the possible call in axidma_write(). So the patch
is ok :)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (11 preceding siblings ...)
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done Fam Zheng
@ 2015-07-16 5:38 ` Jason Wang
2015-07-20 17:09 ` Stefan Hajnoczi
13 siblings, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-07-16 5:38 UTC (permalink / raw
To: Fam Zheng, qemu-devel
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, Michael Walle,
Gerd Hoffmann, stefanha, Edgar E. Iglesias
On 07/15/2015 06:19 PM, Fam Zheng wrote:
> v2: Don't use BH in patches 6 and 12. [Jason]
> Add Stefan's reviewed-by in all others.
> Drop trace point in patch 8 and move function body upward. [Michael]
>
> Since a90a742 "tap: Drop tap_can_send", all nics that returns false from
> .can_receive() are required to explicitly flush the incoming queue when the
> status of it is changing back to true, otherwise the backend will sop
> processing more rx packets.
>
> The purpose of this callback is to tell the peer backend (tap, socket, etc)
> "hold on until guest consumes old data because my buffer is not ready". More
> often than not NICs also do this when driver deactivated the card or disabled
> rx, causing the packets being unnessarily queued, where they should actualy be
> dropped.
>
> This series adds such missing qemu_flush_queued_packets calls for all NICs, and
> drops such unnecessary conditions in .can_receive(), so that NICs now:
>
> - return false from .can_receive when guest buffers are busy.
> - call qemu_flush_queued_packets when buffers are available again.
> - return -1 from .receive when rx is not enabled.
>
> e1000, ne2000, rocker and vmxnet3 are not included because they're fixed by
> other patches on the list and applied to Stefan's tree.
>
> virtio-net is covered by another series:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07377.html ^U
>
> http://patchwork.ozlabs.org/patch/495379/ (the subject should have v2, my bad.)
>
> All other NICs are okay, as they already do the flush on the state transition
> points.
>
> Please review.
Reviewed-by: Jason Wang <jasowang@redhat.com>
Thanks
>
> Fam Zheng (12):
> xgmac: Drop packets with eth_can_rx is false.
> pcnet: Drop pcnet_can_receive
> eepro100: Drop nic_can_receive
> usbnet: Drop usbnet_can_receive
> etsec: Move etsec_can_receive into etsec_receive
> etsec: Flush queue when rx buffer is consumed
> mcf_fec: Drop mcf_fec_can_receive
> milkymist-minimac2: Flush queued packets when link comes up
> mipsnet: Flush queued packets when receiving is enabled
> stellaris_enet: Flush queued packets when read done
> dp8393x: Flush packets when link comes up
> axienet: Flush queued packets when rx is done
>
> hw/net/dp8393x.c | 8 ++++++++
> hw/net/eepro100.c | 11 -----------
> hw/net/fsl_etsec/etsec.c | 20 ++++++++++----------
> hw/net/fsl_etsec/etsec.h | 4 +++-
> hw/net/fsl_etsec/rings.c | 17 +++++++++++------
> hw/net/lance.c | 1 -
> hw/net/mcf_fec.c | 9 +--------
> hw/net/milkymist-minimac2.c | 33 ++++++++++++++++-----------------
> hw/net/mipsnet.c | 9 +++++++--
> hw/net/pcnet-pci.c | 1 -
> hw/net/pcnet.c | 9 ---------
> hw/net/pcnet.h | 1 -
> hw/net/stellaris_enet.c | 14 +++++---------
> hw/net/xgmac.c | 8 ++++----
> hw/net/xilinx_axienet.c | 17 +++++++++++++----
> hw/usb/dev-network.c | 20 ++++----------------
> trace-events | 1 -
> 17 files changed, 82 insertions(+), 101 deletions(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
` (12 preceding siblings ...)
2015-07-16 5:38 ` [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Jason Wang
@ 2015-07-20 17:09 ` Stefan Hajnoczi
13 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2015-07-20 17:09 UTC (permalink / raw
To: Fam Zheng
Cc: Peter Maydell, Peter Crosthwaite, Rob Herring, jasowang,
qemu-devel, Michael Walle, Gerd Hoffmann, stefanha,
Edgar E. Iglesias
[-- Attachment #1: Type: text/plain, Size: 3551 bytes --]
On Wed, Jul 15, 2015 at 06:19:01PM +0800, Fam Zheng wrote:
> v2: Don't use BH in patches 6 and 12. [Jason]
> Add Stefan's reviewed-by in all others.
> Drop trace point in patch 8 and move function body upward. [Michael]
>
> Since a90a742 "tap: Drop tap_can_send", all nics that returns false from
> .can_receive() are required to explicitly flush the incoming queue when the
> status of it is changing back to true, otherwise the backend will sop
> processing more rx packets.
>
> The purpose of this callback is to tell the peer backend (tap, socket, etc)
> "hold on until guest consumes old data because my buffer is not ready". More
> often than not NICs also do this when driver deactivated the card or disabled
> rx, causing the packets being unnessarily queued, where they should actualy be
> dropped.
>
> This series adds such missing qemu_flush_queued_packets calls for all NICs, and
> drops such unnecessary conditions in .can_receive(), so that NICs now:
>
> - return false from .can_receive when guest buffers are busy.
> - call qemu_flush_queued_packets when buffers are available again.
> - return -1 from .receive when rx is not enabled.
>
> e1000, ne2000, rocker and vmxnet3 are not included because they're fixed by
> other patches on the list and applied to Stefan's tree.
>
> virtio-net is covered by another series:
>
> https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg07377.html ^U
>
> http://patchwork.ozlabs.org/patch/495379/ (the subject should have v2, my bad.)
>
> All other NICs are okay, as they already do the flush on the state transition
> points.
>
> Please review.
>
> Fam Zheng (12):
> xgmac: Drop packets with eth_can_rx is false.
> pcnet: Drop pcnet_can_receive
> eepro100: Drop nic_can_receive
> usbnet: Drop usbnet_can_receive
> etsec: Move etsec_can_receive into etsec_receive
> etsec: Flush queue when rx buffer is consumed
> mcf_fec: Drop mcf_fec_can_receive
> milkymist-minimac2: Flush queued packets when link comes up
> mipsnet: Flush queued packets when receiving is enabled
> stellaris_enet: Flush queued packets when read done
> dp8393x: Flush packets when link comes up
> axienet: Flush queued packets when rx is done
>
> hw/net/dp8393x.c | 8 ++++++++
> hw/net/eepro100.c | 11 -----------
> hw/net/fsl_etsec/etsec.c | 20 ++++++++++----------
> hw/net/fsl_etsec/etsec.h | 4 +++-
> hw/net/fsl_etsec/rings.c | 17 +++++++++++------
> hw/net/lance.c | 1 -
> hw/net/mcf_fec.c | 9 +--------
> hw/net/milkymist-minimac2.c | 33 ++++++++++++++++-----------------
> hw/net/mipsnet.c | 9 +++++++--
> hw/net/pcnet-pci.c | 1 -
> hw/net/pcnet.c | 9 ---------
> hw/net/pcnet.h | 1 -
> hw/net/stellaris_enet.c | 14 +++++---------
> hw/net/xgmac.c | 8 ++++----
> hw/net/xilinx_axienet.c | 17 +++++++++++++----
> hw/usb/dev-network.c | 20 ++++----------------
> trace-events | 1 -
> 17 files changed, 82 insertions(+), 101 deletions(-)
I can't merge this until net/hub.c is fixed. Anything that relies on
qemu_can_send_packet() will break if we drop .can_receive() functions.
At this point in the 2.4 release cycle I'm tempted to revert the
relevant qemu_set_fd_handler2() refactoring patches and stage this for
2.5 instead. That way we know networking works as before and we can
tackle all these fixups in 2.5.
[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-07-20 17:10 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-15 10:19 [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 01/12] xgmac: Drop packets with eth_can_rx is false Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 02/12] pcnet: Drop pcnet_can_receive Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 03/12] eepro100: Drop nic_can_receive Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 04/12] usbnet: Drop usbnet_can_receive Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 05/12] etsec: Move etsec_can_receive into etsec_receive Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 06/12] etsec: Flush queue when rx buffer is consumed Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 07/12] mcf_fec: Drop mcf_fec_can_receive Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 08/12] milkymist-minimac2: Flush queued packets when link comes up Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 09/12] mipsnet: Flush queued packets when receiving is enabled Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 10/12] stellaris_enet: Flush queued packets when read done Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 11/12] dp8393x: Flush packets when link comes up Fam Zheng
2015-07-15 10:19 ` [Qemu-devel] [PATCH v2 for-2.4 12/12] axienet: Flush queued packets when rx is done Fam Zheng
2015-07-16 2:58 ` Jason Wang
2015-07-16 3:32 ` Fam Zheng
2015-07-16 5:38 ` Jason Wang
2015-07-16 5:38 ` [Qemu-devel] [PATCH v2 for-2.4 00/12] hw/net: Fix .can_receive() for NICs Jason Wang
2015-07-20 17:09 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.