* [Qemu-devel] [PATCH v7 1/3] rtl8139: cleanup FCS calculation
2011-03-22 23:11 [Qemu-devel] [PATCH v7] rtl8139: add vlan support Benjamin Poirier
@ 2011-03-22 23:11 ` Benjamin Poirier
2011-03-22 23:11 ` [Qemu-devel] [PATCH v7 2/3] rtl8139: add vlan tag extraction Benjamin Poirier
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Poirier @ 2011-03-22 23:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, Jason Wang, Michael S. Tsirkin
clean out ifdef's around ethernet checksum calculation
Signed-off-by: Benjamin Poirier <benjamin.poirier@gmail.com>
Acked-by: 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 0ba51fc..d843aa0 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.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v7 2/3] rtl8139: add vlan tag extraction
2011-03-22 23:11 [Qemu-devel] [PATCH v7] rtl8139: add vlan support Benjamin Poirier
2011-03-22 23:11 ` [Qemu-devel] [PATCH v7 1/3] rtl8139: cleanup FCS calculation Benjamin Poirier
@ 2011-03-22 23:11 ` Benjamin Poirier
2011-03-22 23:11 ` [Qemu-devel] [PATCH v7 3/3] rtl8139: add vlan tag insertion Benjamin Poirier
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Poirier @ 2011-03-22 23:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, Jason Wang, Michael S. Tsirkin
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 | 66 +++++++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 52 insertions(+), 14 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index d843aa0..5b2f489 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -72,6 +72,16 @@
#define MOD2(input, size) \
( ( input ) & ( size - 1 ) )
+#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
@@ -812,11 +822,13 @@ 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)
{
RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
+ /* size is the length of the buffer passed to the driver */
int size = size_;
+ const uint8_t *dot1q_buf = NULL;
uint32_t packet_header = 0;
- uint8_t buf1[60];
+ uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN];
static const uint8_t broadcast_macaddr[6] =
{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
@@ -928,12 +940,15 @@ 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) {
+ /* if too small buffer, then expand it
+ * Include some tailroom in case a vlan tag is later removed. */
+ if (size < MIN_BUF_SIZE + VLAN_HLEN) {
memcpy(buf1, buf, size);
- memset(buf1 + size, 0, MIN_BUF_SIZE - size);
+ memset(buf1 + size, 0, MIN_BUF_SIZE + VLAN_HLEN - size);
buf = buf1;
- size = MIN_BUF_SIZE;
+ if (size < MIN_BUF_SIZE) {
+ size = MIN_BUF_SIZE;
+ }
}
if (rtl8139_cp_receiver_enabled(s))
@@ -996,6 +1011,29 @@ 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. */
+ if (s->CpCmd & CPlusRxVLAN && be16_to_cpup((uint16_t *)
+ &buf[ETHER_ADDR_LEN * 2]) == ETH_P_8021Q) {
+ dot1q_buf = &buf[ETHER_ADDR_LEN * 2];
+ size -= VLAN_HLEN;
+ /* if too small buffer, use the tailroom added duing expansion */
+ if (size < MIN_BUF_SIZE) {
+ size = MIN_BUF_SIZE;
+ }
+
+ 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;
+ }
+
/* TODO: scatter the packet over available receive ring descriptors space */
if (size+4 > rx_space)
@@ -1017,7 +1055,14 @@ 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 (dot1q_buf) {
+ cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
+ cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN,
+ buf + 2 * ETHER_ADDR_LEN + VLAN_HLEN,
+ size - 2 * ETHER_ADDR_LEN);
+ } else {
+ cpu_physical_memory_write(rx_addr, buf, size);
+ }
if (s->CpCmd & CPlusRxChkSum)
{
@@ -1025,7 +1070,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(crc32(0, buf, size_));
cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
/* first segment of received packet flag */
@@ -1070,9 +1115,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);
@@ -2054,10 +2096,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.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v7 3/3] rtl8139: add vlan tag insertion
2011-03-22 23:11 [Qemu-devel] [PATCH v7] rtl8139: add vlan support Benjamin Poirier
2011-03-22 23:11 ` [Qemu-devel] [PATCH v7 1/3] rtl8139: cleanup FCS calculation Benjamin Poirier
2011-03-22 23:11 ` [Qemu-devel] [PATCH v7 2/3] rtl8139: add vlan tag extraction Benjamin Poirier
@ 2011-03-22 23:11 ` Benjamin Poirier
2011-03-24 9:20 ` [Qemu-devel] [PATCH v7] rtl8139: add vlan support Jason Wang
2011-03-26 11:34 ` Blue Swirl
4 siblings, 0 replies; 6+ messages in thread
From: Benjamin Poirier @ 2011-03-22 23:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, Jason Wang, Michael S. Tsirkin
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 | 75 ++++++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 5b2f489..d545933 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -45,6 +45,7 @@
* 2010-Feb-04 Frediano Ziglio: Rewrote timer support using QEMU timer only
* when strictly needed (required for for
* Darwin)
+ * 2011-Mar-22 Benjamin Poirier: Implemented VLAN offloading
*/
/* For crc32 */
@@ -56,6 +57,7 @@
#include "net.h"
#include "loader.h"
#include "sysemu.h"
+#include "iov.h"
/* debug RTL8139 card */
//#define DEBUG_RTL8139 1
@@ -1756,22 +1758,52 @@ 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)
{
+ struct iovec *iov = NULL;
+
if (!size)
{
DEBUG_PRINT(("RTL8139: +++ empty ethernet frame\n"));
return;
}
+ if (dot1q_buf && size >= ETHER_ADDR_LEN * 2) {
+ iov = (struct iovec[3]) {
+ { .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 },
+ };
+ }
+
if (TxLoopBack == (s->TxConfig & TxLoopBack))
{
+ size_t buf2_size;
+ uint8_t *buf2;
+
+ if (iov) {
+ buf2_size = iov_size(iov, 3);
+ buf2 = qemu_malloc(buf2_size);
+ iov_to_buf(iov, 3, buf2, 0, buf2_size);
+ buf = buf2;
+ }
+
DEBUG_PRINT(("RTL8139: +++ transmit loopback mode\n"));
rtl8139_do_receive(&s->nic->nc, buf, size, do_interrupt);
+
+ if (iov) {
+ qemu_free(buf2);
+ }
}
else
{
- qemu_send_packet(&s->nic->nc, buf, size);
+ if (iov) {
+ qemu_sendv_packet(&s->nic->nc, iov, 3);
+ } else {
+ qemu_send_packet(&s->nic->nc, buf, size);
+ }
}
}
@@ -1805,7 +1837,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));
@@ -1932,7 +1964,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);
@@ -1944,9 +1975,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 */
@@ -1970,9 +1998,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 */
@@ -2072,13 +2100,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 */
@@ -2087,6 +2115,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;
@@ -2240,7 +2283,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));
@@ -2313,7 +2357,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.4.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v7] rtl8139: add vlan support
2011-03-22 23:11 [Qemu-devel] [PATCH v7] rtl8139: add vlan support Benjamin Poirier
` (2 preceding siblings ...)
2011-03-22 23:11 ` [Qemu-devel] [PATCH v7 3/3] rtl8139: add vlan tag insertion Benjamin Poirier
@ 2011-03-24 9:20 ` Jason Wang
2011-03-26 11:34 ` Blue Swirl
4 siblings, 0 replies; 6+ messages in thread
From: Jason Wang @ 2011-03-24 9:20 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: qemu-devel
On 03/23/2011 07:11 AM, Benjamin Poirier wrote:
> Hello,
>
> Here is version 7 of my patchset to add vlan support to the emulated rtl8139
> nic.
>
> Changes since v6:
> * added check against guest requesting tagging on frames with len< 12
> * simplified tag extraction in receive function. dot1q_buf arg removed
> from rtl8139_do_receive(). Frame is linearized in transfer_frame()
> when loopback mode is on.
> * added an entry to file header
>
> I've ran the same tests as usual on linux and this time also freebsd 8.2, with
> and without vlanhwtso in the latter case. Jason, you're right that loopback
> mode is seldom used! It seems the bsd driver only uses it at probe time to
> identify a defect in some 8169 [1,2] and even then, that check has been
> disabled [3]. The linux driver doesn't support loopback mode (unless it's well
> hidden.)
>
> [1] http://lists.freebsd.org/pipermail/freebsd-emulation/2006-May/thread.html#2055
> [2] http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/re/if_re.c?rev=1.196;content-type=text%2Fplain
> [3] http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/re/if_re.c#rev1.68
>
> 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
>
Looks good to me.
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v7] rtl8139: add vlan support
2011-03-22 23:11 [Qemu-devel] [PATCH v7] rtl8139: add vlan support Benjamin Poirier
` (3 preceding siblings ...)
2011-03-24 9:20 ` [Qemu-devel] [PATCH v7] rtl8139: add vlan support Jason Wang
@ 2011-03-26 11:34 ` Blue Swirl
4 siblings, 0 replies; 6+ messages in thread
From: Blue Swirl @ 2011-03-26 11:34 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: qemu-devel
Thanks, applied all.
On Wed, Mar 23, 2011 at 1:11 AM, Benjamin Poirier
<benjamin.poirier@gmail.com> wrote:
> Hello,
>
> Here is version 7 of my patchset to add vlan support to the emulated rtl8139
> nic.
>
> Changes since v6:
> * added check against guest requesting tagging on frames with len < 12
> * simplified tag extraction in receive function. dot1q_buf arg removed
> from rtl8139_do_receive(). Frame is linearized in transfer_frame()
> when loopback mode is on.
> * added an entry to file header
>
> I've ran the same tests as usual on linux and this time also freebsd 8.2, with
> and without vlanhwtso in the latter case. Jason, you're right that loopback
> mode is seldom used! It seems the bsd driver only uses it at probe time to
> identify a defect in some 8169 [1,2] and even then, that check has been
> disabled [3]. The linux driver doesn't support loopback mode (unless it's well
> hidden.)
>
> [1] http://lists.freebsd.org/pipermail/freebsd-emulation/2006-May/thread.html#2055
> [2] http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/re/if_re.c?rev=1.196;content-type=text%2Fplain
> [3] http://www.freebsd.org/cgi/cvsweb.cgi/src/sys/dev/re/if_re.c#rev1.68
>
> 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
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread