[Qemu-devel] [PULL 0/9] Net patches

[Qemu-devel] [PULL 0/9] Net patches

[Jason Wang]

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)

----------------------------------------------------------------

----------------------------------------------------------------
Brad Smith (1):
      tap-bsd: OpenBSD uses tap(4) now

Kevin Wolf (1):
      e1000e: Don't zero out buffer address in rx descriptor

Li Qiang (2):
      net: eepro100: fix memory leak in device uninit
      net: vmxnet: initialise local tx descriptor

Prasad J Pandit (4):
      net: pcnet: check rx/tx descriptor ring length
      net: pcnet: fix source formatting and indentation
      net: rocker: set limit to DMA buffer size
      net: rtl8139: limit processing of ring descriptors

Zhang Chen (1):
      colo-proxy: fix memory leak

 hw/net/e1000e_core.c   |   8 +--
 hw/net/eepro100.c      |   1 +
 hw/net/pcnet.c         | 133 ++++++++++++++++++++++++++-----------------------
 hw/net/rocker/rocker.c |   2 +-
 hw/net/rtl8139.c       |   2 +-
 hw/net/vmxnet3.c       |   1 +
 net/colo-compare.c     |  34 ++++++-------
 net/filter-rewriter.c  |  17 ++-----
 net/tap-bsd.c          |   6 ++-
 net/trace-events       |   1 +
 10 files changed, 104 insertions(+), 101 deletions(-)

-- 
2.7.4

[Qemu-devel] [PULL 1/9] net: pcnet: check rx/tx descriptor ring length

[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

[Qemu-devel] [PULL 2/9] net: pcnet: fix source formatting and indentation

[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

[Qemu-devel] [PULL 3/9] tap-bsd: OpenBSD uses tap(4) now

[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

[Qemu-devel] [PULL 4/9] net: eepro100: fix memory leak in device uninit

[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

[Qemu-devel] [PULL 5/9] net: rocker: set limit to DMA buffer size

[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

[Qemu-devel] [PULL 6/9] e1000e: Don't zero out buffer address in rx descriptor

[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

[Qemu-devel] [PULL 7/9] net: vmxnet: initialise local tx descriptor

[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

[Qemu-devel] [PULL 8/9] net: rtl8139: limit processing of ring descriptors

[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

[Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak

[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

Re: [Qemu-devel] [PULL 0/9] Net patches

[Peter Maydell]

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

Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak

[Greg Kurz]

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) ""

Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak

[Zhang Chen]

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

Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak

[Jason Wang]

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

Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak

[Zhang Chen]

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

Re: [Qemu-devel] [PULL 9/9] colo-proxy: fix memory leak

[Greg Kurz]

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