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

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

[Benjamin Poirier]

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

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 tested 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 to the ppc32 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 v3 1/2] 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>
---
 hw/rtl8139.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 96 insertions(+), 27 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index a22530c..35ccd3d 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -47,6 +47,8 @@
  *                                  Darwin)
  */
 
+#include <net/ethernet.h>
+
 #include "hw.h"
 #include "pci.h"
 #include "qemu-timer.h"
@@ -68,6 +70,16 @@
 #if defined(RTL8139_CALCULATE_RXCRC)
 /* For crc32 */
 #include <zlib.h>
+
+static inline uLong rtl8139_crc32(uLong crc, const Bytef *buf, uInt len)
+{
+    return crc32(crc, buf, len);
+}
+#else
+static inline uLong rtl8139_crc32(uLong crc, const Bytef *buf, uInt len)
+{
+    return 0;
+}
 #endif
 
 #define SET_MASKED(input, mask, curr) \
@@ -77,6 +89,9 @@
 #define MOD2(input, size) \
     ( ( input ) & ( size - 1 )  )
 
+#define VLAN_TCI_LEN 2
+#define VLAN_HDR_LEN (ETHER_TYPE_LEN + VLAN_TCI_LEN)
+
 #if defined (DEBUG_RTL8139)
 #  define DEBUG_PRINT(x) do { printf x ; } while (0)
 #else
@@ -814,9 +829,11 @@ 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;
+    int size_ = buf_size + (dot1q_buf ? VLAN_HDR_LEN : 0);
     int size = size_;
 
     uint32_t packet_header = 0;
@@ -935,7 +952,14 @@ 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);
+        if (unlikely(dot1q_buf)) {
+            memcpy(buf1, buf, 2 * ETHER_ADDR_LEN);
+            memcpy(buf1 + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN);
+            memcpy(buf1 + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 *
+                ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN);
+        } else {
+            memcpy(buf1, buf, size);
+        }
         memset(buf1 + size, 0, MIN_BUF_SIZE - size);
         buf = buf1;
         size = MIN_BUF_SIZE;
@@ -1022,7 +1046,21 @@ 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 );
+        if (unlikely(dot1q_buf)) {
+            cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
+            val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN);
+            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, dot1q_buf,
+                VLAN_HDR_LEN);
+            val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_LEN);
+            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN +
+                VLAN_HDR_LEN, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
+                ETHER_ADDR_LEN);
+            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
+                ETHER_ADDR_LEN);
+        } else {
+            cpu_physical_memory_write(rx_addr, buf, size);
+            val = rtl8139_crc32(0, buf, size);
+        }
 
         if (s->CpCmd & CPlusRxChkSum)
         {
@@ -1031,9 +1069,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;
+        val = cpu_to_le32(val);
 #endif
         cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
 
@@ -1133,13 +1169,24 @@ 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 (unlikely(dot1q_buf)) {
+            rtl8139_write_buffer(s, buf, 2 * ETHER_ADDR_LEN);
+            val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN);
+            rtl8139_write_buffer(s, dot1q_buf, VLAN_HDR_LEN);
+            val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_LEN);
+            rtl8139_write_buffer(s, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
+                ETHER_ADDR_LEN);
+            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
+                ETHER_ADDR_LEN);
+        } else {
+            rtl8139_write_buffer(s, buf, size);
+            val = rtl8139_crc32(0, buf, size);
+        }
 
         /* write checksum */
 #if defined (RTL8139_CALCULATE_RXCRC)
-        val = cpu_to_le32(crc32(0, buf, size));
-#else
-        val = 0;
+        val = cpu_to_le32(val);
 #endif
 
         rtl8139_write_buffer(s, (uint8_t *)&val, 4);
@@ -1165,7 +1212,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)
@@ -1740,7 +1787,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)
     {
@@ -1751,11 +1799,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);
+        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_HDR_LEN },
+                { .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);
+        }
     }
 }
 
@@ -1789,7 +1848,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));
 
@@ -1916,7 +1975,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);
@@ -1928,9 +1986,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 */
@@ -1954,9 +2009,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 */
@@ -2056,13 +2111,12 @@ 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)
     {
+        uint16_t *dot1q_buffer;
+
         DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : descriptor %d is last segment descriptor\n", descriptor));
 
         /* can transfer fully assembled packet */
@@ -2071,6 +2125,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 */
+            printf("RTL8139: +++ C+ Tx mode : inserting vlan tag with "
+                "tci: %u\n", bswap16(txdw1 & CP_TX_VLAN_TAG_MASK));
+
+            dot1q_buffer = alloca(VLAN_HDR_LEN);
+            dot1q_buffer[0] = cpu_to_be16(ETHERTYPE_VLAN);
+            /* 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;
@@ -2228,7 +2297,7 @@ 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));
@@ -2301,7 +2370,7 @@ 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] [PATCH v3 2/2] 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>

--

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 |   89 +++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 63 insertions(+), 26 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 35ccd3d..f3aaebc 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -835,10 +835,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
     RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
     int size_ = buf_size + (dot1q_buf ? VLAN_HDR_LEN : 0);
     int size = size_;
+    const uint8_t *next_part;
+    size_t next_part_size;
 
     uint32_t packet_header = 0;
 
-    uint8_t buf1[60];
     static const uint8_t broadcast_macaddr[6] =
         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
 
@@ -950,21 +951,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
         }
     }
 
-    /* if too small buffer, then expand it */
-    if (size < MIN_BUF_SIZE) {
-        if (unlikely(dot1q_buf)) {
-            memcpy(buf1, buf, 2 * ETHER_ADDR_LEN);
-            memcpy(buf1 + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN);
-            memcpy(buf1 + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 *
-                ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN);
-        } else {
-            memcpy(buf1, buf, size);
-        }
-        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
-        buf = buf1;
-        size = MIN_BUF_SIZE;
-    }
-
     if (rtl8139_cp_receiver_enabled(s))
     {
         DEBUG_PRINT(("RTL8139: in C+ Rx mode ================\n"));
@@ -1025,6 +1011,44 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
 
         uint32_t rx_space = rxdw0 & CP_RX_BUFFER_SIZE_MASK;
 
+        /* write VLAN info to descriptor variables */
+        /* next_part starts right after the vlan header (if any), at the
+         * ethertype for the payload */
+        next_part = &buf[ETHER_ADDR_LEN * 2];
+        if (s->CpCmd & CPlusRxVLAN && (dot1q_buf || be16_to_cpup((uint16_t *)
+                    &buf[ETHER_ADDR_LEN * 2]) == ETHERTYPE_VLAN)) {
+            if (!dot1q_buf) {
+                /* the tag is in the buffer */
+                dot1q_buf = &buf[ETHER_ADDR_LEN * 2];
+                next_part += VLAN_HDR_LEN;
+            }
+            size -= VLAN_HDR_LEN;
+
+            rxdw1 &= ~CP_RX_VLAN_TAG_MASK;
+            /* BE + ~le_to_cpu()~ + cpu_to_le() = BE */
+            rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *)
+                &buf[ETHER_HDR_LEN]);
+
+            DEBUG_PRINT(("RTL8139: C+ Rx mode : extracted vlan tag with tci: "
+                    "%u\n", be16_to_cpup((uint16_t *) &buf[ETHER_HDR_LEN])));
+        } else {
+            /* reset VLAN tag flag */
+            rxdw1 &= ~CP_RX_TAVA;
+        }
+        next_part_size = buf + buf_size - next_part;
+
+        /* if too small buffer, then expand it */
+        if (size < MIN_BUF_SIZE) {
+            size_t tmp_size = MIN_BUF_SIZE - ETHER_ADDR_LEN * 2;
+            uint8_t *tmp = alloca(tmp_size);
+
+            memcpy(tmp, next_part, next_part_size);
+            memset(tmp + next_part_size, 0, tmp_size - next_part_size);
+            next_part = tmp;
+            next_part_size = tmp_size;
+            size = MIN_BUF_SIZE;
+        }
+
         /* TODO: scatter the packet over available receive ring descriptors space */
 
         if (size+4 > rx_space)
@@ -1049,14 +1073,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
         if (unlikely(dot1q_buf)) {
             cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
             val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN);
-            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, dot1q_buf,
-                VLAN_HDR_LEN);
             val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_LEN);
-            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN +
-                VLAN_HDR_LEN, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
-                ETHER_ADDR_LEN);
-            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
-                ETHER_ADDR_LEN);
+            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, next_part,
+                next_part_size);
+            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN,
+                next_part_size);
         } else {
             cpu_physical_memory_write(rx_addr, buf, size);
             val = rtl8139_crc32(0, buf, size);
@@ -1115,9 +1136,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
         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);
@@ -1144,6 +1162,25 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
     {
         DEBUG_PRINT(("RTL8139: in ring Rx mode ================\n"));
 
+        /* if too small buffer, then expand it */
+        if (size < MIN_BUF_SIZE) {
+            uint8_t *tmp;
+
+            tmp = alloca(MIN_BUF_SIZE);
+            if (unlikely(dot1q_buf)) {
+                memcpy(tmp, buf, 2 * ETHER_ADDR_LEN);
+                memcpy(tmp + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN);
+                memcpy(tmp + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 *
+                    ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN);
+            } else {
+                memcpy(tmp, buf, size);
+            }
+            memset(tmp + size, 0, MIN_BUF_SIZE - size);
+            buf = tmp;
+            size = MIN_BUF_SIZE;
+            dot1q_buf = NULL;
+        }
+
         /* begin ring receiver mode */
         int avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr, s->RxBufferSize);
 
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH v3 1/2] rtl8139: add vlan tag insertion

[Blue Swirl]

On Sat, Feb 26, 2011 at 2:39 AM, Benjamin Poirier
<benjamin.poirier@gmail.com> wrote:
> 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>
> ---
>  hw/rtl8139.c |  123 +++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 96 insertions(+), 27 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..35ccd3d 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -47,6 +47,8 @@
>  *                                  Darwin)
>  */
>
> +#include <net/ethernet.h>

This header is only available on Linux. Please remove.

Re: [Qemu-devel] [PATCH v3 2/2] rtl8139: add vlan tag extraction

[Blue Swirl]

On Sat, Feb 26, 2011 at 2:40 AM, Benjamin Poirier
<benjamin.poirier@gmail.com> wrote:
> 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>
>
> --
>
> 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 |   89 +++++++++++++++++++++++++++++++++++++++++-----------------
>  1 files changed, 63 insertions(+), 26 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index 35ccd3d..f3aaebc 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -835,10 +835,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>     RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>     int size_ = buf_size + (dot1q_buf ? VLAN_HDR_LEN : 0);
>     int size = size_;
> +    const uint8_t *next_part;
> +    size_t next_part_size;
>
>     uint32_t packet_header = 0;
>
> -    uint8_t buf1[60];
>     static const uint8_t broadcast_macaddr[6] =
>         { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> @@ -950,21 +951,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>         }
>     }
>
> -    /* if too small buffer, then expand it */
> -    if (size < MIN_BUF_SIZE) {
> -        if (unlikely(dot1q_buf)) {
> -            memcpy(buf1, buf, 2 * ETHER_ADDR_LEN);
> -            memcpy(buf1 + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN);
> -            memcpy(buf1 + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 *
> -                ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN);
> -        } else {
> -            memcpy(buf1, buf, size);
> -        }
> -        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
> -        buf = buf1;
> -        size = MIN_BUF_SIZE;
> -    }
> -
>     if (rtl8139_cp_receiver_enabled(s))
>     {
>         DEBUG_PRINT(("RTL8139: in C+ Rx mode ================\n"));
> @@ -1025,6 +1011,44 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>
>         uint32_t rx_space = rxdw0 & CP_RX_BUFFER_SIZE_MASK;
>
> +        /* write VLAN info to descriptor variables */
> +        /* next_part starts right after the vlan header (if any), at the
> +         * ethertype for the payload */
> +        next_part = &buf[ETHER_ADDR_LEN * 2];
> +        if (s->CpCmd & CPlusRxVLAN && (dot1q_buf || be16_to_cpup((uint16_t *)
> +                    &buf[ETHER_ADDR_LEN * 2]) == ETHERTYPE_VLAN)) {
> +            if (!dot1q_buf) {
> +                /* the tag is in the buffer */
> +                dot1q_buf = &buf[ETHER_ADDR_LEN * 2];
> +                next_part += VLAN_HDR_LEN;
> +            }
> +            size -= VLAN_HDR_LEN;
> +
> +            rxdw1 &= ~CP_RX_VLAN_TAG_MASK;
> +            /* BE + ~le_to_cpu()~ + cpu_to_le() = BE */
> +            rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *)
> +                &buf[ETHER_HDR_LEN]);
> +
> +            DEBUG_PRINT(("RTL8139: C+ Rx mode : extracted vlan tag with tci: "
> +                    "%u\n", be16_to_cpup((uint16_t *) &buf[ETHER_HDR_LEN])));
> +        } else {
> +            /* reset VLAN tag flag */
> +            rxdw1 &= ~CP_RX_TAVA;
> +        }
> +        next_part_size = buf + buf_size - next_part;
> +
> +        /* if too small buffer, then expand it */
> +        if (size < MIN_BUF_SIZE) {
> +            size_t tmp_size = MIN_BUF_SIZE - ETHER_ADDR_LEN * 2;
> +            uint8_t *tmp = alloca(tmp_size);

Don't use alloca() but qemu_malloc() or a static buffer.

> +
> +            memcpy(tmp, next_part, next_part_size);
> +            memset(tmp + next_part_size, 0, tmp_size - next_part_size);
> +            next_part = tmp;
> +            next_part_size = tmp_size;
> +            size = MIN_BUF_SIZE;
> +        }
> +
>         /* TODO: scatter the packet over available receive ring descriptors space */
>
>         if (size+4 > rx_space)
> @@ -1049,14 +1073,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>         if (unlikely(dot1q_buf)) {
>             cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
>             val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN);
> -            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, dot1q_buf,
> -                VLAN_HDR_LEN);
>             val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_LEN);
> -            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN +
> -                VLAN_HDR_LEN, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
> -                ETHER_ADDR_LEN);
> -            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
> -                ETHER_ADDR_LEN);
> +            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, next_part,
> +                next_part_size);
> +            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN,
> +                next_part_size);
>         } else {
>             cpu_physical_memory_write(rx_addr, buf, size);
>             val = rtl8139_crc32(0, buf, size);
> @@ -1115,9 +1136,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>         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);
> @@ -1144,6 +1162,25 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>     {
>         DEBUG_PRINT(("RTL8139: in ring Rx mode ================\n"));
>
> +        /* if too small buffer, then expand it */
> +        if (size < MIN_BUF_SIZE) {
> +            uint8_t *tmp;
> +
> +            tmp = alloca(MIN_BUF_SIZE);

Ditto.

Note to self: HACKING forgets to forbid alloca, should be fixed.

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

[Jason Wang]

On 02/26/2011 08:39 AM, Benjamin Poirier wrote:
> I've posted v2 of these patches back in november
> http://article.gmane.org/gmane.comp.emulators.qemu/84252
>
> 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 tested 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 to the ppc32 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.
>

Thanks for the patch and it's better to test tso also.

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

Re: [Qemu-devel] [PATCH v3 2/2] rtl8139: add vlan tag extraction

[Jason Wang]

On 02/26/2011 08:40 AM, Benjamin Poirier wrote:
> 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>
>
> --
>
> 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.)

Linux driver use the NETIF_F_HW_VLAN_RX by default, so it's needed.

> ---
>   hw/rtl8139.c |   89 +++++++++++++++++++++++++++++++++++++++++-----------------
>   1 files changed, 63 insertions(+), 26 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index 35ccd3d..f3aaebc 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -835,10 +835,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>       RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>       int size_ = buf_size + (dot1q_buf ? VLAN_HDR_LEN : 0);
>       int size = size_;
> +    const uint8_t *next_part;
> +    size_t next_part_size;
>
>       uint32_t packet_header = 0;
>
> -    uint8_t buf1[60];
>       static const uint8_t broadcast_macaddr[6] =
>           { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
>
> @@ -950,21 +951,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>           }
>       }
>
> -    /* if too small buffer, then expand it */
> -    if (size<  MIN_BUF_SIZE) {
> -        if (unlikely(dot1q_buf)) {
> -            memcpy(buf1, buf, 2 * ETHER_ADDR_LEN);
> -            memcpy(buf1 + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN);
> -            memcpy(buf1 + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 *
> -                ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN);
> -        } else {
> -            memcpy(buf1, buf, size);
> -        }
> -        memset(buf1 + size, 0, MIN_BUF_SIZE - size);
> -        buf = buf1;
> -        size = MIN_BUF_SIZE;
> -    }
> -

Since the your tx loopback changes depends on rx changes, I suggest you 
to put rx patch first which can also make review easier.

>       if (rtl8139_cp_receiver_enabled(s))
>       {
>           DEBUG_PRINT(("RTL8139: in C+ Rx mode ================\n"));
> @@ -1025,6 +1011,44 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>
>           uint32_t rx_space = rxdw0&  CP_RX_BUFFER_SIZE_MASK;
>
> +        /* write VLAN info to descriptor variables */
> +        /* next_part starts right after the vlan header (if any), at the
> +         * ethertype for the payload */
> +        next_part =&buf[ETHER_ADDR_LEN * 2];
> +        if (s->CpCmd&  CPlusRxVLAN&&  (dot1q_buf || be16_to_cpup((uint16_t *)
> +&buf[ETHER_ADDR_LEN * 2]) == ETHERTYPE_VLAN)) {
> +            if (!dot1q_buf) {
> +                /* the tag is in the buffer */
> +                dot1q_buf =&buf[ETHER_ADDR_LEN * 2];
> +                next_part += VLAN_HDR_LEN;
> +            }
> +            size -= VLAN_HDR_LEN;
> +
> +            rxdw1&= ~CP_RX_VLAN_TAG_MASK;
> +            /* BE + ~le_to_cpu()~ + cpu_to_le() = BE */
> +            rxdw1 |= CP_RX_TAVA | le16_to_cpup((uint16_t *)
> +&buf[ETHER_HDR_LEN]);
> +
> +            DEBUG_PRINT(("RTL8139: C+ Rx mode : extracted vlan tag with tci: "
> +                    "%u\n", be16_to_cpup((uint16_t *)&buf[ETHER_HDR_LEN])));
> +        } else {
> +            /* reset VLAN tag flag */
> +            rxdw1&= ~CP_RX_TAVA;
> +        }
> +        next_part_size = buf + buf_size - next_part;
> +
> +        /* if too small buffer, then expand it */
> +        if (size<  MIN_BUF_SIZE) {
> +            size_t tmp_size = MIN_BUF_SIZE - ETHER_ADDR_LEN * 2;
> +            uint8_t *tmp = alloca(tmp_size);
> +
> +            memcpy(tmp, next_part, next_part_size);
> +            memset(tmp + next_part_size, 0, tmp_size - next_part_size);
> +            next_part = tmp;
> +            next_part_size = tmp_size;
> +            size = MIN_BUF_SIZE;
> +        }
> +
>           /* TODO: scatter the packet over available receive ring descriptors space */
>
>           if (size+4>  rx_space)
> @@ -1049,14 +1073,11 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>           if (unlikely(dot1q_buf)) {
>               cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
>               val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN);
> -            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, dot1q_buf,
> -                VLAN_HDR_LEN);
>               val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_LEN);
> -            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN +
> -                VLAN_HDR_LEN, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
> -                ETHER_ADDR_LEN);
> -            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN, buf_size - 2 *
> -                ETHER_ADDR_LEN);
> +            cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN, next_part,
> +                next_part_size);
> +            val = rtl8139_crc32(val, buf + 2 * ETHER_ADDR_LEN,
> +                next_part_size);
>           } else {
>               cpu_physical_memory_write(rx_addr, buf, size);
>               val = rtl8139_crc32(0, buf, size);
> @@ -1115,9 +1136,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>           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);
> @@ -1144,6 +1162,25 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf,
>       {
>           DEBUG_PRINT(("RTL8139: in ring Rx mode ================\n"));
>
> +        /* if too small buffer, then expand it */
> +        if (size<  MIN_BUF_SIZE) {
> +            uint8_t *tmp;
> +
> +            tmp = alloca(MIN_BUF_SIZE);
> +            if (unlikely(dot1q_buf)) {
> +                memcpy(tmp, buf, 2 * ETHER_ADDR_LEN);
> +                memcpy(tmp + 2 * ETHER_ADDR_LEN, dot1q_buf, VLAN_HDR_LEN);
> +                memcpy(tmp + 2 * ETHER_ADDR_LEN + VLAN_HDR_LEN, buf + 2 *
> +                    ETHER_ADDR_LEN, buf_size - 2 * ETHER_ADDR_LEN);
> +            } else {
> +                memcpy(tmp, buf, size);
> +            }
> +            memset(tmp + size, 0, MIN_BUF_SIZE - size);
> +            buf = tmp;
> +            size = MIN_BUF_SIZE;
> +            dot1q_buf = NULL;
> +        }
> +
>           /* begin ring receiver mode */
>           int avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr, s->RxBufferSize);
>