[Qemu-devel] [PULL 00/18] Net patches

[Qemu-devel] [PULL 00/18] Net patches

[Jason Wang]

The following changes since commit 43ab9a5376c95c61ae898a222c4d04bdf60e239b:

  hw/i386/vmport: fix missing definitions with non-log trace backends (2017-12-21 22:52:28 +0000)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 0065e915192cdf83c2700bb377e5323c2649476e:

  qemu-doc: Update the deprecation information of -tftp, -bootp, -redir and -smb (2017-12-22 10:06:05 +0800)

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

----------------------------------------------------------------
Ed Swierk via Qemu-devel (2):
      e1000, e1000e: Move per-packet TX offload flags out of context state
      e1000: Separate TSO and non-TSO contexts, fixing UDP TX corruption

Mark Cave-Ayland (13):
      net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function
      net: introduce net_crc32_le() function
      pcnet: switch pcnet over to use net_crc32_le()
      eepro100: switch eepro100 e100_compute_mcast_idx() over to use net_crc32()
      sunhme: switch sunhme over to use net_crc32_le()
      sungem: fix multicast filter CRC calculation
      eepro100: use inline net_crc32() and bitshift instead of compute_mcast_idx()
      opencores_eth: use inline net_crc32() and bitshift instead of compute_mcast_idx()
      lan9118: use inline net_crc32() and bitshift instead of compute_mcast_idx()
      ftgmac100: use inline net_crc32() and bitshift instead of compute_mcast_idx()
      ne2000: use inline net_crc32() and bitshift instead of compute_mcast_idx()
      rtl8139: use inline net_crc32() and bitshift instead of compute_mcast_idx()
      net: remove unused compute_mcast_idx() function

Thomas Huth (3):
      net: Remove the legacy "-net channel" parameter
      qemu-doc: The "-net nic" option can be used with "netdev=...", too
      qemu-doc: Update the deprecation information of -tftp, -bootp, -redir and -smb

 hw/net/e1000.c         | 92 ++++++++++++++++++++++++++++----------------------
 hw/net/e1000e.c        |  4 +--
 hw/net/e1000e_core.c   | 16 ++++-----
 hw/net/e1000e_core.h   |  2 ++
 hw/net/e1000x_common.h |  2 --
 hw/net/eepro100.c      | 32 +++---------------
 hw/net/ftgmac100.c     |  2 +-
 hw/net/lan9118.c       |  3 +-
 hw/net/ne2000.c        |  4 ++-
 hw/net/opencores_eth.c |  3 +-
 hw/net/pcnet.c         | 22 ++----------
 hw/net/rtl8139.c       |  2 +-
 hw/net/sungem.c        |  5 ++-
 hw/net/sunhme.c        | 25 +-------------
 include/net/net.h      |  5 ++-
 include/net/slirp.h    |  2 --
 net/net.c              | 40 +++++++++++++++-------
 net/slirp.c            | 34 -------------------
 qemu-doc.texi          | 38 +++++++++++----------
 qemu-options.hx        | 14 ++++----
 20 files changed, 144 insertions(+), 203 deletions(-)

[Qemu-devel] [PULL 01/18] e1000, e1000e: Move per-packet TX offload flags out of context state

[Jason Wang]

From: Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>

sum_needed and cptse flags are received from the guest within each
transmit data descriptor. They are not part of the offload context;
instead, they determine how to apply a previously received context to
the packet being transmitted:

- If cptse is set, perform both segmentation and checksum offload
  using the parameters in the TSO context; otherwise just do checksum
  offload. (Currently the e1000 device incorrectly stores only one
  context, which will be fixed in a subsequent patch.)

- Depending on the bits set in sum_needed, possibly perform L4
  checksum offload and/or IP checksum offload, using the parameters in
  the appropriate context.

Move these flags out of struct e1000x_txd_props, which is otherwise
dedicated to storing values from a context descriptor, and into the
per-packet TX struct.

Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/e1000.c         | 30 ++++++++++++++++--------------
 hw/net/e1000e.c        |  4 ++--
 hw/net/e1000e_core.c   | 16 ++++++++--------
 hw/net/e1000e_core.h   |  2 ++
 hw/net/e1000x_common.h |  2 --
 5 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 05a00cb..30aef93 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -98,6 +98,8 @@ typedef struct E1000State_st {
         unsigned char data[0x10000];
         uint16_t size;
         unsigned char vlan_needed;
+        unsigned char sum_needed;
+        bool cptse;
         e1000x_txd_props props;
         uint16_t tso_frames;
     } tx;
@@ -540,7 +542,7 @@ xmit_seg(E1000State *s)
     unsigned int frames = s->tx.tso_frames, css, sofar;
     struct e1000_tx *tp = &s->tx;
 
-    if (tp->props.tse && tp->props.cptse) {
+    if (tp->props.tse && tp->cptse) {
         css = tp->props.ipcss;
         DBGOUT(TXSUM, "frames %d size %d ipcss %d\n",
                frames, tp->size, css);
@@ -564,7 +566,7 @@ xmit_seg(E1000State *s)
             }
         } else    /* UDP */
             stw_be_p(tp->data+css+4, len);
-        if (tp->props.sum_needed & E1000_TXD_POPTS_TXSM) {
+        if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
             unsigned int phsum;
             // add pseudo-header length before checksum calculation
             void *sp = tp->data + tp->props.tucso;
@@ -576,11 +578,11 @@ xmit_seg(E1000State *s)
         tp->tso_frames++;
     }
 
-    if (tp->props.sum_needed & E1000_TXD_POPTS_TXSM) {
+    if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
         putsum(tp->data, tp->size, tp->props.tucso,
                tp->props.tucss, tp->props.tucse);
     }
-    if (tp->props.sum_needed & E1000_TXD_POPTS_IXSM) {
+    if (tp->sum_needed & E1000_TXD_POPTS_IXSM) {
         putsum(tp->data, tp->size, tp->props.ipcso,
                tp->props.ipcss, tp->props.ipcse);
     }
@@ -624,17 +626,17 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
     } else if (dtype == (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D)) {
         // data descriptor
         if (tp->size == 0) {
-            tp->props.sum_needed = le32_to_cpu(dp->upper.data) >> 8;
+            tp->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
         }
-        tp->props.cptse = (txd_lower & E1000_TXD_CMD_TSE) ? 1 : 0;
+        tp->cptse = (txd_lower & E1000_TXD_CMD_TSE) ? 1 : 0;
     } else {
         // legacy descriptor
-        tp->props.cptse = 0;
+        tp->cptse = 0;
     }
 
     if (e1000x_vlan_enabled(s->mac_reg) &&
         e1000x_is_vlan_txd(txd_lower) &&
-        (tp->props.cptse || txd_lower & E1000_TXD_CMD_EOP)) {
+        (tp->cptse || txd_lower & E1000_TXD_CMD_EOP)) {
         tp->vlan_needed = 1;
         stw_be_p(tp->vlan_header,
                       le16_to_cpu(s->mac_reg[VET]));
@@ -643,7 +645,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
     }
 
     addr = le64_to_cpu(dp->buffer_addr);
-    if (tp->props.tse && tp->props.cptse) {
+    if (tp->props.tse && tp->cptse) {
         msh = tp->props.hdr_len + tp->props.mss;
         do {
             bytes = split_size;
@@ -665,7 +667,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
             }
             split_size -= bytes;
         } while (bytes && split_size);
-    } else if (!tp->props.tse && tp->props.cptse) {
+    } else if (!tp->props.tse && tp->cptse) {
         // context descriptor TSE is not set, while data descriptor TSE is set
         DBGOUT(TXERR, "TCP segmentation error\n");
     } else {
@@ -676,14 +678,14 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
 
     if (!(txd_lower & E1000_TXD_CMD_EOP))
         return;
-    if (!(tp->props.tse && tp->props.cptse && tp->size < tp->props.hdr_len)) {
+    if (!(tp->props.tse && tp->cptse && tp->size < tp->props.hdr_len)) {
         xmit_seg(s);
     }
     tp->tso_frames = 0;
-    tp->props.sum_needed = 0;
+    tp->sum_needed = 0;
     tp->vlan_needed = 0;
     tp->size = 0;
-    tp->props.cptse = 0;
+    tp->cptse = 0;
 }
 
 static uint32_t
@@ -1461,7 +1463,7 @@ static const VMStateDescription vmstate_e1000 = {
         VMSTATE_UINT16(tx.props.mss, E1000State),
         VMSTATE_UINT16(tx.size, E1000State),
         VMSTATE_UINT16(tx.tso_frames, E1000State),
-        VMSTATE_UINT8(tx.props.sum_needed, E1000State),
+        VMSTATE_UINT8(tx.sum_needed, E1000State),
         VMSTATE_INT8(tx.props.ip, E1000State),
         VMSTATE_INT8(tx.props.tcp, E1000State),
         VMSTATE_BUFFER(tx.header, E1000State),
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index f1af279..191398a 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -556,7 +556,7 @@ static const VMStateDescription e1000e_vmstate_tx = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT8(props.sum_needed, struct e1000e_tx),
+        VMSTATE_UINT8(sum_needed, struct e1000e_tx),
         VMSTATE_UINT8(props.ipcss, struct e1000e_tx),
         VMSTATE_UINT8(props.ipcso, struct e1000e_tx),
         VMSTATE_UINT16(props.ipcse, struct e1000e_tx),
@@ -569,7 +569,7 @@ static const VMStateDescription e1000e_vmstate_tx = {
         VMSTATE_INT8(props.ip, struct e1000e_tx),
         VMSTATE_INT8(props.tcp, struct e1000e_tx),
         VMSTATE_BOOL(props.tse, struct e1000e_tx),
-        VMSTATE_BOOL(props.cptse, struct e1000e_tx),
+        VMSTATE_BOOL(cptse, struct e1000e_tx),
         VMSTATE_BOOL(skip_cp, struct e1000e_tx),
         VMSTATE_END_OF_LIST()
     }
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 43a8d89..c93c466 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -632,18 +632,18 @@ e1000e_rss_parse_packet(E1000ECore *core,
 static void
 e1000e_setup_tx_offloads(E1000ECore *core, struct e1000e_tx *tx)
 {
-    if (tx->props.tse && tx->props.cptse) {
+    if (tx->props.tse && tx->cptse) {
         net_tx_pkt_build_vheader(tx->tx_pkt, true, true, tx->props.mss);
         net_tx_pkt_update_ip_checksums(tx->tx_pkt);
         e1000x_inc_reg_if_not_full(core->mac, TSCTC);
         return;
     }
 
-    if (tx->props.sum_needed & E1000_TXD_POPTS_TXSM) {
+    if (tx->sum_needed & E1000_TXD_POPTS_TXSM) {
         net_tx_pkt_build_vheader(tx->tx_pkt, false, true, 0);
     }
 
-    if (tx->props.sum_needed & E1000_TXD_POPTS_IXSM) {
+    if (tx->sum_needed & E1000_TXD_POPTS_IXSM) {
         net_tx_pkt_update_ip_hdr_checksum(tx->tx_pkt);
     }
 }
@@ -715,13 +715,13 @@ e1000e_process_tx_desc(E1000ECore *core,
         return;
     } else if (dtype == (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D)) {
         /* data descriptor */
-        tx->props.sum_needed = le32_to_cpu(dp->upper.data) >> 8;
-        tx->props.cptse = (txd_lower & E1000_TXD_CMD_TSE) ? 1 : 0;
+        tx->sum_needed = le32_to_cpu(dp->upper.data) >> 8;
+        tx->cptse = (txd_lower & E1000_TXD_CMD_TSE) ? 1 : 0;
         e1000e_process_ts_option(core, dp);
     } else {
         /* legacy descriptor */
         e1000e_process_ts_option(core, dp);
-        tx->props.cptse = 0;
+        tx->cptse = 0;
     }
 
     addr = le64_to_cpu(dp->buffer_addr);
@@ -747,8 +747,8 @@ e1000e_process_tx_desc(E1000ECore *core,
         tx->skip_cp = false;
         net_tx_pkt_reset(tx->tx_pkt);
 
-        tx->props.sum_needed = 0;
-        tx->props.cptse = 0;
+        tx->sum_needed = 0;
+        tx->cptse = 0;
     }
 }
 
diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
index 1ff6978..7d8ff41 100644
--- a/hw/net/e1000e_core.h
+++ b/hw/net/e1000e_core.h
@@ -71,6 +71,8 @@ struct E1000Core {
         e1000x_txd_props props;
 
         bool skip_cp;
+        unsigned char sum_needed;
+        bool cptse;
         struct NetTxPkt *tx_pkt;
     } tx[E1000E_NUM_QUEUES];
 
diff --git a/hw/net/e1000x_common.h b/hw/net/e1000x_common.h
index 3072ce9..0268884 100644
--- a/hw/net/e1000x_common.h
+++ b/hw/net/e1000x_common.h
@@ -193,7 +193,6 @@ void e1000x_update_regs_on_autoneg_done(uint32_t *mac, uint16_t *phy);
 void e1000x_increase_size_stats(uint32_t *mac, const int *size_regs, int size);
 
 typedef struct e1000x_txd_props {
-    unsigned char sum_needed;
     uint8_t ipcss;
     uint8_t ipcso;
     uint16_t ipcse;
@@ -206,7 +205,6 @@ typedef struct e1000x_txd_props {
     int8_t ip;
     int8_t tcp;
     bool tse;
-    bool cptse;
 } e1000x_txd_props;
 
 void e1000x_read_tx_ctx_descr(struct e1000_context_desc *d,
-- 
2.7.4

[Qemu-devel] [PULL 02/18] e1000: Separate TSO and non-TSO contexts, fixing UDP TX corruption

[Jason Wang]

From: Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>

The device is supposed to maintain two distinct contexts for transmit
offloads: one has parameters for both segmentation and checksum
offload, the other only for checksum offload. The guest driver can
send two context descriptors, one for each context (the TSE flag
specifies which). Then the guest can refer to one or the other context
in subsequent transmit data descriptors, depending on what offloads it
wants applied to each packet.

Currently the e1000 device stores just one context, and misinterprets
the TSE flags in the context and data descriptors. This is often okay:
Linux happens to send a fresh context descriptor before every data
descriptor, so forgetting the other context doesn't matter. Windows
does rely on separate contexts for TSO vs. non-TSO packets, but for
mostly-TCP traffic the two contexts have identical TCP-specific
offload parameters so confusing them doesn't matter.

One case where this confusion matters is when a Windows guest sets up
a TSO context for TCP and a non-TSO context for UDP, and then
transmits both TCP and UDP traffic in parallel. The e1000 device
sometimes ends up using TCP-specific parameters while doing checksum
offload on a UDP datagram: it writes the checksum to offset 16 (the
correct location for a TCP checksum), stomping on two bytes of UDP
data, and leaving the wrong value in the actual UDP checksum field at
offset 6. (Even worse, the host network stack may then recompute the
UDP checksum, "correcting" it to match the corrupt data before sending
it out a physical interface.)

Correct this by tracking the TSO context independently of the non-TSO
context, and selecting the appropriate context based on the TSE flag
in each transmit data descriptor.

Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/e1000.c | 70 +++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 40 insertions(+), 30 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 30aef93..804ec08 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -101,6 +101,7 @@ typedef struct E1000State_st {
         unsigned char sum_needed;
         bool cptse;
         e1000x_txd_props props;
+        e1000x_txd_props tso_props;
         uint16_t tso_frames;
     } tx;
 
@@ -541,35 +542,37 @@ xmit_seg(E1000State *s)
     uint16_t len;
     unsigned int frames = s->tx.tso_frames, css, sofar;
     struct e1000_tx *tp = &s->tx;
+    struct e1000x_txd_props *props = tp->cptse ? &tp->tso_props : &tp->props;
 
-    if (tp->props.tse && tp->cptse) {
-        css = tp->props.ipcss;
+    if (tp->cptse) {
+        css = props->ipcss;
         DBGOUT(TXSUM, "frames %d size %d ipcss %d\n",
                frames, tp->size, css);
-        if (tp->props.ip) {    /* IPv4 */
+        if (props->ip) {    /* IPv4 */
             stw_be_p(tp->data+css+2, tp->size - css);
             stw_be_p(tp->data+css+4,
                      lduw_be_p(tp->data + css + 4) + frames);
         } else {         /* IPv6 */
             stw_be_p(tp->data+css+4, tp->size - css);
         }
-        css = tp->props.tucss;
+        css = props->tucss;
         len = tp->size - css;
-        DBGOUT(TXSUM, "tcp %d tucss %d len %d\n", tp->props.tcp, css, len);
-        if (tp->props.tcp) {
-            sofar = frames * tp->props.mss;
+        DBGOUT(TXSUM, "tcp %d tucss %d len %d\n", props->tcp, css, len);
+        if (props->tcp) {
+            sofar = frames * props->mss;
             stl_be_p(tp->data+css+4, ldl_be_p(tp->data+css+4)+sofar); /* seq */
-            if (tp->props.paylen - sofar > tp->props.mss) {
+            if (props->paylen - sofar > props->mss) {
                 tp->data[css + 13] &= ~9;    /* PSH, FIN */
             } else if (frames) {
                 e1000x_inc_reg_if_not_full(s->mac_reg, TSCTC);
             }
-        } else    /* UDP */
+        } else {    /* UDP */
             stw_be_p(tp->data+css+4, len);
+        }
         if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
             unsigned int phsum;
             // add pseudo-header length before checksum calculation
-            void *sp = tp->data + tp->props.tucso;
+            void *sp = tp->data + props->tucso;
 
             phsum = lduw_be_p(sp) + len;
             phsum = (phsum >> 16) + (phsum & 0xffff);
@@ -579,12 +582,10 @@ xmit_seg(E1000State *s)
     }
 
     if (tp->sum_needed & E1000_TXD_POPTS_TXSM) {
-        putsum(tp->data, tp->size, tp->props.tucso,
-               tp->props.tucss, tp->props.tucse);
+        putsum(tp->data, tp->size, props->tucso, props->tucss, props->tucse);
     }
     if (tp->sum_needed & E1000_TXD_POPTS_IXSM) {
-        putsum(tp->data, tp->size, tp->props.ipcso,
-               tp->props.ipcss, tp->props.ipcse);
+        putsum(tp->data, tp->size, props->ipcso, props->ipcss, props->ipcse);
     }
     if (tp->vlan_needed) {
         memmove(tp->vlan, tp->data, 4);
@@ -616,11 +617,11 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
 
     s->mit_ide |= (txd_lower & E1000_TXD_CMD_IDE);
     if (dtype == E1000_TXD_CMD_DEXT) {    /* context descriptor */
-        e1000x_read_tx_ctx_descr(xp, &tp->props);
-        tp->tso_frames = 0;
-        if (tp->props.tucso == 0) {    /* this is probably wrong */
-            DBGOUT(TXSUM, "TCP/UDP: cso 0!\n");
-            tp->props.tucso = tp->props.tucss + (tp->props.tcp ? 16 : 6);
+        if (le32_to_cpu(xp->cmd_and_length) & E1000_TXD_CMD_TSE) {
+            e1000x_read_tx_ctx_descr(xp, &tp->tso_props);
+            tp->tso_frames = 0;
+        } else {
+            e1000x_read_tx_ctx_descr(xp, &tp->props);
         }
         return;
     } else if (dtype == (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D)) {
@@ -645,8 +646,8 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
     }
 
     addr = le64_to_cpu(dp->buffer_addr);
-    if (tp->props.tse && tp->cptse) {
-        msh = tp->props.hdr_len + tp->props.mss;
+    if (tp->cptse) {
+        msh = tp->tso_props.hdr_len + tp->tso_props.mss;
         do {
             bytes = split_size;
             if (tp->size + bytes > msh)
@@ -655,21 +656,19 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
             bytes = MIN(sizeof(tp->data) - tp->size, bytes);
             pci_dma_read(d, addr, tp->data + tp->size, bytes);
             sz = tp->size + bytes;
-            if (sz >= tp->props.hdr_len && tp->size < tp->props.hdr_len) {
-                memmove(tp->header, tp->data, tp->props.hdr_len);
+            if (sz >= tp->tso_props.hdr_len
+                && tp->size < tp->tso_props.hdr_len) {
+                memmove(tp->header, tp->data, tp->tso_props.hdr_len);
             }
             tp->size = sz;
             addr += bytes;
             if (sz == msh) {
                 xmit_seg(s);
-                memmove(tp->data, tp->header, tp->props.hdr_len);
-                tp->size = tp->props.hdr_len;
+                memmove(tp->data, tp->header, tp->tso_props.hdr_len);
+                tp->size = tp->tso_props.hdr_len;
             }
             split_size -= bytes;
         } while (bytes && split_size);
-    } else if (!tp->props.tse && tp->cptse) {
-        // context descriptor TSE is not set, while data descriptor TSE is set
-        DBGOUT(TXERR, "TCP segmentation error\n");
     } else {
         split_size = MIN(sizeof(tp->data) - tp->size, split_size);
         pci_dma_read(d, addr, tp->data + tp->size, split_size);
@@ -678,7 +677,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
 
     if (!(txd_lower & E1000_TXD_CMD_EOP))
         return;
-    if (!(tp->props.tse && tp->cptse && tp->size < tp->props.hdr_len)) {
+    if (!(tp->cptse && tp->size < tp->tso_props.hdr_len)) {
         xmit_seg(s);
     }
     tp->tso_frames = 0;
@@ -1437,7 +1436,7 @@ static const VMStateDescription vmstate_e1000_full_mac_state = {
 
 static const VMStateDescription vmstate_e1000 = {
     .name = "e1000",
-    .version_id = 2,
+    .version_id = 3,
     .minimum_version_id = 1,
     .pre_save = e1000_pre_save,
     .post_load = e1000_post_load,
@@ -1510,6 +1509,17 @@ static const VMStateDescription vmstate_e1000 = {
         VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, RA, 32),
         VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128),
         VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
+        VMSTATE_UINT8_V(tx.tso_props.ipcss, E1000State, 3),
+        VMSTATE_UINT8_V(tx.tso_props.ipcso, E1000State, 3),
+        VMSTATE_UINT16_V(tx.tso_props.ipcse, E1000State, 3),
+        VMSTATE_UINT8_V(tx.tso_props.tucss, E1000State, 3),
+        VMSTATE_UINT8_V(tx.tso_props.tucso, E1000State, 3),
+        VMSTATE_UINT16_V(tx.tso_props.tucse, E1000State, 3),
+        VMSTATE_UINT32_V(tx.tso_props.paylen, E1000State, 3),
+        VMSTATE_UINT8_V(tx.tso_props.hdr_len, E1000State, 3),
+        VMSTATE_UINT16_V(tx.tso_props.mss, E1000State, 3),
+        VMSTATE_INT8_V(tx.tso_props.ip, E1000State, 3),
+        VMSTATE_INT8_V(tx.tso_props.tcp, E1000State, 3),
         VMSTATE_END_OF_LIST()
     },
     .subsections = (const VMStateDescription*[]) {
-- 
2.7.4

[Qemu-devel] [PULL 03/18] net: move CRC32 calculation from compute_mcast_idx() into its own net_crc32() function

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Separate out the standard ethernet CRC32 calculation into a new net_crc32()
function, renaming the constant POLYNOMIAL to POLYNOMIAL_BE to make it clear
that this is a big-endian CRC32 calculation.

As part of the constant rename, remove the duplicate definition of POLYNOMIAL
from eepro100.c and use the new POLYNOMIAL_BE constant instead.

Once this is complete remove the existing CRC32 implementation from
compute_mcast_idx() and call the new net_crc32() function in its place.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/eepro100.c |  4 +---
 include/net/net.h |  3 ++-
 net/net.c         | 16 +++++++++++-----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 1c0def5..71cddfe 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -323,8 +323,6 @@ static const uint16_t eepro100_mdi_mask[] = {
     0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
 };
 
-#define POLYNOMIAL 0x04c11db6
-
 static E100PCIDeviceInfo *eepro100_get_class(EEPRO100State *s);
 
 /* From FreeBSD (locally modified). */
@@ -342,7 +340,7 @@ static unsigned e100_compute_mcast_idx(const uint8_t *ep)
             crc <<= 1;
             b >>= 1;
             if (carry) {
-                crc = ((crc ^ POLYNOMIAL) | carry);
+                crc = ((crc ^ POLYNOMIAL_BE) | carry);
             }
         }
     }
diff --git a/include/net/net.h b/include/net/net.h
index 1c55a93..586098c 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -227,7 +227,8 @@ NetClientState *net_hub_port_find(int hub_id);
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
 
-#define POLYNOMIAL 0x04c11db6
+#define POLYNOMIAL_BE 0x04c11db6
+uint32_t net_crc32(const uint8_t *p, int len);
 unsigned compute_mcast_idx(const uint8_t *ep);
 
 #define vmstate_offset_macaddr(_state, _field)                       \
diff --git a/net/net.c b/net/net.c
index 39ef546..a14dc99 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1581,25 +1581,31 @@ int net_client_parse(QemuOptsList *opts_list, const char *optarg)
 
 /* From FreeBSD */
 /* XXX: optimize */
-unsigned compute_mcast_idx(const uint8_t *ep)
+uint32_t net_crc32(const uint8_t *p, int len)
 {
     uint32_t crc;
     int carry, i, j;
     uint8_t b;
 
     crc = 0xffffffff;
-    for (i = 0; i < 6; i++) {
-        b = *ep++;
+    for (i = 0; i < len; i++) {
+        b = *p++;
         for (j = 0; j < 8; j++) {
             carry = ((crc & 0x80000000L) ? 1 : 0) ^ (b & 0x01);
             crc <<= 1;
             b >>= 1;
             if (carry) {
-                crc = ((crc ^ POLYNOMIAL) | carry);
+                crc = ((crc ^ POLYNOMIAL_BE) | carry);
             }
         }
     }
-    return crc >> 26;
+
+    return crc;
+}
+
+unsigned compute_mcast_idx(const uint8_t *ep)
+{
+    return net_crc32(ep, ETH_ALEN) >> 26;
 }
 
 QemuOptsList qemu_netdev_opts = {
-- 
2.7.4

[Qemu-devel] [PULL 04/18] net: introduce net_crc32_le() function

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This provides a standard ethernet CRC32 little-endian implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/net/net.h |  2 ++
 net/net.c         | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 586098c..4afac1a 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -228,7 +228,9 @@ NetClientState *net_hub_port_find(int hub_id);
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
 
 #define POLYNOMIAL_BE 0x04c11db6
+#define POLYNOMIAL_LE 0xedb88320
 uint32_t net_crc32(const uint8_t *p, int len);
+uint32_t net_crc32_le(const uint8_t *p, int len);
 unsigned compute_mcast_idx(const uint8_t *ep);
 
 #define vmstate_offset_macaddr(_state, _field)                       \
diff --git a/net/net.c b/net/net.c
index a14dc99..4ecaf80 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1603,6 +1603,28 @@ uint32_t net_crc32(const uint8_t *p, int len)
     return crc;
 }
 
+uint32_t net_crc32_le(const uint8_t *p, int len)
+{
+    uint32_t crc;
+    int carry, i, j;
+    uint8_t b;
+
+    crc = 0xffffffff;
+    for (i = 0; i < len; i++) {
+        b = *p++;
+        for (j = 0; j < 8; j++) {
+            carry = (crc & 0x1) ^ (b & 0x01);
+            crc >>= 1;
+            b >>= 1;
+            if (carry) {
+                crc ^= POLYNOMIAL_LE;
+            }
+        }
+    }
+
+    return crc;
+}
+
 unsigned compute_mcast_idx(const uint8_t *ep)
 {
     return net_crc32(ep, ETH_ALEN) >> 26;
-- 
2.7.4

[Qemu-devel] [PULL 05/18] pcnet: switch pcnet over to use net_crc32_le()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Instead of lnc_mchash() using its own implementation, we can simply call
net_crc32_le() directly and apply the bit shift inline.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/pcnet.c | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 6544553..39d5d93 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -38,6 +38,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "qemu/timer.h"
 #include "qemu/sockets.h"
 #include "sysemu/sysemu.h"
@@ -522,25 +523,6 @@ static inline void pcnet_rmd_store(PCNetState *s, struct pcnet_RMD *rmd,
            be16_to_cpu(hdr->ether_type));       \
 } while (0)
 
-#define MULTICAST_FILTER_LEN 8
-
-static inline uint32_t lnc_mchash(const uint8_t *ether_addr)
-{
-#define LNC_POLYNOMIAL          0xEDB88320UL
-    uint32_t crc = 0xFFFFFFFF;
-    int idx, bit;
-    uint8_t data;
-
-    for (idx = 0; idx < 6; idx++) {
-        for (data = *ether_addr++, bit = 0; bit < MULTICAST_FILTER_LEN; bit++) {
-            crc = (crc >> 1) ^ (((crc ^ data) & 1) ? LNC_POLYNOMIAL : 0);
-            data >>= 1;
-        }
-    }
-    return crc;
-#undef LNC_POLYNOMIAL
-}
-
 #define CRC(crc, ch)	 (crc = (crc >> 8) ^ crctab[(crc ^ (ch)) & 0xff])
 
 /* generated using the AUTODIN II polynomial
@@ -656,7 +638,7 @@ static inline int ladr_match(PCNetState *s, const uint8_t *buf, int size)
             s->csr[10] & 0xff, s->csr[10] >> 8,
             s->csr[11] & 0xff, s->csr[11] >> 8
         };
-        int index = lnc_mchash(hdr->ether_dhost) >> 26;
+        int index = net_crc32_le(hdr->ether_dhost, ETH_ALEN) >> 26;
         return !!(ladr[index >> 3] & (1 << (index & 7)));
     }
     return 0;
-- 
2.7.4

[Qemu-devel] [PULL 06/18] eepro100: switch eepro100 e100_compute_mcast_idx() over to use net_crc32()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Instead of e100_compute_mcast_idx() using its own implementation, we can
simply call net_crc32() directly and apply the bit shift inline.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/eepro100.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 71cddfe..e30fed8 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -44,6 +44,7 @@
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "hw/nvram/eeprom93xx.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/dma.h"
@@ -325,28 +326,6 @@ static const uint16_t eepro100_mdi_mask[] = {
 
 static E100PCIDeviceInfo *eepro100_get_class(EEPRO100State *s);
 
-/* From FreeBSD (locally modified). */
-static unsigned e100_compute_mcast_idx(const uint8_t *ep)
-{
-    uint32_t crc;
-    int carry, i, j;
-    uint8_t b;
-
-    crc = 0xffffffff;
-    for (i = 0; i < 6; i++) {
-        b = *ep++;
-        for (j = 0; j < 8; j++) {
-            carry = ((crc & 0x80000000L) ? 1 : 0) ^ (b & 0x01);
-            crc <<= 1;
-            b >>= 1;
-            if (carry) {
-                crc = ((crc ^ POLYNOMIAL_BE) | carry);
-            }
-        }
-    }
-    return (crc & BITS(7, 2)) >> 2;
-}
-
 /* Read a 16 bit control/status (CSR) register. */
 static uint16_t e100_read_reg2(EEPRO100State *s, E100RegisterOffset addr)
 {
@@ -843,7 +822,8 @@ static void set_multicast_list(EEPRO100State *s)
         uint8_t multicast_addr[6];
         pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
         TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
-        unsigned mcast_idx = e100_compute_mcast_idx(multicast_addr);
+        unsigned mcast_idx = (net_crc32(multicast_addr, ETH_ALEN) &
+                              BITS(7, 2)) >> 2;
         assert(mcast_idx < 64);
         s->mult[mcast_idx >> 3] |= (1 << (mcast_idx & 7));
     }
@@ -1679,7 +1659,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
         if (s->configuration[21] & BIT(3)) {
           /* Multicast all bit is set, receive all multicast frames. */
         } else {
-          unsigned mcast_idx = e100_compute_mcast_idx(buf);
+          unsigned mcast_idx = (net_crc32(buf, ETH_ALEN) & BITS(7, 2)) >> 2;
           assert(mcast_idx < 64);
           if (s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7))) {
             /* Multicast frame is allowed in hash table. */
-- 
2.7.4

[Qemu-devel] [PULL 07/18] sunhme: switch sunhme over to use net_crc32_le()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Instead of sunhme_crc32_le() using its own implementation, we can simply call
net_crc32_le() directly and apply the bit shift inline.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/sunhme.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/hw/net/sunhme.c b/hw/net/sunhme.c
index b1efa1b..7558fca 100644
--- a/hw/net/sunhme.c
+++ b/hw/net/sunhme.c
@@ -698,29 +698,6 @@ static inline void sunhme_set_rx_ring_nr(SunHMEState *s, int i)
     s->erxregs[HME_ERXI_RING >> 2] = ring;
 }
 
-#define POLYNOMIAL_LE 0xedb88320
-static uint32_t sunhme_crc32_le(const uint8_t *p, int len)
-{
-    uint32_t crc;
-    int carry, i, j;
-    uint8_t b;
-
-    crc = 0xffffffff;
-    for (i = 0; i < len; i++) {
-        b = *p++;
-        for (j = 0; j < 8; j++) {
-            carry = (crc & 0x1) ^ (b & 0x01);
-            crc >>= 1;
-            b >>= 1;
-            if (carry) {
-                crc = crc ^ POLYNOMIAL_LE;
-            }
-        }
-    }
-
-    return crc;
-}
-
 #define MIN_BUF_SIZE 60
 
 static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
@@ -761,7 +738,7 @@ static ssize_t sunhme_receive(NetClientState *nc, const uint8_t *buf,
             trace_sunhme_rx_filter_bcast_match();
         } else if (s->macregs[HME_MACI_RXCFG >> 2] & HME_MAC_RXCFG_HENABLE) {
             /* Didn't match local address, check hash filter */
-            int mcast_idx = sunhme_crc32_le(buf, 6) >> 26;
+            int mcast_idx = net_crc32_le(buf, ETH_ALEN) >> 26;
             if (!(s->macregs[(HME_MACI_HASHTAB0 >> 2) - (mcast_idx >> 4)] &
                     (1 << (mcast_idx & 0xf)))) {
                 /* Didn't match hash filter */
-- 
2.7.4

[Qemu-devel] [PULL 08/18] sungem: fix multicast filter CRC calculation

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

>From the Linux sungem driver, we know that the multicast filter CRC is
implemented using ether_crc_le() which isn't the same as calling zlib's
crc32() function (the zlib implementation requires a complemented initial value
and also returns the complemented result).

Fix the multicast filter by simply using the new net_crc32_le() function.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/sungem.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/net/sungem.c b/hw/net/sungem.c
index 6aa8d11..60f1e47 100644
--- a/hw/net/sungem.c
+++ b/hw/net/sungem.c
@@ -11,12 +11,11 @@
 #include "hw/pci/pci.h"
 #include "qemu/log.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "net/checksum.h"
 #include "hw/net/mii.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
-/* For crc32 */
-#include <zlib.h>
 
 #define TYPE_SUNGEM "sungem"
 
@@ -595,7 +594,7 @@ static ssize_t sungem_receive(NetClientState *nc, const uint8_t *buf,
     }
 
     /* Get MAC crc */
-    mac_crc = crc32(~0, buf, 6);
+    mac_crc = net_crc32_le(buf, ETH_ALEN);
 
     /* Packet isn't for me ? */
     rx_cond = sungem_check_rx_mac(s, buf, mac_crc);
-- 
2.7.4

[Qemu-devel] [PULL 09/18] eepro100: use inline net_crc32() and bitshift instead of compute_mcast_idx()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This makes it much easier to compare the multicast CRC calculation endian and
bitshift against the Linux driver implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/eepro100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index e30fed8..a07a632 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1679,7 +1679,7 @@ static ssize_t nic_receive(NetClientState *nc, const uint8_t * buf, size_t size)
         rfd_status |= 0x0004;
     } else if (s->configuration[20] & BIT(6)) {
         /* Multiple IA bit set. */
-        unsigned mcast_idx = compute_mcast_idx(buf);
+        unsigned mcast_idx = net_crc32(buf, ETH_ALEN) >> 26;
         assert(mcast_idx < 64);
         if (s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7))) {
             TRACE(RXTX, logout("%p accepted, multiple IA bit set\n", s));
-- 
2.7.4

[Qemu-devel] [PULL 10/18] opencores_eth: use inline net_crc32() and bitshift instead of compute_mcast_idx()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This makes it much easier to compare the multicast CRC calculation endian and
bitshift against the Linux driver implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/opencores_eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c
index 268d6a7..d42b79c 100644
--- a/hw/net/opencores_eth.c
+++ b/hw/net/opencores_eth.c
@@ -36,6 +36,7 @@
 #include "hw/net/mii.h"
 #include "hw/sysbus.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
 
@@ -373,7 +374,7 @@ static ssize_t open_eth_receive(NetClientState *nc,
         if (memcmp(buf, bcast_addr, sizeof(bcast_addr)) == 0) {
             miss = GET_REGBIT(s, MODER, BRO);
         } else if ((buf[0] & 0x1) || GET_REGBIT(s, MODER, IAM)) {
-            unsigned mcast_idx = compute_mcast_idx(buf);
+            unsigned mcast_idx = net_crc32(buf, ETH_ALEN) >> 26;
             miss = !(s->regs[HASH0 + mcast_idx / 32] &
                     (1 << (mcast_idx % 32)));
             trace_open_eth_receive_mcast(
-- 
2.7.4

[Qemu-devel] [PULL 11/18] lan9118: use inline net_crc32() and bitshift instead of compute_mcast_idx()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This makes it much easier to compare the multicast CRC calculation endian and
bitshift against the Linux driver implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/lan9118.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/net/lan9118.c b/hw/net/lan9118.c
index 3db8937..b9032da 100644
--- a/hw/net/lan9118.c
+++ b/hw/net/lan9118.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "hw/sysbus.h"
 #include "net/net.h"
+#include "net/eth.h"
 #include "hw/devices.h"
 #include "sysemu/sysemu.h"
 #include "hw/ptimer.h"
@@ -504,7 +505,7 @@ static int lan9118_filter(lan9118_state *s, const uint8_t *addr)
         }
     } else {
         /* Hash matching  */
-        hash = compute_mcast_idx(addr);
+        hash = net_crc32(addr, ETH_ALEN) >> 26;
         if (hash & 0x20) {
             return (s->mac_hashh >> (hash & 0x1f)) & 1;
         } else {
-- 
2.7.4

[Qemu-devel] [PULL 12/18] ftgmac100: use inline net_crc32() and bitshift instead of compute_mcast_idx()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This makes it much easier to compare the multicast CRC calculation endian and
bitshift against the Linux driver implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/ftgmac100.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 3c36ab9..704f452 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -762,7 +762,7 @@ static int ftgmac100_filter(FTGMAC100State *s, const uint8_t *buf, size_t len)
             }
 
             /* TODO: this does not seem to work for ftgmac100 */
-            mcast_idx = compute_mcast_idx(buf);
+            mcast_idx = net_crc32(buf, ETH_ALEN) >> 26;
             if (!(s->math[mcast_idx / 32] & (1 << (mcast_idx % 32)))) {
                 return 0;
             }
-- 
2.7.4

[Qemu-devel] [PULL 13/18] ne2000: use inline net_crc32() and bitshift instead of compute_mcast_idx()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This makes it much easier to compare the multicast CRC calculation endian and
bitshift against the Linux driver implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/ne2000.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/net/ne2000.c b/hw/net/ne2000.c
index 6874c8c..687ef84 100644
--- a/hw/net/ne2000.c
+++ b/hw/net/ne2000.c
@@ -23,6 +23,8 @@
  */
 #include "qemu/osdep.h"
 #include "hw/pci/pci.h"
+#include "net/net.h"
+#include "net/eth.h"
 #include "ne2000.h"
 #include "hw/loader.h"
 #include "sysemu/sysemu.h"
@@ -199,7 +201,7 @@ ssize_t ne2000_receive(NetClientState *nc, const uint8_t *buf, size_t size_)
             /* multicast */
             if (!(s->rxcr & 0x08))
                 return size;
-            mcast_idx = compute_mcast_idx(buf);
+            mcast_idx = net_crc32(buf, ETH_ALEN) >> 26;
             if (!(s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7))))
                 return size;
         } else if (s->mem[0] == buf[0] &&
-- 
2.7.4

[Qemu-devel] [PULL 14/18] rtl8139: use inline net_crc32() and bitshift instead of compute_mcast_idx()

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

This makes it much easier to compare the multicast CRC calculation endian and
bitshift against the Linux driver implementation.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
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 a6b2a9f..1cc95b8 100644
--- a/hw/net/rtl8139.c
+++ b/hw/net/rtl8139.c
@@ -882,7 +882,7 @@ static ssize_t rtl8139_do_receive(NetClientState *nc, const uint8_t *buf, size_t
                 return size;
             }
 
-            int mcast_idx = compute_mcast_idx(buf);
+            int mcast_idx = net_crc32(buf, ETH_ALEN) >> 26;
 
             if (!(s->mult[mcast_idx >> 3] & (1 << (mcast_idx & 7))))
             {
-- 
2.7.4

[Qemu-devel] [PULL 15/18] net: remove unused compute_mcast_idx() function

[Jason Wang]

From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Now that all of the callers have been converted to compute the multicast index
inline using new net CRC functions, this function can now be dropped.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/net.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/net.c b/net/net.c
index 4ecaf80..5bc0a34 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1625,11 +1625,6 @@ uint32_t net_crc32_le(const uint8_t *p, int len)
     return crc;
 }
 
-unsigned compute_mcast_idx(const uint8_t *ep)
-{
-    return net_crc32(ep, ETH_ALEN) >> 26;
-}
-
 QemuOptsList qemu_netdev_opts = {
     .name = "netdev",
     .implied_opt_name = "type",
-- 
2.7.4

[Qemu-devel] [PULL 16/18] net: Remove the legacy "-net channel" parameter

[Jason Wang]

From: Thomas Huth <thuth@redhat.com>

It has never been documented, so hardly anybody knows about this
parameter, and it is marked as deprecated since QEMU v2.6.
Time to let it go now.

Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/net/slirp.h |  2 --
 net/net.c           |  7 -------
 net/slirp.c         | 34 ----------------------------------
 qemu-doc.texi       |  5 -----
 4 files changed, 48 deletions(-)

diff --git a/include/net/slirp.h b/include/net/slirp.h
index 64b795c..0c98e46 100644
--- a/include/net/slirp.h
+++ b/include/net/slirp.h
@@ -36,8 +36,6 @@ void hmp_hostfwd_remove(Monitor *mon, const QDict *qdict);
 
 int net_slirp_redir(const char *redir_str);
 
-int net_slirp_parse_legacy(QemuOptsList *opts_list, const char *optarg, int *ret);
-
 int net_slirp_smb(const char *exported_dir);
 
 void hmp_info_usernet(Monitor *mon, const QDict *qdict);
diff --git a/net/net.c b/net/net.c
index 5bc0a34..2b81c93 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1565,13 +1565,6 @@ int net_init_clients(void)
 
 int net_client_parse(QemuOptsList *opts_list, const char *optarg)
 {
-#if defined(CONFIG_SLIRP)
-    int ret;
-    if (net_slirp_parse_legacy(opts_list, optarg, &ret)) {
-        return ret;
-    }
-#endif
-
     if (!qemu_opts_parse_noisily(opts_list, optarg, true)) {
         return -1;
     }
diff --git a/net/slirp.c b/net/slirp.c
index 318a26e..cb8ca23 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -956,37 +956,3 @@ int net_init_slirp(const Netdev *netdev, const char *name,
 
     return ret;
 }
-
-int net_slirp_parse_legacy(QemuOptsList *opts_list, const char *optarg, int *ret)
-{
-    if (strcmp(opts_list->name, "net") != 0 ||
-        strncmp(optarg, "channel,", strlen("channel,")) != 0) {
-        return 0;
-    }
-
-    error_report("The '-net channel' option is deprecated. "
-                 "Please use '-netdev user,guestfwd=...' instead.");
-
-    /* handle legacy -net channel,port:chr */
-    optarg += strlen("channel,");
-
-    if (QTAILQ_EMPTY(&slirp_stacks)) {
-        struct slirp_config_str *config;
-
-        config = g_malloc(sizeof(*config));
-        pstrcpy(config->str, sizeof(config->str), optarg);
-        config->flags = SLIRP_CFG_LEGACY;
-        config->next = slirp_configs;
-        slirp_configs = config;
-        *ret = 0;
-    } else {
-        Error *err = NULL;
-        *ret = slirp_guestfwd(QTAILQ_FIRST(&slirp_stacks), optarg, 1, &err);
-        if (*ret < 0) {
-            error_report_err(err);
-        }
-    }
-
-    return 1;
-}
-
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 90bea73..140659a 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2670,11 +2670,6 @@ The ``-smb /some/dir'' argument is now a synonym for setting
 the ``-netdev user,smb=/some/dir'' argument instead. The new
 syntax allows different settings to be provided per NIC.
 
-@subsection -net channel (since 2.6.0)
-
-The ``--net channel,ARGS'' argument is now a synonym for setting
-the ``-netdev user,guestfwd=ARGS'' argument instead.
-
 @subsection -net vlan (since 2.9.0)
 
 The ``-net vlan=NN'' argument is partially replaced with the
-- 
2.7.4

[Qemu-devel] [PULL 17/18] qemu-doc: The "-net nic" option can be used with "netdev=...", too

[Jason Wang]

From: Thomas Huth <thuth@redhat.com>

Looks like we missed to document that it is also possible to specify
a netdev with "-net nic" - which is very useful if you want to
configure your on-board NIC to use a backend that has been specified
with "-netdev".

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 qemu-options.hx | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 94647e2..9f4dd3a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2035,9 +2035,10 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "-netdev hubport,id=str,hubid=n\n"
     "                configure a hub port on QEMU VLAN 'n'\n", QEMU_ARCH_ALL)
 DEF("net", HAS_ARG, QEMU_OPTION_net,
-    "-net nic[,vlan=n][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
-    "                old way to create a new NIC and connect it to VLAN 'n'\n"
-    "                (use the '-device devtype,netdev=str' option if possible instead)\n"
+    "-net nic[,vlan=n][,netdev=nd][,macaddr=mac][,model=type][,name=str][,addr=str][,vectors=v]\n"
+    "                configure or create an on-board (or machine default) NIC and\n"
+    "                connect it either to VLAN 'n' or the netdev 'nd' (for pluggable\n"
+    "                NICs please use '-device devtype,netdev=nd' instead)\n"
     "-net dump[,vlan=n][,file=f][,len=n]\n"
     "                dump traffic on vlan 'n' to file 'f' (max n bytes per packet)\n"
     "-net none       use it alone to have zero network devices. If no -net option\n"
@@ -2058,10 +2059,11 @@ DEF("net", HAS_ARG, QEMU_OPTION_net,
     "                old way to initialize a host network interface\n"
     "                (use the -netdev option if possible instead)\n", QEMU_ARCH_ALL)
 STEXI
-@item -net nic[,vlan=@var{n}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
+@item -net nic[,vlan=@var{n}][,netdev=@var{nd}][,macaddr=@var{mac}][,model=@var{type}] [,name=@var{name}][,addr=@var{addr}][,vectors=@var{v}]
 @findex -net
-Create a new Network Interface Card and connect it to VLAN @var{n} (@var{n}
-= 0 is the default). The NIC is an e1000 by default on the PC
+Configure or create an on-board (or machine default) Network Interface Card
+(NIC) and connect it either to VLAN @var{n} (@var{n} = 0 is the default), or
+to the netdev @var{nd}. The NIC is an e1000 by default on the PC
 target. Optionally, the MAC address can be changed to @var{mac}, the
 device address set to @var{addr} (PCI cards only),
 and a @var{name} can be assigned for use in monitor commands.
-- 
2.7.4

[Qemu-devel] [PULL 18/18] qemu-doc: Update the deprecation information of -tftp, -bootp, -redir and -smb

[Jason Wang]

From: Thomas Huth <thuth@redhat.com>

The information how to update the deprecated parameters was too scarce,
so that some people did not update to the new syntax yet. Provide some
more information to make sure that it is clear how to update from the
old syntax to the new one.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 qemu-doc.texi | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index 140659a..ae90f71 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2648,27 +2648,36 @@ combined with ``-vnc tls-creds=tls0'
 
 @subsection -tftp (since 2.6.0)
 
-The ``-tftp /some/dir'' argument is now a synonym for setting
-the ``-netdev user,tftp=/some/dir' argument. The new syntax
-allows different settings to be provided per NIC.
+The ``-tftp /some/dir'' argument is replaced by
+``-netdev user,id=x,tftp=/some/dir'', either accompanied with
+``-device ...,netdev=x'' (for pluggable NICs) or ``-net nic,netdev=x''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
 
 @subsection -bootp (since 2.6.0)
 
-The ``-bootp /some/file'' argument is now a synonym for setting
-the ``-netdev user,bootp=/some/file' argument. The new syntax
-allows different settings to be provided per NIC.
+The ``-bootp /some/file'' argument is replaced by
+``-netdev user,id=x,bootp=/some/file'', either accompanied with
+``-device ...,netdev=x'' (for pluggable NICs) or ``-net nic,netdev=x''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
 
 @subsection -redir (since 2.6.0)
 
-The ``-redir ARGS'' argument is now a synonym for setting
-the ``-netdev user,hostfwd=ARGS'' argument instead. The new
-syntax allows different settings to be provided per NIC.
+The ``-redir [tcp|udp]:hostport:[guestaddr]:guestport'' argument is
+replaced by ``-netdev
+user,id=x,hostfwd=[tcp|udp]:[hostaddr]:hostport-[guestaddr]:guestport'',
+either accompanied with ``-device ...,netdev=x'' (for pluggable NICs) or
+``-net nic,netdev=x'' (for embedded NICs). The new syntax allows different
+settings to be provided per NIC.
 
 @subsection -smb (since 2.6.0)
 
-The ``-smb /some/dir'' argument is now a synonym for setting
-the ``-netdev user,smb=/some/dir'' argument instead. The new
-syntax allows different settings to be provided per NIC.
+The ``-smb /some/dir'' argument is replaced by
+``-netdev user,id=x,smb=/some/dir'', either accompanied with
+``-device ...,netdev=x'' (for pluggable NICs) or ``-net nic,netdev=x''
+(for embedded NICs). The new syntax allows different settings to be
+provided per NIC.
 
 @subsection -net vlan (since 2.9.0)
 
-- 
2.7.4

Re: [Qemu-devel] [PULL 00/18] Net patches

[Peter Maydell]

On 22 December 2017 at 02:15, Jason Wang <jasowang@redhat.com> wrote:
> The following changes since commit 43ab9a5376c95c61ae898a222c4d04bdf60e239b:
>
>   hw/i386/vmport: fix missing definitions with non-log trace backends (2017-12-21 22:52:28 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 0065e915192cdf83c2700bb377e5323c2649476e:
>
>   qemu-doc: Update the deprecation information of -tftp, -bootp, -redir and -smb (2017-12-22 10:06:05 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Applied, thanks.

PS: just noticed, but "Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>"
is a bit of an odd Author string to end up in git commit logs, so you
and/or Ed might like to fix that up for any future patches.

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/18] Net patches

[Ed Swierk]

On Jan 8, 2018 02:16, "Peter Maydell" <peter.maydell@linaro.org> wrote:

On 22 December 2017 at 02:15, Jason Wang <jasowang@redhat.com> wrote:
> The following changes since commit 43ab9a5376c95c61ae898a222c4d04
bdf60e239b:
>
>   hw/i386/vmport: fix missing definitions with non-log trace backends
(2017-12-21 22:52:28 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 0065e915192cdf83c2700bb377e5323c2649476e:
>
>   qemu-doc: Update the deprecation information of -tftp, -bootp, -redir
and -smb (2017-12-22 10:06:05 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Applied, thanks.

PS: just noticed, but "Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>"
is a bit of an odd Author string to end up in git commit logs, so you
and/or Ed might like to fix that up for any future patches.


Thanks for pointing that out. I wonder if it's happening because I let
git-send-email automatically fill in the From: line.

Anyway if it's not too late to fix, the correct address is Ed Swierk <
eswierk@skyportsystems.com> .


thanks
-- PMM

Re: [Qemu-devel] [PULL 00/18] Net patches

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1470 bytes --]

On 01/08/2018 07:30 AM, Ed Swierk via Qemu-devel wrote:

> Applied, thanks.
> 
> PS: just noticed, but "Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>"
> is a bit of an odd Author string to end up in git commit logs, so you
> and/or Ed might like to fix that up for any future patches.
> 
> 
> Thanks for pointing that out. I wonder if it's happening because I let
> git-send-email automatically fill in the From: line.

It's also a factor of how strict your ISP is about DMARC handling; the
list automatically rewrites the 'From:' header to insert the 'via
Qemu-devel' tag if it detects DMARC settings at your ISP that won't
allow your email through as originally written.  Sadly, mailman doesn't
know to insert a manual 'From:' line in the body when it rewrites the
original From: header; but if you know that DMARC settings are going to
munge your original header, you can probably convince git to always
insert an explicit From: line in the message body to override whatever
munging the list does.

> 
> Anyway if it's not too late to fix, the correct address is Ed Swierk <
> eswierk@skyportsystems.com> .

You can also submit a patch against .mailmap so that existing
submissions with the suspect data will at least be credited to the
correct address for the purposes of crawling through git history.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PULL 00/18] Net patches

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1486 bytes --]

On 01/08/2018 09:10 AM, Eric Blake wrote:
> On 01/08/2018 07:30 AM, Ed Swierk via Qemu-devel wrote:
> 
>> Applied, thanks.
>>
>> PS: just noticed, but "Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>"
>> is a bit of an odd Author string to end up in git commit logs, so you
>> and/or Ed might like to fix that up for any future patches.
>>
>>
>> Thanks for pointing that out. I wonder if it's happening because I let
>> git-send-email automatically fill in the From: line.
> 
> It's also a factor of how strict your ISP is about DMARC handling; the
> list automatically rewrites the 'From:' header to insert the 'via
> Qemu-devel' tag if it detects DMARC settings at your ISP that won't
> allow your email through as originally written.  Sadly, mailman doesn't
> know to insert a manual 'From:' line in the body when it rewrites the
> original From: header; but if you know that DMARC settings are going to
> munge your original header, you can probably convince git to always
> insert an explicit From: line in the message body to override whatever
> munging the list does.
> 

Hmm - we probably ought to enhance checkpatch.pl to insist that patches
attributed to qemu-devel@nongnu.org as the author should be flagged as
invalid, since the list munging is a 1:M mapping rather than 1:1, and
therefore not easily invertable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PULL 00/18] Net patches

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

On 01/08/2018 09:18 AM, Eric Blake wrote:

> Hmm - we probably ought to enhance checkpatch.pl to insist that patches
> attributed to qemu-devel@nongnu.org as the author should be flagged as
> invalid, since the list munging is a 1:M mapping rather than 1:1, and
> therefore not easily invertable.

Stefan reminded me on IRC that checkpatch.pl doesn't process From: lines
(it only sees patches, not mboxes) - but that patchew could be taught to
make this sort of check.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PULL 00/18] Net patches

[Ed Swierk]

On Mon, Jan 8, 2018 at 7:10 AM, Eric Blake <eblake@redhat.com> wrote:
>
> On 01/08/2018 07:30 AM, Ed Swierk via Qemu-devel wrote:
>
> > Applied, thanks.
> >
> > PS: just noticed, but "Ed Swierk via Qemu-devel <qemu-devel@nongnu.org>"
> > is a bit of an odd Author string to end up in git commit logs, so you
> > and/or Ed might like to fix that up for any future patches.
> >
> >
> > Thanks for pointing that out. I wonder if it's happening because I let
> > git-send-email automatically fill in the From: line.
>
> It's also a factor of how strict your ISP is about DMARC handling; the
> list automatically rewrites the 'From:' header to insert the 'via
> Qemu-devel' tag if it detects DMARC settings at your ISP that won't
> allow your email through as originally written.  Sadly, mailman doesn't
> know to insert a manual 'From:' line in the body when it rewrites the
> original From: header; but if you know that DMARC settings are going to
> munge your original header, you can probably convince git to always
> insert an explicit From: line in the message body to override whatever
> munging the list does.

I'm trying to figure out what I need to fix on my end. I went back and
looked at the email headers. Here are the two that ended up with the
wrong author:

Return-Path: <eswierk@skyportsystems.com>
Received: from eswierk-sc.localdomain
(67-207-112-138.static.wiline.com. [67.207.112.138])
        by smtp.gmail.com with ESMTPSA id
d9sm20150979pfk.117.2017.11.14.15.23.43
        (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
        Tue, 14 Nov 2017 15:23:44 -0800 (PST)
From: Ed Swierk <eswierk@skyportsystems.com>
To: Jason Wang <jasowang@redhat.com>,
        "Daniel P . Berrange" <berrange@redhat.com>,
        Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-devel@nongnu.org,
        Ed Swierk <eswierk@skyportsystems.com>
Subject: [PATCH 1/2] e1000, e1000e: Move per-packet TX offload flags
out of context state
Date: Tue, 14 Nov 2017 15:23:33 -0800
Message-Id: <1510701814-52973-2-git-send-email-eswierk@skyportsystems.com>
X-Mailer: git-send-email 1.9.1
In-Reply-To: <1510701814-52973-1-git-send-email-eswierk@skyportsystems.com>
References: <1510701814-52973-1-git-send-email-eswierk@skyportsystems.com>

Return-Path: <eswierk@skyportsystems.com>
Received: from eswierk-sc.localdomain
(67-207-112-138.static.wiline.com. [67.207.112.138])
        by smtp.gmail.com with ESMTPSA id
d9sm20150979pfk.117.2017.11.14.15.23.45
        (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
        Tue, 14 Nov 2017 15:23:45 -0800 (PST)
From: Ed Swierk <eswierk@skyportsystems.com>
To: Jason Wang <jasowang@redhat.com>,
        "Daniel P . Berrange" <berrange@redhat.com>,
        Stefan Hajnoczi <stefanha@gmail.com>
Cc: qemu-devel@nongnu.org,
        Ed Swierk <eswierk@skyportsystems.com>
Subject: [PATCH 2/2] e1000: Separate TSO and non-TSO contexts, fixing
UDP TX corruption
Date: Tue, 14 Nov 2017 15:23:34 -0800
Message-Id: <1510701814-52973-3-git-send-email-eswierk@skyportsystems.com>
X-Mailer: git-send-email 1.9.1
In-Reply-To: <1510701814-52973-1-git-send-email-eswierk@skyportsystems.com>
References: <1510701814-52973-1-git-send-email-eswierk@skyportsystems.com>


This one had the correct author:

Return-Path: <eswierk@skyportsystems.com>
Received: from eswierk-sc.localdomain
(67-207-112-138.static.wiline.com. [67.207.112.138])
        by smtp.gmail.com with ESMTPSA id s3sm4082810pfk.7.2017.11.16.06.06.36
        (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
        Thu, 16 Nov 2017 06:06:37 -0800 (PST)
From: Ed Swierk <eswierk@skyportsystems.com>
To: Jason Wang <jasowang@redhat.com>,
        "Daniel P . Berrange" <berrange@redhat.com>,
        Stefan Hajnoczi <stefanha@gmail.com>,
        Dmitry Fleytman <dmitry@daynix.com>
Cc: qemu-devel@nongnu.org,
        Ed Swierk <eswierk@skyportsystems.com>
Subject: [PATCH v3 REPOST] net: Transmit zero UDP checksum as 0xFFFF
Date: Thu, 16 Nov 2017 06:06:06 -0800
Message-Id: <1510841166-40615-1-git-send-email-eswierk@skyportsystems.com>
X-Mailer: git-send-email 1.9.1


Is there any way to tell exactly what mailman didn't like about the
first two, causing it to change the author?

--Ed

Re: [Qemu-devel] [PULL 00/18] Net patches

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 4369 bytes --]

On 01/08/2018 09:54 AM, Ed Swierk wrote:

>> It's also a factor of how strict your ISP is about DMARC handling; the
>> list automatically rewrites the 'From:' header to insert the 'via
>> Qemu-devel' tag if it detects DMARC settings at your ISP that won't
>> allow your email through as originally written.  Sadly, mailman doesn't
>> know to insert a manual 'From:' line in the body when it rewrites the
>> original From: header; but if you know that DMARC settings are going to
>> munge your original header, you can probably convince git to always
>> insert an explicit From: line in the message body to override whatever
>> munging the list does.
> 
> I'm trying to figure out what I need to fix on my end. I went back and
> looked at the email headers. Here are the two that ended up with the
> wrong author:

https://dmarc.org/wiki/FAQ has some more information on DMARC.  There's
two aspects to it: one is that the domain in charge of the policy can
choose default reactions to any mail claiming to be sent from that
domain (valid, none, flag, reject); the other is that recipients can
choose whether to honor DMARC settings (some recipients let all mail
through, even if DMARC said to flag or reject it, others are stricter
and drop mail that DMARC marked as reject).  We had list readers
complaining about not getting emails (tending to come from recipients
that drop mails when DMARC says reject, and only mails from senders
where DMARC was set to reject rather than to flag), so we enabled the
mailman settings that rewrite the From: line based on a DMARC lookup of
the sender's information.

> 
> Return-Path: <eswierk@skyportsystems.com>
> Received: from eswierk-sc.localdomain
> (67-207-112-138.static.wiline.com. [67.207.112.138])
>         by smtp.gmail.com with ESMTPSA id
> d9sm20150979pfk.117.2017.11.14.15.23.43
>         (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
>         Tue, 14 Nov 2017 15:23:44 -0800 (PST)
> From: Ed Swierk <eswierk@skyportsystems.com>

Here, it looks like your local system picked gmail.com as its SMTP
server, and since gmail does not have an IP address in the range that
skyportsystems.com claims under its DMARC listings, your mail is
rejected rather than flagged by recipients that honor DMARC, so mailman
munged the header to let recipients get the mail anyway.

> This one had the correct author:
> 
> Return-Path: <eswierk@skyportsystems.com>
> Received: from eswierk-sc.localdomain
> (67-207-112-138.static.wiline.com. [67.207.112.138])
>         by smtp.gmail.com with ESMTPSA id s3sm4082810pfk.7.2017.11.16.06.06.36
>         (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128);
>         Thu, 16 Nov 2017 06:06:37 -0800 (PST)
> From: Ed Swierk <eswierk@skyportsystems.com>

That shows the same IP address as the sending location and again shows a
path through gmail.com, so I'm not sure why it was handled differently,
unless skyportsystems.com was changing DMARC policies between the two
messages, or if you really did send the two mails through different
setups.  The most annoying thing about DMARC is that most end users do
NOT have control over their domain's choice of DMARC settings; but the
rule of thumb is "if your domain has a strict DMARC policy, then mail
sent claiming to be from that domain must go through the SMTP servers
whitelisted by that domain", coupled with mailman's policy that "if a
message was sent from a domain with a DMARC that rejects the mailing
list IP, then rewrite the header to make the mail appear to come from
the list instead".

Meanwhile, as an example, I used to be able to spoof my redhat.com email
address when sending from my home computer and connecting to my ISP as
the SMTP sender; but about a year ago, Red Hat tightened their DMARC
settings so that if I want to send a mail that purports to be from
redhat.com, I now have to send it through Red Hat's SMTP server, rather
than my personal one, or else I risk my message not reaching the end
recipient.  But Red Hat's DMARC policy merely flags rather than
rejecting spoofed emails, and because it is not marked as reject,
mailman does not munge the headers of mails I send to the list.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]