Re: [Qemu-devel] [PATCH v7] rtl8139: add vlan support
Thanks, applied all. On Wed, Mar 23, 2011 at 1:11 AM, Benjamin Poirier benjamin.poir...@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
Re: [Qemu-devel] [PATCH v7] rtl8139: add vlan support
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 #includenet/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 jasow...@redhat.com
[Qemu-devel] [PATCH v7] rtl8139: add vlan support
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