[PULL V2 00/14] Net patches

[PULL V2 00/14] Net patches

[Jason Wang]

The following changes since commit 2a95551e8b1456aa53ce54fac573df18809340a6:

  Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200330' into staging (2020-03-31 11:20:21 +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 1153cf9f5b67fad41ca6f8571e9a26e2c7c70759:

  qtest: add tulip test case (2020-03-31 21:14:35 +0800)

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

Changes from V1:

- fix the compiling error
- include qtest for tulip OOB

----------------------------------------------------------------
Andrew Melnychenko (1):
      Fixed integer overflow in e1000e

Li Qiang (1):
      qtest: add tulip test case

Peter Maydell (2):
      hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive()
      hw/net/allwinner-sun8i-emac.c: Fix REG_ADDR_HIGH/LOW reads

Philippe Mathieu-Daudé (7):
      hw/net/i82596: Correct command bitmask (CID 1419392)
      hw/net/e1000e_core: Let e1000e_can_receive() return a boolean
      hw/net/smc91c111: Let smc91c111_can_receive() return a boolean
      hw/net/rtl8139: Simplify if/else statement
      hw/net/rtl8139: Update coding style to make checkpatch.pl happy
      hw/net: Make NetCanReceive() return a boolean
      hw/net/can: Make CanBusClientInfo::can_receive() return a boolean

Prasad J Pandit (1):
      net: tulip: check frame size and r/w data length

Zhang Chen (2):
      net/colo-compare.c: Expose "compare_timeout" to users
      net/colo-compare.c: Expose "expired_scan_cycle" to users

 hw/net/allwinner-sun8i-emac.c | 14 +++----
 hw/net/allwinner_emac.c       |  2 +-
 hw/net/cadence_gem.c          |  8 ++--
 hw/net/can/can_sja1000.c      |  8 ++--
 hw/net/can/can_sja1000.h      |  2 +-
 hw/net/dp8393x.c              |  8 ++--
 hw/net/e1000.c                |  2 +-
 hw/net/e1000e.c               |  4 +-
 hw/net/e1000e_core.c          |  2 +-
 hw/net/e1000e_core.h          |  2 +-
 hw/net/ftgmac100.c            |  6 +--
 hw/net/i82596.c               | 66 ++++++++++++++++++++----------
 hw/net/i82596.h               |  2 +-
 hw/net/imx_fec.c              |  2 +-
 hw/net/opencores_eth.c        |  5 +--
 hw/net/rtl8139.c              | 22 +++++-----
 hw/net/smc91c111.c            | 10 ++---
 hw/net/spapr_llan.c           |  4 +-
 hw/net/sungem.c               |  6 +--
 hw/net/sunhme.c               |  4 +-
 hw/net/tulip.c                | 36 ++++++++++++----
 hw/net/virtio-net.c           | 10 ++---
 hw/net/xilinx_ethlite.c       |  2 +-
 include/net/can_emu.h         |  2 +-
 include/net/net.h             |  2 +-
 net/can/can_socketcan.c       |  4 +-
 net/colo-compare.c            | 95 ++++++++++++++++++++++++++++++++++++++++---
 net/filter-buffer.c           |  2 +-
 net/hub.c                     |  6 +--
 qemu-options.hx               | 10 +++--
 tests/qtest/Makefile.include  |  1 +
 tests/qtest/tulip-test.c      | 91 +++++++++++++++++++++++++++++++++++++++++
 32 files changed, 328 insertions(+), 112 deletions(-)
 create mode 100644 tests/qtest/tulip-test.c

[PULL V2 01/14] hw/net/i82596: Correct command bitmask (CID 1419392)

[Jason Wang]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

The command is 32-bit, but we are loading the 16 upper bits with
the 'get_uint16(s->scb + 2)' call.

Once shifted by 16, the command bits match the status bits:

- Command
  Bit 31 ACK-CX   Acknowledges that the CU completed an Action Command.
  Bit 30 ACK-FR   Acknowledges that the RU received a frame.
  Bit 29 ACK-CNA  Acknowledges that the Command Unit became not active.
  Bit 28 ACK-RNR  Acknowledges that the Receive Unit became not ready.

- Status
  Bit 15 CX       The CU finished executing a command with its I(interrupt) bit set.
  Bit 14 FR       The RU finished receiving a frame.
  Bit 13 CNA      The Command Unit left the Active state.
  Bit 12 RNR      The Receive Unit left the Ready state.

Add the SCB_COMMAND_ACK_MASK definition to simplify the code.

This fixes Coverity 1419392 (CONSTANT_EXPRESSION_RESULT):

  /hw/net/i82596.c: 352 in examine_scb()
  346         cuc = (command >> 8) & 0x7;
  347         ruc = (command >> 4) & 0x7;
  348         DBG(printf("MAIN COMMAND %04x  cuc %02x ruc %02x\n", command, cuc, ruc));
  349         /* and clear the scb command word */
  350         set_uint16(s->scb + 2, 0);
  351
  >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
  >>>     "command & (2147483648UL /* 1UL << 31 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
  352         if (command & BIT(31))      /* ACK-CX */
  353             s->scb_status &= ~SCB_STATUS_CX;
  >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
  >>>     "command & (1073741824UL /* 1UL << 30 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
  354         if (command & BIT(30))      /*ACK-FR */
  355             s->scb_status &= ~SCB_STATUS_FR;
  >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
  >>>     "command & (536870912UL /* 1UL << 29 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
  356         if (command & BIT(29))      /*ACK-CNA */
  357             s->scb_status &= ~SCB_STATUS_CNA;
  >>>     CID 1419392:    (CONSTANT_EXPRESSION_RESULT)
  >>>     "command & (268435456UL /* 1UL << 28 */)" is always 0 regardless of the values of its operands. This occurs as the logical operand of "if".
  358         if (command & BIT(28))      /*ACK-RNR */
  359             s->scb_status &= ~SCB_STATUS_RNR;

Fixes: Covertiy CID 1419392 (commit 376b851909)
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/i82596.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index fe9f239..ecdb9aa 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -43,6 +43,9 @@
 #define SCB_STATUS_CNA  0x2000 /* CU left active state */
 #define SCB_STATUS_RNR  0x1000 /* RU left active state */
 
+#define SCB_COMMAND_ACK_MASK \
+        (SCB_STATUS_CX | SCB_STATUS_FR | SCB_STATUS_CNA | SCB_STATUS_RNR)
+
 #define CU_IDLE         0
 #define CU_SUSPENDED    1
 #define CU_ACTIVE       2
@@ -348,14 +351,7 @@ static void examine_scb(I82596State *s)
     /* and clear the scb command word */
     set_uint16(s->scb + 2, 0);
 
-    if (command & BIT(31))      /* ACK-CX */
-        s->scb_status &= ~SCB_STATUS_CX;
-    if (command & BIT(30))      /*ACK-FR */
-        s->scb_status &= ~SCB_STATUS_FR;
-    if (command & BIT(29))      /*ACK-CNA */
-        s->scb_status &= ~SCB_STATUS_CNA;
-    if (command & BIT(28))      /*ACK-RNR */
-        s->scb_status &= ~SCB_STATUS_RNR;
+    s->scb_status &= ~(command & SCB_COMMAND_ACK_MASK);
 
     switch (cuc) {
     case 0:     /* no change */
-- 
2.5.0

[PULL V2 02/14] hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive()

[Jason Wang]

From: Peter Maydell <peter.maydell@linaro.org>

The i82596_receive() function attempts to pass the guest a buffer
which is effectively the concatenation of the data it is passed and a
4 byte CRC value.  However, rather than implementing this as "write
the data; then write the CRC" it instead bumps the length value of
the data by 4, and writes 4 extra bytes from beyond the end of the
buffer, which it then overwrites with the CRC.  It also assumed that
we could always fit all four bytes of the CRC into the final receive
buffer, which might not be true if the CRC needs to be split over two
receive buffers.

Calculate separately how many bytes we need to transfer into the
guest's receive buffer from the source buffer, and how many we need
to transfer from the CRC work.

We add a count 'bufsz' of the number of bytes left in the source
buffer, which we use purely to assert() that we don't overrun.

Spotted by Coverity (CID 1419396) for the specific case when we end
up using a local array as the source buffer.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/i82596.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index ecdb9aa..3b0efd0 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -497,7 +497,8 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
     uint32_t rfd_p;
     uint32_t rbd;
     uint16_t is_broadcast = 0;
-    size_t len = sz;
+    size_t len = sz; /* length of data for guest (including CRC) */
+    size_t bufsz = sz; /* length of data in buf */
     uint32_t crc;
     uint8_t *crc_ptr;
     uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
@@ -591,6 +592,7 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
         if (len < MIN_BUF_SIZE) {
             len = MIN_BUF_SIZE;
         }
+        bufsz = len;
     }
 
     /* Calculate the ethernet checksum (4 bytes) */
@@ -623,6 +625,7 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
         while (len) {
             uint16_t buffer_size, num;
             uint32_t rba;
+            size_t bufcount, crccount;
 
             /* printf("Receive: rbd is %08x\n", rbd); */
             buffer_size = get_uint16(rbd + 12);
@@ -635,14 +638,37 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz)
             }
             rba = get_uint32(rbd + 8);
             /* printf("rba is 0x%x\n", rba); */
-            address_space_write(&address_space_memory, rba,
-                                MEMTXATTRS_UNSPECIFIED, buf, num);
-            rba += num;
-            buf += num;
-            len -= num;
-            if (len == 0) { /* copy crc */
-                address_space_write(&address_space_memory, rba - 4,
-                                    MEMTXATTRS_UNSPECIFIED, crc_ptr, 4);
+            /*
+             * Calculate how many bytes we want from buf[] and how many
+             * from the CRC.
+             */
+            if ((len - num) >= 4) {
+                /* The whole guest buffer, we haven't hit the CRC yet */
+                bufcount = num;
+            } else {
+                /* All that's left of buf[] */
+                bufcount = len - 4;
+            }
+            crccount = num - bufcount;
+
+            if (bufcount > 0) {
+                /* Still some of the actual data buffer to transfer */
+                assert(bufsz >= bufcount);
+                bufsz -= bufcount;
+                address_space_write(&address_space_memory, rba,
+                                    MEMTXATTRS_UNSPECIFIED, buf, bufcount);
+                rba += bufcount;
+                buf += bufcount;
+                len -= bufcount;
+            }
+
+            /* Write as much of the CRC as fits */
+            if (crccount > 0) {
+                address_space_write(&address_space_memory, rba,
+                                    MEMTXATTRS_UNSPECIFIED, crc_ptr, crccount);
+                rba += crccount;
+                crc_ptr += crccount;
+                len -= crccount;
             }
 
             num |= 0x4000; /* set F BIT */
-- 
2.5.0

[PULL V2 03/14] Fixed integer overflow in e1000e

[Jason Wang]

From: Andrew Melnychenko <andrew@daynix.com>

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1737400
Fixed setting max_queue_num if there are no peers in
NICConf. qemu_new_nic() creates NICState with 1 NetClientState(index
0) without peers, set max_queue_num to 0 - It prevents undefined
behavior and possible crashes, especially during pcie hotplug.

Fixes: 6f3fbe4ed06 ("net: Introduce e1000e device emulation")
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Dmitry Fleytman <dmitry.fleytman@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/e1000e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index a91dbdc..f2cc155 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -328,7 +328,7 @@ e1000e_init_net_peer(E1000EState *s, PCIDevice *pci_dev, uint8_t *macaddr)
     s->nic = qemu_new_nic(&net_e1000e_info, &s->conf,
         object_get_typename(OBJECT(s)), dev->id, s);
 
-    s->core.max_queue_num = s->conf.peers.queues - 1;
+    s->core.max_queue_num = s->conf.peers.queues ? s->conf.peers.queues - 1 : 0;
 
     trace_e1000e_mac_set_permanent(MAC_ARG(macaddr));
     memcpy(s->core.permanent_mac, macaddr, sizeof(s->core.permanent_mac));
-- 
2.5.0

[PULL V2 04/14] hw/net/e1000e_core: Let e1000e_can_receive() return a boolean

[Jason Wang]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The e1000e_can_receive() function simply returns a boolean value.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/e1000e_core.c | 2 +-
 hw/net/e1000e_core.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index df957e0..d567687 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -967,7 +967,7 @@ e1000e_start_recv(E1000ECore *core)
     }
 }
 
-int
+bool
 e1000e_can_receive(E1000ECore *core)
 {
     int i;
diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
index 49abb13..aee32f7 100644
--- a/hw/net/e1000e_core.h
+++ b/hw/net/e1000e_core.h
@@ -143,7 +143,7 @@ e1000e_core_set_link_status(E1000ECore *core);
 void
 e1000e_core_pci_uninit(E1000ECore *core);
 
-int
+bool
 e1000e_can_receive(E1000ECore *core);
 
 ssize_t
-- 
2.5.0

[PULL V2 05/14] hw/net/smc91c111: Let smc91c111_can_receive() return a boolean

[Jason Wang]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The smc91c111_can_receive() function simply returns a boolean value.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/smc91c111.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index e9eb6f6..02be60c 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -130,16 +130,16 @@ static void smc91c111_update(smc91c111_state *s)
     qemu_set_irq(s->irq, level);
 }
 
-static int smc91c111_can_receive(smc91c111_state *s)
+static bool smc91c111_can_receive(smc91c111_state *s)
 {
     if ((s->rcr & RCR_RXEN) == 0 || (s->rcr & RCR_SOFT_RST)) {
-        return 1;
+        return true;
     }
     if (s->allocated == (1 << NUM_PACKETS) - 1 ||
         s->rx_fifo_len == NUM_PACKETS) {
-        return 0;
+        return false;
     }
-    return 1;
+    return true;
 }
 
 static inline void smc91c111_flush_queued_packets(smc91c111_state *s)
-- 
2.5.0

[PULL V2 06/14] hw/net/rtl8139: Simplify if/else statement

[Jason Wang]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Rewrite:

      if (E) {
          return A;
      } else {
          return B;
      }
      /* EOF */
  }

as:

      if (E) {
          return A;
      }
      return B;
  }

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/rtl8139.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index ae4739b..ef32115 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -808,11 +808,11 @@ static int rtl8139_can_receive(NetClientState *nc)
         /* ??? Flow control not implemented in c+ mode.
            This is a hack to work around slirp deficiencies anyway.  */
         return 1;
-    } else {
-        avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr,
-                     s->RxBufferSize);
-        return (avail == 0 || avail >= 1514 || (s->IntrMask & RxOverflow));
     }
+
+    avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr,
+                 s->RxBufferSize);
+    return avail == 0 || avail >= 1514 || (s->IntrMask & RxOverflow);
 }
 
 static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t size_, int do_interrupt)
-- 
2.5.0

[PULL V2 07/14] hw/net/rtl8139: Update coding style to make checkpatch.pl happy

[Jason Wang]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

We will modify this code in the next commit. Clean it up
first to avoid checkpatch.pl errors.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/rtl8139.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index ef32115..be9a0af 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -799,10 +799,12 @@ static int rtl8139_can_receive(NetClientState *nc)
     int avail;
 
     /* Receive (drop) packets if card is disabled.  */
-    if (!s->clock_enabled)
-      return 1;
-    if (!rtl8139_receiver_enabled(s))
-      return 1;
+    if (!s->clock_enabled) {
+        return 1;
+    }
+    if (!rtl8139_receiver_enabled(s)) {
+        return 1;
+    }
 
     if (rtl8139_cp_receiver_enabled(s) && rtl8139_cp_rx_valid(s)) {
         /* ??? Flow control not implemented in c+ mode.
-- 
2.5.0

[PULL V2 08/14] hw/net: Make NetCanReceive() return a boolean

[Jason Wang]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The NetCanReceive handler return whether the device can or
can not receive new packets. Make it obvious by returning
a boolean type.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/allwinner_emac.c |  2 +-
 hw/net/cadence_gem.c    |  8 ++++----
 hw/net/dp8393x.c        |  8 +++-----
 hw/net/e1000.c          |  2 +-
 hw/net/e1000e.c         |  2 +-
 hw/net/ftgmac100.c      |  6 +++---
 hw/net/i82596.c         | 10 +++++-----
 hw/net/i82596.h         |  2 +-
 hw/net/imx_fec.c        |  2 +-
 hw/net/opencores_eth.c  |  5 ++---
 hw/net/rtl8139.c        |  8 ++++----
 hw/net/smc91c111.c      |  2 +-
 hw/net/spapr_llan.c     |  4 ++--
 hw/net/sungem.c         |  6 +++---
 hw/net/sunhme.c         |  4 ++--
 hw/net/virtio-net.c     | 10 +++++-----
 hw/net/xilinx_ethlite.c |  2 +-
 include/net/net.h       |  2 +-
 net/filter-buffer.c     |  2 +-
 net/hub.c               |  6 +++---
 20 files changed, 45 insertions(+), 48 deletions(-)

diff --git a/hw/net/allwinner_emac.c b/hw/net/allwinner_emac.c
index e9bbff8..ddddf35 100644
--- a/hw/net/allwinner_emac.c
+++ b/hw/net/allwinner_emac.c
@@ -178,7 +178,7 @@ static uint32_t fifo8_pop_word(Fifo8 *fifo)
     return ret;
 }
 
-static int aw_emac_can_receive(NetClientState *nc)
+static bool aw_emac_can_receive(NetClientState *nc)
 {
     AwEmacState *s = qemu_get_nic_opaque(nc);
 
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 6340c1e..51ec5a0 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -505,7 +505,7 @@ static void phy_update_link(CadenceGEMState *s)
     }
 }
 
-static int gem_can_receive(NetClientState *nc)
+static bool gem_can_receive(NetClientState *nc)
 {
     CadenceGEMState *s;
     int i;
@@ -518,7 +518,7 @@ static int gem_can_receive(NetClientState *nc)
             s->can_rx_state = 1;
             DB_PRINT("can't receive - no enable\n");
         }
-        return 0;
+        return false;
     }
 
     for (i = 0; i < s->num_priority_queues; i++) {
@@ -532,14 +532,14 @@ static int gem_can_receive(NetClientState *nc)
             s->can_rx_state = 2;
             DB_PRINT("can't receive - all the buffer descriptors are busy\n");
         }
-        return 0;
+        return false;
     }
 
     if (s->can_rx_state != 0) {
         s->can_rx_state = 0;
         DB_PRINT("can receive\n");
     }
-    return 1;
+    return true;
 }
 
 /*
diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 1563c11..c54db0d 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -414,7 +414,7 @@ static void dp8393x_do_stop_timer(dp8393xState *s)
     dp8393x_update_wt_regs(s);
 }
 
-static int dp8393x_can_receive(NetClientState *nc);
+static bool dp8393x_can_receive(NetClientState *nc);
 
 static void dp8393x_do_receiver_enable(dp8393xState *s)
 {
@@ -718,13 +718,11 @@ static void dp8393x_watchdog(void *opaque)
     dp8393x_update_irq(s);
 }
 
-static int dp8393x_can_receive(NetClientState *nc)
+static bool dp8393x_can_receive(NetClientState *nc)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
 
-    if (!(s->regs[SONIC_CR] & SONIC_CR_RXEN))
-        return 0;
-    return 1;
+    return !!(s->regs[SONIC_CR] & SONIC_CR_RXEN);
 }
 
 static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 9233248..2a69eee 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -845,7 +845,7 @@ static bool e1000_has_rxbufs(E1000State *s, size_t total_size)
     return total_size <= bufs * s->rxbuf_size;
 }
 
-static int
+static bool
 e1000_can_receive(NetClientState *nc)
 {
     E1000State *s = qemu_get_nic_opaque(nc);
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index f2cc155..79ba158 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -199,7 +199,7 @@ static const MemoryRegionOps io_ops = {
     },
 };
 
-static int
+static bool
 e1000e_nc_can_receive(NetClientState *nc)
 {
     E1000EState *s = qemu_get_nic_opaque(nc);
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 2f92b65..041ed21 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -562,18 +562,18 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
     ftgmac100_update_irq(s);
 }
 
-static int ftgmac100_can_receive(NetClientState *nc)
+static bool ftgmac100_can_receive(NetClientState *nc)
 {
     FTGMAC100State *s = FTGMAC100(qemu_get_nic_opaque(nc));
     FTGMAC100Desc bd;
 
     if ((s->maccr & (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN))
          != (FTGMAC100_MACCR_RXDMA_EN | FTGMAC100_MACCR_RXMAC_EN)) {
-        return 0;
+        return false;
     }
 
     if (ftgmac100_read_bd(&bd, s->rx_descriptor)) {
-        return 0;
+        return false;
     }
     return !(bd.des0 & FTGMAC100_RXDES0_RXPKT_RDY);
 }
diff --git a/hw/net/i82596.c b/hw/net/i82596.c
index 3b0efd0..055c3a1 100644
--- a/hw/net/i82596.c
+++ b/hw/net/i82596.c
@@ -470,23 +470,23 @@ void i82596_h_reset(void *opaque)
     i82596_s_reset(s);
 }
 
-int i82596_can_receive(NetClientState *nc)
+bool i82596_can_receive(NetClientState *nc)
 {
     I82596State *s = qemu_get_nic_opaque(nc);
 
     if (s->rx_status == RX_SUSPENDED) {
-        return 0;
+        return false;
     }
 
     if (!s->lnkst) {
-        return 0;
+        return false;
     }
 
     if (USE_TIMER && !timer_pending(s->flush_queue_timer)) {
-        return 1;
+        return true;
     }
 
-    return 1;
+    return true;
 }
 
 #define MIN_BUF_SIZE 60
diff --git a/hw/net/i82596.h b/hw/net/i82596.h
index 1238ac1..f0bbe81 100644
--- a/hw/net/i82596.h
+++ b/hw/net/i82596.h
@@ -48,7 +48,7 @@ void i82596_ioport_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t i82596_ioport_readl(void *opaque, uint32_t addr);
 uint32_t i82596_bcr_readw(I82596State *s, uint32_t rap);
 ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t size_);
-int i82596_can_receive(NetClientState *nc);
+bool i82596_can_receive(NetClientState *nc);
 void i82596_set_link_status(NetClientState *nc);
 void i82596_common_init(DeviceState *dev, I82596State *s, NetClientInfo *info);
 extern const VMStateDescription vmstate_i82596;
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 5c145a8..a35c336 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -1049,7 +1049,7 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
     imx_eth_update(s);
 }
 
-static int imx_eth_can_receive(NetClientState *nc)
+static bool imx_eth_can_receive(NetClientState *nc)
 {
     IMXFECState *s = IMX_FEC(qemu_get_nic_opaque(nc));
 
diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c
index 6b338c2..2ba0dc8 100644
--- a/hw/net/opencores_eth.c
+++ b/hw/net/opencores_eth.c
@@ -349,12 +349,11 @@ static void open_eth_reset(void *opaque)
     open_eth_set_link_status(qemu_get_queue(s->nic));
 }
 
-static int open_eth_can_receive(NetClientState *nc)
+static bool open_eth_can_receive(NetClientState *nc)
 {
     OpenEthState *s = qemu_get_nic_opaque(nc);
 
-    return GET_REGBIT(s, MODER, RXEN) &&
-        (s->regs[TX_BD_NUM] < 0x80);
+    return GET_REGBIT(s, MODER, RXEN) && (s->regs[TX_BD_NUM] < 0x80);
 }
 
 static ssize_t open_eth_receive(NetClientState *nc,
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c
index be9a0af..70aca7e 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -793,23 +793,23 @@ static bool rtl8139_cp_rx_valid(RTL8139State *s)
     return !(s->RxRingAddrLO == 0 && s->RxRingAddrHI == 0);
 }
 
-static int rtl8139_can_receive(NetClientState *nc)
+static bool rtl8139_can_receive(NetClientState *nc)
 {
     RTL8139State *s = qemu_get_nic_opaque(nc);
     int avail;
 
     /* Receive (drop) packets if card is disabled.  */
     if (!s->clock_enabled) {
-        return 1;
+        return true;
     }
     if (!rtl8139_receiver_enabled(s)) {
-        return 1;
+        return true;
     }
 
     if (rtl8139_cp_receiver_enabled(s) && rtl8139_cp_rx_valid(s)) {
         /* ??? Flow control not implemented in c+ mode.
            This is a hack to work around slirp deficiencies anyway.  */
-        return 1;
+        return true;
     }
 
     avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr,
diff --git a/hw/net/smc91c111.c b/hw/net/smc91c111.c
index 02be60c..b3240b9 100644
--- a/hw/net/smc91c111.c
+++ b/hw/net/smc91c111.c
@@ -667,7 +667,7 @@ static void smc91c111_writefn(void *opaque, hwaddr addr,
     }
 }
 
-static int smc91c111_can_receive_nc(NetClientState *nc)
+static bool smc91c111_can_receive_nc(NetClientState *nc)
 {
     smc91c111_state *s = qemu_get_nic_opaque(nc);
 
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 80f5a1d..a237702 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -110,11 +110,11 @@ typedef struct SpaprVioVlan {
     RxBufPool *rx_pool[RX_MAX_POOLS];  /* Receive buffer descriptor pools */
 } SpaprVioVlan;
 
-static int spapr_vlan_can_receive(NetClientState *nc)
+static bool spapr_vlan_can_receive(NetClientState *nc)
 {
     SpaprVioVlan *dev = qemu_get_nic_opaque(nc);
 
-    return (dev->isopen && dev->rx_bufs > 0);
+    return dev->isopen && dev->rx_bufs > 0;
 }
 
 /**
diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 89da51f..b01197d 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -433,7 +433,7 @@ static bool sungem_rx_full(SunGEMState *s, uint32_t kick, uint32_t done)
     return kick == ((done + 1) & s->rx_mask);
 }
 
-static int sungem_can_receive(NetClientState *nc)
+static bool sungem_can_receive(NetClientState *nc)
 {
     SunGEMState *s = qemu_get_nic_opaque(nc);
     uint32_t kick, done, rxdma_cfg, rxmac_cfg;
@@ -445,11 +445,11 @@ static int sungem_can_receive(NetClientState *nc)
     /* If MAC disabled, can't receive */
     if ((rxmac_cfg & MAC_RXCFG_ENAB) == 0) {
         trace_sungem_rx_mac_disabled();
-        return 0;
+        return false;
     }
     if ((rxdma_cfg & RXDMA_CFG_ENABLE) == 0) {
         trace_sungem_rx_txdma_disabled();
-        return 0;
+        return false;
     }
 
     /* Check RX availability */
diff --git a/hw/net/sunhme.c b/hw/net/sunhme.c
index 8863601..9c38583 100644
--- a/hw/net/sunhme.c
+++ b/hw/net/sunhme.c
@@ -657,11 +657,11 @@ static void sunhme_transmit(SunHMEState *s)
     sunhme_update_irq(s);
 }
 
-static int sunhme_can_receive(NetClientState *nc)
+static bool sunhme_can_receive(NetClientState *nc)
 {
     SunHMEState *s = qemu_get_nic_opaque(nc);
 
-    return s->macregs[HME_MACI_RXCFG >> 2] & HME_MAC_RXCFG_ENABLE;
+    return !!(s->macregs[HME_MACI_RXCFG >> 2] & HME_MAC_RXCFG_ENABLE);
 }
 
 static void sunhme_link_status_changed(NetClientState *nc)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3627bb1..a46e3b3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1234,26 +1234,26 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq)
     qemu_flush_queued_packets(qemu_get_subqueue(n->nic, queue_index));
 }
 
-static int virtio_net_can_receive(NetClientState *nc)
+static bool virtio_net_can_receive(NetClientState *nc)
 {
     VirtIONet *n = qemu_get_nic_opaque(nc);
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     VirtIONetQueue *q = virtio_net_get_subqueue(nc);
 
     if (!vdev->vm_running) {
-        return 0;
+        return false;
     }
 
     if (nc->queue_index >= n->curr_queues) {
-        return 0;
+        return false;
     }
 
     if (!virtio_queue_ready(q->rx_vq) ||
         !(vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)) {
-        return 0;
+        return false;
     }
 
-    return 1;
+    return true;
 }
 
 static int virtio_net_has_buffers(VirtIONetQueue *q, int bufsize)
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index cf07e69..71d16fe 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -175,7 +175,7 @@ static const MemoryRegionOps eth_ops = {
     }
 };
 
-static int eth_can_rx(NetClientState *nc)
+static bool eth_can_rx(NetClientState *nc)
 {
     struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
     unsigned int rxbase = s->rxbuf * (0x800 / 4);
diff --git a/include/net/net.h b/include/net/net.h
index 094e966..39085d9 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -42,7 +42,7 @@ typedef struct NICConf {
 /* Net clients */
 
 typedef void (NetPoll)(NetClientState *, bool enable);
-typedef int (NetCanReceive)(NetClientState *);
+typedef bool (NetCanReceive)(NetClientState *);
 typedef ssize_t (NetReceive)(NetClientState *, const uint8_t *, size_t);
 typedef ssize_t (NetReceiveIOV)(NetClientState *, const struct iovec *, int);
 typedef void (NetCleanup) (NetClientState *);
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 88da78f..12e0254 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -74,7 +74,7 @@ static ssize_t filter_buffer_receive_iov(NetFilterState *nf,
      * the filter can still accept packets until its internal queue is full.
      * For example:
      *   For some reason, receiver could not receive more packets
-     * (.can_receive() returns zero). Without a filter, at most one packet
+     * (.can_receive() returns false). Without a filter, at most one packet
      * will be queued in incoming queue and sender's poll will be disabled
      * unit its sent_cb() was called. With a filter, it will keep receiving
      * the packets without caring about the receiver. This is suboptimal.
diff --git a/net/hub.c b/net/hub.c
index 88cfb87..1375738 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -90,7 +90,7 @@ static NetHub *net_hub_new(int id)
     return hub;
 }
 
-static int net_hub_port_can_receive(NetClientState *nc)
+static bool net_hub_port_can_receive(NetClientState *nc)
 {
     NetHubPort *port;
     NetHubPort *src_port = DO_UPCAST(NetHubPort, nc, nc);
@@ -102,11 +102,11 @@ static int net_hub_port_can_receive(NetClientState *nc)
         }
 
         if (qemu_can_send_packet(&port->nc)) {
-            return 1;
+            return true;
         }
     }
 
-    return 0;
+    return false;
 }
 
 static ssize_t net_hub_port_receive(NetClientState *nc,
-- 
2.5.0

[PULL V2 09/14] hw/net/can: Make CanBusClientInfo::can_receive() return a boolean

[Jason Wang]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The CanBusClientInfo::can_receive handler return whether the
device can or can not receive new frames. Make it obvious by
returning a boolean type.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/allwinner-sun8i-emac.c | 2 +-
 hw/net/can/can_sja1000.c      | 8 ++++----
 hw/net/can/can_sja1000.h      | 2 +-
 include/net/can_emu.h         | 2 +-
 net/can/can_socketcan.c       | 4 ++--
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
index 3fc5e34..033eaa8 100644
--- a/hw/net/allwinner-sun8i-emac.c
+++ b/hw/net/allwinner-sun8i-emac.c
@@ -395,7 +395,7 @@ static void allwinner_sun8i_emac_flush_desc(FrameDescriptor *desc,
     cpu_physical_memory_write(phys_addr, desc, sizeof(*desc));
 }
 
-static int allwinner_sun8i_emac_can_receive(NetClientState *nc)
+static bool allwinner_sun8i_emac_can_receive(NetClientState *nc)
 {
     AwSun8iEmacState *s = qemu_get_nic_opaque(nc);
     FrameDescriptor desc;
diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
index 39c78fa..ea915a0 100644
--- a/hw/net/can/can_sja1000.c
+++ b/hw/net/can/can_sja1000.c
@@ -733,21 +733,21 @@ uint64_t can_sja_mem_read(CanSJA1000State *s, hwaddr addr, unsigned size)
     return temp;
 }
 
-int can_sja_can_receive(CanBusClientState *client)
+bool can_sja_can_receive(CanBusClientState *client)
 {
     CanSJA1000State *s = container_of(client, CanSJA1000State, bus_client);
 
     if (s->clock & 0x80) { /* PeliCAN Mode */
         if (s->mode & 0x01) { /* reset mode. */
-            return 0;
+            return false;
         }
     } else { /* BasicCAN mode */
         if (s->control & 0x01) {
-            return 0;
+            return false;
         }
     }
 
-    return 1; /* always return 1, when operation mode */
+    return true; /* always return true, when operation mode */
 }
 
 ssize_t can_sja_receive(CanBusClientState *client, const qemu_can_frame *frames,
diff --git a/hw/net/can/can_sja1000.h b/hw/net/can/can_sja1000.h
index 220a622..7ca9cd6 100644
--- a/hw/net/can/can_sja1000.h
+++ b/hw/net/can/can_sja1000.h
@@ -137,7 +137,7 @@ void can_sja_disconnect(CanSJA1000State *s);
 
 int can_sja_init(CanSJA1000State *s, qemu_irq irq);
 
-int can_sja_can_receive(CanBusClientState *client);
+bool can_sja_can_receive(CanBusClientState *client);
 
 ssize_t can_sja_receive(CanBusClientState *client,
                         const qemu_can_frame *frames, size_t frames_cnt);
diff --git a/include/net/can_emu.h b/include/net/can_emu.h
index d4fc51b..fce9770 100644
--- a/include/net/can_emu.h
+++ b/include/net/can_emu.h
@@ -83,7 +83,7 @@ typedef struct CanBusClientState CanBusClientState;
 typedef struct CanBusState CanBusState;
 
 typedef struct CanBusClientInfo {
-    int (*can_receive)(CanBusClientState *);
+    bool (*can_receive)(CanBusClientState *);
     ssize_t (*receive)(CanBusClientState *,
         const struct qemu_can_frame *frames, size_t frames_cnt);
 } CanBusClientInfo;
diff --git a/net/can/can_socketcan.c b/net/can/can_socketcan.c
index 29bfacd..807f31f 100644
--- a/net/can/can_socketcan.c
+++ b/net/can/can_socketcan.c
@@ -110,9 +110,9 @@ static void can_host_socketcan_read(void *opaque)
     }
 }
 
-static int can_host_socketcan_can_receive(CanBusClientState *client)
+static bool can_host_socketcan_can_receive(CanBusClientState *client)
 {
-    return 1;
+    return true;
 }
 
 static ssize_t can_host_socketcan_receive(CanBusClientState *client,
-- 
2.5.0

[PULL V2 10/14] net/colo-compare.c: Expose "compare_timeout" to users

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

The "compare_timeout" determines the maximum time to hold the primary net packet.
This patch expose the "compare_timeout", make user have ability to
adjest the value according to application scenarios.

QMP command demo:
    { "execute": "qom-get",
         "arguments": { "path": "/objects/comp0",
                        "property": "compare_timeout" } }

    { "execute": "qom-set",
         "arguments": { "path": "/objects/comp0",
                        "property": "compare_timeout",
                        "value": 5000} }

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 qemu-options.hx    |  8 +++++---
 2 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 7ee17f2..ec09b2a 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -50,6 +50,7 @@ static NotifierList colo_compare_notifiers =
 
 /* TODO: Should be configurable */
 #define REGULAR_PACKET_CHECK_MS 3000
+#define DEFAULT_TIME_OUT_MS 3000
 
 static QemuMutex event_mtx;
 static QemuCond event_complete_cond;
@@ -92,6 +93,7 @@ typedef struct CompareState {
     SocketReadState sec_rs;
     SocketReadState notify_rs;
     bool vnet_hdr;
+    uint32_t compare_timeout;
 
     /*
      * Record the connection that through the NIC
@@ -607,10 +609,9 @@ static int colo_old_packet_check_one_conn(Connection *conn,
                                           CompareState *s)
 {
     GList *result = NULL;
-    int64_t check_time = REGULAR_PACKET_CHECK_MS;
 
     result = g_queue_find_custom(&conn->primary_list,
-                                 &check_time,
+                                 &s->compare_timeout,
                                  (GCompareFunc)colo_old_packet_check_one);
 
     if (result) {
@@ -984,6 +985,39 @@ static void compare_set_notify_dev(Object *obj, const char *value, Error **errp)
     s->notify_dev = g_strdup(value);
 }
 
+static void compare_get_timeout(Object *obj, Visitor *v,
+                                const char *name, void *opaque,
+                                Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+    uint32_t value = s->compare_timeout;
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void compare_set_timeout(Object *obj, Visitor *v,
+                                const char *name, void *opaque,
+                                Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+    Error *local_err = NULL;
+    uint32_t value;
+
+    visit_type_uint32(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    if (!value) {
+        error_setg(&local_err, "Property '%s.%s' requires a positive value",
+                   object_get_typename(obj), name);
+        goto out;
+    }
+    s->compare_timeout = value;
+
+out:
+    error_propagate(errp, local_err);
+}
+
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 {
     CompareState *s = container_of(pri_rs, CompareState, pri_rs);
@@ -1090,6 +1124,11 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
         return;
     }
 
+    if (!s->compare_timeout) {
+        /* Set default value to 3000 MS */
+        s->compare_timeout = DEFAULT_TIME_OUT_MS;
+    }
+
     if (find_and_check_chardev(&chr, s->pri_indev, errp) ||
         !qemu_chr_fe_init(&s->chr_pri_in, chr, errp)) {
         return;
@@ -1185,6 +1224,10 @@ static void colo_compare_init(Object *obj)
                             compare_get_notify_dev, compare_set_notify_dev,
                             NULL);
 
+    object_property_add(obj, "compare_timeout", "uint32",
+                        compare_get_timeout,
+                        compare_set_timeout, NULL, NULL, NULL);
+
     s->vnet_hdr = false;
     object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
                              compare_set_vnet_hdr, NULL);
diff --git a/qemu-options.hx b/qemu-options.hx
index 962a5eb..9e48e13 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4615,7 +4615,7 @@ SRST
         stored. The file format is libpcap, so it can be analyzed with
         tools such as tcpdump or Wireshark.
 
-    ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id]``
+    ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id][,compare_timeout=@var{ms}]``
         Colo-compare gets packet from primary\_inchardevid and
         secondary\_inchardevid, than compare primary packet with
         secondary packet. If the packets are same, we will output
@@ -4624,8 +4624,10 @@ SRST
         outdevchardevid. In order to improve efficiency, we need to put
         the task of comparison in another thread. If it has the
         vnet\_hdr\_support flag, colo compare will send/recv packet with
-        vnet\_hdr\_len. If you want to use Xen COLO, will need the
-        notify\_dev to notify Xen colo-frame to do checkpoint.
+        vnet\_hdr\_len. Then compare\_timeout=@var{ms} determines the
+        maximum delay colo-compare wait for the packet.
+        If you want to use Xen COLO, will need the notify\_dev to
+        notify Xen colo-frame to do checkpoint.
 
         we must use it with the help of filter-mirror and
         filter-redirector.
-- 
2.5.0

[PULL V2 11/14] net/colo-compare.c: Expose "expired_scan_cycle" to users

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

The "expired_scan_cycle" determines period of scanning expired
primary node net packets.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 qemu-options.hx    |  4 +++-
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index ec09b2a..10c0239 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -48,7 +48,6 @@ static NotifierList colo_compare_notifiers =
 #define COLO_COMPARE_FREE_PRIMARY     0x01
 #define COLO_COMPARE_FREE_SECONDARY   0x02
 
-/* TODO: Should be configurable */
 #define REGULAR_PACKET_CHECK_MS 3000
 #define DEFAULT_TIME_OUT_MS 3000
 
@@ -94,6 +93,7 @@ typedef struct CompareState {
     SocketReadState notify_rs;
     bool vnet_hdr;
     uint32_t compare_timeout;
+    uint32_t expired_scan_cycle;
 
     /*
      * Record the connection that through the NIC
@@ -823,7 +823,7 @@ static void check_old_packet_regular(void *opaque)
     /* if have old packet we will notify checkpoint */
     colo_old_packet_check(s);
     timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
-                REGULAR_PACKET_CHECK_MS);
+              s->expired_scan_cycle);
 }
 
 /* Public API, Used for COLO frame to notify compare event */
@@ -853,7 +853,7 @@ static void colo_compare_timer_init(CompareState *s)
                                 SCALE_MS, check_old_packet_regular,
                                 s);
     timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
-                    REGULAR_PACKET_CHECK_MS);
+              s->expired_scan_cycle);
 }
 
 static void colo_compare_timer_del(CompareState *s)
@@ -1018,6 +1018,39 @@ out:
     error_propagate(errp, local_err);
 }
 
+static void compare_get_expired_scan_cycle(Object *obj, Visitor *v,
+                                           const char *name, void *opaque,
+                                           Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+    uint32_t value = s->expired_scan_cycle;
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void compare_set_expired_scan_cycle(Object *obj, Visitor *v,
+                                           const char *name, void *opaque,
+                                           Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+    Error *local_err = NULL;
+    uint32_t value;
+
+    visit_type_uint32(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    if (!value) {
+        error_setg(&local_err, "Property '%s.%s' requires a positive value",
+                   object_get_typename(obj), name);
+        goto out;
+    }
+    s->expired_scan_cycle = value;
+
+out:
+    error_propagate(errp, local_err);
+}
+
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 {
     CompareState *s = container_of(pri_rs, CompareState, pri_rs);
@@ -1129,6 +1162,11 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
         s->compare_timeout = DEFAULT_TIME_OUT_MS;
     }
 
+    if (!s->expired_scan_cycle) {
+        /* Set default value to 3000 MS */
+        s->expired_scan_cycle = REGULAR_PACKET_CHECK_MS;
+    }
+
     if (find_and_check_chardev(&chr, s->pri_indev, errp) ||
         !qemu_chr_fe_init(&s->chr_pri_in, chr, errp)) {
         return;
@@ -1228,6 +1266,10 @@ static void colo_compare_init(Object *obj)
                         compare_get_timeout,
                         compare_set_timeout, NULL, NULL, NULL);
 
+    object_property_add(obj, "expired_scan_cycle", "uint32",
+                        compare_get_expired_scan_cycle,
+                        compare_set_expired_scan_cycle, NULL, NULL, NULL);
+
     s->vnet_hdr = false;
     object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
                              compare_set_vnet_hdr, NULL);
diff --git a/qemu-options.hx b/qemu-options.hx
index 9e48e13..16debd0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4615,7 +4615,7 @@ SRST
         stored. The file format is libpcap, so it can be analyzed with
         tools such as tcpdump or Wireshark.
 
-    ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id][,compare_timeout=@var{ms}]``
+    ``-object colo-compare,id=id,primary_in=chardevid,secondary_in=chardevid,outdev=chardevid,iothread=id[,vnet_hdr_support][,notify_dev=id][,compare_timeout=@var{ms}][,expired_scan_cycle=@var{ms}``
         Colo-compare gets packet from primary\_inchardevid and
         secondary\_inchardevid, than compare primary packet with
         secondary packet. If the packets are same, we will output
@@ -4626,6 +4626,8 @@ SRST
         vnet\_hdr\_support flag, colo compare will send/recv packet with
         vnet\_hdr\_len. Then compare\_timeout=@var{ms} determines the
         maximum delay colo-compare wait for the packet.
+        The expired\_scan\_cycle=@var{ms} to set the period of scanning
+        expired primary node network packets.
         If you want to use Xen COLO, will need the notify\_dev to
         notify Xen colo-frame to do checkpoint.
 
-- 
2.5.0

[PULL V2 12/14] net: tulip: check frame size and r/w data length

[Jason Wang]

From: Prasad J Pandit <pjp@fedoraproject.org>

Tulip network driver while copying tx/rx buffers does not check
frame size against r/w data length. This may lead to OOB buffer
access. Add check to avoid it.

Limit iterations over descriptors to avoid potential infinite
loop issue in tulip_xmit_list_update.

Reported-by: Li Qiang <pangpei.lq@antfin.com>
Reported-by: Ziming Zhang <ezrakiez@gmail.com>
Reported-by: Jason Wang <jasowang@redhat.com>
Tested-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/tulip.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/hw/net/tulip.c b/hw/net/tulip.c
index cfac271..1295f51 100644
--- a/hw/net/tulip.c
+++ b/hw/net/tulip.c
@@ -170,6 +170,10 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
         } else {
             len = s->rx_frame_len;
         }
+
+        if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
+            return;
+        }
         pci_dma_write(&s->dev, desc->buf_addr1, s->rx_frame +
             (s->rx_frame_size - s->rx_frame_len), len);
         s->rx_frame_len -= len;
@@ -181,6 +185,10 @@ static void tulip_copy_rx_bytes(TULIPState *s, struct tulip_descriptor *desc)
         } else {
             len = s->rx_frame_len;
         }
+
+        if (s->rx_frame_len + len > sizeof(s->rx_frame)) {
+            return;
+        }
         pci_dma_write(&s->dev, desc->buf_addr2, s->rx_frame +
             (s->rx_frame_size - s->rx_frame_len), len);
         s->rx_frame_len -= len;
@@ -227,7 +235,8 @@ static ssize_t tulip_receive(TULIPState *s, const uint8_t *buf, size_t size)
 
     trace_tulip_receive(buf, size);
 
-    if (size < 14 || size > 2048 || s->rx_frame_len || tulip_rx_stopped(s)) {
+    if (size < 14 || size > sizeof(s->rx_frame) - 4
+        || s->rx_frame_len || tulip_rx_stopped(s)) {
         return 0;
     }
 
@@ -275,7 +284,6 @@ static ssize_t tulip_receive_nc(NetClientState *nc,
     return tulip_receive(qemu_get_nic_opaque(nc), buf, size);
 }
 
-
 static NetClientInfo net_tulip_info = {
     .type = NET_CLIENT_DRIVER_NIC,
     .size = sizeof(NICState),
@@ -558,7 +566,7 @@ static void tulip_tx(TULIPState *s, struct tulip_descriptor *desc)
         if ((s->csr[6] >> CSR6_OM_SHIFT) & CSR6_OM_MASK) {
             /* Internal or external Loopback */
             tulip_receive(s, s->tx_frame, s->tx_frame_len);
-        } else {
+        } else if (s->tx_frame_len <= sizeof(s->tx_frame)) {
             qemu_send_packet(qemu_get_queue(s->nic),
                 s->tx_frame, s->tx_frame_len);
         }
@@ -570,23 +578,31 @@ static void tulip_tx(TULIPState *s, struct tulip_descriptor *desc)
     }
 }
 
-static void tulip_copy_tx_buffers(TULIPState *s, struct tulip_descriptor *desc)
+static int tulip_copy_tx_buffers(TULIPState *s, struct tulip_descriptor *desc)
 {
     int len1 = (desc->control >> TDES1_BUF1_SIZE_SHIFT) & TDES1_BUF1_SIZE_MASK;
     int len2 = (desc->control >> TDES1_BUF2_SIZE_SHIFT) & TDES1_BUF2_SIZE_MASK;
 
+    if (s->tx_frame_len + len1 > sizeof(s->tx_frame)) {
+        return -1;
+    }
     if (len1) {
         pci_dma_read(&s->dev, desc->buf_addr1,
             s->tx_frame + s->tx_frame_len, len1);
         s->tx_frame_len += len1;
     }
 
+    if (s->tx_frame_len + len2 > sizeof(s->tx_frame)) {
+        return -1;
+    }
     if (len2) {
         pci_dma_read(&s->dev, desc->buf_addr2,
             s->tx_frame + s->tx_frame_len, len2);
         s->tx_frame_len += len2;
     }
     desc->status = (len1 + len2) ? 0 : 0x7fffffff;
+
+    return 0;
 }
 
 static void tulip_setup_filter_addr(TULIPState *s, uint8_t *buf, int n)
@@ -651,13 +667,15 @@ static uint32_t tulip_ts(TULIPState *s)
 
 static void tulip_xmit_list_update(TULIPState *s)
 {
+#define TULIP_DESC_MAX 128
+    uint8_t i = 0;
     struct tulip_descriptor desc;
 
     if (tulip_ts(s) != CSR5_TS_SUSPENDED) {
         return;
     }
 
-    for (;;) {
+    for (i = 0; i < TULIP_DESC_MAX; i++) {
         tulip_desc_read(s, s->current_tx_desc, &desc);
         tulip_dump_tx_descriptor(s, &desc);
 
@@ -675,10 +693,10 @@ static void tulip_xmit_list_update(TULIPState *s)
                 s->tx_frame_len = 0;
             }
 
-            tulip_copy_tx_buffers(s, &desc);
-
-            if (desc.control & TDES1_LS) {
-                tulip_tx(s, &desc);
+            if (!tulip_copy_tx_buffers(s, &desc)) {
+                if (desc.control & TDES1_LS) {
+                    tulip_tx(s, &desc);
+                }
             }
         }
         tulip_desc_write(s, s->current_tx_desc, &desc);
-- 
2.5.0

[PULL V2 13/14] hw/net/allwinner-sun8i-emac.c: Fix REG_ADDR_HIGH/LOW reads

[Jason Wang]

From: Peter Maydell <peter.maydell@linaro.org>

Coverity points out (CID 1421926) that the read code for
REG_ADDR_HIGH reads off the end of the buffer, because it does a
32-bit read from byte 4 of a 6-byte buffer.

The code also has an endianness issue for both REG_ADDR_HIGH and
REG_ADDR_LOW, because it will do the wrong thing on a big-endian
host.

Rewrite the read code to use ldl_le_p() and lduw_le_p() to fix this;
the write code is not incorrect, but for consistency we make it use
stl_le_p() and stw_le_p().

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/allwinner-sun8i-emac.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/net/allwinner-sun8i-emac.c b/hw/net/allwinner-sun8i-emac.c
index 033eaa8..28637ff 100644
--- a/hw/net/allwinner-sun8i-emac.c
+++ b/hw/net/allwinner-sun8i-emac.c
@@ -611,10 +611,10 @@ static uint64_t allwinner_sun8i_emac_read(void *opaque, hwaddr offset,
         value = s->mii_data;
         break;
     case REG_ADDR_HIGH:         /* MAC Address High */
-        value = *(((uint32_t *) (s->conf.macaddr.a)) + 1);
+        value = lduw_le_p(s->conf.macaddr.a + 4);
         break;
     case REG_ADDR_LOW:          /* MAC Address Low */
-        value = *(uint32_t *) (s->conf.macaddr.a);
+        value = ldl_le_p(s->conf.macaddr.a);
         break;
     case REG_TX_DMA_STA:        /* Transmit DMA Status */
         break;
@@ -728,14 +728,10 @@ static void allwinner_sun8i_emac_write(void *opaque, hwaddr offset,
         s->mii_data = value;
         break;
     case REG_ADDR_HIGH:         /* MAC Address High */
-        s->conf.macaddr.a[4] = (value & 0xff);
-        s->conf.macaddr.a[5] = (value & 0xff00) >> 8;
+        stw_le_p(s->conf.macaddr.a + 4, value);
         break;
     case REG_ADDR_LOW:          /* MAC Address Low */
-        s->conf.macaddr.a[0] = (value & 0xff);
-        s->conf.macaddr.a[1] = (value & 0xff00) >> 8;
-        s->conf.macaddr.a[2] = (value & 0xff0000) >> 16;
-        s->conf.macaddr.a[3] = (value & 0xff000000) >> 24;
+        stl_le_p(s->conf.macaddr.a, value);
         break;
     case REG_TX_DMA_STA:        /* Transmit DMA Status */
     case REG_TX_CUR_DESC:       /* Transmit Current Descriptor */
-- 
2.5.0

[PULL V2 14/14] qtest: add tulip test case

[Jason Wang]

From: Li Qiang <liq3ea@163.com>

The tulip networking card emulation has an OOB issue in
'tulip_copy_tx_buffers' when the guest provide malformed descriptor.
This test will trigger a ASAN heap overflow crash. To trigger this
issue we can construct the data as following:

1. construct a 'tulip_descriptor'. Its control is set to
'0x7ff | 0x7ff << 11', this will make the 'tulip_copy_tx_buffers's
'len1' and 'len2' to 0x7ff(2047). So 'len1+len2' will overflow
'TULIPState's 'tx_frame' field. This descriptor's 'buf_addr1' and
'buf_addr2' should set to a guest address.

2. write this descriptor to tulip device's CSR4 register. This will
set the 'TULIPState's 'current_tx_desc' field.

3. write 'CSR6_ST' to tulip device's CSR6 register. This will trigger
'tulip_xmit_list_update' and finally calls 'tulip_copy_tx_buffers'.

Following shows the backtrack of crash:

==31781==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x628000007cd0 at pc 0x7fe03c5a077a bp 0x7fff05b46770 sp 0x7fff05b45f18
WRITE of size 2047 at 0x628000007cd0 thread T0
    #0 0x7fe03c5a0779  (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x79779)
    #1 0x5575fb6daa6a in flatview_read_continue /home/test/qemu/exec.c:3194
    #2 0x5575fb6daccb in flatview_read /home/test/qemu/exec.c:3227
    #3 0x5575fb6dae66 in address_space_read_full /home/test/qemu/exec.c:3240
    #4 0x5575fb6db0cb in address_space_rw /home/test/qemu/exec.c:3268
    #5 0x5575fbdfd460 in dma_memory_rw_relaxed /home/test/qemu/include/sysemu/dma.h:87
    #6 0x5575fbdfd4b5 in dma_memory_rw /home/test/qemu/include/sysemu/dma.h:110
    #7 0x5575fbdfd866 in pci_dma_rw /home/test/qemu/include/hw/pci/pci.h:787
    #8 0x5575fbdfd8a3 in pci_dma_read /home/test/qemu/include/hw/pci/pci.h:794
    #9 0x5575fbe02761 in tulip_copy_tx_buffers hw/net/tulip.c:585
    #10 0x5575fbe0366b in tulip_xmit_list_update hw/net/tulip.c:678
    #11 0x5575fbe04073 in tulip_write hw/net/tulip.c:783

Signed-off-by: Li Qiang <liq3ea@163.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 tests/qtest/Makefile.include |  1 +
 tests/qtest/tulip-test.c     | 91 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)
 create mode 100644 tests/qtest/tulip-test.c

diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 10a28de..9e5a51d 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -217,6 +217,7 @@ qos-test-obj-y += tests/qtest/es1370-test.o
 qos-test-obj-y += tests/qtest/ipoctal232-test.o
 qos-test-obj-y += tests/qtest/megasas-test.o
 qos-test-obj-y += tests/qtest/ne2000-test.o
+qos-test-obj-y += tests/qtest/tulip-test.o
 qos-test-obj-y += tests/qtest/nvme-test.o
 qos-test-obj-y += tests/qtest/pca9552-test.o
 qos-test-obj-y += tests/qtest/pci-test.o
diff --git a/tests/qtest/tulip-test.c b/tests/qtest/tulip-test.c
new file mode 100644
index 0000000..2fb6c4d
--- /dev/null
+++ b/tests/qtest/tulip-test.c
@@ -0,0 +1,91 @@
+/*
+ * QTest testcase for DEC/Intel Tulip 21143
+ *
+ * Copyright (c) 2020 Li Qiang <liq3ea@gmail.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/module.h"
+#include "libqos/qgraph.h"
+#include "libqos/pci.h"
+#include "qemu/bitops.h"
+#include "hw/net/tulip.h"
+
+typedef struct QTulip_pci QTulip_pci;
+
+struct QTulip_pci {
+    QOSGraphObject obj;
+    QPCIDevice dev;
+};
+
+static void *tulip_pci_get_driver(void *obj, const char *interface)
+{
+    QTulip_pci *tulip_pci = obj;
+
+    if (!g_strcmp0(interface, "pci-device")) {
+        return &tulip_pci->dev;
+    }
+
+    fprintf(stderr, "%s not present in tulip_pci\n", interface);
+    g_assert_not_reached();
+}
+
+static void *tulip_pci_create(void *pci_bus, QGuestAllocator *alloc, void *addr)
+{
+    QTulip_pci *tulip_pci = g_new0(QTulip_pci, 1);
+    QPCIBus *bus = pci_bus;
+
+    qpci_device_init(&tulip_pci->dev, bus, addr);
+    tulip_pci->obj.get_driver = tulip_pci_get_driver;
+
+    return &tulip_pci->obj;
+}
+
+static void tulip_large_tx(void *obj, void *data, QGuestAllocator *alloc)
+{
+    QTulip_pci *tulip_pci = obj;
+    QPCIDevice *dev = &tulip_pci->dev;
+    QPCIBar bar;
+    struct tulip_descriptor context;
+    char guest_data[4096];
+    uint64_t context_pa;
+    uint64_t guest_pa;
+
+    qpci_device_enable(dev);
+    bar = qpci_iomap(dev, 0, NULL);
+    context_pa = guest_alloc(alloc, sizeof(context));
+    guest_pa = guest_alloc(alloc, 4096);
+    memset(guest_data, 'A', sizeof(guest_data));
+    context.status = TDES0_OWN;
+    context.control = TDES1_BUF2_SIZE_MASK << TDES1_BUF2_SIZE_SHIFT |
+                      TDES1_BUF1_SIZE_MASK << TDES1_BUF1_SIZE_SHIFT;
+    context.buf_addr2 = guest_pa;
+    context.buf_addr1 = guest_pa;
+
+    qtest_memwrite(dev->bus->qts, context_pa, &context, sizeof(context));
+    qtest_memwrite(dev->bus->qts, guest_pa, guest_data, sizeof(guest_data));
+    qpci_io_writel(dev, bar, 0x20, context_pa);
+    qpci_io_writel(dev, bar, 0x30, CSR6_ST);
+    guest_free(alloc, context_pa);
+    guest_free(alloc, guest_pa);
+}
+
+static void tulip_register_nodes(void)
+{
+    QOSGraphEdgeOptions opts = {
+        .extra_device_opts = "addr=04.0",
+    };
+    add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
+
+    qos_node_create_driver("tulip", tulip_pci_create);
+    qos_node_consumes("tulip", "pci-bus", &opts);
+    qos_node_produces("tulip", "pci-device");
+
+    qos_add_test("tulip_large_tx", "tulip", tulip_large_tx, NULL);
+}
+
+libqos_init(tulip_register_nodes);
-- 
2.5.0

Re: [PULL V2 00/14] Net patches

[Peter Maydell]

On Tue, 31 Mar 2020 at 14:21, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 2a95551e8b1456aa53ce54fac573df18809340a6:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20200330' into staging (2020-03-31 11:20:21 +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 1153cf9f5b67fad41ca6f8571e9a26e2c7c70759:
>
>   qtest: add tulip test case (2020-03-31 21:14:35 +0800)
>
> ----------------------------------------------------------------
>
> Changes from V1:
>
> - fix the compiling error
> - include qtest for tulip OOB


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM