From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=37362 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PFd9O-0002rp-SF for qemu-devel@nongnu.org; Mon, 08 Nov 2010 20:37:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PFd9N-0005a7-8j for qemu-devel@nongnu.org; Mon, 08 Nov 2010 20:37:30 -0500 Received: from mail-vw0-f45.google.com ([209.85.212.45]:50883) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PFd9N-0005Zv-49 for qemu-devel@nongnu.org; Mon, 08 Nov 2010 20:37:29 -0500 Received: by vws4 with SMTP id 4so2578769vws.4 for ; Mon, 08 Nov 2010 17:37:28 -0800 (PST) Sender: Benjamin Poirier Message-ID: <4CD8A5D5.5050707@polymtl.ca> Date: Mon, 08 Nov 2010 20:37:25 -0500 From: Benjamin Poirier MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] rtl8139: add vlan tag insertion References: <1289165136-28207-1-git-send-email-benjamin.poirier@polymtl.ca> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-devel@nongnu.org On 08/11/10 11:34 AM, Stefan Hajnoczi wrote: > On Sun, Nov 7, 2010 at 9:25 PM, Benjamin Poirier > wrote: >> Add support to the emulated hardware to add vlan tags in packets going >> from the guest to the network. >> >> Signed-off-by: Benjamin Poirier >> Cc: Igor V. Kovalenko >> --- >> hw/rtl8139.c | 46 +++++++++++++++++++++++++++++++++++----------- >> 1 files changed, 35 insertions(+), 11 deletions(-) >> >> diff --git a/hw/rtl8139.c b/hw/rtl8139.c >> index d92981d..ac294da 100644 >> --- a/hw/rtl8139.c >> +++ b/hw/rtl8139.c >> @@ -47,6 +47,8 @@ >> * Darwin) >> */ >> >> +#include >> + >> #include "hw.h" >> #include "pci.h" >> #include "qemu-timer.h" >> @@ -58,6 +60,10 @@ >> >> #define PCI_FREQUENCY 33000000L >> >> +/* bytes in VLAN tag */ >> +#define VLAN_TCI_LEN 2 >> +#define VLAN_HDR_LEN (ETHER_TYPE_LEN + VLAN_TCI_LEN) >> + >> /* debug RTL8139 card C+ mode only */ >> //#define DEBUG_RTL8139CP 1 >> >> @@ -1913,7 +1919,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); >> @@ -1925,9 +1930,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 */ >> @@ -1951,8 +1953,8 @@ 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 add tag flag */ >> +#define CP_TX_TAGC (1<<17) >> /* w1 bits 0...15 : VLAN tag */ >> #define CP_TX_VLAN_TAG_MASK ((1<<16) - 1) >> /* w2 low 32bit of Rx buffer ptr */ >> @@ -1978,12 +1980,22 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) >> >> DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : transmitting from descriptor %d\n", descriptor)); >> >> + int vlan_extra_size = 0; >> if (txdw0 & CP_TX_FS) >> { >> DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : descriptor %d is first segment descriptor\n", descriptor)); >> >> + DEBUG_PRINT(("RTL8139: +++ C+ Tx mode : add vlan tag: %u tci: %u\n", >> + !!(txdw1 & CP_TX_TAGC), bswap16(txdw1 & >> + CP_TX_VLAN_TAG_MASK))); > > Please use leXX_to_cpu() and cpu_to_leXX() instead of bswapXX() for > little-endian conversion. This is more explicit than unconditional > byteswapping and work on both big- and little-endian QEMU host > architectures. I believe the use of flat out swapping is warranted here. The data has been adjusted for host-endianness when the register was read: txdw1 = le32_to_cpu(val); We have to mask and swap to accomodate the format used by the card's registers, which is always the same regardless of machine endianness. txdw1 is extracted from a 32 bit register like such (when the descriptor specifies tx tagging), in increasing address order: tci8-15 tci0-7 0x01 0x00 -Ben > > Stefan >