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

[Blue Swirl]

On Thu, Mar 3, 2011 at 12:36 AM, Benjamin Poirier
<address@hidden> 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 <address@hidden>
> Cc: Igor V. Kovalenko <address@hidden>
> Cc: Jason Wang <address@hidden>
> Cc: Michael S. Tsirkin <address@hidden>
>
> --
>
> 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 |  100 
> +++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 files changed, 81 insertions(+), 19 deletions(-)
>
> diff --git a/hw/rtl8139.c b/hw/rtl8139.c
> index a22530c..6e770bd 100644
> --- a/hw/rtl8139.c
> +++ b/hw/rtl8139.c
> @@ -68,6 +68,16 @@
>  #if defined(RTL8139_CALCULATE_RXCRC)
>  /* For crc32 */
>  #include <zlib.h>
> +
> +static inline uLong rtl8139_crc32(uLong crc, const Bytef *buf, uInt len)

Don't use non-standard uLong or Bytef etc. but uint32_t and uint8_t.

> +{
> +    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 +87,14 @@
>  #define MOD2(input, size) \
>     ( ( input ) & ( size - 1 )  )
>
> +#define ETHER_ADDR_LEN 6
> +#define ETHER_TYPE_LEN 2
> +#define ETHER_HDR_LEN (ETHER_ADDR_LEN * 2 + ETHER_TYPE_LEN)
> +#define ETHERTYPE_VLAN 0x8100
> +
> +#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
> @@ -818,10 +836,13 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
> const uint8_t *buf, size_
>  {
>     RTL8139State *s = DO_UPCAST(NICState, nc, nc)->opaque;
>     int size = size_;
> +    const uint8_t *dot1q_buf;
> +    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 };
>
> @@ -933,14 +954,6 @@ 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);
> -        buf = buf1;
> -        size = MIN_BUF_SIZE;
> -    }
> -
>     if (rtl8139_cp_receiver_enabled(s))
>     {
>         DEBUG_PRINT(("RTL8139: in C+ Rx mode ================\n"));
> @@ -1001,6 +1014,41 @@ 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 */
> +        /* 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 && be16_to_cpup((uint16_t *)
> +                &buf[ETHER_ADDR_LEN * 2]) == ETHERTYPE_VLAN) {
> +            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;
> +            dot1q_buf = NULL;
> +        }
> +        next_part_size = 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;
> +
> +            memcpy(buf1, next_part, next_part_size);
> +            memset(buf1 + next_part_size, 0, tmp_size - next_part_size);
> +            next_part = buf1;
> +            next_part_size = tmp_size;
> +            size = MIN_BUF_SIZE;
> +        }
> +
>         /* TODO: scatter the packet over available receive ring descriptors 
> space */
>
>         if (size+4 > rx_space)
> @@ -1022,7 +1070,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 );
> +        if (unlikely(dot1q_buf)) {
> +            cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
> +            val = rtl8139_crc32(0, buf, 2 * ETHER_ADDR_LEN);
> +            val = rtl8139_crc32(val, dot1q_buf, VLAN_HDR_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);
> +        }
>
>         if (s->CpCmd & CPlusRxChkSum)
>         {
> @@ -1031,9 +1090,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);
>
> @@ -1079,9 +1136,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);
> @@ -1108,6 +1162,14 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, 
> const uint8_t *buf, size_
>     {
>         DEBUG_PRINT(("RTL8139: in ring Rx mode ================\n"));
>
> +        /* if too small buffer, then expand it */
> +        if (size < MIN_BUF_SIZE) {
> +            memcpy(buf1, buf, size);
> +            memset(buf1 + size, 0, MIN_BUF_SIZE - size);
> +            buf = buf1;
> +            size = MIN_BUF_SIZE;
> +        }
> +
>         /* begin ring receiver mode */
>         int avail = MOD2(s->RxBufferSize + s->RxBufPtr - s->RxBufAddr, 
> s->RxBufferSize);
>
> @@ -1133,13 +1195,13 @@ static ssize_t rtl8139_do_receive(VLANClientState 
> *nc, const uint8_t *buf, size_
>
>         rtl8139_write_buffer(s, (uint8_t *)&val, 4);
>
> +        /* receive/copy to target memory */
>         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

The #ifdeffery is probably not needed.