* [PULL V2 01/20] virtio-net: calculating proper msix vectors on init
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 02/20] net: Fix build error when DEBUG_NET is on Jason Wang
` (19 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-devel
Currently, the default msix vectors for virtio-net-pci is 3 which is
obvious not suitable for multiqueue guest, so we depends on the user
or management tools to pass a correct vectors parameter. In fact, we
can simplifying this by calculating the number of vectors on realize.
Consider we have N queues, the number of vectors needed is 2*N + 2
(#queue pairs + plus one config interrupt and control vq). We didn't
check whether or not host support control vq because it was added
unconditionally by qemu to avoid breaking legacy guests such as Minix.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/core/machine.c | 1 +
hw/virtio/virtio-net-pci.c | 10 +++++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4386f57..979133f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,7 @@
GlobalProperty hw_compat_5_2[] = {
{ "ICH9-LPC", "smm-compat", "on"},
{ "PIIX4_PM", "smm-compat", "on"},
+ { "virtio-net-pci", "vectors", "3"},
};
const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
diff --git a/hw/virtio/virtio-net-pci.c b/hw/virtio/virtio-net-pci.c
index 292d13d..aa0b3ca 100644
--- a/hw/virtio/virtio-net-pci.c
+++ b/hw/virtio/virtio-net-pci.c
@@ -41,7 +41,8 @@ struct VirtIONetPCI {
static Property virtio_net_properties[] = {
DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
- DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
+ DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
+ DEV_NVECTORS_UNSPECIFIED),
DEFINE_PROP_END_OF_LIST(),
};
@@ -50,6 +51,13 @@ static void virtio_net_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
DeviceState *qdev = DEVICE(vpci_dev);
VirtIONetPCI *dev = VIRTIO_NET_PCI(vpci_dev);
DeviceState *vdev = DEVICE(&dev->vdev);
+ VirtIONet *net = VIRTIO_NET(vdev);
+
+ if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
+ vpci_dev->nvectors = 2 * MAX(net->nic_conf.peers.queues, 1)
+ + 1 /* Config interrupt */
+ + 1 /* Control vq */;
+ }
virtio_net_set_netclient_name(&dev->vdev, qdev->id,
object_get_typename(OBJECT(qdev)));
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 02/20] net: Fix build error when DEBUG_NET is on
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
2021-03-15 9:14 ` [PULL V2 01/20] virtio-net: calculating proper msix vectors on init Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 03/20] net: validate that ids are well formed Jason Wang
` (18 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, Bin Meng, qemu-devel
From: Bin Meng <bin.meng@windriver.com>
"qemu-common.h" should be included to provide the forward declaration
of qemu_hexdump() when DEBUG_NET is on.
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/net.c b/net/net.c
index 9a2a4c9..16a87cc 100644
--- a/net/net.c
+++ b/net/net.c
@@ -23,6 +23,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu-common.h"
#include "net/net.h"
#include "clients.h"
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 03/20] net: validate that ids are well formed
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
2021-03-15 9:14 ` [PULL V2 01/20] virtio-net: calculating proper msix vectors on init Jason Wang
2021-03-15 9:14 ` [PULL V2 02/20] net: Fix build error when DEBUG_NET is on Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 04/20] e1000: fail early for evil descriptor Jason Wang
` (17 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Paolo Bonzini, Jason Wang, qemu-devel
From: Paolo Bonzini <pbonzini@redhat.com>
When a network or network device is created from the command line or HMP,
QemuOpts ensures that the id passes the id_wellformed check. However,
QMP skips this:
$ qemu-system-x86_64 -qmp stdio -S -nic user,id=123/456
qemu-system-x86_64: -nic user,id=123/456: Parameter id expects an identifier
Identifiers consist of letters, digits, -, ., _, starting with a letter.
$ qemu-system-x86_64 -qmp stdio -S
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"netdev_add", "arguments": {"type": "user", "id": "123/456"}}
{"return": {}}
After:
$ qemu-system-x86_64 -qmp stdio -S
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"netdev_add", "arguments": {"type": "user", "id": "123/456"}}
{"error": {"class": "GenericError", "desc": "Parameter "id" expects an identifier"}}
Validity checks should be performed always at the bottom of the call chain,
because QMP skips all the steps above. At the same time we know that every
call chain should go through either QMP or (for legacy) through QemuOpts.
Because the id for -net and -nic is automatically generated and not
well-formed by design, just add the check to QMP.
Cc: Jason Wang <jasowang@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/net.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/net.c b/net/net.c
index 16a87cc..77b35ea 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1134,6 +1134,11 @@ void netdev_add(QemuOpts *opts, Error **errp)
void qmp_netdev_add(Netdev *netdev, Error **errp)
{
+ if (!id_wellformed(netdev->id)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "id", "an identifier");
+ return;
+ }
+
net_client_init1(netdev, true, errp);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 04/20] e1000: fail early for evil descriptor
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (2 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 03/20] net: validate that ids are well formed Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 05/20] net: introduce qemu_receive_packet() Jason Wang
` (16 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
During procss_tx_desc(), driver can try to chain data descriptor with
legacy descriptor, when will lead underflow for the following
calculation in process_tx_desc() for bytes:
if (tp->size + bytes > msh)
bytes = msh - tp->size;
This will lead a infinite loop. So check and fail early if tp->size if
greater or equal to msh.
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr>
Reported-by: Ruhr-University Bochum <bugs-syssec@rub.de>
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/e1000.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index d8da2f6..4345d86 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -670,6 +670,9 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
msh = tp->tso_props.hdr_len + tp->tso_props.mss;
do {
bytes = split_size;
+ if (tp->size >= msh) {
+ goto eop;
+ }
if (tp->size + bytes > msh)
bytes = msh - tp->size;
@@ -695,6 +698,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
tp->size += split_size;
}
+eop:
if (!(txd_lower & E1000_TXD_CMD_EOP))
return;
if (!(tp->cptse && tp->size < tp->tso_props.hdr_len)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 05/20] net: introduce qemu_receive_packet()
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (3 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 04/20] e1000: fail early for evil descriptor Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 06/20] e1000: switch to use qemu_receive_packet() for loopback Jason Wang
` (15 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
Some NIC supports loopback mode and this is done by calling
nc->info->receive() directly which in fact suppresses the effort of
reentrancy check that is done in qemu_net_queue_send().
Unfortunately we can't use qemu_net_queue_send() here since for
loopback there's no sender as peer, so this patch introduce a
qemu_receive_packet() which is used for implementing loopback mode
for a NIC with this check.
NIC that supports loopback mode will be converted to this helper.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/net/net.h | 5 +++++
include/net/queue.h | 8 ++++++++
net/net.c | 38 +++++++++++++++++++++++++++++++-------
net/queue.c | 22 ++++++++++++++++++++++
4 files changed, 66 insertions(+), 7 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 919faca..4f56cae 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -144,12 +144,17 @@ void *qemu_get_nic_opaque(NetClientState *nc);
void qemu_del_net_client(NetClientState *nc);
typedef void (*qemu_nic_foreach)(NICState *nic, void *opaque);
void qemu_foreach_nic(qemu_nic_foreach func, void *opaque);
+int qemu_can_receive_packet(NetClientState *nc);
int qemu_can_send_packet(NetClientState *nc);
ssize_t qemu_sendv_packet(NetClientState *nc, const struct iovec *iov,
int iovcnt);
ssize_t qemu_sendv_packet_async(NetClientState *nc, const struct iovec *iov,
int iovcnt, NetPacketSent *sent_cb);
ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size);
+ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size);
+ssize_t qemu_receive_packet_iov(NetClientState *nc,
+ const struct iovec *iov,
+ int iovcnt);
ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size);
ssize_t qemu_send_packet_async(NetClientState *nc, const uint8_t *buf,
int size, NetPacketSent *sent_cb);
diff --git a/include/net/queue.h b/include/net/queue.h
index c0269bb..9f2f289 100644
--- a/include/net/queue.h
+++ b/include/net/queue.h
@@ -55,6 +55,14 @@ void qemu_net_queue_append_iov(NetQueue *queue,
void qemu_del_net_queue(NetQueue *queue);
+ssize_t qemu_net_queue_receive(NetQueue *queue,
+ const uint8_t *data,
+ size_t size);
+
+ssize_t qemu_net_queue_receive_iov(NetQueue *queue,
+ const struct iovec *iov,
+ int iovcnt);
+
ssize_t qemu_net_queue_send(NetQueue *queue,
NetClientState *sender,
unsigned flags,
diff --git a/net/net.c b/net/net.c
index 77b35ea..edf9b95 100644
--- a/net/net.c
+++ b/net/net.c
@@ -529,6 +529,17 @@ int qemu_set_vnet_be(NetClientState *nc, bool is_be)
#endif
}
+int qemu_can_receive_packet(NetClientState *nc)
+{
+ if (nc->receive_disabled) {
+ return 0;
+ } else if (nc->info->can_receive &&
+ !nc->info->can_receive(nc)) {
+ return 0;
+ }
+ return 1;
+}
+
int qemu_can_send_packet(NetClientState *sender)
{
int vm_running = runstate_is_running();
@@ -541,13 +552,7 @@ int qemu_can_send_packet(NetClientState *sender)
return 1;
}
- if (sender->peer->receive_disabled) {
- return 0;
- } else if (sender->peer->info->can_receive &&
- !sender->peer->info->can_receive(sender->peer)) {
- return 0;
- }
- return 1;
+ return qemu_can_receive_packet(sender->peer);
}
static ssize_t filter_receive_iov(NetClientState *nc,
@@ -680,6 +685,25 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
return qemu_send_packet_async(nc, buf, size, NULL);
}
+ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
+{
+ if (!qemu_can_receive_packet(nc)) {
+ return 0;
+ }
+
+ return qemu_net_queue_receive(nc->incoming_queue, buf, size);
+}
+
+ssize_t qemu_receive_packet_iov(NetClientState *nc, const struct iovec *iov,
+ int iovcnt)
+{
+ if (!qemu_can_receive_packet(nc)) {
+ return 0;
+ }
+
+ return qemu_net_queue_receive_iov(nc->incoming_queue, iov, iovcnt);
+}
+
ssize_t qemu_send_packet_raw(NetClientState *nc, const uint8_t *buf, int size)
{
return qemu_send_packet_async_with_flags(nc, QEMU_NET_PACKET_FLAG_RAW,
diff --git a/net/queue.c b/net/queue.c
index 19e32c8..c872d51 100644
--- a/net/queue.c
+++ b/net/queue.c
@@ -182,6 +182,28 @@ static ssize_t qemu_net_queue_deliver_iov(NetQueue *queue,
return ret;
}
+ssize_t qemu_net_queue_receive(NetQueue *queue,
+ const uint8_t *data,
+ size_t size)
+{
+ if (queue->delivering) {
+ return 0;
+ }
+
+ return qemu_net_queue_deliver(queue, NULL, 0, data, size);
+}
+
+ssize_t qemu_net_queue_receive_iov(NetQueue *queue,
+ const struct iovec *iov,
+ int iovcnt)
+{
+ if (queue->delivering) {
+ return 0;
+ }
+
+ return qemu_net_queue_deliver_iov(queue, NULL, 0, iov, iovcnt);
+}
+
ssize_t qemu_net_queue_send(NetQueue *queue,
NetClientState *sender,
unsigned flags,
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 06/20] e1000: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (4 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 05/20] net: introduce qemu_receive_packet() Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 07/20] dp8393x: switch to use qemu_receive_packet() for loopback packet Jason Wang
` (14 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/e1000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 4345d86..4f75b44 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -546,7 +546,7 @@ e1000_send_packet(E1000State *s, const uint8_t *buf, int size)
NetClientState *nc = qemu_get_queue(s->nic);
if (s->phy_reg[PHY_CTRL] & MII_CR_LOOPBACK) {
- nc->info->receive(nc, buf, size);
+ qemu_receive_packet(nc, buf, size);
} else {
qemu_send_packet(nc, buf, size);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 07/20] dp8393x: switch to use qemu_receive_packet() for loopback packet
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (5 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 06/20] e1000: switch to use qemu_receive_packet() for loopback Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 08/20] msf2-mac: switch to use qemu_receive_packet() for loopback Jason Wang
` (13 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/dp8393x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 205c0de..533a830 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -506,7 +506,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
s->regs[SONIC_TCR] |= SONIC_TCR_CRSL;
if (nc->info->can_receive(nc)) {
s->loopback_packet = 1;
- nc->info->receive(nc, s->tx_buffer, tx_len);
+ qemu_receive_packet(nc, s->tx_buffer, tx_len);
}
} else {
/* Transmit packet */
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 08/20] msf2-mac: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (6 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 07/20] dp8393x: switch to use qemu_receive_packet() for loopback packet Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 09/20] sungem: " Jason Wang
` (12 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/msf2-emac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/msf2-emac.c b/hw/net/msf2-emac.c
index 32ba9e8..3e62060 100644
--- a/hw/net/msf2-emac.c
+++ b/hw/net/msf2-emac.c
@@ -158,7 +158,7 @@ static void msf2_dma_tx(MSF2EmacState *s)
* R_CFG1 bit 0 is set.
*/
if (s->regs[R_CFG1] & R_CFG1_LB_EN_MASK) {
- nc->info->receive(nc, buf, size);
+ qemu_receive_packet(nc, buf, size);
} else {
qemu_send_packet(nc, buf, size);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 09/20] sungem: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (7 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 08/20] msf2-mac: switch to use qemu_receive_packet() for loopback Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 10/20] tx_pkt: switch to use qemu_receive_packet_iov() " Jason Wang
` (11 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/sungem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 33c3722..3684a4d 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -306,7 +306,7 @@ static void sungem_send_packet(SunGEMState *s, const uint8_t *buf,
NetClientState *nc = qemu_get_queue(s->nic);
if (s->macregs[MAC_XIFCFG >> 2] & MAC_XIFCFG_LBCK) {
- nc->info->receive(nc, buf, size);
+ qemu_receive_packet(nc, buf, size);
} else {
qemu_send_packet(nc, buf, size);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 10/20] tx_pkt: switch to use qemu_receive_packet_iov() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (8 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 09/20] sungem: " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 11/20] rtl8139: switch to use qemu_receive_packet() " Jason Wang
` (10 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, qemu-stable, qemu-devel, Prasad J Pandit
This patch switches to use qemu_receive_receive_iov() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/net_tx_pkt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
index da262ed..1f9aa59 100644
--- a/hw/net/net_tx_pkt.c
+++ b/hw/net/net_tx_pkt.c
@@ -553,7 +553,7 @@ static inline void net_tx_pkt_sendv(struct NetTxPkt *pkt,
NetClientState *nc, const struct iovec *iov, int iov_cnt)
{
if (pkt->is_loopback) {
- nc->info->receive_iov(nc, iov, iov_cnt);
+ qemu_receive_packet_iov(nc, iov, iov_cnt);
} else {
qemu_sendv_packet(nc, iov, iov_cnt);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 11/20] rtl8139: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (9 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 10/20] tx_pkt: switch to use qemu_receive_packet_iov() " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 12/20] pcnet: " Jason Wang
` (9 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell
Cc: Jason Wang, Alexander Bulekov, qemu-stable, qemu-devel, Prasad J Pandit
From: Alexander Bulekov <alxndr@bu.edu>
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Buglink: https://bugs.launchpad.net/qemu/+bug/1910826
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/rtl8139.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index 4675ac8..90b4fc6 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -1795,7 +1795,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
}
DPRINTF("+++ transmit loopback mode\n");
- rtl8139_do_receive(qemu_get_queue(s->nic), buf, size, do_interrupt);
+ qemu_receive_packet(qemu_get_queue(s->nic), buf, size);
if (iov) {
g_free(buf2);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 12/20] pcnet: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (10 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 11/20] rtl8139: switch to use qemu_receive_packet() " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 13/20] cadence_gem: " Jason Wang
` (8 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell
Cc: Jason Wang, Alexander Bulekov, qemu-stable, qemu-devel, Prasad J Pandit
From: Alexander Bulekov <alxndr@bu.edu>
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Buglink: https://bugs.launchpad.net/qemu/+bug/1917085
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/pcnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index f3f18d8..dcd3fc4 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1250,7 +1250,7 @@ txagain:
if (BCR_SWSTYLE(s) == 1)
add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS);
s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC;
- pcnet_receive(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
+ qemu_receive_packet(qemu_get_queue(s->nic), s->buffer, s->xmit_pos);
s->looptest = 0;
} else {
if (s->nic) {
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 13/20] cadence_gem: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (11 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 12/20] pcnet: " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 14/20] lan9118: " Jason Wang
` (7 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell
Cc: Jason Wang, Alexander Bulekov, qemu-stable, qemu-devel, Prasad J Pandit
From: Alexander Bulekov <alxndr@bu.edu>
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/cadence_gem.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 9a4474a..24b3a0f 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1275,8 +1275,8 @@ static void gem_transmit(CadenceGEMState *s)
/* Send the packet somewhere */
if (s->phy_loop || (s->regs[GEM_NWCTRL] &
GEM_NWCTRL_LOCALLOOP)) {
- gem_receive(qemu_get_queue(s->nic), s->tx_packet,
- total_bytes);
+ qemu_receive_packet(qemu_get_queue(s->nic), s->tx_packet,
+ total_bytes);
} else {
qemu_send_packet(qemu_get_queue(s->nic), s->tx_packet,
total_bytes);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 14/20] lan9118: switch to use qemu_receive_packet() for loopback
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (12 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 13/20] cadence_gem: " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 15/20] pvrdma: wean code off pvrdma_ring.h kernel header Jason Wang
` (6 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell
Cc: Jason Wang, Alexander Bulekov, qemu-stable, qemu-devel, Prasad J Pandit
From: Alexander Bulekov <alxndr@bu.edu>
This patch switches to use qemu_receive_packet() which can detect
reentrancy and return early.
This is intended to address CVE-2021-3416.
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: qemu-stable@nongnu.org
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com
Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/lan9118.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index abc7962..6aff424 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -680,7 +680,7 @@ static void do_tx_packet(lan9118_state *s)
/* FIXME: Honor TX disable, and allow queueing of packets. */
if (s->phy_control & 0x4000) {
/* This assumes the receive routine doesn't touch the VLANClient. */
- lan9118_receive(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
+ qemu_receive_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
} else {
qemu_send_packet(qemu_get_queue(s->nic), s->txp->data, s->txp->len);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 15/20] pvrdma: wean code off pvrdma_ring.h kernel header
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (13 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 14/20] lan9118: " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 16/20] qapi: net: Add query-netdev command Jason Wang
` (5 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Jason Wang, Cornelia Huck, qemu-devel
From: Cornelia Huck <cohuck@redhat.com>
The pvrdma code relies on the pvrdma_ring.h kernel header for some
basic ring buffer handling. The content of that header isn't very
exciting, but contains some (q)atomic_*() invocations that (a)
cause manual massaging when doing a headers update, and (b) are
an indication that we probably should not be importing that header
at all.
Let's reimplement the ring buffer handling directly in the pvrdma
code instead. This arguably also improves readability of the code.
Importing the header can now be dropped.
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
Tested-by: Yuval Shaia <yuval.shaia.ml@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/rdma/vmw/pvrdma.h | 5 +-
hw/rdma/vmw/pvrdma_cmd.c | 6 +-
hw/rdma/vmw/pvrdma_dev_ring.c | 41 ++++----
hw/rdma/vmw/pvrdma_dev_ring.h | 9 +-
hw/rdma/vmw/pvrdma_main.c | 4 +-
.../drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h | 114 ---------------------
| 3 +-
7 files changed, 38 insertions(+), 144 deletions(-)
delete mode 100644 include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
index 1d36a76..d08965d 100644
--- a/hw/rdma/vmw/pvrdma.h
+++ b/hw/rdma/vmw/pvrdma.h
@@ -26,7 +26,6 @@
#include "../rdma_backend_defs.h"
#include "../rdma_rm_defs.h"
-#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
#include "pvrdma_dev_ring.h"
#include "qom/object.h"
@@ -64,10 +63,10 @@ typedef struct DSRInfo {
union pvrdma_cmd_req *req;
union pvrdma_cmd_resp *rsp;
- struct pvrdma_ring *async_ring_state;
+ PvrdmaRingState *async_ring_state;
PvrdmaRing async;
- struct pvrdma_ring *cq_ring_state;
+ PvrdmaRingState *cq_ring_state;
PvrdmaRing cq;
} DSRInfo;
diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index 692125a..f59879e 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -262,7 +262,7 @@ static int create_cq_ring(PCIDevice *pci_dev , PvrdmaRing **ring,
r = g_malloc(sizeof(*r));
*ring = r;
- r->ring_state = (struct pvrdma_ring *)
+ r->ring_state = (PvrdmaRingState *)
rdma_pci_dma_map(pci_dev, tbl[0], TARGET_PAGE_SIZE);
if (!r->ring_state) {
@@ -398,7 +398,7 @@ static int create_qp_rings(PCIDevice *pci_dev, uint64_t pdir_dma,
*rings = sr;
/* Create send ring */
- sr->ring_state = (struct pvrdma_ring *)
+ sr->ring_state = (PvrdmaRingState *)
rdma_pci_dma_map(pci_dev, tbl[0], TARGET_PAGE_SIZE);
if (!sr->ring_state) {
rdma_error_report("Failed to map to QP ring state");
@@ -639,7 +639,7 @@ static int create_srq_ring(PCIDevice *pci_dev, PvrdmaRing **ring,
r = g_malloc(sizeof(*r));
*ring = r;
- r->ring_state = (struct pvrdma_ring *)
+ r->ring_state = (PvrdmaRingState *)
rdma_pci_dma_map(pci_dev, tbl[0], TARGET_PAGE_SIZE);
if (!r->ring_state) {
rdma_error_report("Failed to map tp SRQ ring state");
diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
index f0bcde7..074ac59 100644
--- a/hw/rdma/vmw/pvrdma_dev_ring.c
+++ b/hw/rdma/vmw/pvrdma_dev_ring.c
@@ -22,11 +22,10 @@
#include "trace.h"
#include "../rdma_utils.h"
-#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
#include "pvrdma_dev_ring.h"
int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
- struct pvrdma_ring *ring_state, uint32_t max_elems,
+ PvrdmaRingState *ring_state, uint32_t max_elems,
size_t elem_sz, dma_addr_t *tbl, uint32_t npages)
{
int i;
@@ -73,48 +72,54 @@ out:
void *pvrdma_ring_next_elem_read(PvrdmaRing *ring)
{
- int e;
- unsigned int idx = 0, offset;
+ unsigned int idx, offset;
+ const uint32_t tail = qatomic_read(&ring->ring_state->prod_tail);
+ const uint32_t head = qatomic_read(&ring->ring_state->cons_head);
- e = pvrdma_idx_ring_has_data(ring->ring_state, ring->max_elems, &idx);
- if (e <= 0) {
+ if (tail & ~((ring->max_elems << 1) - 1) ||
+ head & ~((ring->max_elems << 1) - 1) ||
+ tail == head) {
trace_pvrdma_ring_next_elem_read_no_data(ring->name);
return NULL;
}
+ idx = head & (ring->max_elems - 1);
offset = idx * ring->elem_sz;
return ring->pages[offset / TARGET_PAGE_SIZE] + (offset % TARGET_PAGE_SIZE);
}
void pvrdma_ring_read_inc(PvrdmaRing *ring)
{
- pvrdma_idx_ring_inc(&ring->ring_state->cons_head, ring->max_elems);
+ uint32_t idx = qatomic_read(&ring->ring_state->cons_head);
+
+ idx = (idx + 1) & ((ring->max_elems << 1) - 1);
+ qatomic_set(&ring->ring_state->cons_head, idx);
}
void *pvrdma_ring_next_elem_write(PvrdmaRing *ring)
{
- int idx;
- unsigned int offset, tail;
+ unsigned int idx, offset;
+ const uint32_t tail = qatomic_read(&ring->ring_state->prod_tail);
+ const uint32_t head = qatomic_read(&ring->ring_state->cons_head);
- idx = pvrdma_idx_ring_has_space(ring->ring_state, ring->max_elems, &tail);
- if (idx <= 0) {
+ if (tail & ~((ring->max_elems << 1) - 1) ||
+ head & ~((ring->max_elems << 1) - 1) ||
+ tail == (head ^ ring->max_elems)) {
rdma_error_report("CQ is full");
return NULL;
}
- idx = pvrdma_idx(&ring->ring_state->prod_tail, ring->max_elems);
- if (idx < 0 || tail != idx) {
- rdma_error_report("Invalid idx %d", idx);
- return NULL;
- }
-
+ idx = tail & (ring->max_elems - 1);
offset = idx * ring->elem_sz;
return ring->pages[offset / TARGET_PAGE_SIZE] + (offset % TARGET_PAGE_SIZE);
}
void pvrdma_ring_write_inc(PvrdmaRing *ring)
{
- pvrdma_idx_ring_inc(&ring->ring_state->prod_tail, ring->max_elems);
+ uint32_t idx = qatomic_read(&ring->ring_state->prod_tail);
+
+ idx = (idx + 1) & ((ring->max_elems << 1) - 1);
+ qatomic_set(&ring->ring_state->prod_tail, idx);
}
void pvrdma_ring_free(PvrdmaRing *ring)
diff --git a/hw/rdma/vmw/pvrdma_dev_ring.h b/hw/rdma/vmw/pvrdma_dev_ring.h
index 5f2a0cf..d231588 100644
--- a/hw/rdma/vmw/pvrdma_dev_ring.h
+++ b/hw/rdma/vmw/pvrdma_dev_ring.h
@@ -19,18 +19,23 @@
#define MAX_RING_NAME_SZ 32
+typedef struct PvrdmaRingState {
+ int prod_tail; /* producer tail */
+ int cons_head; /* consumer head */
+} PvrdmaRingState;
+
typedef struct PvrdmaRing {
char name[MAX_RING_NAME_SZ];
PCIDevice *dev;
uint32_t max_elems;
size_t elem_sz;
- struct pvrdma_ring *ring_state; /* used only for unmap */
+ PvrdmaRingState *ring_state; /* used only for unmap */
int npages;
void **pages;
} PvrdmaRing;
int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
- struct pvrdma_ring *ring_state, uint32_t max_elems,
+ PvrdmaRingState *ring_state, uint32_t max_elems,
size_t elem_sz, dma_addr_t *tbl, uint32_t npages);
void *pvrdma_ring_next_elem_read(PvrdmaRing *ring);
void pvrdma_ring_read_inc(PvrdmaRing *ring);
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 8593570..84ae802 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -85,7 +85,7 @@ static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
rdma_pci_dma_unmap(pci_dev, ring_state, TARGET_PAGE_SIZE);
}
-static int init_dev_ring(PvrdmaRing *ring, struct pvrdma_ring **ring_state,
+static int init_dev_ring(PvrdmaRing *ring, PvrdmaRingState **ring_state,
const char *name, PCIDevice *pci_dev,
dma_addr_t dir_addr, uint32_t num_pages)
{
@@ -114,7 +114,7 @@ static int init_dev_ring(PvrdmaRing *ring, struct pvrdma_ring **ring_state,
/* RX ring is the second */
(*ring_state)++;
rc = pvrdma_ring_init(ring, name, pci_dev,
- (struct pvrdma_ring *)*ring_state,
+ (PvrdmaRingState *)*ring_state,
(num_pages - 1) * TARGET_PAGE_SIZE /
sizeof(struct pvrdma_cqne),
sizeof(struct pvrdma_cqne),
diff --git a/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h b/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
deleted file mode 100644
index 7b4062a..0000000
--- a/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
+++ /dev/null
@@ -1,114 +0,0 @@
-/*
- * Copyright (c) 2012-2016 VMware, Inc. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of EITHER the GNU General Public License
- * version 2 as published by the Free Software Foundation or the BSD
- * 2-Clause License. This program is distributed in the hope that it
- * will be useful, but WITHOUT ANY WARRANTY; WITHOUT EVEN THE IMPLIED
- * WARRANTY OF MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE.
- * See the GNU General Public License version 2 for more details at
- * http://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program available in the file COPYING in the main
- * directory of this source tree.
- *
- * The BSD 2-Clause License
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * - Redistributions of source code must retain the above
- * copyright notice, this list of conditions and the following
- * disclaimer.
- *
- * - Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer in the documentation and/or other materials
- * provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
- * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
- * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
- * OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef __PVRDMA_RING_H__
-#define __PVRDMA_RING_H__
-
-#include "standard-headers/linux/types.h"
-
-#define PVRDMA_INVALID_IDX -1 /* Invalid index. */
-
-struct pvrdma_ring {
- int prod_tail; /* Producer tail. */
- int cons_head; /* Consumer head. */
-};
-
-struct pvrdma_ring_state {
- struct pvrdma_ring tx; /* Tx ring. */
- struct pvrdma_ring rx; /* Rx ring. */
-};
-
-static inline int pvrdma_idx_valid(uint32_t idx, uint32_t max_elems)
-{
- /* Generates fewer instructions than a less-than. */
- return (idx & ~((max_elems << 1) - 1)) == 0;
-}
-
-static inline int32_t pvrdma_idx(int *var, uint32_t max_elems)
-{
- const unsigned int idx = qatomic_read(var);
-
- if (pvrdma_idx_valid(idx, max_elems))
- return idx & (max_elems - 1);
- return PVRDMA_INVALID_IDX;
-}
-
-static inline void pvrdma_idx_ring_inc(int *var, uint32_t max_elems)
-{
- uint32_t idx = qatomic_read(var) + 1; /* Increment. */
-
- idx &= (max_elems << 1) - 1; /* Modulo size, flip gen. */
- qatomic_set(var, idx);
-}
-
-static inline int32_t pvrdma_idx_ring_has_space(const struct pvrdma_ring *r,
- uint32_t max_elems, uint32_t *out_tail)
-{
- const uint32_t tail = qatomic_read(&r->prod_tail);
- const uint32_t head = qatomic_read(&r->cons_head);
-
- if (pvrdma_idx_valid(tail, max_elems) &&
- pvrdma_idx_valid(head, max_elems)) {
- *out_tail = tail & (max_elems - 1);
- return tail != (head ^ max_elems);
- }
- return PVRDMA_INVALID_IDX;
-}
-
-static inline int32_t pvrdma_idx_ring_has_data(const struct pvrdma_ring *r,
- uint32_t max_elems, uint32_t *out_head)
-{
- const uint32_t tail = qatomic_read(&r->prod_tail);
- const uint32_t head = qatomic_read(&r->cons_head);
-
- if (pvrdma_idx_valid(tail, max_elems) &&
- pvrdma_idx_valid(head, max_elems)) {
- *out_head = head & (max_elems - 1);
- return tail != head;
- }
- return PVRDMA_INVALID_IDX;
-}
-
-#endif /* __PVRDMA_RING_H__ */
--git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index fa6f2b6..1050e36 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -215,8 +215,7 @@ sed -e '1h;2,$H;$!d;g' -e 's/[^};]*pvrdma[^(| ]*([^)]*);//g' \
"$linux/drivers/infiniband/hw/vmw_pvrdma/pvrdma_verbs.h" > \
"$tmp_pvrdma_verbs";
-for i in "$linux/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h" \
- "$linux/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h" \
+for i in "$linux/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h" \
"$tmp_pvrdma_verbs"; do \
cp_portable "$i" \
"$output/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/"
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 16/20] qapi: net: Add query-netdev command
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (14 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 15/20] pvrdma: wean code off pvrdma_ring.h kernel header Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-16 21:27 ` Peter Maydell
2021-03-16 21:37 ` Peter Maydell
2021-03-15 9:14 ` [PULL V2 17/20] tests: Add tests for " Jason Wang
` (4 subsequent siblings)
20 siblings, 2 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Alexey Kirillov, Jason Wang, qemu-devel
From: Alexey Kirillov <lekiravi@yandex-team.ru>
The query-netdev command is used to get the configuration of the current
network device backends (netdevs).
This is the QMP analog of the HMP command "info network" but only for
netdevs (i.e. excluding NIC and hubports).
The query-netdev command returns an array of objects of the NetdevInfo
type, which are an extension of Netdev type. It means that response can
be used for netdev-add after small modification. This can be useful for
recreate the same netdev configuration.
Information about the network device is filled in when it is created or
modified and is available through the NetClientState->stored_config.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/net/net.h | 3 ++
net/l2tpv3.c | 7 ++++
net/net.c | 30 +++++++++++++-
net/netmap.c | 7 ++++
net/slirp.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
net/socket.c | 71 +++++++++++++++++++++++++++++++
net/tap-win32.c | 9 ++++
net/tap.c | 103 ++++++++++++++++++++++++++++++++++++++++++---
net/vde.c | 22 ++++++++++
net/vhost-user.c | 18 ++++++--
net/vhost-vdpa.c | 14 +++++++
qapi/net.json | 80 +++++++++++++++++++++++++++++++++++
12 files changed, 477 insertions(+), 9 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 4f56cae..dc3679f 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -5,6 +5,8 @@
#include "qapi/qapi-types-net.h"
#include "net/queue.h"
#include "hw/qdev-properties-system.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-net.h"
#define MAC_FMT "%02X:%02X:%02X:%02X:%02X:%02X"
#define MAC_ARG(x) ((uint8_t *)(x))[0], ((uint8_t *)(x))[1], \
@@ -93,6 +95,7 @@ struct NetClientState {
char *model;
char *name;
char info_str[256];
+ NetdevInfo *stored_config;
unsigned receive_disabled : 1;
NetClientDestructor *destructor;
unsigned int queue_index;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index e4d4218..8aa0a3e 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -723,6 +723,13 @@ int net_init_l2tpv3(const Netdev *netdev,
l2tpv3_read_poll(s, true);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_L2TPV3;
+
+ QAPI_CLONE_MEMBERS(NetdevL2TPv3Options,
+ &nc->stored_config->u.l2tpv3, l2tpv3);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"l2tpv3: connected");
return 0;
diff --git a/net/net.c b/net/net.c
index edf9b95..9a2a6ab 100644
--- a/net/net.c
+++ b/net/net.c
@@ -36,7 +36,6 @@
#include "monitor/monitor.h"
#include "qemu/help_option.h"
#include "qapi/qapi-commands-net.h"
-#include "qapi/qapi-visit-net.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qemu/error-report.h"
@@ -353,6 +352,7 @@ static void qemu_free_net_client(NetClientState *nc)
}
g_free(nc->name);
g_free(nc->model);
+ qapi_free_NetdevInfo(nc->stored_config);
if (nc->destructor) {
nc->destructor(nc);
}
@@ -1289,6 +1289,34 @@ RxFilterInfoList *qmp_query_rx_filter(bool has_name, const char *name,
return filter_list;
}
+NetdevInfoList *qmp_query_netdev(Error **errp)
+{
+ NetdevInfoList *list = NULL;
+ NetClientState *nc;
+
+ QTAILQ_FOREACH(nc, &net_clients, next) {
+ /*
+ * Only look at netdevs (backend network devices), not for each queue
+ * or NIC / hubport
+ */
+ if (nc->stored_config) {
+ NetdevInfo *element = QAPI_CLONE(NetdevInfo, nc->stored_config);
+
+ g_free(element->id); /* Need to dealloc empty id after clone */
+ element->id = g_strdup(nc->name);
+
+ element->has_peer_id = nc->peer != NULL;
+ if (element->has_peer_id) {
+ element->peer_id = g_strdup(nc->peer->name);
+ }
+
+ QAPI_LIST_PREPEND(list, element);
+ }
+ }
+
+ return list;
+}
+
void hmp_info_network(Monitor *mon, const QDict *qdict)
{
NetClientState *nc, *peer;
diff --git a/net/netmap.c b/net/netmap.c
index 350f097..ad59d4a 100644
--- a/net/netmap.c
+++ b/net/netmap.c
@@ -427,6 +427,13 @@ int net_init_netmap(const Netdev *netdev,
pstrcpy(s->ifname, sizeof(s->ifname), netmap_opts->ifname);
netmap_read_poll(s, true); /* Initially only poll for reads. */
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_NETMAP;
+
+ QAPI_CLONE_MEMBERS(NetdevNetmapOptions,
+ &nc->stored_config->u.netmap, netmap_opts);
+
return 0;
}
diff --git a/net/slirp.c b/net/slirp.c
index be914c0..6ab348b 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -377,6 +377,9 @@ static int net_slirp_init(NetClientState *peer, const char *model,
int shift;
char *end;
struct slirp_config_str *config;
+ NetdevUserOptions *stored;
+ StringList **stored_hostfwd;
+ StringList **stored_guestfwd;
if (!ipv4 && (vnetwork || vhost || vnameserver)) {
error_setg(errp, "IPv4 disabled but netmask/host/dns provided");
@@ -552,6 +555,115 @@ static int net_slirp_init(NetClientState *peer, const char *model,
nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_USER;
+ stored = &nc->stored_config->u.user;
+
+ if (vhostname) {
+ stored->has_hostname = true;
+ stored->hostname = g_strdup(vhostname);
+ }
+
+ stored->has_q_restrict = true;
+ stored->q_restrict = restricted;
+
+ stored->has_ipv4 = true;
+ stored->ipv4 = ipv4;
+
+ stored->has_ipv6 = true;
+ stored->ipv6 = ipv6;
+
+ if (ipv4) {
+ uint8_t *net_bytes = (uint8_t *)&net;
+ uint8_t *mask_bytes = (uint8_t *)&mask;
+
+ stored->has_net = true;
+ stored->net = g_strdup_printf("%d.%d.%d.%d/%d.%d.%d.%d",
+ net_bytes[0], net_bytes[1],
+ net_bytes[2], net_bytes[3],
+ mask_bytes[0], mask_bytes[1],
+ mask_bytes[2], mask_bytes[3]);
+
+ stored->has_host = true;
+ stored->host = g_strdup(inet_ntoa(host));
+ }
+
+ if (tftp_export) {
+ stored->has_tftp = true;
+ stored->tftp = g_strdup(tftp_export);
+ }
+
+ if (bootfile) {
+ stored->has_bootfile = true;
+ stored->bootfile = g_strdup(bootfile);
+ }
+
+ if (vdhcp_start) {
+ stored->has_dhcpstart = true;
+ stored->dhcpstart = g_strdup(vdhcp_start);
+ }
+
+ if (ipv4) {
+ stored->has_dns = true;
+ stored->dns = g_strdup(inet_ntoa(dns));
+ }
+
+ if (dnssearch) {
+ stored->has_dnssearch = true;
+ StringList **stored_list = &stored->dnssearch;
+
+ for (int i = 0; dnssearch[i]; i++) {
+ String *element = g_new0(String, 1);
+
+ element->str = g_strdup(dnssearch[i]);
+ QAPI_LIST_APPEND(stored_list, element);
+ }
+ }
+
+ if (vdomainname) {
+ stored->has_domainname = true;
+ stored->domainname = g_strdup(vdomainname);
+ }
+
+ if (ipv6) {
+ char addrstr[INET6_ADDRSTRLEN];
+ const char *res;
+
+ stored->has_ipv6_prefix = true;
+ stored->ipv6_prefix = g_strdup(vprefix6);
+
+ stored->has_ipv6_prefixlen = true;
+ stored->ipv6_prefixlen = vprefix6_len;
+
+ res = inet_ntop(AF_INET6, &ip6_host,
+ addrstr, sizeof(addrstr));
+
+ stored->has_ipv6_host = true;
+ stored->ipv6_host = g_strdup(res);
+
+ res = inet_ntop(AF_INET6, &ip6_dns,
+ addrstr, sizeof(addrstr));
+
+ stored->has_ipv6_dns = true;
+ stored->ipv6_dns = g_strdup(res);
+ }
+
+ if (smb_export) {
+ stored->has_smb = true;
+ stored->smb = g_strdup(smb_export);
+ }
+
+ if (vsmbserver) {
+ stored->has_smbserver = true;
+ stored->smbserver = g_strdup(vsmbserver);
+ }
+
+ if (tftp_server_name) {
+ stored->has_tftp_server_name = true;
+ stored->tftp_server_name = g_strdup(tftp_server_name);
+ }
+
snprintf(nc->info_str, sizeof(nc->info_str),
"net=%s,restrict=%s", inet_ntoa(net),
restricted ? "on" : "off");
@@ -581,15 +693,25 @@ static int net_slirp_init(NetClientState *peer, const char *model,
s->poll_notifier.notify = net_slirp_poll_notify;
main_loop_poll_add_notifier(&s->poll_notifier);
+ stored_hostfwd = &stored->hostfwd;
+ stored_guestfwd = &stored->guestfwd;
+
for (config = slirp_configs; config; config = config->next) {
+ String *element = g_new0(String, 1);
+
+ element->str = g_strdup(config->str);
if (config->flags & SLIRP_CFG_HOSTFWD) {
if (slirp_hostfwd(s, config->str, errp) < 0) {
goto error;
}
+ stored->has_hostfwd = true;
+ QAPI_LIST_APPEND(stored_hostfwd, element);
} else {
if (slirp_guestfwd(s, config->str, errp) < 0) {
goto error;
}
+ stored->has_guestfwd = true;
+ QAPI_LIST_APPEND(stored_guestfwd, element);
}
}
#ifndef _WIN32
diff --git a/net/socket.c b/net/socket.c
index 15b410e..1614523 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -342,6 +342,7 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
NetSocketState *s;
SocketAddress *sa;
SocketAddressType sa_type;
+ NetdevSocketOptions *stored;
sa = socket_local_address(fd, errp);
if (!sa) {
@@ -385,8 +386,19 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
net_socket_read_poll(s, true);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_SOCKET;
+ stored = &nc->stored_config->u.socket;
+
+ stored->has_fd = true;
+ stored->fd = g_strdup_printf("%d", fd);
+
/* mcast: save bound address as dst */
if (is_connected && mcast != NULL) {
+ stored->has_mcast = true;
+ stored->mcast = g_strdup(mcast);
+
s->dgram_dst = saddr;
snprintf(nc->info_str, sizeof(nc->info_str),
"socket: fd=%d (cloned mcast=%s:%d)",
@@ -428,6 +440,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
{
NetClientState *nc;
NetSocketState *s;
+ NetdevSocketOptions *stored;
nc = qemu_new_net_client(&net_socket_info, peer, model, name);
@@ -447,6 +460,15 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
} else {
qemu_set_fd_handler(s->fd, NULL, net_socket_connect, s);
}
+
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_SOCKET;
+ stored = &nc->stored_config->u.socket;
+
+ stored->has_fd = true;
+ stored->fd = g_strdup_printf("%d", fd);
+
return s;
}
@@ -483,6 +505,7 @@ static void net_socket_accept(void *opaque)
struct sockaddr_in saddr;
socklen_t len;
int fd;
+ NetdevSocketOptions *stored;
for(;;) {
len = sizeof(saddr);
@@ -498,6 +521,13 @@ static void net_socket_accept(void *opaque)
s->fd = fd;
s->nc.link_down = false;
net_socket_connect(s);
+
+ /* Store additional startup parameters (extend net_socket_listen_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ stored->has_fd = true;
+ stored->fd = g_strdup_printf("%d", fd);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: connection from %s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
@@ -513,6 +543,7 @@ static int net_socket_listen_init(NetClientState *peer,
NetSocketState *s;
struct sockaddr_in saddr;
int fd, ret;
+ NetdevSocketOptions *stored;
if (parse_host_port(&saddr, host_str, errp) < 0) {
return -1;
@@ -549,6 +580,15 @@ static int net_socket_listen_init(NetClientState *peer,
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
qemu_set_fd_handler(s->listen_fd, net_socket_accept, NULL, s);
+
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_SOCKET;
+ stored = &nc->stored_config->u.socket;
+
+ stored->has_listen = true;
+ stored->listen = g_strdup(host_str);
+
return 0;
}
@@ -561,6 +601,7 @@ static int net_socket_connect_init(NetClientState *peer,
NetSocketState *s;
int fd, connected, ret;
struct sockaddr_in saddr;
+ NetdevSocketOptions *stored;
if (parse_host_port(&saddr, host_str, errp) < 0) {
return -1;
@@ -598,6 +639,12 @@ static int net_socket_connect_init(NetClientState *peer,
return -1;
}
+ /* Store additional startup parameters (extend net_socket_fd_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ stored->has_connect = true;
+ stored->connect = g_strdup(host_str);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: connect to %s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
@@ -615,6 +662,7 @@ static int net_socket_mcast_init(NetClientState *peer,
int fd;
struct sockaddr_in saddr;
struct in_addr localaddr, *param_localaddr;
+ NetdevSocketOptions *stored;
if (parse_host_port(&saddr, host_str, errp) < 0) {
return -1;
@@ -643,6 +691,19 @@ static int net_socket_mcast_init(NetClientState *peer,
s->dgram_dst = saddr;
+ /* Store additional startup parameters (extend net_socket_fd_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ if (!stored->has_mcast) {
+ stored->has_mcast = true;
+ stored->mcast = g_strdup(host_str);
+ }
+
+ if (localaddr_str) {
+ stored->has_localaddr = true;
+ stored->localaddr = g_strdup(localaddr_str);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: mcast=%s:%d",
inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
@@ -660,6 +721,7 @@ static int net_socket_udp_init(NetClientState *peer,
NetSocketState *s;
int fd, ret;
struct sockaddr_in laddr, raddr;
+ NetdevSocketOptions *stored;
if (parse_host_port(&laddr, lhost, errp) < 0) {
return -1;
@@ -698,6 +760,15 @@ static int net_socket_udp_init(NetClientState *peer,
s->dgram_dst = raddr;
+ /* Store additional startup parameters (extend net_socket_fd_init) */
+ stored = &s->nc.stored_config->u.socket;
+
+ stored->has_localaddr = true;
+ stored->localaddr = g_strdup(lhost);
+
+ stored->has_udp = true;
+ stored->udp = g_strdup(rhost);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"socket: udp=%s:%d",
inet_ntoa(raddr.sin_addr), ntohs(raddr.sin_port));
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 2b5dcda..b60933b 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -768,6 +768,7 @@ static int tap_win32_init(NetClientState *peer, const char *model,
NetClientState *nc;
TAPState *s;
tap_win32_overlapped_t *handle;
+ NetdevTapOptions *stored;
if (tap_win32_open(&handle, ifname) < 0) {
printf("tap: Could not open '%s'\n", ifname);
@@ -778,6 +779,14 @@ static int tap_win32_init(NetClientState *peer, const char *model,
s = DO_UPCAST(TAPState, nc, nc);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_TAP;
+ stored = &nc->stored_config->u.tap;
+
+ stored->has_ifname = true;
+ stored->ifname = g_strdup(ifname);
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"tap: ifname=%s", ifname);
diff --git a/net/tap.c b/net/tap.c
index b751285..8041245 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -590,6 +590,7 @@ int net_init_bridge(const Netdev *netdev, const char *name,
const char *helper, *br;
TAPState *s;
int fd, vnet_hdr;
+ NetdevBridgeOptions *stored;
assert(netdev->type == NET_CLIENT_DRIVER_BRIDGE);
bridge = &netdev->u.bridge;
@@ -609,6 +610,21 @@ int net_init_bridge(const Netdev *netdev, const char *name,
}
s = net_tap_fd_init(peer, "bridge", name, fd, vnet_hdr);
+ /* Store startup parameters */
+ s->nc.stored_config = g_new0(NetdevInfo, 1);
+ s->nc.stored_config->type = NET_BACKEND_BRIDGE;
+ stored = &s->nc.stored_config->u.bridge;
+
+ if (br) {
+ stored->has_br = true;
+ stored->br = g_strdup(br);
+ }
+
+ if (helper) {
+ stored->has_helper = true;
+ stored->helper = g_strdup(helper);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
br);
@@ -656,11 +672,13 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
const char *model, const char *name,
const char *ifname, const char *script,
const char *downscript, const char *vhostfdname,
- int vnet_hdr, int fd, Error **errp)
+ int vnet_hdr, int fd, NetdevInfo **common_stored,
+ Error **errp)
{
Error *err = NULL;
TAPState *s = net_tap_fd_init(peer, model, name, fd, vnet_hdr);
int vhostfd;
+ NetdevTapOptions *stored;
tap_set_sndbuf(s->fd, tap, &err);
if (err) {
@@ -668,12 +686,65 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
return;
}
+ /* Store startup parameters */
+ if (!*common_stored) {
+ *common_stored = g_new0(NetdevInfo, 1);
+ (*common_stored)->type = NET_BACKEND_TAP;
+ s->nc.stored_config = *common_stored;
+ }
+ stored = &(*common_stored)->u.tap;
+
+ if (tap->has_sndbuf && !stored->has_sndbuf) {
+ stored->has_sndbuf = true;
+ stored->sndbuf = tap->sndbuf;
+ }
+
+ if (vnet_hdr && !stored->has_vnet_hdr) {
+ stored->has_vnet_hdr = true;
+ stored->vnet_hdr = true;
+ }
+
if (tap->has_fd || tap->has_fds) {
+ if (!stored->has_fds) {
+ stored->has_fds = true;
+ stored->fds = g_strdup_printf("%d", fd);
+ } else {
+ char *tmp_s = stored->fds;
+ stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
+ g_free(tmp_s);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd);
} else if (tap->has_helper) {
+ if (!stored->has_helper) {
+ stored->has_helper = true;
+ stored->helper = g_strdup(tap->helper);
+ }
+
+ if (!stored->has_br) {
+ stored->has_br = true;
+ stored->br = tap->has_br ? g_strdup(tap->br) :
+ g_strdup(DEFAULT_BRIDGE_INTERFACE);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s",
tap->helper);
} else {
+ if (ifname && !stored->has_ifname) {
+ stored->has_ifname = true;
+ stored->ifname = g_strdup(ifname);
+ }
+
+ if (script && !stored->has_script) {
+ stored->has_script = true;
+ stored->script = g_strdup(script);
+ }
+
+ if (downscript && !stored->has_downscript) {
+ stored->has_downscript = true;
+ stored->downscript = g_strdup(downscript);
+ }
+
snprintf(s->nc.info_str, sizeof(s->nc.info_str),
"ifname=%s,script=%s,downscript=%s", ifname, script,
downscript);
@@ -689,9 +760,20 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
vhostfdname || (tap->has_vhostforce && tap->vhostforce)) {
VhostNetOptions options;
+ stored->has_vhost = true;
+ stored->vhost = true;
+
+ if (tap->has_vhostforce && tap->vhostforce) {
+ stored->has_vhostforce = true;
+ stored->vhostforce = true;
+ }
+
options.backend_type = VHOST_BACKEND_TYPE_KERNEL;
options.net_backend = &s->nc;
if (tap->has_poll_us) {
+ stored->has_poll_us = true;
+ stored->poll_us = tap->poll_us;
+
options.busyloop_timeout = tap->poll_us;
} else {
options.busyloop_timeout = 0;
@@ -731,6 +813,15 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
}
options.opaque = (void *)(uintptr_t)vhostfd;
+ if (!stored->has_vhostfds) {
+ stored->has_vhostfds = true;
+ stored->vhostfds = g_strdup_printf("%d", vhostfd);
+ } else {
+ char *tmp_s = stored->vhostfds;
+ stored->vhostfds = g_strdup_printf("%s:%d", stored->fds, vhostfd);
+ g_free(tmp_s);
+ }
+
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
if (tap->has_vhostforce && tap->vhostforce) {
@@ -783,6 +874,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
const char *vhostfdname;
char ifname[128];
int ret = 0;
+ NetdevInfo *common_stored = NULL; /* will store configuration */
assert(netdev->type == NET_CLIENT_DRIVER_TAP);
tap = &netdev->u.tap;
@@ -829,7 +921,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
net_init_tap_one(tap, peer, "tap", name, NULL,
script, downscript,
- vhostfdname, vnet_hdr, fd, &err);
+ vhostfdname, vnet_hdr, fd, &common_stored, &err);
if (err) {
error_propagate(errp, err);
close(fd);
@@ -892,7 +984,7 @@ int net_init_tap(const Netdev *netdev, const char *name,
net_init_tap_one(tap, peer, "tap", name, ifname,
script, downscript,
tap->has_vhostfds ? vhost_fds[i] : NULL,
- vnet_hdr, fd, &err);
+ vnet_hdr, fd, &common_stored, &err);
if (err) {
error_propagate(errp, err);
ret = -1;
@@ -935,7 +1027,7 @@ free_fail:
net_init_tap_one(tap, peer, "bridge", name, ifname,
script, downscript, vhostfdname,
- vnet_hdr, fd, &err);
+ vnet_hdr, fd, &common_stored, &err);
if (err) {
error_propagate(errp, err);
close(fd);
@@ -981,7 +1073,8 @@ free_fail:
net_init_tap_one(tap, peer, "tap", name, ifname,
i >= 1 ? "no" : script,
i >= 1 ? "no" : downscript,
- vhostfdname, vnet_hdr, fd, &err);
+ vhostfdname, vnet_hdr, fd,
+ &common_stored, &err);
if (err) {
error_propagate(errp, err);
close(fd);
diff --git a/net/vde.c b/net/vde.c
index 99189cc..b0b8800 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -84,6 +84,7 @@ static int net_vde_init(NetClientState *peer, const char *model,
VDECONN *vde;
char *init_group = (char *)group;
char *init_sock = (char *)sock;
+ NetdevVdeOptions *stored;
struct vde_open_args args = {
.port = port,
@@ -108,6 +109,27 @@ static int net_vde_init(NetClientState *peer, const char *model,
qemu_set_fd_handler(vde_datafd(s->vde), vde_to_qemu, NULL, s);
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_VDE;
+ stored = &nc->stored_config->u.vde;
+
+ if (sock) {
+ stored->has_sock = true;
+ stored->sock = g_strdup(sock);
+ }
+
+ stored->has_port = true;
+ stored->port = port;
+
+ if (group) {
+ stored->has_group = true;
+ stored->group = g_strdup(group);
+ }
+
+ stored->has_mode = true;
+ stored->mode = mode;
+
return 0;
}
diff --git a/net/vhost-user.c b/net/vhost-user.c
index ffbd94d..5b7056b 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -311,14 +311,15 @@ static void net_vhost_user_event(void *opaque, QEMUChrEvent event)
}
static int net_vhost_user_init(NetClientState *peer, const char *device,
- const char *name, Chardev *chr,
- int queues)
+ const char *name, const char *chardev,
+ Chardev *chr, int queues)
{
Error *err = NULL;
NetClientState *nc, *nc0 = NULL;
NetVhostUserState *s = NULL;
VhostUserState *user;
int i;
+ NetdevVhostUserOptions *stored;
assert(name);
assert(queues > 0);
@@ -355,6 +356,16 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
assert(s->vhost_net);
+ /* Store startup parameters */
+ nc0->stored_config = g_new0(NetdevInfo, 1);
+ nc0->stored_config->type = NET_BACKEND_VHOST_USER;
+ stored = &nc0->stored_config->u.vhost_user;
+
+ stored->chardev = g_strdup(chardev);
+
+ stored->has_queues = true;
+ stored->queues = queues;
+
return 0;
err:
@@ -446,5 +457,6 @@ int net_init_vhost_user(const Netdev *netdev, const char *name,
return -1;
}
- return net_vhost_user_init(peer, "vhost_user", name, chr, queues);
+ return net_vhost_user_init(peer, "vhost_user", name,
+ vhost_user_opts->chardev, chr, queues);
}
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index fe659ec..8c27ea0 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -184,8 +184,22 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
VhostVDPAState *s;
int vdpa_device_fd = -1;
int ret = 0;
+ NetdevVhostVDPAOptions *stored;
+
assert(name);
nc = qemu_new_net_client(&net_vhost_vdpa_info, peer, device, name);
+
+ /* Store startup parameters */
+ nc->stored_config = g_new0(NetdevInfo, 1);
+ nc->stored_config->type = NET_BACKEND_VHOST_VDPA;
+ stored = &nc->stored_config->u.vhost_vdpa;
+
+ stored->has_vhostdev = true;
+ stored->vhostdev = g_strdup(vhostdev);
+
+ stored->has_queues = true;
+ stored->queues = 1; /* TODO: change when support multiqueue */
+
snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
nc->queue_index = 0;
s = DO_UPCAST(VhostVDPAState, nc, nc);
diff --git a/qapi/net.json b/qapi/net.json
index c31748c8..87361eb 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -714,3 +714,83 @@
##
{ 'event': 'FAILOVER_NEGOTIATED',
'data': {'device-id': 'str'} }
+
+##
+# @NetBackend:
+#
+# Available netdev backend drivers.
+#
+# Since: 6.0
+##
+{ 'enum': 'NetBackend',
+ 'data': [ 'bridge', 'l2tpv3', 'netmap', 'socket', 'tap', 'user', 'vde',
+ 'vhost-user', 'vhost-vdpa' ] }
+
+##
+# @NetdevInfo:
+#
+# Configuration of a network backend device (netdev).
+#
+# @id: Device identifier.
+#
+# @type: Specify the driver used for interpreting remaining arguments.
+#
+# @peer-id: The connected frontend network device name (absent if no frontend
+# is connected).
+#
+# Since: 6.0
+##
+{ 'union': 'NetdevInfo',
+ 'base': { 'id': 'str',
+ 'type': 'NetBackend',
+ '*peer-id': 'str' },
+ 'discriminator': 'type',
+ 'data': {
+ 'bridge': 'NetdevBridgeOptions',
+ 'l2tpv3': 'NetdevL2TPv3Options',
+ 'netmap': 'NetdevNetmapOptions',
+ 'socket': 'NetdevSocketOptions',
+ 'tap': 'NetdevTapOptions',
+ 'user': 'NetdevUserOptions',
+ 'vde': 'NetdevVdeOptions',
+ 'vhost-user': 'NetdevVhostUserOptions',
+ 'vhost-vdpa': 'NetdevVhostVDPAOptions' } }
+
+##
+# @query-netdev:
+#
+# Get a list of @NetdevInfo for all virtual network backend devices (netdevs).
+#
+# Returns: a list of @NetdevInfo describing each netdev.
+#
+# Since: 6.0
+#
+# Example:
+#
+# -> { "execute": "query-netdev" }
+# <- { "return": [
+# {
+# "ipv6": true,
+# "ipv4": true,
+# "host": "10.0.2.2",
+# "ipv6-dns": "fec0::3",
+# "ipv6-prefix": "fec0::",
+# "net": "10.0.2.0/255.255.255.0",
+# "ipv6-host": "fec0::2",
+# "type": "user",
+# "peer-id": "net0",
+# "dns": "10.0.2.3",
+# "hostfwd": [
+# {
+# "str": "tcp::20004-:22"
+# }
+# ],
+# "ipv6-prefixlen": 64,
+# "id": "netdev0",
+# "restrict": false
+# }
+# ]
+# }
+#
+##
+{ 'command': 'query-netdev', 'returns': ['NetdevInfo'] }
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PULL V2 16/20] qapi: net: Add query-netdev command
2021-03-15 9:14 ` [PULL V2 16/20] qapi: net: Add query-netdev command Jason Wang
@ 2021-03-16 21:27 ` Peter Maydell
2021-03-17 4:16 ` Jason Wang
2021-03-16 21:37 ` Peter Maydell
1 sibling, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2021-03-16 21:27 UTC (permalink / raw)
To: Jason Wang; +Cc: Alexey Kirillov, QEMU Developers
On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
>
> From: Alexey Kirillov <lekiravi@yandex-team.ru>
>
> The query-netdev command is used to get the configuration of the current
> network device backends (netdevs).
> This is the QMP analog of the HMP command "info network" but only for
> netdevs (i.e. excluding NIC and hubports).
>
> The query-netdev command returns an array of objects of the NetdevInfo
> type, which are an extension of Netdev type. It means that response can
> be used for netdev-add after small modification. This can be useful for
> recreate the same netdev configuration.
>
> Information about the network device is filled in when it is created or
> modified and is available through the NetClientState->stored_config.
>
> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Hi; Coverity is doubtful (CID 1450841) about this code:
> @@ -668,12 +686,65 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
> + if (!stored->has_fds) {
> + stored->has_fds = true;
> + stored->fds = g_strdup_printf("%d", fd);
> + } else {
> + char *tmp_s = stored->fds;
> + stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
> + g_free(tmp_s);
> + }
Here we have a bit of code which maintains stored->fds as a
colon-separated string of integers, by tacking the new fd onto
the end of the old string if it's already present.
> @@ -731,6 +813,15 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
> }
> options.opaque = (void *)(uintptr_t)vhostfd;
>
> + if (!stored->has_vhostfds) {
> + stored->has_vhostfds = true;
> + stored->vhostfds = g_strdup_printf("%d", vhostfd);
> + } else {
> + char *tmp_s = stored->vhostfds;
> + stored->vhostfds = g_strdup_printf("%s:%d", stored->fds, vhostfd);
> + g_free(tmp_s);
> + }
Here we have a bit of code that's kind of similar, except that
the first argument to g_strdup_printf() is 'stored->fds', not
'stored->vhostfds'.
Coverity suspects cut-n-paste error -- is it right ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PULL V2 16/20] qapi: net: Add query-netdev command
2021-03-16 21:27 ` Peter Maydell
@ 2021-03-17 4:16 ` Jason Wang
0 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-17 4:16 UTC (permalink / raw)
To: Peter Maydell; +Cc: Alexey Kirillov, QEMU Developers
在 2021/3/17 上午5:27, Peter Maydell 写道:
> On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
>> From: Alexey Kirillov <lekiravi@yandex-team.ru>
>>
>> The query-netdev command is used to get the configuration of the current
>> network device backends (netdevs).
>> This is the QMP analog of the HMP command "info network" but only for
>> netdevs (i.e. excluding NIC and hubports).
>>
>> The query-netdev command returns an array of objects of the NetdevInfo
>> type, which are an extension of Netdev type. It means that response can
>> be used for netdev-add after small modification. This can be useful for
>> recreate the same netdev configuration.
>>
>> Information about the network device is filled in when it is created or
>> modified and is available through the NetClientState->stored_config.
>>
>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Hi; Coverity is doubtful (CID 1450841) about this code:
>
>> @@ -668,12 +686,65 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
>> + if (!stored->has_fds) {
>> + stored->has_fds = true;
>> + stored->fds = g_strdup_printf("%d", fd);
>> + } else {
>> + char *tmp_s = stored->fds;
>> + stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
>> + g_free(tmp_s);
>> + }
> Here we have a bit of code which maintains stored->fds as a
> colon-separated string of integers, by tacking the new fd onto
> the end of the old string if it's already present.
>
>> @@ -731,6 +813,15 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
>> }
>> options.opaque = (void *)(uintptr_t)vhostfd;
>>
>> + if (!stored->has_vhostfds) {
>> + stored->has_vhostfds = true;
>> + stored->vhostfds = g_strdup_printf("%d", vhostfd);
>> + } else {
>> + char *tmp_s = stored->vhostfds;
>> + stored->vhostfds = g_strdup_printf("%s:%d", stored->fds, vhostfd);
>> + g_free(tmp_s);
>> + }
> Here we have a bit of code that's kind of similar, except that
> the first argument to g_strdup_printf() is 'stored->fds', not
> 'stored->vhostfds'.
>
> Coverity suspects cut-n-paste error -- is it right ?
Yes, stored->vhostfds should be used here.
Alexey, please send patch to fix this.
Thanks
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL V2 16/20] qapi: net: Add query-netdev command
2021-03-15 9:14 ` [PULL V2 16/20] qapi: net: Add query-netdev command Jason Wang
2021-03-16 21:27 ` Peter Maydell
@ 2021-03-16 21:37 ` Peter Maydell
2021-03-17 3:34 ` Jason Wang
1 sibling, 1 reply; 27+ messages in thread
From: Peter Maydell @ 2021-03-16 21:37 UTC (permalink / raw)
To: Jason Wang; +Cc: Alexey Kirillov, QEMU Developers
On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
>
> From: Alexey Kirillov <lekiravi@yandex-team.ru>
>
> The query-netdev command is used to get the configuration of the current
> network device backends (netdevs).
> This is the QMP analog of the HMP command "info network" but only for
> netdevs (i.e. excluding NIC and hubports).
>
> The query-netdev command returns an array of objects of the NetdevInfo
> type, which are an extension of Netdev type. It means that response can
> be used for netdev-add after small modification. This can be useful for
> recreate the same netdev configuration.
>
> Information about the network device is filled in when it is created or
> modified and is available through the NetClientState->stored_config.
>
> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
Hi; Coverity complains about a memory leak in this code
(CID 1450842):
> @@ -581,15 +693,25 @@ static int net_slirp_init(NetClientState *peer, const char *model,
> s->poll_notifier.notify = net_slirp_poll_notify;
> main_loop_poll_add_notifier(&s->poll_notifier);
>
> + stored_hostfwd = &stored->hostfwd;
> + stored_guestfwd = &stored->guestfwd;
> +
> for (config = slirp_configs; config; config = config->next) {
> + String *element = g_new0(String, 1);
Here we allocate memory...
> +
> + element->str = g_strdup(config->str);
> if (config->flags & SLIRP_CFG_HOSTFWD) {
> if (slirp_hostfwd(s, config->str, errp) < 0) {
> goto error;
...but if we take this error-exit path we have neither freed nor
kept a pointer to that memory.
> }
> + stored->has_hostfwd = true;
> + QAPI_LIST_APPEND(stored_hostfwd, element);
> } else {
> if (slirp_guestfwd(s, config->str, errp) < 0) {
> goto error;
Similarly here.
> }
> + stored->has_guestfwd = true;
> + QAPI_LIST_APPEND(stored_guestfwd, element);
> }
> }
> #ifndef _WIN32
More generally, what state is the net backend init function
supposed to leave 'stored' in if it fails? Is it the backend's
responsibility to free everything that it might have allocated
and left a pointer to? eg if we did
stored->hostname = g_strdup(vhostname);
do we need to go back and free(stored->hostname) ? Or is the caller
guaranteeing to clean up 'stored' somehow ? Or is the backend
supposed to not touch 'stored' until it's sure it's going to
succeed ? (presumably not, as the current code does not do this...)
This commit has no comments describing or documenting the
API requirements the new functionality imposes on a net backend:
could we have a followup patch which adds some documentation,
please, so that authors of future backends know what they have to
implement ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PULL V2 16/20] qapi: net: Add query-netdev command
2021-03-16 21:37 ` Peter Maydell
@ 2021-03-17 3:34 ` Jason Wang
2021-03-25 2:36 ` Jason Wang
0 siblings, 1 reply; 27+ messages in thread
From: Jason Wang @ 2021-03-17 3:34 UTC (permalink / raw)
To: Peter Maydell; +Cc: Alexey Kirillov, QEMU Developers
在 2021/3/17 上午5:37, Peter Maydell 写道:
> On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
>> From: Alexey Kirillov <lekiravi@yandex-team.ru>
>>
>> The query-netdev command is used to get the configuration of the current
>> network device backends (netdevs).
>> This is the QMP analog of the HMP command "info network" but only for
>> netdevs (i.e. excluding NIC and hubports).
>>
>> The query-netdev command returns an array of objects of the NetdevInfo
>> type, which are an extension of Netdev type. It means that response can
>> be used for netdev-add after small modification. This can be useful for
>> recreate the same netdev configuration.
>>
>> Information about the network device is filled in when it is created or
>> modified and is available through the NetClientState->stored_config.
>>
>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>> Acked-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
> Hi; Coverity complains about a memory leak in this code
> (CID 1450842):
>
>> @@ -581,15 +693,25 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>> s->poll_notifier.notify = net_slirp_poll_notify;
>> main_loop_poll_add_notifier(&s->poll_notifier);
>>
>> + stored_hostfwd = &stored->hostfwd;
>> + stored_guestfwd = &stored->guestfwd;
>> +
>> for (config = slirp_configs; config; config = config->next) {
>> + String *element = g_new0(String, 1);
> Here we allocate memory...
>
>> +
>> + element->str = g_strdup(config->str);
>> if (config->flags & SLIRP_CFG_HOSTFWD) {
>> if (slirp_hostfwd(s, config->str, errp) < 0) {
>> goto error;
> ...but if we take this error-exit path we have neither freed nor
> kept a pointer to that memory.
Yes.
>
>> }
>> + stored->has_hostfwd = true;
>> + QAPI_LIST_APPEND(stored_hostfwd, element);
>> } else {
>> if (slirp_guestfwd(s, config->str, errp) < 0) {
>> goto error;
> Similarly here.
>
>> }
>> + stored->has_guestfwd = true;
>> + QAPI_LIST_APPEND(stored_guestfwd, element);
>> }
>> }
>> #ifndef _WIN32
> More generally, what state is the net backend init function
> supposed to leave 'stored' in if it fails? Is it the backend's
> responsibility to free everything that it might have allocated
> and left a pointer to? eg if we did
> stored->hostname = g_strdup(vhostname);
> do we need to go back and free(stored->hostname) ? Or is the caller
> guaranteeing to clean up 'stored' somehow ? Or is the backend
> supposed to not touch 'stored' until it's sure it's going to
> succeed ? (presumably not, as the current code does not do this...)
Clean and free in the function that do the allocation seems better
(self-conatined).
>
> This commit has no comments describing or documenting the
> API requirements the new functionality imposes on a net backend:
> could we have a followup patch which adds some documentation,
> please, so that authors of future backends know what they have to
> implement ?
Alexey, plase send patches to fix the above issues and document the API.
Thanks
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PULL V2 16/20] qapi: net: Add query-netdev command
2021-03-17 3:34 ` Jason Wang
@ 2021-03-25 2:36 ` Jason Wang
0 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-25 2:36 UTC (permalink / raw)
To: Peter Maydell; +Cc: Alexey Kirillov, QEMU Developers
在 2021/3/17 上午11:34, Jason Wang 写道:
>
> 在 2021/3/17 上午5:37, Peter Maydell 写道:
>> On Mon, 15 Mar 2021 at 09:15, Jason Wang <jasowang@redhat.com> wrote:
>>> From: Alexey Kirillov <lekiravi@yandex-team.ru>
>>>
>>> The query-netdev command is used to get the configuration of the
>>> current
>>> network device backends (netdevs).
>>> This is the QMP analog of the HMP command "info network" but only for
>>> netdevs (i.e. excluding NIC and hubports).
>>>
>>> The query-netdev command returns an array of objects of the NetdevInfo
>>> type, which are an extension of Netdev type. It means that response can
>>> be used for netdev-add after small modification. This can be useful for
>>> recreate the same netdev configuration.
>>>
>>> Information about the network device is filled in when it is created or
>>> modified and is available through the NetClientState->stored_config.
>>>
>>> Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
>>> Acked-by: Markus Armbruster <armbru@redhat.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>> Hi; Coverity complains about a memory leak in this code
>> (CID 1450842):
>>
>>> @@ -581,15 +693,25 @@ static int net_slirp_init(NetClientState
>>> *peer, const char *model,
>>> s->poll_notifier.notify = net_slirp_poll_notify;
>>> main_loop_poll_add_notifier(&s->poll_notifier);
>>>
>>> + stored_hostfwd = &stored->hostfwd;
>>> + stored_guestfwd = &stored->guestfwd;
>>> +
>>> for (config = slirp_configs; config; config = config->next) {
>>> + String *element = g_new0(String, 1);
>> Here we allocate memory...
>>
>>> +
>>> + element->str = g_strdup(config->str);
>>> if (config->flags & SLIRP_CFG_HOSTFWD) {
>>> if (slirp_hostfwd(s, config->str, errp) < 0) {
>>> goto error;
>> ...but if we take this error-exit path we have neither freed nor
>> kept a pointer to that memory.
>
>
> Yes.
>
>
>>
>>> }
>>> + stored->has_hostfwd = true;
>>> + QAPI_LIST_APPEND(stored_hostfwd, element);
>>> } else {
>>> if (slirp_guestfwd(s, config->str, errp) < 0) {
>>> goto error;
>> Similarly here.
>>
>>> }
>>> + stored->has_guestfwd = true;
>>> + QAPI_LIST_APPEND(stored_guestfwd, element);
>>> }
>>> }
>>> #ifndef _WIN32
>> More generally, what state is the net backend init function
>> supposed to leave 'stored' in if it fails? Is it the backend's
>> responsibility to free everything that it might have allocated
>> and left a pointer to? eg if we did
>> stored->hostname = g_strdup(vhostname);
>> do we need to go back and free(stored->hostname) ? Or is the caller
>> guaranteeing to clean up 'stored' somehow ? Or is the backend
>> supposed to not touch 'stored' until it's sure it's going to
>> succeed ? (presumably not, as the current code does not do this...)
>
>
> Clean and free in the function that do the allocation seems better
> (self-conatined).
>
>
>>
>> This commit has no comments describing or documenting the
>> API requirements the new functionality imposes on a net backend:
>> could we have a followup patch which adds some documentation,
>> please, so that authors of future backends know what they have to
>> implement ?
>
>
> Alexey, plase send patches to fix the above issues and document the API.
>
> Thanks
Alexey, any update on this. If it takes time I tend to revert this and
let's re-try for 6.1?
Thanks
>
>
>>
>> thanks
>> -- PMM
>>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PULL V2 17/20] tests: Add tests for query-netdev command
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (15 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 16/20] qapi: net: Add query-netdev command Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 18/20] net: Move NetClientState.info_str to dynamic allocations Jason Wang
` (3 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Alexey Kirillov, Jason Wang, qemu-devel
From: Alexey Kirillov <lekiravi@yandex-team.ru>
A simply qtest that checks for correct number of netdevs in the response
of the query-netdev.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
tests/qtest/meson.build | 3 +
tests/qtest/test-query-netdev.c | 120 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 123 insertions(+)
create mode 100644 tests/qtest/test-query-netdev.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 2688e1b..66ee9fb 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -30,6 +30,9 @@ qtests_generic = [
if config_host.has_key('CONFIG_MODULES')
qtests_generic += [ 'modules-test' ]
endif
+if slirp.found()
+ qtests_generic += [ 'test-query-netdev' ]
+endif
qtests_pci = \
(config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : []) + \
diff --git a/tests/qtest/test-query-netdev.c b/tests/qtest/test-query-netdev.c
new file mode 100644
index 0000000..1118537
--- /dev/null
+++ b/tests/qtest/test-query-netdev.c
@@ -0,0 +1,120 @@
+/*
+ * QTest testcase for the query-netdev
+ *
+ * Copyright Yandex N.V., 2019
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "libqos/libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+
+/*
+ * Events can get in the way of responses we are actually waiting for.
+ */
+GCC_FMT_ATTR(2, 3)
+static QObject *wait_command(QTestState *who, const char *command, ...)
+{
+ va_list ap;
+ QDict *response;
+ QObject *result;
+
+ va_start(ap, command);
+ qtest_qmp_vsend(who, command, ap);
+ va_end(ap);
+
+ response = qtest_qmp_receive(who);
+
+ result = qdict_get(response, "return");
+ g_assert(result);
+ qobject_ref(result);
+ qobject_unref(response);
+
+ return result;
+}
+
+static void qmp_query_netdev_no_error(QTestState *qts, size_t netdevs_count)
+{
+ QObject *resp;
+ QList *netdevs;
+
+ resp = wait_command(qts, "{'execute': 'query-netdev'}");
+
+ netdevs = qobject_to(QList, resp);
+ g_assert(netdevs);
+ g_assert(qlist_size(netdevs) == netdevs_count);
+
+ qobject_unref(resp);
+}
+
+static void test_query_netdev(void)
+{
+ const char *arch = qtest_get_arch();
+ QObject *resp;
+ QTestState *state;
+
+ /* Choosing machine for platforms without default one */
+ if (g_str_equal(arch, "arm") ||
+ g_str_equal(arch, "aarch64")) {
+ state = qtest_init(
+ "-nodefaults "
+ "-M virt "
+ "-netdev user,id=slirp0");
+ } else if (g_str_equal(arch, "tricore")) {
+ state = qtest_init(
+ "-nodefaults "
+ "-M tricore_testboard "
+ "-netdev user,id=slirp0");
+ } else if (g_str_equal(arch, "avr")) {
+ state = qtest_init(
+ "-nodefaults "
+ "-M mega2560 "
+ "-netdev user,id=slirp0");
+ } else if (g_str_equal(arch, "rx")) {
+ state = qtest_init(
+ "-nodefaults "
+ "-M gdbsim-r5f562n8 "
+ "-netdev user,id=slirp0");
+ } else {
+ state = qtest_init(
+ "-nodefaults "
+ "-netdev user,id=slirp0");
+ }
+ g_assert(state);
+
+ qmp_query_netdev_no_error(state, 1);
+
+ resp = wait_command(state,
+ "{'execute': 'netdev_add', 'arguments': {"
+ " 'id': 'slirp1',"
+ " 'type': 'user'}}");
+ qobject_unref(resp);
+
+ qmp_query_netdev_no_error(state, 2);
+
+ resp = wait_command(state,
+ "{'execute': 'netdev_del', 'arguments': {"
+ " 'id': 'slirp1'}}");
+ qobject_unref(resp);
+
+ qmp_query_netdev_no_error(state, 1);
+
+ qtest_quit(state);
+}
+
+int main(int argc, char **argv)
+{
+ int ret = 0;
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/net/qapi/query_netdev", test_query_netdev);
+
+ ret = g_test_run();
+
+ return ret;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 18/20] net: Move NetClientState.info_str to dynamic allocations
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (16 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 17/20] tests: Add tests for " Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 19/20] hmp: Use QAPI NetdevInfo in hmp_info_network Jason Wang
` (2 subsequent siblings)
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Alexey Kirillov, Jason Wang, qemu-devel
From: Alexey Kirillov <lekiravi@yandex-team.ru>
The info_str field of the NetClientState structure is static and has a size
of 256 bytes. This amount is often unclaimed, and the field itself is used
exclusively for HMP "info network".
The patch translates info_str to dynamic memory allocation.
This action is also allows us to painlessly discard usage of this field
for backend devices.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/xen_nic.c | 5 ++---
include/net/net.h | 2 +-
net/l2tpv3.c | 3 +--
net/net.c | 14 ++++++++------
net/slirp.c | 5 ++---
net/socket.c | 43 ++++++++++++++++++++++++-------------------
net/tap-win32.c | 3 +--
net/tap.c | 13 +++++--------
net/vde.c | 3 +--
net/vhost-user.c | 3 +--
net/vhost-vdpa.c | 2 +-
11 files changed, 47 insertions(+), 49 deletions(-)
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 5c815b4..8431808 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -296,9 +296,8 @@ static int net_init(struct XenLegacyDevice *xendev)
netdev->nic = qemu_new_nic(&net_xen_info, &netdev->conf,
"xen", NULL, netdev);
- snprintf(qemu_get_queue(netdev->nic)->info_str,
- sizeof(qemu_get_queue(netdev->nic)->info_str),
- "nic: xenbus vif macaddr=%s", netdev->mac);
+ qemu_get_queue(netdev->nic)->info_str = g_strdup_printf(
+ "nic: xenbus vif macaddr=%s", netdev->mac);
/* fill info */
xenstore_write_be_int(&netdev->xendev, "feature-rx-copy", 1);
diff --git a/include/net/net.h b/include/net/net.h
index dc3679f..a02949f 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -94,7 +94,7 @@ struct NetClientState {
NetQueue *incoming_queue;
char *model;
char *name;
- char info_str[256];
+ char *info_str;
NetdevInfo *stored_config;
unsigned receive_disabled : 1;
NetClientDestructor *destructor;
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 8aa0a3e..96611cb 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -730,8 +730,7 @@ int net_init_l2tpv3(const Netdev *netdev,
QAPI_CLONE_MEMBERS(NetdevL2TPv3Options,
&nc->stored_config->u.l2tpv3, l2tpv3);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "l2tpv3: connected");
+ s->nc.info_str = g_strdup_printf("l2tpv3: connected");
return 0;
outerr:
qemu_del_net_client(nc);
diff --git a/net/net.c b/net/net.c
index 9a2a6ab..277da71 100644
--- a/net/net.c
+++ b/net/net.c
@@ -129,11 +129,12 @@ char *qemu_mac_strdup_printf(const uint8_t *macaddr)
void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6])
{
- snprintf(nc->info_str, sizeof(nc->info_str),
- "model=%s,macaddr=%02x:%02x:%02x:%02x:%02x:%02x",
- nc->model,
- macaddr[0], macaddr[1], macaddr[2],
- macaddr[3], macaddr[4], macaddr[5]);
+ g_free(nc->info_str);
+ nc->info_str = g_strdup_printf(
+ "model=%s,macaddr=%02x:%02x:%02x:%02x:%02x:%02x",
+ nc->model,
+ macaddr[0], macaddr[1], macaddr[2],
+ macaddr[3], macaddr[4], macaddr[5]);
}
static int mac_table[256] = {0};
@@ -352,6 +353,7 @@ static void qemu_free_net_client(NetClientState *nc)
}
g_free(nc->name);
g_free(nc->model);
+ g_free(nc->info_str);
qapi_free_NetdevInfo(nc->stored_config);
if (nc->destructor) {
nc->destructor(nc);
@@ -1226,7 +1228,7 @@ void print_net_client(Monitor *mon, NetClientState *nc)
monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
nc->queue_index,
NetClientDriver_str(nc->info->type),
- nc->info_str);
+ nc->info_str ? nc->info_str : "");
if (!QTAILQ_EMPTY(&nc->filters)) {
monitor_printf(mon, "filters:\n");
}
diff --git a/net/slirp.c b/net/slirp.c
index 6ab348b..bfa07e3 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -664,9 +664,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
stored->tftp_server_name = g_strdup(tftp_server_name);
}
- snprintf(nc->info_str, sizeof(nc->info_str),
- "net=%s,restrict=%s", inet_ntoa(net),
- restricted ? "on" : "off");
+ nc->info_str = g_strdup_printf("net=%s,restrict=%s", inet_ntoa(net),
+ restricted ? "on" : "off");
s = DO_UPCAST(SlirpState, nc, nc);
diff --git a/net/socket.c b/net/socket.c
index 1614523..9817234 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -180,7 +180,8 @@ static void net_socket_send(void *opaque)
s->fd = -1;
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
s->nc.link_down = true;
- memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
+ g_free(s->nc.info_str);
+ s->nc.info_str = g_new0(char, 1);
return;
}
@@ -400,16 +401,16 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
stored->mcast = g_strdup(mcast);
s->dgram_dst = saddr;
- snprintf(nc->info_str, sizeof(nc->info_str),
- "socket: fd=%d (cloned mcast=%s:%d)",
- fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+ nc->info_str = g_strdup_printf("socket: fd=%d (cloned mcast=%s:%d)",
+ fd, inet_ntoa(saddr.sin_addr),
+ ntohs(saddr.sin_port));
} else {
if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
s->dgram_dst.sin_family = AF_UNIX;
}
- snprintf(nc->info_str, sizeof(nc->info_str),
- "socket: fd=%d %s", fd, SocketAddressType_str(sa_type));
+ nc->info_str = g_strdup_printf("socket: fd=%d %s",
+ fd, SocketAddressType_str(sa_type));
}
return s;
@@ -444,7 +445,7 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
nc = qemu_new_net_client(&net_socket_info, peer, model, name);
- snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd);
+ nc->info_str = g_strdup_printf("socket: fd=%d", fd);
s = DO_UPCAST(NetSocketState, nc, nc);
@@ -528,9 +529,10 @@ static void net_socket_accept(void *opaque)
stored->has_fd = true;
stored->fd = g_strdup_printf("%d", fd);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: connection from %s:%d",
- inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+ g_free(s->nc.info_str);
+ s->nc.info_str = g_strdup_printf("socket: connection from %s:%d",
+ inet_ntoa(saddr.sin_addr),
+ ntohs(saddr.sin_port));
}
static int net_socket_listen_init(NetClientState *peer,
@@ -645,9 +647,10 @@ static int net_socket_connect_init(NetClientState *peer,
stored->has_connect = true;
stored->connect = g_strdup(host_str);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: connect to %s:%d",
- inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+ g_free(s->nc.info_str);
+ s->nc.info_str = g_strdup_printf("socket: connect to %s:%d",
+ inet_ntoa(saddr.sin_addr),
+ ntohs(saddr.sin_port));
return 0;
}
@@ -704,9 +707,10 @@ static int net_socket_mcast_init(NetClientState *peer,
stored->localaddr = g_strdup(localaddr_str);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: mcast=%s:%d",
- inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port));
+ g_free(s->nc.info_str);
+ s->nc.info_str = g_strdup_printf("socket: mcast=%s:%d",
+ inet_ntoa(saddr.sin_addr),
+ ntohs(saddr.sin_port));
return 0;
}
@@ -769,9 +773,10 @@ static int net_socket_udp_init(NetClientState *peer,
stored->has_udp = true;
stored->udp = g_strdup(rhost);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "socket: udp=%s:%d",
- inet_ntoa(raddr.sin_addr), ntohs(raddr.sin_port));
+ g_free(s->nc.info_str);
+ s->nc.info_str = g_strdup_printf("socket: udp=%s:%d",
+ inet_ntoa(raddr.sin_addr),
+ ntohs(raddr.sin_port));
return 0;
}
diff --git a/net/tap-win32.c b/net/tap-win32.c
index b60933b..0888db8 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -787,8 +787,7 @@ static int tap_win32_init(NetClientState *peer, const char *model,
stored->has_ifname = true;
stored->ifname = g_strdup(ifname);
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "tap: ifname=%s", ifname);
+ s->nc.info_str = g_strdup_printf("tap: ifname=%s", ifname);
s->handle = handle;
diff --git a/net/tap.c b/net/tap.c
index 8041245..f864f43 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -625,8 +625,7 @@ int net_init_bridge(const Netdev *netdev, const char *name,
stored->helper = g_strdup(helper);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s,br=%s", helper,
- br);
+ s->nc.info_str = g_strdup_printf("helper=%s,br=%s", helper, br);
return 0;
}
@@ -714,7 +713,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
g_free(tmp_s);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str), "fd=%d", fd);
+ s->nc.info_str = g_strdup_printf("fd=%d", fd);
} else if (tap->has_helper) {
if (!stored->has_helper) {
stored->has_helper = true;
@@ -727,8 +726,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
g_strdup(DEFAULT_BRIDGE_INTERFACE);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str), "helper=%s",
- tap->helper);
+ s->nc.info_str = g_strdup_printf("helper=%s", tap->helper);
} else {
if (ifname && !stored->has_ifname) {
stored->has_ifname = true;
@@ -745,9 +743,8 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->downscript = g_strdup(downscript);
}
- snprintf(s->nc.info_str, sizeof(s->nc.info_str),
- "ifname=%s,script=%s,downscript=%s", ifname, script,
- downscript);
+ s->nc.info_str = g_strdup_printf("ifname=%s,script=%s,downscript=%s",
+ ifname, script, downscript);
if (strcmp(downscript, "no") != 0) {
snprintf(s->down_script, sizeof(s->down_script), "%s", downscript);
diff --git a/net/vde.c b/net/vde.c
index b0b8800..67de6eb 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -100,8 +100,7 @@ static int net_vde_init(NetClientState *peer, const char *model,
nc = qemu_new_net_client(&net_vde_info, peer, model, name);
- snprintf(nc->info_str, sizeof(nc->info_str), "sock=%s,fd=%d",
- sock, vde_datafd(vde));
+ nc->info_str = g_strdup_printf("sock=%s,fd=%d", sock, vde_datafd(vde));
s = DO_UPCAST(VDEState, nc, nc);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 5b7056b..49c9a74 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -327,8 +327,7 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
user = g_new0(struct VhostUserState, 1);
for (i = 0; i < queues; i++) {
nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
- snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user%d to %s",
- i, chr->label);
+ nc->info_str = g_strdup_printf("vhost-user%d to %s", i, chr->label);
nc->queue_index = i;
if (!nc0) {
nc0 = nc;
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 8c27ea0..423d717 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -200,7 +200,7 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
stored->has_queues = true;
stored->queues = 1; /* TODO: change when support multiqueue */
- snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
+ nc->info_str = g_strdup_printf(TYPE_VHOST_VDPA);
nc->queue_index = 0;
s = DO_UPCAST(VhostVDPAState, nc, nc);
vdpa_device_fd = qemu_open_old(vhostdev, O_RDWR);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 19/20] hmp: Use QAPI NetdevInfo in hmp_info_network
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (17 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 18/20] net: Move NetClientState.info_str to dynamic allocations Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-15 9:14 ` [PULL V2 20/20] net: Do not fill legacy info_str for backends Jason Wang
2021-03-16 12:30 ` [PULL V2 00/20] Net patches Peter Maydell
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Alexey Kirillov, Jason Wang, qemu-devel
From: Alexey Kirillov <lekiravi@yandex-team.ru>
Replace usage of legacy field info_str of NetClientState for backend
network devices with QAPI NetdevInfo stored_config that already used
in QMP query-netdev.
This change increases the detail of the "info network" output and takes
a more general approach to composing the output.
NIC and hubports still use legacy info_str field.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/qapi/hmp-output-visitor.h | 30 ++++++
net/net.c | 31 +++++-
qapi/hmp-output-visitor.c | 193 ++++++++++++++++++++++++++++++++++++++
qapi/meson.build | 1 +
4 files changed, 254 insertions(+), 1 deletion(-)
create mode 100644 include/qapi/hmp-output-visitor.h
create mode 100644 qapi/hmp-output-visitor.c
diff --git a/include/qapi/hmp-output-visitor.h b/include/qapi/hmp-output-visitor.h
new file mode 100644
index 0000000..541e400
--- /dev/null
+++ b/include/qapi/hmp-output-visitor.h
@@ -0,0 +1,30 @@
+/*
+ * HMP string output Visitor
+ *
+ * Copyright Yandex N.V., 2021
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef HMP_OUTPUT_VISITOR_H
+#define HMP_OUTPUT_VISITOR_H
+
+#include "qapi/visitor.h"
+
+typedef struct HMPOutputVisitor HMPOutputVisitor;
+
+/**
+ * Create a HMP string output visitor for @obj
+ *
+ * Flattens dicts/structures, only shows arrays borders.
+ *
+ * Errors are not expected to happen.
+ *
+ * The caller is responsible for freeing the visitor with
+ * visit_free().
+ */
+Visitor *hmp_output_visitor_new(char **result);
+
+#endif
diff --git a/net/net.c b/net/net.c
index 277da71..725a4e1 100644
--- a/net/net.c
+++ b/net/net.c
@@ -55,6 +55,7 @@
#include "sysemu/sysemu.h"
#include "net/filter.h"
#include "qapi/string-output-visitor.h"
+#include "qapi/hmp-output-visitor.h"
/* Net bridge is currently not supported for W32. */
#if !defined(_WIN32)
@@ -1221,14 +1222,42 @@ static void netfilter_print_info(Monitor *mon, NetFilterState *nf)
monitor_printf(mon, "\n");
}
+static char *generate_info_str(NetClientState *nc)
+{
+ NetdevInfo *ni = nc->stored_config;
+ char *ret_out = NULL;
+ Visitor *v;
+
+ /* Use legacy field info_str for NIC and hubports */
+ if ((nc->info->type == NET_CLIENT_DRIVER_NIC) ||
+ (nc->info->type == NET_CLIENT_DRIVER_HUBPORT)) {
+ return g_strdup(nc->info_str ? nc->info_str : "");
+ }
+
+ if (!ni) {
+ return g_malloc0(1);
+ }
+
+ v = hmp_output_visitor_new(&ret_out);
+ if (visit_type_NetdevInfo(v, "", &ni, NULL)) {
+ visit_complete(v, &ret_out);
+ }
+ visit_free(v);
+
+ return ret_out;
+}
+
void print_net_client(Monitor *mon, NetClientState *nc)
{
NetFilterState *nf;
+ char *info_str = generate_info_str(nc);
monitor_printf(mon, "%s: index=%d,type=%s,%s\n", nc->name,
nc->queue_index,
NetClientDriver_str(nc->info->type),
- nc->info_str ? nc->info_str : "");
+ info_str);
+ g_free(info_str);
+
if (!QTAILQ_EMPTY(&nc->filters)) {
monitor_printf(mon, "filters:\n");
}
diff --git a/qapi/hmp-output-visitor.c b/qapi/hmp-output-visitor.c
new file mode 100644
index 0000000..8036605
--- /dev/null
+++ b/qapi/hmp-output-visitor.c
@@ -0,0 +1,193 @@
+/*
+ * HMP string output Visitor
+ *
+ * Copyright Yandex N.V., 2021
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qapi/hmp-output-visitor.h"
+#include "qapi/visitor-impl.h"
+
+struct HMPOutputVisitor {
+ Visitor visitor;
+ char **result;
+ GString *buffer;
+ bool is_continue;
+};
+
+static HMPOutputVisitor *to_hov(Visitor *v)
+{
+ return container_of(v, HMPOutputVisitor, visitor);
+}
+
+static void hmp_output_append_formatted(Visitor *v, const char *fmt, ...)
+{
+ HMPOutputVisitor *ov = to_hov(v);
+ va_list args;
+
+ if (ov->is_continue) {
+ g_string_append(ov->buffer, ",");
+ } else {
+ ov->is_continue = true;
+ }
+
+ va_start(args, fmt);
+ g_string_append_vprintf(ov->buffer, fmt, args);
+ va_end(args);
+}
+
+static void hmp_output_skip_comma(Visitor *v)
+{
+ HMPOutputVisitor *ov = to_hov(v);
+
+ ov->is_continue = false;
+}
+
+static bool hmp_output_start_struct(Visitor *v, const char *name,
+ void **obj, size_t unused, Error **errp)
+{
+ return true;
+}
+
+static void hmp_output_end_struct(Visitor *v, void **obj) {}
+
+static bool hmp_output_start_list(Visitor *v, const char *name,
+ GenericList **listp, size_t size,
+ Error **errp)
+{
+ hmp_output_append_formatted(v, "%s=[", name);
+ /* First element in array without comma before it */
+ hmp_output_skip_comma(v);
+
+ return true;
+}
+
+static GenericList *hmp_output_next_list(Visitor *v, GenericList *tail,
+ size_t size)
+{
+ return tail->next;
+}
+
+static void hmp_output_end_list(Visitor *v, void **obj)
+{
+ /* Don't need comma after last array element */
+ hmp_output_skip_comma(v);
+ hmp_output_append_formatted(v, "]");
+}
+
+static bool hmp_output_type_int64(Visitor *v, const char *name,
+ int64_t *obj, Error **errp)
+{
+ hmp_output_append_formatted(v, "%s=%" PRId64, name, *obj);
+
+ return true;
+}
+
+static bool hmp_output_type_uint64(Visitor *v, const char *name,
+ uint64_t *obj, Error **errp)
+{
+ hmp_output_append_formatted(v, "%s=%" PRIu64, name, *obj);
+
+ return true;
+}
+
+static bool hmp_output_type_bool(Visitor *v, const char *name, bool *obj,
+ Error **errp)
+{
+ hmp_output_append_formatted(v, "%s=%s", name, *obj ? "true" : "false");
+
+ return true;
+}
+
+static bool hmp_output_type_str(Visitor *v, const char *name, char **obj,
+ Error **errp)
+{
+ /* Skip already printed or unused fields */
+ if (!*obj || g_str_equal(name, "id") || g_str_equal(name, "type")) {
+ return true;
+ }
+
+ /* Do not print stub name for StringList elements */
+ if (g_str_equal(name, "str")) {
+ hmp_output_append_formatted(v, "%s", *obj);
+ } else {
+ hmp_output_append_formatted(v, "%s=%s", name, *obj);
+ }
+
+ return true;
+}
+
+static bool hmp_output_type_number(Visitor *v, const char *name,
+ double *obj, Error **errp)
+{
+ hmp_output_append_formatted(v, "%s=%.17g", name, *obj);
+
+ return true;
+}
+
+/* TODO: remove this function? */
+static bool hmp_output_type_any(Visitor *v, const char *name,
+ QObject **obj, Error **errp)
+{
+ return true;
+}
+
+static bool hmp_output_type_null(Visitor *v, const char *name,
+ QNull **obj, Error **errp)
+{
+ hmp_output_append_formatted(v, "%s=NULL", name);
+
+ return true;
+}
+
+static void hmp_output_complete(Visitor *v, void *opaque)
+{
+ HMPOutputVisitor *ov = to_hov(v);
+
+ *ov->result = g_string_free(ov->buffer, false);
+ ov->buffer = NULL;
+}
+
+static void hmp_output_free(Visitor *v)
+{
+ HMPOutputVisitor *ov = to_hov(v);
+
+ if (ov->buffer) {
+ g_string_free(ov->buffer, true);
+ }
+ g_free(v);
+}
+
+Visitor *hmp_output_visitor_new(char **result)
+{
+ HMPOutputVisitor *v;
+
+ v = g_malloc0(sizeof(*v));
+
+ v->visitor.type = VISITOR_OUTPUT;
+ v->visitor.start_struct = hmp_output_start_struct;
+ v->visitor.end_struct = hmp_output_end_struct;
+ v->visitor.start_list = hmp_output_start_list;
+ v->visitor.next_list = hmp_output_next_list;
+ v->visitor.end_list = hmp_output_end_list;
+ v->visitor.type_int64 = hmp_output_type_int64;
+ v->visitor.type_uint64 = hmp_output_type_uint64;
+ v->visitor.type_bool = hmp_output_type_bool;
+ v->visitor.type_str = hmp_output_type_str;
+ v->visitor.type_number = hmp_output_type_number;
+ v->visitor.type_any = hmp_output_type_any;
+ v->visitor.type_null = hmp_output_type_null;
+ v->visitor.complete = hmp_output_complete;
+ v->visitor.free = hmp_output_free;
+
+ v->result = result;
+ v->buffer = g_string_new("");
+ v->is_continue = false;
+
+ return &v->visitor;
+}
diff --git a/qapi/meson.build b/qapi/meson.build
index fcb15a7..d4424ae 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -8,6 +8,7 @@ util_ss.add(files(
'qobject-output-visitor.c',
'string-input-visitor.c',
'string-output-visitor.c',
+ 'hmp-output-visitor.c',
))
if have_system or have_tools
util_ss.add(files(
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL V2 20/20] net: Do not fill legacy info_str for backends
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (18 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 19/20] hmp: Use QAPI NetdevInfo in hmp_info_network Jason Wang
@ 2021-03-15 9:14 ` Jason Wang
2021-03-16 12:30 ` [PULL V2 00/20] Net patches Peter Maydell
20 siblings, 0 replies; 27+ messages in thread
From: Jason Wang @ 2021-03-15 9:14 UTC (permalink / raw)
To: peter.maydell; +Cc: Alexey Kirillov, Jason Wang, qemu-devel
From: Alexey Kirillov <lekiravi@yandex-team.ru>
As we use QAPI NetClientState->stored_config to store and get information
about backend network devices, we can drop fill of legacy field info_str
for them.
We still use info_str field for NIC and hubports, so we can not completely
remove it.
Signed-off-by: Alexey Kirillov <lekiravi@yandex-team.ru>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/l2tpv3.c | 2 --
net/slirp.c | 3 ---
net/socket.c | 28 ----------------------------
net/tap-win32.c | 2 --
net/tap.c | 9 ---------
net/vde.c | 2 --
net/vhost-user.c | 1 -
net/vhost-vdpa.c | 1 -
8 files changed, 48 deletions(-)
diff --git a/net/l2tpv3.c b/net/l2tpv3.c
index 96611cb..b7e1d84 100644
--- a/net/l2tpv3.c
+++ b/net/l2tpv3.c
@@ -729,8 +729,6 @@ int net_init_l2tpv3(const Netdev *netdev,
QAPI_CLONE_MEMBERS(NetdevL2TPv3Options,
&nc->stored_config->u.l2tpv3, l2tpv3);
-
- s->nc.info_str = g_strdup_printf("l2tpv3: connected");
return 0;
outerr:
qemu_del_net_client(nc);
diff --git a/net/slirp.c b/net/slirp.c
index bfa07e3..9454a67 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -664,9 +664,6 @@ static int net_slirp_init(NetClientState *peer, const char *model,
stored->tftp_server_name = g_strdup(tftp_server_name);
}
- nc->info_str = g_strdup_printf("net=%s,restrict=%s", inet_ntoa(net),
- restricted ? "on" : "off");
-
s = DO_UPCAST(SlirpState, nc, nc);
s->slirp = slirp_init(restricted, ipv4, net, mask, host,
diff --git a/net/socket.c b/net/socket.c
index 9817234..c0de10c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -180,8 +180,6 @@ static void net_socket_send(void *opaque)
s->fd = -1;
net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
s->nc.link_down = true;
- g_free(s->nc.info_str);
- s->nc.info_str = g_new0(char, 1);
return;
}
@@ -401,16 +399,10 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer,
stored->mcast = g_strdup(mcast);
s->dgram_dst = saddr;
- nc->info_str = g_strdup_printf("socket: fd=%d (cloned mcast=%s:%d)",
- fd, inet_ntoa(saddr.sin_addr),
- ntohs(saddr.sin_port));
} else {
if (sa_type == SOCKET_ADDRESS_TYPE_UNIX) {
s->dgram_dst.sin_family = AF_UNIX;
}
-
- nc->info_str = g_strdup_printf("socket: fd=%d %s",
- fd, SocketAddressType_str(sa_type));
}
return s;
@@ -445,8 +437,6 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer,
nc = qemu_new_net_client(&net_socket_info, peer, model, name);
- nc->info_str = g_strdup_printf("socket: fd=%d", fd);
-
s = DO_UPCAST(NetSocketState, nc, nc);
s->fd = fd;
@@ -528,11 +518,6 @@ static void net_socket_accept(void *opaque)
stored->has_fd = true;
stored->fd = g_strdup_printf("%d", fd);
-
- g_free(s->nc.info_str);
- s->nc.info_str = g_strdup_printf("socket: connection from %s:%d",
- inet_ntoa(saddr.sin_addr),
- ntohs(saddr.sin_port));
}
static int net_socket_listen_init(NetClientState *peer,
@@ -647,10 +632,6 @@ static int net_socket_connect_init(NetClientState *peer,
stored->has_connect = true;
stored->connect = g_strdup(host_str);
- g_free(s->nc.info_str);
- s->nc.info_str = g_strdup_printf("socket: connect to %s:%d",
- inet_ntoa(saddr.sin_addr),
- ntohs(saddr.sin_port));
return 0;
}
@@ -707,12 +688,7 @@ static int net_socket_mcast_init(NetClientState *peer,
stored->localaddr = g_strdup(localaddr_str);
}
- g_free(s->nc.info_str);
- s->nc.info_str = g_strdup_printf("socket: mcast=%s:%d",
- inet_ntoa(saddr.sin_addr),
- ntohs(saddr.sin_port));
return 0;
-
}
static int net_socket_udp_init(NetClientState *peer,
@@ -773,10 +749,6 @@ static int net_socket_udp_init(NetClientState *peer,
stored->has_udp = true;
stored->udp = g_strdup(rhost);
- g_free(s->nc.info_str);
- s->nc.info_str = g_strdup_printf("socket: udp=%s:%d",
- inet_ntoa(raddr.sin_addr),
- ntohs(raddr.sin_port));
return 0;
}
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 0888db8..21e4511 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -787,8 +787,6 @@ static int tap_win32_init(NetClientState *peer, const char *model,
stored->has_ifname = true;
stored->ifname = g_strdup(ifname);
- s->nc.info_str = g_strdup_printf("tap: ifname=%s", ifname);
-
s->handle = handle;
qemu_add_wait_object(s->handle->tap_semaphore, tap_win32_send, s);
diff --git a/net/tap.c b/net/tap.c
index f864f43..12a08d5 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -625,8 +625,6 @@ int net_init_bridge(const Netdev *netdev, const char *name,
stored->helper = g_strdup(helper);
}
- s->nc.info_str = g_strdup_printf("helper=%s,br=%s", helper, br);
-
return 0;
}
@@ -712,8 +710,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->fds = g_strdup_printf("%s:%d", stored->fds, fd);
g_free(tmp_s);
}
-
- s->nc.info_str = g_strdup_printf("fd=%d", fd);
} else if (tap->has_helper) {
if (!stored->has_helper) {
stored->has_helper = true;
@@ -725,8 +721,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->br = tap->has_br ? g_strdup(tap->br) :
g_strdup(DEFAULT_BRIDGE_INTERFACE);
}
-
- s->nc.info_str = g_strdup_printf("helper=%s", tap->helper);
} else {
if (ifname && !stored->has_ifname) {
stored->has_ifname = true;
@@ -743,9 +737,6 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer,
stored->downscript = g_strdup(downscript);
}
- s->nc.info_str = g_strdup_printf("ifname=%s,script=%s,downscript=%s",
- ifname, script, downscript);
-
if (strcmp(downscript, "no") != 0) {
snprintf(s->down_script, sizeof(s->down_script), "%s", downscript);
snprintf(s->down_script_arg, sizeof(s->down_script_arg),
diff --git a/net/vde.c b/net/vde.c
index 67de6eb..64bdb93 100644
--- a/net/vde.c
+++ b/net/vde.c
@@ -100,8 +100,6 @@ static int net_vde_init(NetClientState *peer, const char *model,
nc = qemu_new_net_client(&net_vde_info, peer, model, name);
- nc->info_str = g_strdup_printf("sock=%s,fd=%d", sock, vde_datafd(vde));
-
s = DO_UPCAST(VDEState, nc, nc);
s->vde = vde;
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 49c9a74..e443c4b 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -327,7 +327,6 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
user = g_new0(struct VhostUserState, 1);
for (i = 0; i < queues; i++) {
nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name);
- nc->info_str = g_strdup_printf("vhost-user%d to %s", i, chr->label);
nc->queue_index = i;
if (!nc0) {
nc0 = nc;
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 423d717..5a28bbc 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -200,7 +200,6 @@ static int net_vhost_vdpa_init(NetClientState *peer, const char *device,
stored->has_queues = true;
stored->queues = 1; /* TODO: change when support multiqueue */
- nc->info_str = g_strdup_printf(TYPE_VHOST_VDPA);
nc->queue_index = 0;
s = DO_UPCAST(VhostVDPAState, nc, nc);
vdpa_device_fd = qemu_open_old(vhostdev, O_RDWR);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PULL V2 00/20] Net patches
2021-03-15 9:14 [PULL V2 00/20] Net patches Jason Wang
` (19 preceding siblings ...)
2021-03-15 9:14 ` [PULL V2 20/20] net: Do not fill legacy info_str for backends Jason Wang
@ 2021-03-16 12:30 ` Peter Maydell
20 siblings, 0 replies; 27+ messages in thread
From: Peter Maydell @ 2021-03-16 12:30 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers
On Mon, 15 Mar 2021 at 09:14, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 6157b0e19721aadb4c7fdcfe57b2924af6144b14:
>
> Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-request' into staging (2021-03-14 17:47:49 +0000)
>
> are available in the git repository at:
>
> https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to f2e8319d456724c3d8514d943dc4607e2f08e88a:
>
> net: Do not fill legacy info_str for backends (2021-03-15 16:41:22 +0800)
>
> ----------------------------------------------------------------
>
> Changes since V1:
> - drop the workaound of "-nic" id and fix the merge
> - add the series of query-netdev
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 27+ messages in thread