On 2016-11-23 at 10:00 brho <[email protected]> wrote:
> Commit 24f29c1568ab ("Fix virtio net handling of the header.") breaks my
> ability to ssh into a Linux guest.
>
> From the guest, I see:
>
> ```
> tc@box:~$ Sending discover...
> Sending discover...
> Sending discover...
> Sending discover...
> Sending discover...
> Sending discover...
> ```
>
> where the 'sending discover' is new.
>
> This is on my vmm branch. If I revert 24f29c15, things work as usual.
>
> One thing I noticed about that commit is that previously we'd always strip
> the VIRTIO_HEADER_SIZE from every iov, not just the first one.
>
> Which behavior is correct? Is the header a set of bytes at the beginning of
> the area mapped by the IOVs? Or is it something prepended to every IOV? If
> not the latter (old behavior), then why does the guest's networking fail in
> such a way that stripping 12 bytes off every IOV makes it work?
>
here's a packet from snoopy/tcpdump that went out the same time that
'Sending Discover' went out.
(this is from running snoopy -N0. on mlx4, i think you'll need -p too,
due to some promisc errors.)
028288 ms
ether(s=1c:b7:2c:ee:52:69 d=00:00:00:00:00:00 pr=ffff ln=354)
dump(ff ff ff ff 1c b7 2c ee 52 69 08 00 45 00 01 48 .......,.Ri..E..
00 00 00 00 40 11 79 a6 00 00 00 00 ff ff ff ff [email protected]........
00 44 00 43 01 34 d5 7f 01 01 06 00 ce 30 d1 7f ..D.C.4.......0.
06 41 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ..A.............
00 00 00 00 1c b7 2c ee 52 69 00 00 00 00 00 00 .......,.Ri.....
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 63 82 53 63 35 01 01 3d 07 01 1c b7 .....c.Sc5..=...
2c ee 52 69 32 04 64 6b 0a 64 39 02 02 40 37 07 .,.Ri2.dk.d9..@7
01 03 06 0c 0f 1c 2a 3c 0c 75 64 68 63 70 20 31 .......*<.udhcp
2e 32 34 2e 32 0c 03 62 6f 78 ff 00 00 00 00 00 1.24.2..box.....
00 00 00 00)
tc@box:~$ ifconfig
eth0 Link encap:Ethernet HWaddr 1C:B7:2C:EE:52:69
the source addr is right, though akaros wrote that in there. the dest
is all 0s, which is wrong.
the protocol is 0xff 0xff, which is also wrong. the dump() is the
payload of the packet. the first 4 bytes are 0xff. add those to the
protocol, and it looks like that should be the destination field (6
0xff bytes for the broadcast).
later on in the dump, we have the MAC from linux (5th byte in the dump,
0x1c 0xb7, etc).
so it looks like we had an extra 12 bytes of probably zeros at the
front of the packet. then probably akaros (in devether) wrote the
source addr, and pumped it down to the NIC.
i added this patch to virtio-net:
diff --git a/user/vmm/virtio_net.c b/user/vmm/virtio_net.c
index 6201acb8358d..600d2416c1de 100644
--- a/user/vmm/virtio_net.c
+++ b/user/vmm/virtio_net.c
@@ -32,6 +32,7 @@
#include <vmm/virtio.h>
#include <vmm/virtio_mmio.h>
#include <vmm/virtio_net.h>
+#include <parlib/ros_debug.h>
#define VIRTIO_HEADER_SIZE 12
@@ -202,6 +203,9 @@ void net_transmitq_fn(void *_vq)
/* Find the beginning of the ethernet frame. */
for (int i = 0; i < olen; i++) {
+ fprintf(stderr, "IOV %d, base %p, len
%d\n-----------\n", i,
+ iov[i].iov_base, iov[i].iov_len);
+ hexdump(stderr, iov[i].iov_base, iov[i].iov_len);
/* This conditional also catches cases when iov_len is
0 */
if (iov[i].iov_len <= bytestostrip) {
/* The virtio net header is bigger or equal to
the current iov.
@@ -211,6 +215,12 @@ void net_transmitq_fn(void *_vq)
}
/* The virtio net header is smaller than the full iov.
* We stop advancing here and write. */
+
+ fprintf(stderr, " bytestostrip now %d\n",
bytestostrip);
+ fprintf(stderr, " writing to FD %d base %p, len
%d\n",
+ fd, iov[i].iov_base + bytestostrip,
+ iov[i].iov_len - bytestostrip);
+
ret = write(fd, iov[i].iov_base + bytestostrip,
iov[i].iov_len - bytestostrip);
assert(ret == iov[i].iov_len - bytestostrip);
and get: (they are all just one IOV)
IOV 0, base 0x3bce7006, len 354
-----------
3bce7006: 00 00 00 00 00 00 00 00 00 00 10 00 ff ff ff ff ................
3bce7016: ff ff 1c b7 2c ee 52 69 08 00 45 00 01 48 00 00 ....,.Ri..E..H..
3bce7026: 00 00 40 11 79 a6 00 00 00 00 ff ff ff ff 00 44 [email protected]
3bce7036: 00 43 01 34 07 74 01 01 06 00 94 eb 38 59 00 06 .C.4.t......8Y..
3bce7046: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
3bce7056: 00 00 1c b7 2c ee 52 69 00 00 00 00 00 00 00 00 ....,.Ri........
3bce7066: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
...
3bce7126: 00 00 63 82 53 63 35 01 03 3d 07 01 1c b7 2c ee ..c.Sc5..=....,.
3bce7136: 52 69 32 04 64 6b 0a 64 36 04 64 6b 0a 43 39 02 Ri2.dk.d6.dk.C9.
3bce7146: 02 40 37 07 01 03 06 0c 0f 1c 2a 3c 0c 75 64 68 .@7.......*<.udh
3bce7156: 63 70 20 31 2e 32 34 2e 32 0c 03 62 6f 78 ff 00 cp 1.24.2..box..
3bce7166: 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 ................
bytestostrip now 0
writing to FD 20 base 0x3bce7006, len 354
so pretty weird. bytes to strip is always 0, though i'd expect it to
be 12, since iov_len > bytestostrip.
then, we write to the kernel the original iovbase. the kernel probably
writes in the src in the first part of the field, but i'd expect that
0x10 to show up in the destination. it turns out that the packet
pasted above has 0x10, but many others do not, and they just have 0x00
for the first 12 bytes.
anyway, i printed a little more, and bytestostrip is 0 at the top of the loop!
the problem is that we were setting it == 12 at the top of
net_transmitq_fn(), but we should be setting it == 12 for every loop
around the infinite loop near L189 (for the
virtio_next_avaiil_vq_desc()).
the following diff fixed it:
diff --git a/user/vmm/virtio_net.c b/user/vmm/virtio_net.c
index 6201acb8358d..18687e04e87c 100644
--- a/user/vmm/virtio_net.c
+++ b/user/vmm/virtio_net.c
@@ -172,7 +172,7 @@ void net_transmitq_fn(void *_vq)
void *stripped;
int ret;
int fd = open(data_path, O_RDWR);
- int bytestostrip = VIRTIO_HEADER_SIZE;
+ int bytestostrip;
if (fd == -1)
VIRTIO_DEV_ERRX(vq->vqdev, "Could not open data file for
ether1.");
@@ -201,6 +201,7 @@ void net_transmitq_fn(void *_vq)
*/
/* Find the beginning of the ethernet frame. */
+ bytestostrip = VIRTIO_HEADER_SIZE;
for (int i = 0; i < olen; i++) {
/* This conditional also catches cases when iov_len is
0 */
if (iov[i].iov_len <= bytestostrip) {
Barret
--
You received this message because you are subscribed to the Google Groups
"Akaros" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
For more options, visit https://groups.google.com/d/optout.
