[Qemu-devel] [PATCH v6] rtl8139: add vlan support

[Qemu-devel] [PATCH v6] rtl8139: add vlan support

[Benjamin Poirier]

Here is version 6 of my patchset to add vlan support to the emulated rtl8139
nic.

Changes since v5:
	* moved all receive changes to "add vlan tag extraction"
	* fixed checkpatch.pl style issues
	* fixed bugs in receive case related to small buffers and loopback
	  mode. Moved "too small buffer" code back where it used to be, though
	  it is changed in content.

Changes since v4:
	* removed alloca(), for real. Thanks to the reviewers for their
	  patience. This patchset now has more versions than the vlan header
	  has bytes!
	* corrected the unlikely, debug printf and long lines, as per comments
	* cleaned out ifdef's pertaining to ethernet checksum calculation.
	  According to a comment since removed they were related to an
	  "optimization":
	  > RTL8139 provides frame CRC with received packet, this feature
	  > seems to be ignored by most drivers, disabled by default
	  see commit ccf1d14

I've tested v5 using x86_64 host/guest with the usual procedure. I've also ran
the clang analyzer on the qemu code base, just for fun.

Changes since v3:
	* removed alloca() and #include <net/ethernet.h> as per comments
	* reordered patches to put extraction before insertion. Extraction
	  touches only the receive path but insertion touches both. The two
	  patches are now needed to have vlan functionnality.

I've tested v4 with x86_64 host/guest. I used the same testing procedure as
before. I've tested a plain configuration as well as one with tso + vlan
offload, successfully.

I had to hack around the Linux 8139cp driver to be able to enable tso on vlan
which leads me to wonder, can someone with access to the C+ spec or a real
card confirm that it can do tso and vlan offload at the same time? The patch
I used for the kernel is at https://gist.github.com/851895.

Changes since v2:
insertion:
	* moved insertion later in the process, to handle tso
	* use qemu_sendv_packet() to insert the tag for us
	* added dot1q_buf parameter to rtl8139_do_receive() to avoid some
	  memcpy() in loopback mode. Note that the code path through that
	  function is unchanged when dot1q_buf is NULL.

extraction:
	* reduced the amount of copying by moving the "frame too short" logic
	  after the removal of the vlan tag (as is done in e1000.c for
	  example). Unfortunately, that logic can no longer be shared betwen
	  C+ and C mode.

I've posted v2 of these patches back in November
http://article.gmane.org/gmane.comp.emulators.qemu/84252

I've tested v3 on the following combinations of guest and hosts:
host: x86_64, guest: x86_64
host: x86_64, guest: ppc32
host: ppc32, guest: ppc32

Testing on the x86_64 host used '-net tap' and consisted of:
* making an http transfert on the untagged interface.
* ping -s 0-1472 to another host on a vlan.
* making an scp upload to another host on a vlan.

Testing on the ppc32 host used '-net socket' connected to an x86_64 qemu-kvm
running the virtio nic and consisted of:
* establishing an ssh connection between the two using an untagged interface.
* ping -s 0-1472 between the two using a vlan.
* making an scp transfer in both directions using a vlan.

All that was successful. Nevertheless, it doesn't exercise all code paths so
care is in order.

Please note that the lack of vlan support in rtl8139 has taken a few people
aback:
https://bugzilla.redhat.com/show_bug.cgi?id=516587
http://article.gmane.org/gmane.linux.network.general/14266

Thanks,
-Ben

[Qemu-devel] [PATCH v6 1/3] rtl8139: cleanup FCS calculation

[Benjamin Poirier]

clean out ifdef's around ethernet checksum calculation

Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
---
 hw/rtl8139.c |   20 +++-----------------
 1 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..3772ac1 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -47,6 +47,9 @@
  *                                  Darwin)
  */
 
+/* For crc32 */
+#include <zlib.h>
+
 #include "hw.h"
 #include "pci.h"
 #include "qemu-timer.h"
@@ -62,14 +65,6 @@
 /* debug RTL8139 card C+ mode only */
 //#define DEBUG_RTL8139CP 1
 
-/* Calculate CRCs properly on Rx packets */
-#define RTL8139_CALCULATE_RXCRC 1
-
-#if defined(RTL8139_CALCULATE_RXCRC)
-/* For crc32 */
-#include <zlib.h>
-#endif
-
 #define SET_MASKED(input, mask, curr) \
     ( ( (input) & ~(mask) ) | ( (curr) & (mask) ) )
 
@@ -1030,11 +1025,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
         }
 
         /* write checksum */
-#if defined (RTL8139_CALCULATE_RXCRC)
         val = cpu_to_le32(crc32(0, buf, size));
-#else
-        val = 0;
-#endif
         cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
 
 /* first segment of received packet flag */
@@ -1136,12 +1127,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
         rtl8139_write_buffer(s, buf, size);
 
         /* write checksum */
-#if defined (RTL8139_CALCULATE_RXCRC)
         val = cpu_to_le32(crc32(0, buf, size));
-#else
-        val = 0;
-#endif
-
         rtl8139_write_buffer(s, (uint8_t *)&val, 4);
 
         /* correct buffer write pointer */
-- 
1.7.2.3

[Qemu-devel] [PATCH v6 2/3] rtl8139: add vlan tag extraction

[Benjamin Poirier]

Add support to the emulated hardware to extract vlan tags in packets
going from the network to the guest.

Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>

--

AFAIK, extraction is optional to get vlans working. The driver
requests rx detagging but should not assume that it was done. Under
Linux, the mac layer will catch the vlan ethertype. I only added this
part for completeness (to emulate the hardware more truthfully...)
---
 hw/rtl8139.c |  125 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 108 insertions(+), 17 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 3772ac1..312d362 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -72,6 +72,22 @@
 #define MOD2(input, size) \
     ( ( input ) & ( size - 1 )  )
 
+#define min(x, y) ({                \
+    typeof(x) _min1 = (x);          \
+    typeof(y) _min2 = (y);          \
+    (void) (&_min1 == &_min2);      \
+    _min1 < _min2 ? _min1 : _min2; })
+
+#define ETHER_ADDR_LEN 6
+#define ETHER_TYPE_LEN 2
+#define ETH_HLEN (ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN)
+#define ETH_P_IP    0x0800      /* Internet Protocol packet */
+#define ETH_P_8021Q 0x8100      /* 802.1Q VLAN Extended Header  */
+#define ETH_MTU     1500
+
+#define VLAN_TCI_LEN 2
+#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
+
 #if defined (DEBUG_RTL8139)
 #  define DEBUG_PRINT(x) do { printf x ; } while (0)
 #else
@@ -777,6 +793,14 @@ static void rtl8139_write_buffer(RTL8139State *s, const void *buf, int size)
     s->RxBufAddr += size;
 }
 
+static unsigned long rtl8139_write_buffer_crc(RTL8139State *s, const void
+    *buf, int size, unsigned long crc)
+{
+    rtl8139_write_buffer(s, buf, size);
+    return crc32(crc, buf, size);
+}
+
+
 #define MIN_BUF_SIZE 60
 static inline target_phys_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
 {
@@ -809,14 +833,20 @@ static int rtl8139_can_receive(VLANClientState *nc)
     }
 }
 
-static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_t size_, int do_interrupt)
+static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
+    size_t buf_size, int do_interrupt, const uint8_t *dot1q_buf)
 {
     RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
+    /* size_ is the total length of argument buffers */
+    const int size_ = buf_size + (dot1q_buf ? VLAN_HLEN : 0);
+    /* size is the length of the buffer passed to the driver */
     int size = size_;
+    const uint8_t *next_part;
+    size_t next_part_size;
 
     uint32_t packet_header = 0;
 
-    uint8_t buf1[60];
+    uint8_t buf1[MIN_BUF_SIZE];
     static const uint8_t broadcast_macaddr[6] =
         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -930,10 +960,28 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 
     /* if too small buffer, then expand it */
     if (size < MIN_BUF_SIZE) {
-        memcpy(buf1, buf, size);
-        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
+        size_t copied_size;
+
+        copied_size = min((size_t) ETHER_ADDR_LEN * 2, buf_size);
+        memcpy(buf1, buf, copied_size);
+        buf_size -= copied_size;
+
+        if (dot1q_buf) {
+            if (copied_size >= ETHER_ADDR_LEN * 2) {
+                memcpy(buf1 + copied_size, dot1q_buf, VLAN_HLEN);
+                copied_size += VLAN_HLEN;
+            }
+            dot1q_buf = NULL;
+        }
+
+        if (buf_size > 0) {
+            memcpy(buf1 + copied_size, &buf[ETHER_ADDR_LEN * 2], buf_size);
+            copied_size += buf_size;
+        }
+        memset(buf1 + copied_size, 0, MIN_BUF_SIZE - copied_size);
         buf = buf1;
         size = MIN_BUF_SIZE;
+        buf_size = MIN_BUF_SIZE;
     }
 
     if (rtl8139_cp_receiver_enabled(s))
@@ -996,6 +1044,37 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 
         uint32_t rx_space = rxdw0 & CP_RX_BUFFER_SIZE_MASK;
 
+        /* write VLAN info to descriptor variables.
+         * The "logical" packet is formed of:
+         * * buf[0..12[,
+         * * dot1q_buf (if offloading is enabled and tag is present or
+         *   dot1q_buf is passed in argument),
+         * * next_part[0..next_part_size[.
+         */
+        next_part = &buf[ETHER_ADDR_LEN * 2];
+        if (s->CpCmd & CPlusRxVLAN && (dot1q_buf || be16_to_cpup((uint16_t *)
+                    next_part) == ETH_P_8021Q)) {
+            if (!dot1q_buf) {
+                /* the tag is in the buffer */
+                dot1q_buf = next_part;
+                next_part += VLAN_HLEN;
+            }
+            size -= VLAN_HLEN;
+
+            rxdw1 &= ~CP_RX_VLAN_TAG_MASK;
+            /* BE + ~le_to_cpu()~ + cpu_to_le() = BE */
+            rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *)
+                &dot1q_buf[ETHER_TYPE_LEN]);
+
+            DEBUG_PRINT(("RTL8139: C+ Rx mode : extracted vlan tag with tci: "
+                    "%u\n", be16_to_cpup((uint16_t *)
+                        &dot1q_buf[ETHER_TYPE_LEN])));
+        } else {
+            /* reset VLAN tag flag */
+            rxdw1 &= ~CP_RX_TAVA;
+        }
+        next_part_size = buf + buf_size - next_part;
+
         /* TODO: scatter the packet over available receive ring descriptors space */
 
         if (size+4 > rx_space)
@@ -1017,7 +1096,18 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
         target_phys_addr_t rx_addr = rtl8139_addr64(rxbufLO, rxbufHI);
 
         /* receive/copy to target memory */
-        cpu_physical_memory_write( rx_addr, buf, size );
+        cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
+        val = crc32(0, buf, 2 * ETHER_ADDR_LEN);
+        if (dot1q_buf) {
+            if (!(s->CpCmd & CPlusRxVLAN)) {
+                cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN,
+                    dot1q_buf, VLAN_HLEN);
+            }
+            val = crc32(val, dot1q_buf, VLAN_HLEN);
+        }
+        cpu_physical_memory_write(rx_addr + size - next_part_size, next_part,
+            next_part_size);
+        val = crc32(val, next_part, next_part_size);
 
         if (s->CpCmd & CPlusRxChkSum)
         {
@@ -1025,7 +1115,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
         }
 
         /* write checksum */
-        val = cpu_to_le32(crc32(0, buf, size));
+        val = cpu_to_le32(val);
         cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
 
 /* first segment of received packet flag */
@@ -1070,9 +1160,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
         rxdw0 &= ~CP_RX_BUFFER_SIZE_MASK;
         rxdw0 |= (size+4);
 
-        /* reset VLAN tag flag */
-        rxdw1 &= ~CP_RX_TAVA;
-
         /* update ring data */
         val = cpu_to_le32(rxdw0);
         cpu_physical_memory_write(cplus_rx_ring_desc,    (uint8_t *)&val, 4);
@@ -1124,10 +1211,18 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 
         rtl8139_write_buffer(s, (uint8_t *)&val, 4);
 
-        rtl8139_write_buffer(s, buf, size);
+        /* receive/copy to target memory */
+        if (dot1q_buf) {
+            val = rtl8139_write_buffer_crc(s, buf, 2 * ETHER_ADDR_LEN, 0);
+            val = rtl8139_write_buffer_crc(s, dot1q_buf, VLAN_HLEN, val);
+            val = rtl8139_write_buffer_crc(s, buf + 2 * ETHER_ADDR_LEN,
+                buf_size - 2 * ETHER_ADDR_LEN, val);
+        } else {
+            val = rtl8139_write_buffer_crc(s, buf, size, 0);
+        }
 
         /* write checksum */
-        val = cpu_to_le32(crc32(0, buf, size));
+        val = cpu_to_le32(val);
         rtl8139_write_buffer(s, (uint8_t *)&val, 4);
 
         /* correct buffer write pointer */
@@ -1151,7 +1246,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 
 static ssize_t rtl8139_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
 {
-    return rtl8139_do_receive(nc, buf, size, 1);
+    return rtl8139_do_receive(nc, buf, size, 1, NULL);
 }
 
 static void rtl8139_reset_rxring(RTL8139State *s, uint32_t bufferSize)
@@ -1737,7 +1832,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size
     if (TxLoopBack == (s->TxConfig & TxLoopBack))
     {
         DEBUG_PRINT(("RTL8139: +++ transmit loopback mode\n"));
-        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt);
+        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt, NULL);
     }
     else
     {
@@ -2066,10 +2161,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
         {
             DEBUG_PRINT(("RTL8139: +++ C+ mode offloaded task checksum\n"));
 
-            #define ETH_P_IP	0x0800		/* Internet Protocol packet	*/
-            #define ETH_HLEN    14
-            #define ETH_MTU     1500
-
             /* ip packet header */
             ip_header *ip = NULL;
             int hlen = 0;
-- 
1.7.2.3

[Qemu-devel] [PATCH v6 3/3] rtl8139: add vlan tag insertion

[Benjamin Poirier]

Add support to the emulated hardware to insert vlan tags in packets
going from the guest to the network.

Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Blue Swirl <blauwirbel@gmail.com>
---
 hw/rtl8139.c |   57 +++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 312d362..11034fb 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1821,7 +1821,8 @@ static uint32_t rtl8139_RxConfig_read(RTL8139State *s)
     return ret;
 }
 
-static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size, int do_interrupt)
+static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
+    int do_interrupt, const uint8_t *dot1q_buf)
 {
     if (!size)
     {
@@ -1832,11 +1833,22 @@ static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size
     if (TxLoopBack == (s->TxConfig & TxLoopBack))
     {
         DEBUG_PRINT(("RTL8139: +++ transmit loopback mode\n"));
-        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt, NULL);
+        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt, dot1q_buf);
     }
     else
     {
-        qemu_send_packet(&s->nic->nc, buf, size);
+        if (dot1q_buf) {
+            struct iovec iov[] = {
+                { .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
+                { .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
+                { .iov_base = buf + ETHER_ADDR_LEN * 2,
+                    .iov_len = size - ETHER_ADDR_LEN * 2 },
+            };
+
+            qemu_sendv_packet(&s->nic->nc, iov, ARRAY_SIZE(iov));
+        } else {
+            qemu_send_packet(&s->nic->nc, buf, size);
+        }
     }
 }
 
@@ -1870,7 +1882,7 @@ static int rtl8139_transmit_one(RTL8139State *s, int descriptor)
     s->TxStatus[descriptor] |= TxHostOwns;
     s->TxStatus[descriptor] |= TxStatOK;
 
-    rtl8139_transfer_frame(s, txbuffer, txsize, 0);
+    rtl8139_transfer_frame(s, txbuffer, txsize, 0, NULL);
 
     DEBUG_PRINT(("RTL8139: +++ transmitted %d bytes from descriptor %d\n", txsize, descriptor));
 
@@ -1997,7 +2009,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 
     cpu_physical_memory_read(cplus_tx_ring_desc,    (uint8_t *)&val, 4);
     txdw0 = le32_to_cpu(val);
-    /* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
     cpu_physical_memory_read(cplus_tx_ring_desc+4,  (uint8_t *)&val, 4);
     txdw1 = le32_to_cpu(val);
     cpu_physical_memory_read(cplus_tx_ring_desc+8,  (uint8_t *)&val, 4);
@@ -2009,9 +2020,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
            descriptor,
            txdw0, txdw1, txbufLO, txbufHI));
 
-    /* TODO: the following discard cast should clean clang analyzer output */
-    (void)txdw1;
-
 /* w0 ownership flag */
 #define CP_TX_OWN (1<<31)
 /* w0 end of ring flag */
@@ -2035,9 +2043,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 /* w0 bits 0...15 : buffer size */
 #define CP_TX_BUFFER_SIZE (1<<16)
 #define CP_TX_BUFFER_SIZE_MASK (CP_TX_BUFFER_SIZE - 1)
-/* w1 tag available flag */
-#define CP_RX_TAGC (1<<17)
-/* w1 bits 0...15 : VLAN tag */
+/* w1 add tag flag */
+#define CP_TX_TAGC (1<<17)
+/* w1 bits 0...15 : VLAN tag (big endian) */
 #define CP_TX_VLAN_TAG_MASK ((1<<16) - 1)
 /* w2 low  32bit of Rx buffer ptr */
 /* w3 high 32bit of Rx buffer ptr */
@@ -2137,13 +2145,13 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
     /* update ring data */
     val = cpu_to_le32(txdw0);
     cpu_physical_memory_write(cplus_tx_ring_desc,    (uint8_t *)&val, 4);
-    /* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
-//    val = cpu_to_le32(txdw1);
-//    cpu_physical_memory_write(cplus_tx_ring_desc+4,  &val, 4);
 
     /* Now decide if descriptor being processed is holding the last segment of packet */
     if (txdw0 & CP_TX_LS)
     {
+        uint8_t dot1q_buffer_space[VLAN_HLEN];
+        uint16_t *dot1q_buffer;
+
         DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : descriptor %d is last segment descriptor\n", descriptor));
 
         /* can transfer fully assembled packet */
@@ -2152,6 +2160,21 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
         int      saved_size    = s->cplus_txbuffer_offset;
         int      saved_buffer_len = s->cplus_txbuffer_len;
 
+        /* create vlan tag */
+        if (txdw1 & CP_TX_TAGC) {
+            /* the vlan tag is in BE byte order in the descriptor
+             * BE + le_to_cpu() + ~swap()~ = cpu */
+            DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : inserting vlan tag with "
+                    "tci: %u\n", bswap16(txdw1 & CP_TX_VLAN_TAG_MASK)));
+
+            dot1q_buffer = (uint16_t *) dot1q_buffer_space;
+            dot1q_buffer[0] = cpu_to_be16(ETH_P_8021Q);
+            /* BE + le_to_cpu() + ~cpu_to_le()~ = BE */
+            dot1q_buffer[1] = cpu_to_le16(txdw1 & CP_TX_VLAN_TAG_MASK);
+        } else {
+            dot1q_buffer = NULL;
+        }
+
         /* reset the card space to protect from recursive call */
         s->cplus_txbuffer = NULL;
         s->cplus_txbuffer_offset = 0;
@@ -2305,7 +2328,8 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 
                         int tso_send_size = ETH_HLEN + hlen + tcp_hlen + chunk_size;
                         DEBUG_PRINT(("RTL8139: +++ C+ mode TSO transferring packet size %d\n", tso_send_size));
-                        rtl8139_transfer_frame(s, saved_buffer, tso_send_size, 0);
+                        rtl8139_transfer_frame(s, saved_buffer, tso_send_size,
+                            0, (uint8_t *) dot1q_buffer);
 
                         /* add transferred count to TCP sequence number */
                         p_tcp_hdr->th_seq = cpu_to_be32(chunk_size + be32_to_cpu(p_tcp_hdr->th_seq));
@@ -2378,7 +2402,8 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 
         DEBUG_PRINT(("RTL8139: +++ C+ mode transmitting %d bytes packet\n", saved_size));
 
-        rtl8139_transfer_frame(s, saved_buffer, saved_size, 1);
+        rtl8139_transfer_frame(s, saved_buffer, saved_size, 1,
+            (uint8_t *) dot1q_buffer);
 
         /* restore card space if there was no recursion and reset offset */
         if (!s->cplus_txbuffer)
-- 
1.7.2.3

[Qemu-devel] Re: [PATCH v6 1/3] rtl8139: cleanup FCS calculation

[Igor Kovalenko]

On Fri, Mar 11, 2011 at 3:35 AM, Benjamin Poirier
<benjamin.poirier@gmail.com> wrote:
> clean out ifdef's around ethernet checksum calculation
>
> Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
> Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/rtl8139.c |   20 +++-----------------
>  1 files changed, 3 insertions(+), 17 deletions(-)

Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

-- 
Kind regards,
Igor V. Kovalenko

[Qemu-devel] Re: [PATCH v6 1/3] rtl8139: cleanup FCS calculation

[Michael S. Tsirkin]

On Fri, Mar 11, 2011 at 08:35:10AM +0300, Igor Kovalenko wrote:
> On Fri, Mar 11, 2011 at 3:35 AM, Benjamin Poirier
> <benjamin.poirier@gmail.com> wrote:
> > clean out ifdef's around ethernet checksum calculation
> >
> > Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
> > Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> > Cc: Jason Wang <jasowang@redhat.com>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Blue Swirl <blauwirbel@gmail.com>
> > ---
> >  hw/rtl8139.c |   20 +++-----------------
> >  1 files changed, 3 insertions(+), 17 deletions(-)
> 
> Signed-off-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

I think you mean Acked-by or Tested-by.  Signed-off-by is for when you
forward someone's patch to the list.

> -- 
> Kind regards,
> Igor V. Kovalenko

[Qemu-devel] Re: [PATCH v6 1/3] rtl8139: cleanup FCS calculation

[Igor Kovalenko]

On Fri, Mar 11, 2011 at 3:35 AM, Benjamin Poirier
<benjamin.poirier@gmail.com> wrote:
> clean out ifdef's around ethernet checksum calculation
>
> Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
> Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/rtl8139.c |   20 +++-----------------
>  1 files changed, 3 insertions(+), 17 deletions(-)
>

Acked-by: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>

-- 
Kind regards,
Igor V. Kovalenko

Re: [Qemu-devel] [PATCH v6] rtl8139: add vlan support

[Jason Wang]

On 03/11/2011 08:35 AM, Benjamin Poirier wrote:
> Here is version 6 of my patchset to add vlan support to the emulated rtl8139
> nic.
>
> Changes since v5:
> 	* moved all receive changes to "add vlan tag extraction"
> 	* fixed checkpatch.pl style issues
> 	* fixed bugs in receive case related to small buffers and loopback
> 	  mode. Moved "too small buffer" code back where it used to be, though
> 	  it is changed in content.
>
> Changes since v4:
> 	* removed alloca(), for real. Thanks to the reviewers for their
> 	  patience. This patchset now has more versions than the vlan header
> 	  has bytes!
> 	* corrected the unlikely, debug printf and long lines, as per comments
> 	* cleaned out ifdef's pertaining to ethernet checksum calculation.
> 	  According to a comment since removed they were related to an
> 	  "optimization":
> 	>  RTL8139 provides frame CRC with received packet, this feature
> 	>  seems to be ignored by most drivers, disabled by default
> 	  see commit ccf1d14
>
> I've tested v5 using x86_64 host/guest with the usual procedure. I've also ran
> the clang analyzer on the qemu code base, just for fun.
>
> Changes since v3:
> 	* removed alloca() and #include<net/ethernet.h>  as per comments
> 	* reordered patches to put extraction before insertion. Extraction
> 	  touches only the receive path but insertion touches both. The two
> 	  patches are now needed to have vlan functionnality.
>
> I've tested v4 with x86_64 host/guest. I used the same testing procedure as
> before. I've tested a plain configuration as well as one with tso + vlan
> offload, successfully.
>
> I had to hack around the Linux 8139cp driver to be able to enable tso on vlan
> which leads me to wonder, can someone with access to the C+ spec or a real
> card confirm that it can do tso and vlan offload at the same time? The patch
> I used for the kernel is at https://gist.github.com/851895.

8139cp does have the support for VLAN with TSO. See IFCAP_VLAN_HWTSO 
capabilities in FreeBSD re(4) driver: 
http://svn.freebsd.org/viewvc/base/release/8.2.0/sys/dev/re/if_re.c?revision=218742&view=markup. 
You can use it in your testing for both vlan tso and loopback TX. 
Additionally, you can submit your 8139 modifications to linux upstream I 
think.

> Changes since v2:
> insertion:
> 	* moved insertion later in the process, to handle tso
> 	* use qemu_sendv_packet() to insert the tag for us
> 	* added dot1q_buf parameter to rtl8139_do_receive() to avoid some
> 	  memcpy() in loopback mode. Note that the code path through that
> 	  function is unchanged when dot1q_buf is NULL.
>
> extraction:
> 	* reduced the amount of copying by moving the "frame too short" logic
> 	  after the removal of the vlan tag (as is done in e1000.c for
> 	  example). Unfortunately, that logic can no longer be shared betwen
> 	  C+ and C mode.
>
> I've posted v2 of these patches back in November
> http://article.gmane.org/gmane.comp.emulators.qemu/84252
>
> I've tested v3 on the following combinations of guest and hosts:
> host: x86_64, guest: x86_64
> host: x86_64, guest: ppc32
> host: ppc32, guest: ppc32
>
> Testing on the x86_64 host used '-net tap' and consisted of:
> * making an http transfert on the untagged interface.
> * ping -s 0-1472 to another host on a vlan.
> * making an scp upload to another host on a vlan.
>
> Testing on the ppc32 host used '-net socket' connected to an x86_64 qemu-kvm
> running the virtio nic and consisted of:
> * establishing an ssh connection between the two using an untagged interface.
> * ping -s 0-1472 between the two using a vlan.
> * making an scp transfer in both directions using a vlan.
>
> All that was successful. Nevertheless, it doesn't exercise all code paths so
> care is in order.
>
> Please note that the lack of vlan support in rtl8139 has taken a few people
> aback:
> https://bugzilla.redhat.com/show_bug.cgi?id=516587
> http://article.gmane.org/gmane.linux.network.general/14266
>
> Thanks,
> -Ben
>

[Qemu-devel] [PATCH v6 2/3] rtl8139: add vlan tag extraction

[Jason Wang]

Benjamin Poirier writes:
 > Add support to the emulated hardware to extract vlan tags in packets
 > going from the network to the guest.
 > 
 > Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
 > Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
 > Cc: Jason Wang <jasowang@redhat.com>
 > Cc: Michael S. Tsirkin <mst@redhat.com>
 > Cc: Blue Swirl <blauwirbel@gmail.com>
 > 

This patch looks functionally right, and one thing needed to be considered is
the dot1q_buf optimization for TX loopback (which is rerely used I think). I
notice it would bring extra complexity for both crc and padding. Maybe we could
just add the vlan tag in tx and pass a whole buffer to rx (Though it may bring
one more copy, but it could greatly simplifized the code).

Any thoughts?

 > --
 > 
 > AFAIK, extraction is optional to get vlans working. The driver
 > requests rx detagging but should not assume that it was done. Under
 > Linux, the mac layer will catch the vlan ethertype. I only added this
 > part for completeness (to emulate the hardware more truthfully...)
 > ---
 >  hw/rtl8139.c |  125 ++++++++++++++++++++++++++++++++++++++++++++++++++--------
 >  1 files changed, 108 insertions(+), 17 deletions(-)
 > 
 > diff --git a/hw/rtl8139.c b/hw/rtl8139.c
 > index 3772ac1..312d362 100644
 > --- a/hw/rtl8139.c
 > +++ b/hw/rtl8139.c
 > @@ -72,6 +72,22 @@
 >  #define MOD2(input, size) \
 >      ( ( input ) & ( size - 1 )  )
 >  
 > +#define min(x, y) ({                \
 > +    typeof(x) _min1 = (x);          \
 > +    typeof(y) _min2 = (y);          \
 > +    (void) (&_min1 == &_min2);      \
 > +    _min1 < _min2 ? _min1 : _min2; })
 > +
 > +#define ETHER_ADDR_LEN 6
 > +#define ETHER_TYPE_LEN 2
 > +#define ETH_HLEN (ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN)
 > +#define ETH_P_IP    0x0800      /* Internet Protocol packet */
 > +#define ETH_P_8021Q 0x8100      /* 802.1Q VLAN Extended Header  */
 > +#define ETH_MTU     1500
 > +
 > +#define VLAN_TCI_LEN 2
 > +#define VLAN_HLEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
 > +
 >  #if defined (DEBUG_RTL8139)
 >  #  define DEBUG_PRINT(x) do { printf x ; } while (0)
 >  #else
 > @@ -777,6 +793,14 @@ static void rtl8139_write_buffer(RTL8139State *s, const void *buf, int size)
 >      s->RxBufAddr += size;
 >  }
 >  
 > +static unsigned long rtl8139_write_buffer_crc(RTL8139State *s, const void
 > +    *buf, int size, unsigned long crc)
 > +{
 > +    rtl8139_write_buffer(s, buf, size);
 > +    return crc32(crc, buf, size);
 > +}
 > +
 > +
 >  #define MIN_BUF_SIZE 60
 >  static inline target_phys_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
 >  {
 > @@ -809,14 +833,20 @@ static int rtl8139_can_receive(VLANClientState *nc)
 >      }
 >  }
 >  
 > -static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_t size_, int do_interrupt)
 > +static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
 > +    size_t buf_size, int do_interrupt, const uint8_t *dot1q_buf)
 >  {
 >      RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
 > +    /* size_ is the total length of argument buffers */
 > +    const int size_ = buf_size + (dot1q_buf ? VLAN_HLEN : 0);
 > +    /* size is the length of the buffer passed to the driver */
 >      int size = size_;
 > +    const uint8_t *next_part;
 > +    size_t next_part_size;
 >  
 >      uint32_t packet_header = 0;
 >  
 > -    uint8_t buf1[60];
 > +    uint8_t buf1[MIN_BUF_SIZE];
 >      static const uint8_t broadcast_macaddr[6] =
 >          { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 >  
 > @@ -930,10 +960,28 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >  
 >      /* if too small buffer, then expand it */
 >      if (size < MIN_BUF_SIZE) {
 > -        memcpy(buf1, buf, size);
 > -        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
 > +        size_t copied_size;
 > +
 > +        copied_size = min((size_t) ETHER_ADDR_LEN * 2, buf_size);
 > +        memcpy(buf1, buf, copied_size);
 > +        buf_size -= copied_size;
 > +
 > +        if (dot1q_buf) {
 > +            if (copied_size >= ETHER_ADDR_LEN * 2) {
 > +                memcpy(buf1 + copied_size, dot1q_buf, VLAN_HLEN);
 > +                copied_size += VLAN_HLEN;
 > +            }
 > +            dot1q_buf = NULL;
 > +        }
 > +
 > +        if (buf_size > 0) {
 > +            memcpy(buf1 + copied_size, &buf[ETHER_ADDR_LEN * 2], buf_size);
 > +            copied_size += buf_size;
 > +        }
 > +        memset(buf1 + copied_size, 0, MIN_BUF_SIZE - copied_size);
 >          buf = buf1;
 >          size = MIN_BUF_SIZE;
 > +        buf_size = MIN_BUF_SIZE;
 >      }
 >  
 >      if (rtl8139_cp_receiver_enabled(s))
 > @@ -996,6 +1044,37 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >  
 >          uint32_t rx_space = rxdw0 & CP_RX_BUFFER_SIZE_MASK;
 >  
 > +        /* write VLAN info to descriptor variables.
 > +         * The "logical" packet is formed of:
 > +         * * buf[0..12[,
 > +         * * dot1q_buf (if offloading is enabled and tag is present or
 > +         *   dot1q_buf is passed in argument),
 > +         * * next_part[0..next_part_size[.
 > +         */
 > +        next_part = &buf[ETHER_ADDR_LEN * 2];
 > +        if (s->CpCmd & CPlusRxVLAN && (dot1q_buf || be16_to_cpup((uint16_t *)
 > +                    next_part) == ETH_P_8021Q)) {
 > +            if (!dot1q_buf) {
 > +                /* the tag is in the buffer */
 > +                dot1q_buf = next_part;
 > +                next_part += VLAN_HLEN;
 > +            }
 > +            size -= VLAN_HLEN;
 > +
 > +            rxdw1 &= ~CP_RX_VLAN_TAG_MASK;
 > +            /* BE + ~le_to_cpu()~ + cpu_to_le() = BE */
 > +            rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *)
 > +                &dot1q_buf[ETHER_TYPE_LEN]);
 > +
 > +            DEBUG_PRINT(("RTL8139: C+ Rx mode : extracted vlan tag with tci: "
 > +                    "%u\n", be16_to_cpup((uint16_t *)
 > +                        &dot1q_buf[ETHER_TYPE_LEN])));
 > +        } else {
 > +            /* reset VLAN tag flag */
 > +            rxdw1 &= ~CP_RX_TAVA;
 > +        }
 > +        next_part_size = buf + buf_size - next_part;
 > +
 >          /* TODO: scatter the packet over available receive ring descriptors space */
 >  
 >          if (size+4 > rx_space)
 > @@ -1017,7 +1096,18 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >          target_phys_addr_t rx_addr = rtl8139_addr64(rxbufLO, rxbufHI);
 >  
 >          /* receive/copy to target memory */
 > -        cpu_physical_memory_write( rx_addr, buf, size );
 > +        cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
 > +        val = crc32(0, buf, 2 * ETHER_ADDR_LEN);
 > +        if (dot1q_buf) {
 > +            if (!(s->CpCmd & CPlusRxVLAN)) {
 > +                cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN,
 > +                    dot1q_buf, VLAN_HLEN);
 > +            }
 > +            val = crc32(val, dot1q_buf, VLAN_HLEN);
 > +        }
 > +        cpu_physical_memory_write(rx_addr + size - next_part_size, next_part,
 > +            next_part_size);
 > +        val = crc32(val, next_part, next_part_size);
 >  
 >          if (s->CpCmd & CPlusRxChkSum)
 >          {
 > @@ -1025,7 +1115,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >          }
 >  
 >          /* write checksum */
 > -        val = cpu_to_le32(crc32(0, buf, size));
 > +        val = cpu_to_le32(val);
 >          cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
 >  
 >  /* first segment of received packet flag */
 > @@ -1070,9 +1160,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >          rxdw0 &= ~CP_RX_BUFFER_SIZE_MASK;
 >          rxdw0 |= (size+4);
 >  
 > -        /* reset VLAN tag flag */
 > -        rxdw1 &= ~CP_RX_TAVA;
 > -
 >          /* update ring data */
 >          val = cpu_to_le32(rxdw0);
 >          cpu_physical_memory_write(cplus_rx_ring_desc,    (uint8_t *)&val, 4);
 > @@ -1124,10 +1211,18 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >  
 >          rtl8139_write_buffer(s, (uint8_t *)&val, 4);
 >  
 > -        rtl8139_write_buffer(s, buf, size);
 > +        /* receive/copy to target memory */
 > +        if (dot1q_buf) {
 > +            val = rtl8139_write_buffer_crc(s, buf, 2 * ETHER_ADDR_LEN, 0);
 > +            val = rtl8139_write_buffer_crc(s, dot1q_buf, VLAN_HLEN, val);
 > +            val = rtl8139_write_buffer_crc(s, buf + 2 * ETHER_ADDR_LEN,
 > +                buf_size - 2 * ETHER_ADDR_LEN, val);
 > +        } else {
 > +            val = rtl8139_write_buffer_crc(s, buf, size, 0);
 > +        }
 >  
 >          /* write checksum */
 > -        val = cpu_to_le32(crc32(0, buf, size));
 > +        val = cpu_to_le32(val);
 >          rtl8139_write_buffer(s, (uint8_t *)&val, 4);
 >  
 >          /* correct buffer write pointer */
 > @@ -1151,7 +1246,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
 >  
 >  static ssize_t rtl8139_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
 >  {
 > -    return rtl8139_do_receive(nc, buf, size, 1);
 > +    return rtl8139_do_receive(nc, buf, size, 1, NULL);
 >  }
 >  
 >  static void rtl8139_reset_rxring(RTL8139State *s, uint32_t bufferSize)
 > @@ -1737,7 +1832,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size
 >      if (TxLoopBack == (s->TxConfig & TxLoopBack))
 >      {
 >          DEBUG_PRINT(("RTL8139: +++ transmit loopback mode\n"));
 > -        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt);
 > +        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt, NULL);
 >      }
 >      else
 >      {
 > @@ -2066,10 +2161,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >          {
 >              DEBUG_PRINT(("RTL8139: +++ C+ mode offloaded task checksum\n"));
 >  
 > -            #define ETH_P_IP	0x0800		/* Internet Protocol packet	*/
 > -            #define ETH_HLEN    14
 > -            #define ETH_MTU     1500
 > -
 >              /* ip packet header */
 >              ip_header *ip = NULL;
 >              int hlen = 0;
 > -- 
 > 1.7.2.3
 > 

[Qemu-devel] [PATCH v6 3/3] rtl8139: add vlan tag insertion

[Jason Wang]

Benjamin Poirier writes:
 > Add support to the emulated hardware to insert vlan tags in packets
 > going from the guest to the network.
 > 
 > Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
 > Cc: Igor V. Kovalenko <igor.v.kovalenko@gmail.com>
 > Cc: Jason Wang <jasowang@redhat.com>
 > Cc: Michael S. Tsirkin <mst@redhat.com>
 > Cc: Blue Swirl <blauwirbel@gmail.com>
 > ---
 >  hw/rtl8139.c |   57 +++++++++++++++++++++++++++++++++++++++++----------------
 >  1 files changed, 41 insertions(+), 16 deletions(-)
 > 
 > diff --git a/hw/rtl8139.c b/hw/rtl8139.c
 > index 312d362..11034fb 100644
 > --- a/hw/rtl8139.c
 > +++ b/hw/rtl8139.c
 > @@ -1821,7 +1821,8 @@ static uint32_t rtl8139_RxConfig_read(RTL8139State *s)
 >      return ret;
 >  }
 >  
 > -static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size, int do_interrupt)
 > +static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size,
 > +    int do_interrupt, const uint8_t *dot1q_buf)
 >  {
 >      if (!size)
 >      {
 > @@ -1832,11 +1833,22 @@ static void rtl8139_transfer_frame(RTL8139State *s, const uint8_t *buf, int size
 >      if (TxLoopBack == (s->TxConfig & TxLoopBack))
 >      {
 >          DEBUG_PRINT(("RTL8139: +++ transmit loopback mode\n"));
 > -        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt, NULL);
 > +        rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt, dot1q_buf);
 >      }
 >      else
 >      {
 > -        qemu_send_packet(&s->nic->nc, buf, size);
 > +        if (dot1q_buf) {
 > +            struct iovec iov[] = {
 > +                { .iov_base = buf, .iov_len = ETHER_ADDR_LEN * 2 },
 > +                { .iov_base = (void *) dot1q_buf, .iov_len = VLAN_HLEN },
 > +                { .iov_base = buf + ETHER_ADDR_LEN * 2,
 > +                    .iov_len = size - ETHER_ADDR_LEN * 2 },

Need to protect againt the malicious guest to send packet whose size is less
than ETHER_ADDR_LEN*2. Other looks good.


 > +            };
 > +
 > +            qemu_sendv_packet(&s->nic->nc, iov, ARRAY_SIZE(iov));
 > +        } else {
 > +            qemu_send_packet(&s->nic->nc, buf, size);
 > +        }
 >      }
 >  }
 >  
 > @@ -1870,7 +1882,7 @@ static int rtl8139_transmit_one(RTL8139State *s, int descriptor)
 >      s->TxStatus[descriptor] |= TxHostOwns;
 >      s->TxStatus[descriptor] |= TxStatOK;
 >  
 > -    rtl8139_transfer_frame(s, txbuffer, txsize, 0);
 > +    rtl8139_transfer_frame(s, txbuffer, txsize, 0, NULL);
 >  
 >      DEBUG_PRINT(("RTL8139: +++ transmitted %d bytes from descriptor %d\n", txsize, descriptor));
 >  
 > @@ -1997,7 +2009,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >  
 >      cpu_physical_memory_read(cplus_tx_ring_desc,    (uint8_t *)&val, 4);
 >      txdw0 = le32_to_cpu(val);
 > -    /* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
 >      cpu_physical_memory_read(cplus_tx_ring_desc+4,  (uint8_t *)&val, 4);
 >      txdw1 = le32_to_cpu(val);
 >      cpu_physical_memory_read(cplus_tx_ring_desc+8,  (uint8_t *)&val, 4);
 > @@ -2009,9 +2020,6 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >             descriptor,
 >             txdw0, txdw1, txbufLO, txbufHI));
 >  
 > -    /* TODO: the following discard cast should clean clang analyzer output */
 > -    (void)txdw1;
 > -
 >  /* w0 ownership flag */
 >  #define CP_TX_OWN (1<<31)
 >  /* w0 end of ring flag */
 > @@ -2035,9 +2043,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >  /* w0 bits 0...15 : buffer size */
 >  #define CP_TX_BUFFER_SIZE (1<<16)
 >  #define CP_TX_BUFFER_SIZE_MASK (CP_TX_BUFFER_SIZE - 1)
 > -/* w1 tag available flag */
 > -#define CP_RX_TAGC (1<<17)
 > -/* w1 bits 0...15 : VLAN tag */
 > +/* w1 add tag flag */
 > +#define CP_TX_TAGC (1<<17)
 > +/* w1 bits 0...15 : VLAN tag (big endian) */
 >  #define CP_TX_VLAN_TAG_MASK ((1<<16) - 1)
 >  /* w2 low  32bit of Rx buffer ptr */
 >  /* w3 high 32bit of Rx buffer ptr */
 > @@ -2137,13 +2145,13 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >      /* update ring data */
 >      val = cpu_to_le32(txdw0);
 >      cpu_physical_memory_write(cplus_tx_ring_desc,    (uint8_t *)&val, 4);
 > -    /* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
 > -//    val = cpu_to_le32(txdw1);
 > -//    cpu_physical_memory_write(cplus_tx_ring_desc+4,  &val, 4);
 >  
 >      /* Now decide if descriptor being processed is holding the last segment of packet */
 >      if (txdw0 & CP_TX_LS)
 >      {
 > +        uint8_t dot1q_buffer_space[VLAN_HLEN];
 > +        uint16_t *dot1q_buffer;
 > +
 >          DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : descriptor %d is last segment descriptor\n", descriptor));
 >  
 >          /* can transfer fully assembled packet */
 > @@ -2152,6 +2160,21 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >          int      saved_size    = s->cplus_txbuffer_offset;
 >          int      saved_buffer_len = s->cplus_txbuffer_len;
 >  
 > +        /* create vlan tag */
 > +        if (txdw1 & CP_TX_TAGC) {
 > +            /* the vlan tag is in BE byte order in the descriptor
 > +             * BE + le_to_cpu() + ~swap()~ = cpu */
 > +            DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : inserting vlan tag with "
 > +                    "tci: %u\n", bswap16(txdw1 & CP_TX_VLAN_TAG_MASK)));
 > +
 > +            dot1q_buffer = (uint16_t *) dot1q_buffer_space;
 > +            dot1q_buffer[0] = cpu_to_be16(ETH_P_8021Q);
 > +            /* BE + le_to_cpu() + ~cpu_to_le()~ = BE */
 > +            dot1q_buffer[1] = cpu_to_le16(txdw1 & CP_TX_VLAN_TAG_MASK);
 > +        } else {
 > +            dot1q_buffer = NULL;
 > +        }
 > +
 >          /* reset the card space to protect from recursive call */
 >          s->cplus_txbuffer = NULL;
 >          s->cplus_txbuffer_offset = 0;
 > @@ -2305,7 +2328,8 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >  
 >                          int tso_send_size = ETH_HLEN + hlen + tcp_hlen + chunk_size;
 >                          DEBUG_PRINT(("RTL8139: +++ C+ mode TSO transferring packet size %d\n", tso_send_size));
 > -                        rtl8139_transfer_frame(s, saved_buffer, tso_send_size, 0);
 > +                        rtl8139_transfer_frame(s, saved_buffer, tso_send_size,
 > +                            0, (uint8_t *) dot1q_buffer);
 >  
 >                          /* add transferred count to TCP sequence number */
 >                          p_tcp_hdr->th_seq = cpu_to_be32(chunk_size + be32_to_cpu(p_tcp_hdr->th_seq));
 > @@ -2378,7 +2402,8 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 >  
 >          DEBUG_PRINT(("RTL8139: +++ C+ mode transmitting %d bytes packet\n", saved_size));
 >  
 > -        rtl8139_transfer_frame(s, saved_buffer, saved_size, 1);
 > +        rtl8139_transfer_frame(s, saved_buffer, saved_size, 1,
 > +            (uint8_t *) dot1q_buffer);
 >  
 >          /* restore card space if there was no recursion and reset offset */
 >          if (!s->cplus_txbuffer)
 > -- 
 > 1.7.2.3
 >