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

[Jason Wang <jasowang@redhat.com>]

On 03/11/2011 08:35 AM, Benjamin Poirier wrote:
> Here is version 6 of my patchset to add vlan support to the emulated rtl8139
> nic.
>
> 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.

8139cp does have the support for VLAN with TSO. See IFCAP_VLAN_HWTSO 
capabilities in FreeBSD re(4) driver: 
http://svn.freebsd.org/viewvc/base/release/8.2.0/sys/dev/re/if_re.c?revision=218742&view=markup. 
You can use it in your testing for both vlan tso and loopback TX. 
Additionally, you can submit your 8139 modifications to linux upstream I 
think.

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