From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=45392 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PtzEW-0005xv-2Z for qemu-devel@nongnu.org; Mon, 28 Feb 2011 04:17:37 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PtzBK-0005AH-Jp for qemu-devel@nongnu.org; Mon, 28 Feb 2011 04:14:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55750) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PtzBK-0005A2-AS for qemu-devel@nongnu.org; Mon, 28 Feb 2011 04:14:18 -0500 Message-ID: <4D6B6760.5050909@redhat.com> Date: Mon, 28 Feb 2011 17:14:08 +0800 From: Jason Wang MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v3 2/2] rtl8139: add vlan tag extraction References: <1298680800-22995-1-git-send-email-benjamin.poirier@gmail.com> <1298680800-22995-3-git-send-email-benjamin.poirier@gmail.com> In-Reply-To: <1298680800-22995-3-git-send-email-benjamin.poirier@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Poirier Cc: qemu-devel@nongnu.org, "Michael S. Tsirkin" 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 > Cc: Igor V. Kovalenko > Cc: Jason Wang > Cc: Michael S. Tsirkin > > -- > > 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); >