* [Qemu-devel] [PULL 1/9] net: pcnet: check rx/tx descriptor ring length
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 2/9] net: pcnet: fix source formatting and indentation Jason Wang
` (8 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Prasad J Pandit, Jason Wang
From: Prasad J Pandit <pjp@fedoraproject.org>
The AMD PC-Net II emulator has set of control and status(CSR)
registers. Of these, CSR76 and CSR78 hold receive and transmit
descriptor ring length respectively. This ring length could range
from 1 to 65535. Setting ring length to zero leads to an infinite
loop in pcnet_rdra_addr() or pcnet_transmit(). Add check to avoid it.
Reported-by: Li Qiang <liqiang6-s@360.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/pcnet.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 198a01f..3078de8 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -1429,8 +1429,11 @@ static void pcnet_csr_writew(PCNetState *s, uint32_t rap, uint32_t new_value)
case 47: /* POLLINT */
case 72:
case 74:
+ break;
case 76: /* RCVRL */
case 78: /* XMTRL */
+ val = (val > 0) ? val : 512;
+ break;
case 112:
if (CSR_STOP(s) || CSR_SPND(s))
break;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 2/9] net: pcnet: fix source formatting and indentation
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 1/9] net: pcnet: check rx/tx descriptor ring length Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 3/9] tap-bsd: OpenBSD uses tap(4) now Jason Wang
` (7 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Prasad J Pandit, Jason Wang
From: Prasad J Pandit <pjp@fedoraproject.org>
Fix indentations and source format at few places. Add braces
around 'if' and 'while' statements.
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/pcnet.c | 130 +++++++++++++++++++++++++++++----------------------------
1 file changed, 67 insertions(+), 63 deletions(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 3078de8..6544553 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -302,7 +302,7 @@ static inline void pcnet_tmd_load(PCNetState *s, struct pcnet_TMD *tmd,
uint32_t tbadr;
int16_t length;
int16_t status;
- } xda;
+ } xda;
s->phys_mem_read(s->dma_opaque, addr, (void *)&xda, sizeof(xda), 0);
tmd->tbadr = le32_to_cpu(xda.tbadr) & 0xffffff;
tmd->length = le16_to_cpu(xda.length);
@@ -664,7 +664,9 @@ static inline int ladr_match(PCNetState *s, const uint8_t *buf, int size)
static inline hwaddr pcnet_rdra_addr(PCNetState *s, int idx)
{
- while (idx < 1) idx += CSR_RCVRL(s);
+ while (idx < 1) {
+ idx += CSR_RCVRL(s);
+ }
return s->rdra + ((CSR_RCVRL(s) - idx) * (BCR_SWSTYLE(s) ? 16 : 8));
}
@@ -672,8 +674,10 @@ static inline int64_t pcnet_get_next_poll_time(PCNetState *s, int64_t current_ti
{
int64_t next_time = current_time +
(65536 - (CSR_SPND(s) ? 0 : CSR_POLL(s))) * 30;
- if (next_time <= current_time)
+
+ if (next_time <= current_time) {
next_time = current_time + 1;
+ }
return next_time;
}
@@ -795,13 +799,13 @@ static void pcnet_init(PCNetState *s)
mode = le16_to_cpu(initblk.mode);
rlen = initblk.rlen >> 4;
tlen = initblk.tlen >> 4;
- ladrf[0] = le16_to_cpu(initblk.ladrf[0]);
- ladrf[1] = le16_to_cpu(initblk.ladrf[1]);
- ladrf[2] = le16_to_cpu(initblk.ladrf[2]);
- ladrf[3] = le16_to_cpu(initblk.ladrf[3]);
- padr[0] = le16_to_cpu(initblk.padr[0]);
- padr[1] = le16_to_cpu(initblk.padr[1]);
- padr[2] = le16_to_cpu(initblk.padr[2]);
+ ladrf[0] = le16_to_cpu(initblk.ladrf[0]);
+ ladrf[1] = le16_to_cpu(initblk.ladrf[1]);
+ ladrf[2] = le16_to_cpu(initblk.ladrf[2]);
+ ladrf[3] = le16_to_cpu(initblk.ladrf[3]);
+ padr[0] = le16_to_cpu(initblk.padr[0]);
+ padr[1] = le16_to_cpu(initblk.padr[1]);
+ padr[2] = le16_to_cpu(initblk.padr[2]);
rdra = le32_to_cpu(initblk.rdra);
tdra = le32_to_cpu(initblk.tdra);
} else {
@@ -809,13 +813,13 @@ static void pcnet_init(PCNetState *s)
s->phys_mem_read(s->dma_opaque, PHYSADDR(s,CSR_IADR(s)),
(uint8_t *)&initblk, sizeof(initblk), 0);
mode = le16_to_cpu(initblk.mode);
- ladrf[0] = le16_to_cpu(initblk.ladrf[0]);
- ladrf[1] = le16_to_cpu(initblk.ladrf[1]);
- ladrf[2] = le16_to_cpu(initblk.ladrf[2]);
- ladrf[3] = le16_to_cpu(initblk.ladrf[3]);
- padr[0] = le16_to_cpu(initblk.padr[0]);
- padr[1] = le16_to_cpu(initblk.padr[1]);
- padr[2] = le16_to_cpu(initblk.padr[2]);
+ ladrf[0] = le16_to_cpu(initblk.ladrf[0]);
+ ladrf[1] = le16_to_cpu(initblk.ladrf[1]);
+ ladrf[2] = le16_to_cpu(initblk.ladrf[2]);
+ ladrf[3] = le16_to_cpu(initblk.ladrf[3]);
+ padr[0] = le16_to_cpu(initblk.padr[0]);
+ padr[1] = le16_to_cpu(initblk.padr[1]);
+ padr[2] = le16_to_cpu(initblk.padr[2]);
rdra = le32_to_cpu(initblk.rdra);
tdra = le32_to_cpu(initblk.tdra);
rlen = rdra >> 29;
@@ -858,12 +862,12 @@ static void pcnet_start(PCNetState *s)
printf("pcnet_start\n");
#endif
- if (!CSR_DTX(s))
+ if (!CSR_DTX(s)) {
s->csr[0] |= 0x0010; /* set TXON */
-
- if (!CSR_DRX(s))
+ }
+ if (!CSR_DRX(s)) {
s->csr[0] |= 0x0020; /* set RXON */
-
+ }
s->csr[0] &= ~0x0004; /* clear STOP bit */
s->csr[0] |= 0x0002;
pcnet_poll_timer(s);
@@ -925,8 +929,7 @@ static void pcnet_rdte_poll(PCNetState *s)
crda);
}
} else {
- printf("pcnet: BAD RMD RDA=0x" TARGET_FMT_plx "\n",
- crda);
+ printf("pcnet: BAD RMD RDA=0x" TARGET_FMT_plx "\n", crda);
#endif
}
}
@@ -1168,10 +1171,11 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
#endif
while (pktcount--) {
- if (CSR_RCVRC(s) <= 1)
+ if (CSR_RCVRC(s) <= 1) {
CSR_RCVRC(s) = CSR_RCVRL(s);
- else
+ } else {
CSR_RCVRC(s)--;
+ }
}
pcnet_rdte_poll(s);
@@ -1207,7 +1211,7 @@ static void pcnet_transmit(PCNetState *s)
s->tx_busy = 1;
- txagain:
+txagain:
if (pcnet_tdte_poll(s)) {
struct pcnet_TMD tmd;
@@ -1251,7 +1255,7 @@ static void pcnet_transmit(PCNetState *s)
s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr),
s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s));
s->xmit_pos += bcnt;
-
+
if (!GET_FIELD(tmd.status, TMDS, ENP)) {
goto txdone;
}
@@ -1276,21 +1280,22 @@ static void pcnet_transmit(PCNetState *s)
s->csr[4] |= 0x0004; /* set TXSTRT */
s->xmit_pos = -1;
- txdone:
+txdone:
SET_FIELD(&tmd.status, TMDS, OWN, 0);
TMDSTORE(&tmd, PHYSADDR(s,CSR_CXDA(s)));
- if (!CSR_TOKINTD(s) || (CSR_LTINTEN(s) && GET_FIELD(tmd.status, TMDS, LTINT)))
+ if (!CSR_TOKINTD(s)
+ || (CSR_LTINTEN(s) && GET_FIELD(tmd.status, TMDS, LTINT))) {
s->csr[0] |= 0x0200; /* set TINT */
-
- if (CSR_XMTRC(s)<=1)
+ }
+ if (CSR_XMTRC(s) <= 1) {
CSR_XMTRC(s) = CSR_XMTRL(s);
- else
+ } else {
CSR_XMTRC(s)--;
- if (count--)
+ }
+ if (count--) {
goto txagain;
-
- } else
- if (s->xmit_pos >= 0) {
+ }
+ } else if (s->xmit_pos >= 0) {
struct pcnet_TMD tmd;
TMDLOAD(&tmd, xmit_cxda);
SET_FIELD(&tmd.misc, TMDM, BUFF, 1);
@@ -1301,9 +1306,9 @@ static void pcnet_transmit(PCNetState *s)
s->csr[0] |= 0x0200; /* set TINT */
if (!CSR_DXSUFLO(s)) {
s->csr[0] &= ~0x0010;
- } else
- if (count--)
- goto txagain;
+ } else if (count--) {
+ goto txagain;
+ }
}
s->tx_busy = 0;
@@ -1315,13 +1320,11 @@ static void pcnet_poll(PCNetState *s)
pcnet_rdte_poll(s);
}
- if (CSR_TDMD(s) ||
- (CSR_TXON(s) && !CSR_DPOLL(s) && pcnet_tdte_poll(s)))
- {
+ if (CSR_TDMD(s) || (CSR_TXON(s) && !CSR_DPOLL(s) && pcnet_tdte_poll(s))) {
/* prevent recursion */
- if (s->tx_busy)
+ if (s->tx_busy) {
return;
-
+ }
pcnet_transmit(s);
}
}
@@ -1340,15 +1343,16 @@ static void pcnet_poll_timer(void *opaque)
if (!CSR_STOP(s) && !CSR_SPND(s) && !CSR_DPOLL(s)) {
uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) * 33;
- if (!s->timer || !now)
+ if (!s->timer || !now) {
s->timer = now;
- else {
+ } else {
uint64_t t = now - s->timer + CSR_POLL(s);
if (t > 0xffffLL) {
pcnet_poll(s);
CSR_POLL(s) = CSR_PINT(s);
- } else
+ } else {
CSR_POLL(s) = t;
+ }
}
timer_mod(s->poll_timer,
pcnet_get_next_poll_time(s,qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)));
@@ -1371,21 +1375,21 @@ static void pcnet_csr_writew(PCNetState *s, uint32_t rap, uint32_t new_value)
val = (val & 0x007f) | (s->csr[0] & 0x7f00);
/* IFF STOP, STRT and INIT are set, clear STRT and INIT */
- if ((val&7) == 7)
- val &= ~3;
-
- if (!CSR_STOP(s) && (val & 4))
+ if ((val & 7) == 7) {
+ val &= ~3;
+ }
+ if (!CSR_STOP(s) && (val & 4)) {
pcnet_stop(s);
-
- if (!CSR_INIT(s) && (val & 1))
+ }
+ if (!CSR_INIT(s) && (val & 1)) {
pcnet_init(s);
-
- if (!CSR_STRT(s) && (val & 2))
+ }
+ if (!CSR_STRT(s) && (val & 2)) {
pcnet_start(s);
-
- if (CSR_TDMD(s))
+ }
+ if (CSR_TDMD(s)) {
pcnet_transmit(s);
-
+ }
return;
case 1:
case 2:
@@ -1435,9 +1439,10 @@ static void pcnet_csr_writew(PCNetState *s, uint32_t rap, uint32_t new_value)
val = (val > 0) ? val : 512;
break;
case 112:
- if (CSR_STOP(s) || CSR_SPND(s))
- break;
- return;
+ if (CSR_STOP(s) || CSR_SPND(s)) {
+ break;
+ }
+ return;
case 3:
break;
case 4:
@@ -1654,8 +1659,7 @@ void pcnet_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
pcnet_bcr_writew(s, s->rap, val & 0xffff);
break;
}
- } else
- if ((addr & 0x0f) == 0) {
+ } else if ((addr & 0x0f) == 0) {
/* switch device to dword i/o mode */
pcnet_bcr_writew(s, BCR_BSBC, pcnet_bcr_readw(s, BCR_BSBC) | 0x0080);
#ifdef PCNET_DEBUG_IO
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 3/9] tap-bsd: OpenBSD uses tap(4) now
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 1/9] net: pcnet: check rx/tx descriptor ring length Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 2/9] net: pcnet: fix source formatting and indentation Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 4/9] net: eepro100: fix memory leak in device uninit Jason Wang
` (6 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Brad Smith, Jason Wang
From: Brad Smith <brad@comstyle.com>
Update the tap-bsd code now that OpenBSD uses tap(4).
Signed-off-by: Brad Smith <brad@comstyle.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/tap-bsd.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/tap-bsd.c b/net/tap-bsd.c
index c506ac3..6c96922 100644
--- a/net/tap-bsd.c
+++ b/net/tap-bsd.c
@@ -35,6 +35,10 @@
#include <net/if_tap.h>
#endif
+#if defined(__OpenBSD__)
+#include <sys/param.h>
+#endif
+
#ifndef __FreeBSD__
int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
int vnet_hdr_required, int mq_required, Error **errp)
@@ -55,7 +59,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr,
if (*ifname) {
snprintf(dname, sizeof dname, "/dev/%s", ifname);
} else {
-#if defined(__OpenBSD__)
+#if defined(__OpenBSD__) && OpenBSD < 201605
snprintf(dname, sizeof dname, "/dev/tun%d", i);
#else
snprintf(dname, sizeof dname, "/dev/tap%d", i);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 4/9] net: eepro100: fix memory leak in device uninit
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (2 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 3/9] tap-bsd: OpenBSD uses tap(4) now Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 5/9] net: rocker: set limit to DMA buffer size Jason Wang
` (5 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Li Qiang, Jason Wang
From: Li Qiang <liqiang6-s@360.cn>
The exit dispatch of eepro100 network card device doesn't free
the 's->vmstate' field which was allocated in device realize thus
leading a host memory leak. This patch avoid this.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/eepro100.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index bab4dbf..4bf71f2 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1843,6 +1843,7 @@ static void pci_nic_uninit(PCIDevice *pci_dev)
EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
vmstate_unregister(&pci_dev->qdev, s->vmstate, s);
+ g_free(s->vmstate);
eeprom93xx_free(&pci_dev->qdev, s->eeprom);
qemu_del_nic(s->nic);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 5/9] net: rocker: set limit to DMA buffer size
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (3 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 4/9] net: eepro100: fix memory leak in device uninit Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 6/9] e1000e: Don't zero out buffer address in rx descriptor Jason Wang
` (4 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Prasad J Pandit, Jason Wang
From: Prasad J Pandit <pjp@fedoraproject.org>
Rocker network switch emulator has test registers to help debug
DMA operations. While testing host DMA access, a buffer address
is written to register 'TEST_DMA_ADDR' and its size is written to
register 'TEST_DMA_SIZE'. When performing TEST_DMA_CTRL_INVERT
test, if DMA buffer size was greater than 'INT_MAX', it leads to
an invalid buffer access. Limit the DMA buffer size to avoid it.
Reported-by: Huawei PSIRT <psirt@huawei.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/rocker/rocker.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 30f2ce4..e9d215a 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -860,7 +860,7 @@ static void rocker_io_writel(void *opaque, hwaddr addr, uint32_t val)
rocker_msix_irq(r, val);
break;
case ROCKER_TEST_DMA_SIZE:
- r->test_dma_size = val;
+ r->test_dma_size = val & 0xFFFF;
break;
case ROCKER_TEST_DMA_ADDR + 4:
r->test_dma_addr = ((uint64_t)val) << 32 | r->lower32;
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 6/9] e1000e: Don't zero out buffer address in rx descriptor
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (4 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 5/9] net: rocker: set limit to DMA buffer size Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 7/9] net: vmxnet: initialise local tx descriptor Jason Wang
` (3 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Kevin Wolf, Jason Wang
From: Kevin Wolf <mail@kevin-wolf.de>
The e1000e emulation zeroes out any used rx descriptor and then writes a
completely newly constructed value there. By doing this, it doesn't only
update the write-back area of the descriptors (as it's supposed to do),
but it also clears the buffer address, which real hardware doesn't do.
The spec explicitly mentions in chapter 7.1.8 that it is valid for a
driver to reuse a descriptor and only update the status field while
doing so, i.e. reusing the old buffer address:
If software statically allocates buffers, and uses memory read to
check for completed descriptors, it simply has to zero the status
byte in the descriptor to make it ready for reuse by hardware.
This patch fixes the behaviour to leave the buffer address in
descriptors unchanged even after the descriptor has been used.
Signed-off-by: Kevin Wolf <mail@kevin-wolf.de>
Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/e1000e_core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 6505983..2b11499 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1278,11 +1278,10 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t *desc,
struct e1000_rx_desc *d = (struct e1000_rx_desc *) desc;
- memset(d, 0, sizeof(*d));
-
assert(!rss_info->enabled);
d->length = cpu_to_le16(length);
+ d->csum = 0;
e1000e_build_rx_metadata(core, pkt, pkt != NULL,
rss_info,
@@ -1291,6 +1290,7 @@ e1000e_write_lgcy_rx_descr(E1000ECore *core, uint8_t *desc,
&d->special);
d->errors = (uint8_t) (le32_to_cpu(status_flags) >> 24);
d->status = (uint8_t) le32_to_cpu(status_flags);
+ d->special = 0;
}
static inline void
@@ -1301,7 +1301,7 @@ e1000e_write_ext_rx_descr(E1000ECore *core, uint8_t *desc,
{
union e1000_rx_desc_extended *d = (union e1000_rx_desc_extended *) desc;
- memset(d, 0, sizeof(*d));
+ memset(&d->wb, 0, sizeof(d->wb));
d->wb.upper.length = cpu_to_le16(length);
@@ -1325,7 +1325,7 @@ e1000e_write_ps_rx_descr(E1000ECore *core, uint8_t *desc,
union e1000_rx_desc_packet_split *d =
(union e1000_rx_desc_packet_split *) desc;
- memset(d, 0, sizeof(*d));
+ memset(&d->wb, 0, sizeof(d->wb));
d->wb.middle.length0 = cpu_to_le16((*written)[0]);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 7/9] net: vmxnet: initialise local tx descriptor
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (5 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 6/9] e1000e: Don't zero out buffer address in rx descriptor Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 8/9] net: rtl8139: limit processing of ring descriptors Jason Wang
` (2 subsequent siblings)
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Li Qiang, Prasad J Pandit, Jason Wang
From: Li Qiang <liqiang6-s@360.cn>
In Vmxnet3 device emulator while processing transmit(tx) queue,
when it reaches end of packet, it calls vmxnet3_complete_packet.
In that local 'txcq_descr' object is not initialised, which could
leak host memory bytes a guest.
Reported-by: Li Qiang <liqiang6-s@360.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
hw/net/vmxnet3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 90f6943..92f6af9 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -531,6 +531,7 @@ static void vmxnet3_complete_packet(VMXNET3State *s, int qidx, uint32_t tx_ridx)
VMXNET3_RING_DUMP(VMW_RIPRN, "TXC", qidx, &s->txq_descr[qidx].comp_ring);
+ memset(&txcq_descr, 0, sizeof(txcq_descr));
txcq_descr.txdIdx = tx_ridx;
txcq_descr.gen = vmxnet3_ring_curr_gen(&s->txq_descr[qidx].comp_ring);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 8/9] net: rtl8139: limit processing of ring descriptors
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (6 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 7/9] net: vmxnet: initialise local tx descriptor Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-26 2:24 ` [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak Jason Wang
2016-10-27 12:31 ` [Qemu-devel] [PULL 0/9] Net patches Peter Maydell
9 siblings, 0 replies; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Prasad J Pandit, Jason Wang
From: Prasad J Pandit <pjp@fedoraproject.org>
RTL8139 ethernet controller in C+ mode supports multiple
descriptor rings, each with maximum of 64 descriptors. While
processing transmit descriptor ring in 'rtl8139_cplus_transmit',
it does not limit the descriptor count and runs forever. Add
check to avoid it.
Reported-by: Andrew Henderson <hendersa@icculus.org>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
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 3345bc6..f05e59c 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -2350,7 +2350,7 @@ static void rtl8139_cplus_transmit(RTL8139State *s)
{
int txcount = 0;
- while (rtl8139_cplus_transmit_one(s))
+ while (txcount < 64 && rtl8139_cplus_transmit_one(s))
{
++txcount;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (7 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 8/9] net: rtl8139: limit processing of ring descriptors Jason Wang
@ 2016-10-26 2:24 ` Jason Wang
2016-10-30 12:13 ` Greg Kurz
2016-10-27 12:31 ` [Qemu-devel] [PULL 0/9] Net patches Peter Maydell
9 siblings, 1 reply; 16+ messages in thread
From: Jason Wang @ 2016-10-26 2:24 UTC (permalink / raw)
To: qemu-devel, peter.maydell; +Cc: Zhang Chen, Jason Wang
From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Fix memory leak in colo-compare.c and filter-rewriter.c
Report by Coverity and add some comments.
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/colo-compare.c | 34 +++++++++++++++-------------------
net/filter-rewriter.c | 17 +++++------------
net/trace-events | 1 +
3 files changed, 21 insertions(+), 31 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 109990f..f791383 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -188,7 +188,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
{
struct tcphdr *ptcp, *stcp;
int res;
- char *sdebug, *ddebug;
trace_colo_compare_main("compare tcp");
if (ppkt->size != spkt->size) {
@@ -219,24 +218,21 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
(spkt->size - ETH_HLEN));
if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
- sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
- ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
- fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
- " s: seq/ack=%u/%u res=%d flags=%x/%x\n",
- __func__, sdebug, ddebug,
- (unsigned int)ntohl(ptcp->th_seq),
- (unsigned int)ntohl(ptcp->th_ack),
- (unsigned int)ntohl(stcp->th_seq),
- (unsigned int)ntohl(stcp->th_ack),
- res, ptcp->th_flags, stcp->th_flags);
-
- fprintf(stderr, "Primary len = %d\n", ppkt->size);
- qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
- fprintf(stderr, "Secondary len = %d\n", spkt->size);
- qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
-
- g_free(sdebug);
- g_free(ddebug);
+ trace_colo_compare_pkt_info(inet_ntoa(ppkt->ip->ip_src),
+ inet_ntoa(ppkt->ip->ip_dst),
+ ntohl(ptcp->th_seq),
+ ntohl(ptcp->th_ack),
+ ntohl(stcp->th_seq),
+ ntohl(stcp->th_ack),
+ res, ptcp->th_flags,
+ stcp->th_flags,
+ ppkt->size,
+ spkt->size);
+
+ qemu_hexdump((char *)ppkt->data, stderr,
+ "colo-compare ppkt", ppkt->size);
+ qemu_hexdump((char *)spkt->data, stderr,
+ "colo-compare spkt", spkt->size);
}
return res;
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 89abe72..c4ab91c 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -68,15 +68,11 @@ static int handle_primary_tcp_pkt(NetFilterState *nf,
tcp_pkt = (struct tcphdr *)pkt->transport_header;
if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
- char *sdebug, *ddebug;
- sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
- ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
- trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
+ trace_colo_filter_rewriter_pkt_info(__func__,
+ inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
tcp_pkt->th_flags);
trace_colo_filter_rewriter_conn_offset(conn->offset);
- g_free(sdebug);
- g_free(ddebug);
}
if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
@@ -116,15 +112,11 @@ static int handle_secondary_tcp_pkt(NetFilterState *nf,
tcp_pkt = (struct tcphdr *)pkt->transport_header;
if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
- char *sdebug, *ddebug;
- sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
- ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
- trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
+ trace_colo_filter_rewriter_pkt_info(__func__,
+ inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
tcp_pkt->th_flags);
trace_colo_filter_rewriter_conn_offset(conn->offset);
- g_free(sdebug);
- g_free(ddebug);
}
if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
@@ -162,6 +154,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
iov_to_buf(iov, iovcnt, 0, buf, size);
pkt = packet_new(buf, size);
+ g_free(buf);
/*
* if we get tcp packet
diff --git a/net/trace-events b/net/trace-events
index d67f048..b1913a6 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -13,6 +13,7 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
colo_old_packet_check_found(int64_t old_time) "%" PRId64
colo_compare_miscompare(void) ""
+colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag, uint32_t sflag, int psize, int ssize) "src/dst: %s/%s p: seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x ppkt_size: %d spkt_size: %d\n"
# net/filter-rewriter.c
colo_filter_rewriter_debug(void) ""
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak
2016-10-26 2:24 ` [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak Jason Wang
@ 2016-10-30 12:13 ` Greg Kurz
2016-10-31 6:37 ` Zhang Chen
0 siblings, 1 reply; 16+ messages in thread
From: Greg Kurz @ 2016-10-30 12:13 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, peter.maydell, Zhang Chen
On Wed, 26 Oct 2016 10:24:11 +0800
Jason Wang <jasowang@redhat.com> wrote:
> From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>
> Fix memory leak in colo-compare.c and filter-rewriter.c
> Report by Coverity and add some comments.
>
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
Hi,
This commit breaks --enable-trace-backends=ust builds in travis (#17):
In file included from ./trace/generated-tracers.h:5966:0,
from /home/travis/build/gkurz/qemu/include/trace.h:4,
from qapi/qapi-visit-core.c:22:
./trace/generated-ust-provider.h:18625:3: error: unknown type name ‘_TP_EXPROTOint’
In file included from /home/travis/build/gkurz/qemu/include/trace.h:4:0,
from qapi/qapi-visit-core.c:22:
./trace/generated-tracers.h: In function ‘trace_colo_compare_pkt_info’:
./trace/generated-tracers.h:19249:432: error: expected string literal before ‘_SDT_ASM_OPERANDS_ssize’
./trace/generated-tracers.h:19249:432: error: implicit declaration of function ‘__tracepoint_cb_qemu___colo_compare_pkt_info’ [-Werror=implicit-function-declaration]
./trace/generated-tracers.h:19249:432: error: nested extern declaration of ‘__tracepoint_cb_qemu___colo_compare_pkt_info’ [-Werror=nested-externs]
cc1: all warnings being treated as errors
make: *** [qapi/qapi-visit-core.o] Error 1
make: *** Waiting for unfinished jobs....
https://travis-ci.org/gkurz/qemu/jobs/171641119
Cheers.
--
Greg
> net/colo-compare.c | 34 +++++++++++++++-------------------
> net/filter-rewriter.c | 17 +++++------------
> net/trace-events | 1 +
> 3 files changed, 21 insertions(+), 31 deletions(-)
>
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 109990f..f791383 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -188,7 +188,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> {
> struct tcphdr *ptcp, *stcp;
> int res;
> - char *sdebug, *ddebug;
>
> trace_colo_compare_main("compare tcp");
> if (ppkt->size != spkt->size) {
> @@ -219,24 +218,21 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
> (spkt->size - ETH_HLEN));
>
> if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
> - sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
> - ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
> - fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
> - " s: seq/ack=%u/%u res=%d flags=%x/%x\n",
> - __func__, sdebug, ddebug,
> - (unsigned int)ntohl(ptcp->th_seq),
> - (unsigned int)ntohl(ptcp->th_ack),
> - (unsigned int)ntohl(stcp->th_seq),
> - (unsigned int)ntohl(stcp->th_ack),
> - res, ptcp->th_flags, stcp->th_flags);
> -
> - fprintf(stderr, "Primary len = %d\n", ppkt->size);
> - qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
> - fprintf(stderr, "Secondary len = %d\n", spkt->size);
> - qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
> -
> - g_free(sdebug);
> - g_free(ddebug);
> + trace_colo_compare_pkt_info(inet_ntoa(ppkt->ip->ip_src),
> + inet_ntoa(ppkt->ip->ip_dst),
> + ntohl(ptcp->th_seq),
> + ntohl(ptcp->th_ack),
> + ntohl(stcp->th_seq),
> + ntohl(stcp->th_ack),
> + res, ptcp->th_flags,
> + stcp->th_flags,
> + ppkt->size,
> + spkt->size);
> +
> + qemu_hexdump((char *)ppkt->data, stderr,
> + "colo-compare ppkt", ppkt->size);
> + qemu_hexdump((char *)spkt->data, stderr,
> + "colo-compare spkt", spkt->size);
> }
>
> return res;
> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
> index 89abe72..c4ab91c 100644
> --- a/net/filter-rewriter.c
> +++ b/net/filter-rewriter.c
> @@ -68,15 +68,11 @@ static int handle_primary_tcp_pkt(NetFilterState *nf,
>
> tcp_pkt = (struct tcphdr *)pkt->transport_header;
> if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
> - char *sdebug, *ddebug;
> - sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
> - ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
> - trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
> + trace_colo_filter_rewriter_pkt_info(__func__,
> + inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
> ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
> tcp_pkt->th_flags);
> trace_colo_filter_rewriter_conn_offset(conn->offset);
> - g_free(sdebug);
> - g_free(ddebug);
> }
>
> if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
> @@ -116,15 +112,11 @@ static int handle_secondary_tcp_pkt(NetFilterState *nf,
> tcp_pkt = (struct tcphdr *)pkt->transport_header;
>
> if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
> - char *sdebug, *ddebug;
> - sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
> - ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
> - trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
> + trace_colo_filter_rewriter_pkt_info(__func__,
> + inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
> ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
> tcp_pkt->th_flags);
> trace_colo_filter_rewriter_conn_offset(conn->offset);
> - g_free(sdebug);
> - g_free(ddebug);
> }
>
> if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
> @@ -162,6 +154,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>
> iov_to_buf(iov, iovcnt, 0, buf, size);
> pkt = packet_new(buf, size);
> + g_free(buf);
>
> /*
> * if we get tcp packet
> diff --git a/net/trace-events b/net/trace-events
> index d67f048..b1913a6 100644
> --- a/net/trace-events
> +++ b/net/trace-events
> @@ -13,6 +13,7 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
> colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
> colo_old_packet_check_found(int64_t old_time) "%" PRId64
> colo_compare_miscompare(void) ""
> +colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag, uint32_t sflag, int psize, int ssize) "src/dst: %s/%s p: seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x ppkt_size: %d spkt_size: %d\n"
>
> # net/filter-rewriter.c
> colo_filter_rewriter_debug(void) ""
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak
2016-10-30 12:13 ` Greg Kurz
@ 2016-10-31 6:37 ` Zhang Chen
2016-10-31 7:40 ` Jason Wang
0 siblings, 1 reply; 16+ messages in thread
From: Zhang Chen @ 2016-10-31 6:37 UTC (permalink / raw)
To: Greg Kurz, Jason Wang; +Cc: qemu-devel, peter.maydell, Stefan Hajnoczi
On 10/30/2016 08:13 PM, Greg Kurz wrote:
> On Wed, 26 Oct 2016 10:24:11 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>
>> Fix memory leak in colo-compare.c and filter-rewriter.c
>> Report by Coverity and add some comments.
>>
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
> Hi,
>
> This commit breaks --enable-trace-backends=ust builds in travis (#17):
>
> In file included from ./trace/generated-tracers.h:5966:0,
> from /home/travis/build/gkurz/qemu/include/trace.h:4,
> from qapi/qapi-visit-core.c:22:
> ./trace/generated-ust-provider.h:18625:3: error: unknown type name ‘_TP_EXPROTOint’
> In file included from /home/travis/build/gkurz/qemu/include/trace.h:4:0,
> from qapi/qapi-visit-core.c:22:
> ./trace/generated-tracers.h: In function ‘trace_colo_compare_pkt_info’:
> ./trace/generated-tracers.h:19249:432: error: expected string literal before ‘_SDT_ASM_OPERANDS_ssize’
> ./trace/generated-tracers.h:19249:432: error: implicit declaration of function ‘__tracepoint_cb_qemu___colo_compare_pkt_info’ [-Werror=implicit-function-declaration]
> ./trace/generated-tracers.h:19249:432: error: nested extern declaration of ‘__tracepoint_cb_qemu___colo_compare_pkt_info’ [-Werror=nested-externs]
> cc1: all warnings being treated as errors
> make: *** [qapi/qapi-visit-core.o] Error 1
> make: *** Waiting for unfinished jobs....
>
> https://travis-ci.org/gkurz/qemu/jobs/171641119
I got your point and test it, same problem in here.
I found that when trace args >10 it not work(this trace have 11 args),
else it works well like that:
colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq,
uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag,
uint32_t sflag, const char *src1) "src/dst: %s/%s p: seq/ack=%u/%u s:
seq/ack=%u/%u res=%d flags=%x/%x %s\n"
So I think this bug could be related to trace system, but I review trace
codes nothing be found about this.
CC: stefan
Thanks
Zhang Chen
>
> Cheers.
>
> --
> Greg
>
>> net/colo-compare.c | 34 +++++++++++++++-------------------
>> net/filter-rewriter.c | 17 +++++------------
>> net/trace-events | 1 +
>> 3 files changed, 21 insertions(+), 31 deletions(-)
>>
>> diff --git a/net/colo-compare.c b/net/colo-compare.c
>> index 109990f..f791383 100644
>> --- a/net/colo-compare.c
>> +++ b/net/colo-compare.c
>> @@ -188,7 +188,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>> {
>> struct tcphdr *ptcp, *stcp;
>> int res;
>> - char *sdebug, *ddebug;
>>
>> trace_colo_compare_main("compare tcp");
>> if (ppkt->size != spkt->size) {
>> @@ -219,24 +218,21 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt)
>> (spkt->size - ETH_HLEN));
>>
>> if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) {
>> - sdebug = strdup(inet_ntoa(ppkt->ip->ip_src));
>> - ddebug = strdup(inet_ntoa(ppkt->ip->ip_dst));
>> - fprintf(stderr, "%s: src/dst: %s/%s p: seq/ack=%u/%u"
>> - " s: seq/ack=%u/%u res=%d flags=%x/%x\n",
>> - __func__, sdebug, ddebug,
>> - (unsigned int)ntohl(ptcp->th_seq),
>> - (unsigned int)ntohl(ptcp->th_ack),
>> - (unsigned int)ntohl(stcp->th_seq),
>> - (unsigned int)ntohl(stcp->th_ack),
>> - res, ptcp->th_flags, stcp->th_flags);
>> -
>> - fprintf(stderr, "Primary len = %d\n", ppkt->size);
>> - qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size);
>> - fprintf(stderr, "Secondary len = %d\n", spkt->size);
>> - qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size);
>> -
>> - g_free(sdebug);
>> - g_free(ddebug);
>> + trace_colo_compare_pkt_info(inet_ntoa(ppkt->ip->ip_src),
>> + inet_ntoa(ppkt->ip->ip_dst),
>> + ntohl(ptcp->th_seq),
>> + ntohl(ptcp->th_ack),
>> + ntohl(stcp->th_seq),
>> + ntohl(stcp->th_ack),
>> + res, ptcp->th_flags,
>> + stcp->th_flags,
>> + ppkt->size,
>> + spkt->size);
>> +
>> + qemu_hexdump((char *)ppkt->data, stderr,
>> + "colo-compare ppkt", ppkt->size);
>> + qemu_hexdump((char *)spkt->data, stderr,
>> + "colo-compare spkt", spkt->size);
>> }
>>
>> return res;
>> diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
>> index 89abe72..c4ab91c 100644
>> --- a/net/filter-rewriter.c
>> +++ b/net/filter-rewriter.c
>> @@ -68,15 +68,11 @@ static int handle_primary_tcp_pkt(NetFilterState *nf,
>>
>> tcp_pkt = (struct tcphdr *)pkt->transport_header;
>> if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
>> - char *sdebug, *ddebug;
>> - sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
>> - ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
>> - trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
>> + trace_colo_filter_rewriter_pkt_info(__func__,
>> + inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
>> ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
>> tcp_pkt->th_flags);
>> trace_colo_filter_rewriter_conn_offset(conn->offset);
>> - g_free(sdebug);
>> - g_free(ddebug);
>> }
>>
>> if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_SYN)) {
>> @@ -116,15 +112,11 @@ static int handle_secondary_tcp_pkt(NetFilterState *nf,
>> tcp_pkt = (struct tcphdr *)pkt->transport_header;
>>
>> if (trace_event_get_state(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
>> - char *sdebug, *ddebug;
>> - sdebug = strdup(inet_ntoa(pkt->ip->ip_src));
>> - ddebug = strdup(inet_ntoa(pkt->ip->ip_dst));
>> - trace_colo_filter_rewriter_pkt_info(__func__, sdebug, ddebug,
>> + trace_colo_filter_rewriter_pkt_info(__func__,
>> + inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
>> ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
>> tcp_pkt->th_flags);
>> trace_colo_filter_rewriter_conn_offset(conn->offset);
>> - g_free(sdebug);
>> - g_free(ddebug);
>> }
>>
>> if (((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == (TH_ACK | TH_SYN))) {
>> @@ -162,6 +154,7 @@ static ssize_t colo_rewriter_receive_iov(NetFilterState *nf,
>>
>> iov_to_buf(iov, iovcnt, 0, buf, size);
>> pkt = packet_new(buf, size);
>> + g_free(buf);
>>
>> /*
>> * if we get tcp packet
>> diff --git a/net/trace-events b/net/trace-events
>> index d67f048..b1913a6 100644
>> --- a/net/trace-events
>> +++ b/net/trace-events
>> @@ -13,6 +13,7 @@ colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
>> colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, spkt size = %d, ip_src = %s, ip_dst = %s"
>> colo_old_packet_check_found(int64_t old_time) "%" PRId64
>> colo_compare_miscompare(void) ""
>> +colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag, uint32_t sflag, int psize, int ssize) "src/dst: %s/%s p: seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x ppkt_size: %d spkt_size: %d\n"
>>
>> # net/filter-rewriter.c
>> colo_filter_rewriter_debug(void) ""
>
>
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak
2016-10-31 6:37 ` Zhang Chen
@ 2016-10-31 7:40 ` Jason Wang
2016-10-31 8:05 ` Zhang Chen
0 siblings, 1 reply; 16+ messages in thread
From: Jason Wang @ 2016-10-31 7:40 UTC (permalink / raw)
To: Zhang Chen, Greg Kurz; +Cc: qemu-devel, peter.maydell, Stefan Hajnoczi
On 2016年10月31日 14:37, Zhang Chen wrote:
>
>
> On 10/30/2016 08:13 PM, Greg Kurz wrote:
>> On Wed, 26 Oct 2016 10:24:11 +0800
>> Jason Wang <jasowang@redhat.com> wrote:
>>
>>> From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>
>>> Fix memory leak in colo-compare.c and filter-rewriter.c
>>> Report by Coverity and add some comments.
>>>
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>> ---
>> Hi,
>>
>> This commit breaks --enable-trace-backends=ust builds in travis (#17):
>>
>> In file included from ./trace/generated-tracers.h:5966:0,
>> from /home/travis/build/gkurz/qemu/include/trace.h:4,
>> from qapi/qapi-visit-core.c:22:
>> ./trace/generated-ust-provider.h:18625:3: error: unknown type name
>> ‘_TP_EXPROTOint’
>> In file included from /home/travis/build/gkurz/qemu/include/trace.h:4:0,
>> from qapi/qapi-visit-core.c:22:
>> ./trace/generated-tracers.h: In function ‘trace_colo_compare_pkt_info’:
>> ./trace/generated-tracers.h:19249:432: error: expected string literal
>> before ‘_SDT_ASM_OPERANDS_ssize’
>> ./trace/generated-tracers.h:19249:432: error: implicit declaration of
>> function ‘__tracepoint_cb_qemu___colo_compare_pkt_info’
>> [-Werror=implicit-function-declaration]
>> ./trace/generated-tracers.h:19249:432: error: nested extern
>> declaration of ‘__tracepoint_cb_qemu___colo_compare_pkt_info’
>> [-Werror=nested-externs]
>> cc1: all warnings being treated as errors
>> make: *** [qapi/qapi-visit-core.o] Error 1
>> make: *** Waiting for unfinished jobs....
>>
>> https://travis-ci.org/gkurz/qemu/jobs/171641119
>
> I got your point and test it, same problem in here.
> I found that when trace args >10 it not work(this trace have 11 args),
> else it works well like that:
>
> colo_compare_pkt_info(const char *src, const char *dst, uint32_t pseq,
> uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t pflag,
> uint32_t sflag, const char *src1) "src/dst: %s/%s p: seq/ack=%u/%u
> s: seq/ack=%u/%u res=%d flags=%x/%x %s\n"
>
> So I think this bug could be related to trace system, but I review
> trace codes nothing be found about this.
> CC: stefan
>
> Thanks
We met similar issue in the past which looks like a limitation of the
backend.
Btw, Alex Bennee has posted a workaround for this, could you please
review or ack on that patch?
Thanks
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak
2016-10-31 7:40 ` Jason Wang
@ 2016-10-31 8:05 ` Zhang Chen
2016-10-31 9:30 ` Greg Kurz
0 siblings, 1 reply; 16+ messages in thread
From: Zhang Chen @ 2016-10-31 8:05 UTC (permalink / raw)
To: Jason Wang, Greg Kurz; +Cc: qemu-devel, peter.maydell, Stefan Hajnoczi
On 10/31/2016 03:40 PM, Jason Wang wrote:
>
>
> On 2016年10月31日 14:37, Zhang Chen wrote:
>>
>>
>> On 10/30/2016 08:13 PM, Greg Kurz wrote:
>>> On Wed, 26 Oct 2016 10:24:11 +0800
>>> Jason Wang <jasowang@redhat.com> wrote:
>>>
>>>> From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>>
>>>> Fix memory leak in colo-compare.c and filter-rewriter.c
>>>> Report by Coverity and add some comments.
>>>>
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>> ---
>>> Hi,
>>>
>>> This commit breaks --enable-trace-backends=ust builds in travis (#17):
>>>
>>> In file included from ./trace/generated-tracers.h:5966:0,
>>> from /home/travis/build/gkurz/qemu/include/trace.h:4,
>>> from qapi/qapi-visit-core.c:22:
>>> ./trace/generated-ust-provider.h:18625:3: error: unknown type name
>>> ‘_TP_EXPROTOint’
>>> In file included from
>>> /home/travis/build/gkurz/qemu/include/trace.h:4:0,
>>> from qapi/qapi-visit-core.c:22:
>>> ./trace/generated-tracers.h: In function ‘trace_colo_compare_pkt_info’:
>>> ./trace/generated-tracers.h:19249:432: error: expected string
>>> literal before ‘_SDT_ASM_OPERANDS_ssize’
>>> ./trace/generated-tracers.h:19249:432: error: implicit declaration
>>> of function ‘__tracepoint_cb_qemu___colo_compare_pkt_info’
>>> [-Werror=implicit-function-declaration]
>>> ./trace/generated-tracers.h:19249:432: error: nested extern
>>> declaration of ‘__tracepoint_cb_qemu___colo_compare_pkt_info’
>>> [-Werror=nested-externs]
>>> cc1: all warnings being treated as errors
>>> make: *** [qapi/qapi-visit-core.o] Error 1
>>> make: *** Waiting for unfinished jobs....
>>>
>>> https://travis-ci.org/gkurz/qemu/jobs/171641119
>>
>> I got your point and test it, same problem in here.
>> I found that when trace args >10 it not work(this trace have 11 args),
>> else it works well like that:
>>
>> colo_compare_pkt_info(const char *src, const char *dst, uint32_t
>> pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t
>> pflag, uint32_t sflag, const char *src1) "src/dst: %s/%s p:
>> seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x %s\n"
>>
>> So I think this bug could be related to trace system, but I review
>> trace codes nothing be found about this.
>> CC: stefan
>>
>> Thanks
>
> We met similar issue in the past which looks like a limitation of the
> backend.
>
> Btw, Alex Bennee has posted a workaround for this, could you please
> review or ack on that patch?
Can you give me a patch name ? and about this patch, should I send a
patch split this trace_event
to two trace_event first ?
Thanks
Zhang Chen
>
> Thanks
>
>
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak
2016-10-31 8:05 ` Zhang Chen
@ 2016-10-31 9:30 ` Greg Kurz
0 siblings, 0 replies; 16+ messages in thread
From: Greg Kurz @ 2016-10-31 9:30 UTC (permalink / raw)
To: Zhang Chen; +Cc: Jason Wang, qemu-devel, peter.maydell, Stefan Hajnoczi
On Mon, 31 Oct 2016 16:05:00 +0800
Zhang Chen <zhangchen.fnst@cn.fujitsu.com> wrote:
> On 10/31/2016 03:40 PM, Jason Wang wrote:
> >
> >
> > On 2016年10月31日 14:37, Zhang Chen wrote:
> >>
> >>
> >> On 10/30/2016 08:13 PM, Greg Kurz wrote:
> >>> On Wed, 26 Oct 2016 10:24:11 +0800
> >>> Jason Wang <jasowang@redhat.com> wrote:
> >>>
> >>>> From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> >>>>
> >>>> Fix memory leak in colo-compare.c and filter-rewriter.c
> >>>> Report by Coverity and add some comments.
> >>>>
> >>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> >>>> Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>>> ---
> >>> Hi,
> >>>
> >>> This commit breaks --enable-trace-backends=ust builds in travis (#17):
> >>>
> >>> In file included from ./trace/generated-tracers.h:5966:0,
> >>> from /home/travis/build/gkurz/qemu/include/trace.h:4,
> >>> from qapi/qapi-visit-core.c:22:
> >>> ./trace/generated-ust-provider.h:18625:3: error: unknown type name
> >>> ‘_TP_EXPROTOint’
> >>> In file included from
> >>> /home/travis/build/gkurz/qemu/include/trace.h:4:0,
> >>> from qapi/qapi-visit-core.c:22:
> >>> ./trace/generated-tracers.h: In function ‘trace_colo_compare_pkt_info’:
> >>> ./trace/generated-tracers.h:19249:432: error: expected string
> >>> literal before ‘_SDT_ASM_OPERANDS_ssize’
> >>> ./trace/generated-tracers.h:19249:432: error: implicit declaration
> >>> of function ‘__tracepoint_cb_qemu___colo_compare_pkt_info’
> >>> [-Werror=implicit-function-declaration]
> >>> ./trace/generated-tracers.h:19249:432: error: nested extern
> >>> declaration of ‘__tracepoint_cb_qemu___colo_compare_pkt_info’
> >>> [-Werror=nested-externs]
> >>> cc1: all warnings being treated as errors
> >>> make: *** [qapi/qapi-visit-core.o] Error 1
> >>> make: *** Waiting for unfinished jobs....
> >>>
> >>> https://travis-ci.org/gkurz/qemu/jobs/171641119
> >>
> >> I got your point and test it, same problem in here.
> >> I found that when trace args >10 it not work(this trace have 11 args),
> >> else it works well like that:
> >>
> >> colo_compare_pkt_info(const char *src, const char *dst, uint32_t
> >> pseq, uint32_t pack, uint32_t sseq, uint32_t sack, int res, uint32_t
> >> pflag, uint32_t sflag, const char *src1) "src/dst: %s/%s p:
> >> seq/ack=%u/%u s: seq/ack=%u/%u res=%d flags=%x/%x %s\n"
> >>
> >> So I think this bug could be related to trace system, but I review
> >> trace codes nothing be found about this.
> >> CC: stefan
> >>
> >> Thanks
> >
> > We met similar issue in the past which looks like a limitation of the
> > backend.
> >
> > Btw, Alex Bennee has posted a workaround for this, could you please
> > review or ack on that patch?
>
> Can you give me a patch name ? and about this patch, should I send a
> patch split this trace_event
> to two trace_event first ?
>
This is what Alex's patch does :)
net: split colo_compare_pkt_info into two trace events
<20161028132559.8324-1-alex.bennee@linaro.org>
> Thanks
> Zhang Chen
>
Cheers.
--
Greg
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PULL 0/9] Net patches
2016-10-26 2:24 [Qemu-devel] [PULL 0/9] Net patches Jason Wang
` (8 preceding siblings ...)
2016-10-26 2:24 ` [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak Jason Wang
@ 2016-10-27 12:31 ` Peter Maydell
9 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-10-27 12:31 UTC (permalink / raw)
To: Jason Wang; +Cc: QEMU Developers
On 26 October 2016 at 03:24, Jason Wang <jasowang@redhat.com> wrote:
> The following changes since commit ede0cbeb7892bdf4a19128853a3a3c61a17fb068:
>
> Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-10-25' into staging (2016-10-25 17:03:11 +0100)
>
> are available in the git repository at:
>
> https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 2061c14c9bea67f8f1fc6bc7acb33c903a0586c1:
>
> colo-proxy: fix memory leak (2016-10-26 09:58:02 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread