Re: [PATCH 11/15] ubd: use bvec_virt
On 04/08/2021 10:56, Christoph Hellwig wrote:
Use bvec_virt instead of open coding it.
Signed-off-by: Christoph Hellwig
---
arch/um/drivers/ubd_kern.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index e497185dd393..cd9dc0556e91 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1268,8 +1268,7 @@ static void ubd_map_req(struct ubd *dev, struct
io_thread_req *io_req,
rq_for_each_segment(bvec, req, iter) {
BUG_ON(i >= io_req->desc_cnt);
- io_req->io_desc[i].buffer =
- page_address(bvec.bv_page) + bvec.bv_offset;
+ io_req->io_desc[i].buffer = bvec_virt(&bvec);
io_req->io_desc[i].length = bvec.bv_len;
i++;
}
Acked-By: Anton Ivanov
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3] virtio: Work around frames incorrectly marked as gso
On 25/02/2020 16:26, Willem de Bruijn wrote:
An skb_dump() + dump_stack() when the packet socket gets such a
packet may point us to the root cause and fix that.
We tried dump stack, it was not informative - it was just the recvmmsg
call stack coming from the UML until it hits the relevant recv bit in
af_packet - it does not tell us where the packet is coming from.
Quoting from the message earlier in the thread:
[ 2334.180854] Call Trace:
[ 2334.181947] dump_stack+0x5c/0x80
[ 2334.183021] packet_recvmsg.cold+0x23/0x49
[ 2334.184063] ___sys_recvmsg+0xe1/0x1f0
[ 2334.185034] ? packet_poll+0xca/0x130
[ 2334.186014] ? sock_poll+0x77/0xb0
[ 2334.186977] ? ep_item_poll.isra.0+0x3f/0xb0
[ 2334.187936] ? ep_send_events_proc+0xf1/0x240
[ 2334.188901] ? dequeue_signal+0xdb/0x180
[ 2334.189848] do_recvmmsg+0xc8/0x2d0
[ 2334.190728] ? ep_poll+0x8c/0x470
[ 2334.191581] __sys_recvmmsg+0x108/0x150
[ 2334.192441] __x64_sys_recvmmsg+0x25/0x30
[ 2334.193346] do_syscall_64+0x53/0x140
[ 2334.194262] entry_SYSCALL_64_after_hwframe+0x44/0xa9
That makes sense. skb_dump might show more interesting details about
the packet.
I will add that and retest later today.
skb len=818 headroom=2 headlen=818 tailroom=908
mac=(2,14) net=(16,0) trans=16
shinfo(txflags=0 nr_frags=0 gso(size=752 type=0 segs=1))
csum(0x100024 ip_summed=3 complete_sw=0 valid=0 level=0)
hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=4 iif=0
sk family=17 type=3 proto=0
Deciphering the actual packet data gives a
TCP packet, ACK and PSH set.
The PSH flag looks like the only "interesting" thing about it in first read.
Thanks.
TCP always sets the PSH bit on a GSO packet as of commit commit
051ba67447de ("tcp: force a PSH flag on TSO packets"), so that is
definitely informative.
The lower gso size might come from a path mtu probing depending on
tcp_base_mss, but that's definitely wild speculation. Increasing that
value to, say, 1024, could tell us.
In this case it may indeed not be a GSO packet. As 752 is the MSS + 28
B TCP header including timestamp + 20 B IPv4 header + 14B Eth header.
Which adds up to 814 already.
Not sure what those 2 B between skb->data and mac_header are. Was this
captured inside packet_rcv?
af_packet, packet_rcv
https://elixir.bootlin.com/linux/latest/source/net/packet/af_packet.c#L2026
network_header and transport_header both
at 16B offset is also sketchy, but again may be an artifact of where
exactly this is being read.
Perhaps this is a segment of a larger GSO packet that is retransmitted
in part. Like an mtu probe or loss probe. See for instance this in
tcp_send_loss_probe for how a single MSS is extracted:
if ((pcount > 1) && (skb->len > (pcount - 1) * mss)) {
if (unlikely(tcp_fragment(sk, TCP_FRAG_IN_RTX_QUEUE, skb,
(pcount - 1) * mss, mss,
GFP_ATOMIC)))
goto rearm_timer;
skb = skb_rb_next(skb);
}
Note that I'm not implicating this specific code. I don't see anything
wrong with it. Just an indication that a trace would be very
informative, as it could tell if any of these edge cases is being hit.
I will be honest, I have found it a bit difficult to trace.
At the point where this is detected, the packet is already in the vEth
interface queue and is being read by recvmmsg on a raw socket.
The flags + gso size combination happened long before that - even before
it was being placed in the queue.
What is clear so far is that while the packet has invalid
gso_size/gso_type combination, it is an otherwise valid tcp frame.
I will stick the debug into is_gso (with a backtrace) instead and re-run
it later today to see if this can pick it up elsewhere in the stack.
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3] virtio: Work around frames incorrectly marked as gso
On 25/02/2020 07:48, Anton Ivanov wrote:
On 24/02/2020 22:22, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 4:00 PM Anton Ivanov
wrote:
On 24/02/2020 20:20, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 2:55 PM Anton Ivanov
wrote:
On 24/02/2020 19:27, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 8:26 AM wrote:
From: Anton Ivanov
Some of the locally generated frames marked as GSO which
arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
Do we understand how these packets are generated?
No, we have not been able to trace them.
The only thing we know is that this is specific to locally generated
packets. Something arriving from the network does not show this.
Else it seems this
might be papering over a deeper problem.
The stack should not create GSO packets less than or equal to
skb_shinfo(skb)->gso_size. See for instance the check in
tcp_gso_segment after pulling the tcp header:
mss = skb_shinfo(skb)->gso_size;
if (unlikely(skb->len <= mss))
goto out;
What is the gso_type, and does it include SKB_GSO_DODGY?
0 - not set.
Thanks for the follow-up details. Is this something that you can trigger easily?
Yes, if you have a UML instance handy.
Running iperf between the host and a UML guest using raw socket
transport triggers it immediately.
This is my UML command line:
vmlinux mem=2048M umid=OPX \
ubd0=OPX-3.0-Work.img \
vec0:transport=raw,ifname=p-veth0,depth=128,gro=1,mac=92:9b:36:5e:38:69 \
root=/dev/ubda ro con=null con0=null,fd:2 con1=fd:0,fd:1
p-right is a part of a vEth pair:
ip link add l-veth0 type veth peer name p-veth0 && ifconfig p-veth0 up
iperf server is on host, iperf -c in the guest.
An skb_dump() + dump_stack() when the packet socket gets such a
packet may point us to the root cause and fix that.
We tried dump stack, it was not informative - it was just the recvmmsg
call stack coming from the UML until it hits the relevant recv bit in
af_packet - it does not tell us where the packet is coming from.
Quoting from the message earlier in the thread:
[ 2334.180854] Call Trace:
[ 2334.181947] dump_stack+0x5c/0x80
[ 2334.183021] packet_recvmsg.cold+0x23/0x49
[ 2334.184063] ___sys_recvmsg+0xe1/0x1f0
[ 2334.185034] ? packet_poll+0xca/0x130
[ 2334.186014] ? sock_poll+0x77/0xb0
[ 2334.186977] ? ep_item_poll.isra.0+0x3f/0xb0
[ 2334.187936] ? ep_send_events_proc+0xf1/0x240
[ 2334.188901] ? dequeue_signal+0xdb/0x180
[ 2334.189848] do_recvmmsg+0xc8/0x2d0
[ 2334.190728] ? ep_poll+0x8c/0x470
[ 2334.191581] __sys_recvmmsg+0x108/0x150
[ 2334.192441] __x64_sys_recvmmsg+0x25/0x30
[ 2334.193346] do_syscall_64+0x53/0x140
[ 2334.194262] entry_SYSCALL_64_after_hwframe+0x44/0xa9
That makes sense. skb_dump might show more interesting details about
the packet.
I will add that and retest later today.
skb len=818 headroom=2 headlen=818 tailroom=908
mac=(2,14) net=(16,0) trans=16
shinfo(txflags=0 nr_frags=0 gso(size=752 type=0 segs=1))
csum(0x100024 ip_summed=3 complete_sw=0 valid=0 level=0)
hash(0x0 sw=0 l4=0) proto=0x0800 pkttype=4 iif=0
sk family=17 type=3 proto=0
Deciphering the actual packet data gives a
TCP packet, ACK and PSH set.
The PSH flag looks like the only "interesting" thing about it in first read.
From the previous thread, these are assumed to be TCP
packets?
Yes
I had missed the original thread. If the packet has
sinfo(skb)->gso_size = 752.
skb->len = 818
then this is a GSO packet. Even though UML will correctly process it
as a normal 818 B packet if psock_rcv pretends that it is, treating it
like that is not strictly correct. A related question is how the setup
arrived at that low MTU size, assuming that is not explicitly
configured that low.
The mtu on the interface is normal. I suspect it is one of the first packets
in the stream or something iperf uses for communication between the server and
the client which always ends up that size.
As of commit 51466a7545b7 ("tcp: fill shinfo->gso_type at last
moment") tcp unconditionally sets gso_type, even for non gso packets.
So either this is not a tcp packet or the field gets zeroed somewhere
along the way. I could not quickly find a possible path to
skb_gso_reset or a raw write.
Same. I have tried to trace a possible origin and I have not seen anything
which may cause it.
It may be useful to insert tests for this condition (skb_is_gso(skb)
&& !skb_shinfo(skb)->gso_type) that call skb_dump at other points in
the network stack. For instance in __ip_queue_xmit and
__dev_queue_xmit.
Since skb segmentation fails in tcp_gso_segment for such packets, it
may also be informative to disable TSO on the veth device and see if
the test fails.
Ack.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in Eng
Re: [PATCH v3] virtio: Work around frames incorrectly marked as gso
On 24/02/2020 22:22, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 4:00 PM Anton Ivanov
wrote:
On 24/02/2020 20:20, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 2:55 PM Anton Ivanov
wrote:
On 24/02/2020 19:27, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 8:26 AM wrote:
From: Anton Ivanov
Some of the locally generated frames marked as GSO which
arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
Do we understand how these packets are generated?
No, we have not been able to trace them.
The only thing we know is that this is specific to locally generated
packets. Something arriving from the network does not show this.
Else it seems this
might be papering over a deeper problem.
The stack should not create GSO packets less than or equal to
skb_shinfo(skb)->gso_size. See for instance the check in
tcp_gso_segment after pulling the tcp header:
mss = skb_shinfo(skb)->gso_size;
if (unlikely(skb->len <= mss))
goto out;
What is the gso_type, and does it include SKB_GSO_DODGY?
0 - not set.
Thanks for the follow-up details. Is this something that you can trigger easily?
Yes, if you have a UML instance handy.
Running iperf between the host and a UML guest using raw socket
transport triggers it immediately.
This is my UML command line:
vmlinux mem=2048M umid=OPX \
ubd0=OPX-3.0-Work.img \
vec0:transport=raw,ifname=p-veth0,depth=128,gro=1,mac=92:9b:36:5e:38:69 \
root=/dev/ubda ro con=null con0=null,fd:2 con1=fd:0,fd:1
p-right is a part of a vEth pair:
ip link add l-veth0 type veth peer name p-veth0 && ifconfig p-veth0 up
iperf server is on host, iperf -c in the guest.
An skb_dump() + dump_stack() when the packet socket gets such a
packet may point us to the root cause and fix that.
We tried dump stack, it was not informative - it was just the recvmmsg
call stack coming from the UML until it hits the relevant recv bit in
af_packet - it does not tell us where the packet is coming from.
Quoting from the message earlier in the thread:
[ 2334.180854] Call Trace:
[ 2334.181947] dump_stack+0x5c/0x80
[ 2334.183021] packet_recvmsg.cold+0x23/0x49
[ 2334.184063] ___sys_recvmsg+0xe1/0x1f0
[ 2334.185034] ? packet_poll+0xca/0x130
[ 2334.186014] ? sock_poll+0x77/0xb0
[ 2334.186977] ? ep_item_poll.isra.0+0x3f/0xb0
[ 2334.187936] ? ep_send_events_proc+0xf1/0x240
[ 2334.188901] ? dequeue_signal+0xdb/0x180
[ 2334.189848] do_recvmmsg+0xc8/0x2d0
[ 2334.190728] ? ep_poll+0x8c/0x470
[ 2334.191581] __sys_recvmmsg+0x108/0x150
[ 2334.192441] __x64_sys_recvmmsg+0x25/0x30
[ 2334.193346] do_syscall_64+0x53/0x140
[ 2334.194262] entry_SYSCALL_64_after_hwframe+0x44/0xa9
That makes sense. skb_dump might show more interesting details about
the packet.
I will add that and retest later today.
From the previous thread, these are assumed to be TCP
packets?
Yes
I had missed the original thread. If the packet has
sinfo(skb)->gso_size = 752.
skb->len = 818
then this is a GSO packet. Even though UML will correctly process it
as a normal 818 B packet if psock_rcv pretends that it is, treating it
like that is not strictly correct. A related question is how the setup
arrived at that low MTU size, assuming that is not explicitly
configured that low.
The mtu on the interface is normal. I suspect it is one of the first packets
in the stream or something iperf uses for communication between the server and
the client which always ends up that size.
As of commit 51466a7545b7 ("tcp: fill shinfo->gso_type at last
moment") tcp unconditionally sets gso_type, even for non gso packets.
So either this is not a tcp packet or the field gets zeroed somewhere
along the way. I could not quickly find a possible path to
skb_gso_reset or a raw write.
Same. I have tried to trace a possible origin and I have not seen anything
which may cause it.
It may be useful to insert tests for this condition (skb_is_gso(skb)
&& !skb_shinfo(skb)->gso_type) that call skb_dump at other points in
the network stack. For instance in __ip_queue_xmit and
__dev_queue_xmit.
Since skb segmentation fails in tcp_gso_segment for such packets, it
may also be informative to disable TSO on the veth device and see if
the test fails.
Ack.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3] virtio: Work around frames incorrectly marked as gso
On 25/02/2020 04:02, Jason Wang wrote:
On 2020/2/25 上午6:22, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 4:00 PM Anton Ivanov
wrote:
On 24/02/2020 20:20, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 2:55 PM Anton Ivanov
wrote:
On 24/02/2020 19:27, Willem de Bruijn wrote:
On Mon, Feb 24, 2020 at 8:26 AM wrote:
From: Anton Ivanov
Some of the locally generated frames marked as GSO which
arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
Do we understand how these packets are generated?
No, we have not been able to trace them.
The only thing we know is that this is specific to locally generated
packets. Something arriving from the network does not show this.
Else it seems this
might be papering over a deeper problem.
The stack should not create GSO packets less than or equal to
skb_shinfo(skb)->gso_size. See for instance the check in
tcp_gso_segment after pulling the tcp header:
mss = skb_shinfo(skb)->gso_size;
if (unlikely(skb->len <= mss))
goto out;
What is the gso_type, and does it include SKB_GSO_DODGY?
0 - not set.
Thanks for the follow-up details. Is this something that you can trigger easily?
Yes, if you have a UML instance handy.
Running iperf between the host and a UML guest using raw socket
transport triggers it immediately.
This is my UML command line:
vmlinux mem=2048M umid=OPX \
ubd0=OPX-3.0-Work.img \
vec0:transport=raw,ifname=p-veth0,depth=128,gro=1,mac=92:9b:36:5e:38:69 \
root=/dev/ubda ro con=null con0=null,fd:2 con1=fd:0,fd:1
p-right is a part of a vEth pair:
ip link add l-veth0 type veth peer name p-veth0 && ifconfig p-veth0 up
iperf server is on host, iperf -c in the guest.
An skb_dump() + dump_stack() when the packet socket gets such a
packet may point us to the root cause and fix that.
We tried dump stack, it was not informative - it was just the recvmmsg
call stack coming from the UML until it hits the relevant recv bit in
af_packet - it does not tell us where the packet is coming from.
Quoting from the message earlier in the thread:
[ 2334.180854] Call Trace:
[ 2334.181947] dump_stack+0x5c/0x80
[ 2334.183021] packet_recvmsg.cold+0x23/0x49
[ 2334.184063] ___sys_recvmsg+0xe1/0x1f0
[ 2334.185034] ? packet_poll+0xca/0x130
[ 2334.186014] ? sock_poll+0x77/0xb0
[ 2334.186977] ? ep_item_poll.isra.0+0x3f/0xb0
[ 2334.187936] ? ep_send_events_proc+0xf1/0x240
[ 2334.188901] ? dequeue_signal+0xdb/0x180
[ 2334.189848] do_recvmmsg+0xc8/0x2d0
[ 2334.190728] ? ep_poll+0x8c/0x470
[ 2334.191581] __sys_recvmmsg+0x108/0x150
[ 2334.192441] __x64_sys_recvmmsg+0x25/0x30
[ 2334.193346] do_syscall_64+0x53/0x140
[ 2334.194262] entry_SYSCALL_64_after_hwframe+0x44/0xa9
That makes sense. skb_dump might show more interesting details about
the packet. From the previous thread, these are assumed to be TCP
packets?
I had missed the original thread. If the packet has
sinfo(skb)->gso_size = 752.
skb->len = 818
then this is a GSO packet. Even though UML will correctly process it
as a normal 818 B packet if psock_rcv pretends that it is, treating it
like that is not strictly correct. A related question is how the setup
arrived at that low MTU size, assuming that is not explicitly
configured that low.
As of commit 51466a7545b7 ("tcp: fill shinfo->gso_type at last
moment") tcp unconditionally sets gso_type, even for non gso packets.
So either this is not a tcp packet or the field gets zeroed somewhere
along the way. I could not quickly find a possible path to
skb_gso_reset or a raw write.
It may be useful to insert tests for this condition (skb_is_gso(skb)
&& !skb_shinfo(skb)->gso_type) that call skb_dump at other points in
the network stack. For instance in __ip_queue_xmit and
__dev_queue_xmit.
+1
We meet some customer hit such condition as well which lead over MTU packet to
be queued by TAP which crashes their buggy userspace application.
We suspect it's the issue of wrong gso_type vs gso_size.
Well, we now have a test case where all the code is available and 100% under
our control :)
Brgds,
Thanks
Since skb segmentation fails in tcp_gso_segment for such packets, it
may also be informative to disable TSO on the veth device and see if
the test fails.
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3] virtio: Work around frames incorrectly marked as gso
On 24/02/2020 20:20, Willem de Bruijn wrote: On Mon, Feb 24, 2020 at 2:55 PM Anton Ivanov wrote: On 24/02/2020 19:27, Willem de Bruijn wrote: On Mon, Feb 24, 2020 at 8:26 AM wrote: From: Anton Ivanov Some of the locally generated frames marked as GSO which arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no fragments (data_len = 0) and length significantly shorter than the MTU (752 in my experiments). Do we understand how these packets are generated? No, we have not been able to trace them. The only thing we know is that this is specific to locally generated packets. Something arriving from the network does not show this. Else it seems this might be papering over a deeper problem. The stack should not create GSO packets less than or equal to skb_shinfo(skb)->gso_size. See for instance the check in tcp_gso_segment after pulling the tcp header: mss = skb_shinfo(skb)->gso_size; if (unlikely(skb->len <= mss)) goto out; What is the gso_type, and does it include SKB_GSO_DODGY? 0 - not set. Thanks for the follow-up details. Is this something that you can trigger easily? Yes, if you have a UML instance handy. Running iperf between the host and a UML guest using raw socket transport triggers it immediately. This is my UML command line: vmlinux mem=2048M umid=OPX \ ubd0=OPX-3.0-Work.img \ vec0:transport=raw,ifname=p-veth0,depth=128,gro=1,mac=92:9b:36:5e:38:69 \ root=/dev/ubda ro con=null con0=null,fd:2 con1=fd:0,fd:1 p-right is a part of a vEth pair: ip link add l-veth0 type veth peer name p-veth0 && ifconfig p-veth0 up iperf server is on host, iperf -c in the guest. An skb_dump() + dump_stack() when the packet socket gets such a packet may point us to the root cause and fix that. We tried dump stack, it was not informative - it was just the recvmmsg call stack coming from the UML until it hits the relevant recv bit in af_packet - it does not tell us where the packet is coming from. Quoting from the message earlier in the thread: [ 2334.180854] Call Trace: [ 2334.181947] dump_stack+0x5c/0x80 [ 2334.183021] packet_recvmsg.cold+0x23/0x49 [ 2334.184063] ___sys_recvmsg+0xe1/0x1f0 [ 2334.185034] ? packet_poll+0xca/0x130 [ 2334.186014] ? sock_poll+0x77/0xb0 [ 2334.186977] ? ep_item_poll.isra.0+0x3f/0xb0 [ 2334.187936] ? ep_send_events_proc+0xf1/0x240 [ 2334.188901] ? dequeue_signal+0xdb/0x180 [ 2334.189848] do_recvmmsg+0xc8/0x2d0 [ 2334.190728] ? ep_poll+0x8c/0x470 [ 2334.191581] __sys_recvmmsg+0x108/0x150 [ 2334.192441] __x64_sys_recvmmsg+0x25/0x30 [ 2334.193346] do_syscall_64+0x53/0x140 [ 2334.194262] entry_SYSCALL_64_after_hwframe+0x44/0xa9 -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3] virtio: Work around frames incorrectly marked as gso
On 24/02/2020 19:27, Willem de Bruijn wrote: On Mon, Feb 24, 2020 at 8:26 AM wrote: From: Anton Ivanov Some of the locally generated frames marked as GSO which arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no fragments (data_len = 0) and length significantly shorter than the MTU (752 in my experiments). Do we understand how these packets are generated? No, we have not been able to trace them. The only thing we know is that this is specific to locally generated packets. Something arriving from the network does not show this. Else it seems this might be papering over a deeper problem. The stack should not create GSO packets less than or equal to skb_shinfo(skb)->gso_size. See for instance the check in tcp_gso_segment after pulling the tcp header: mss = skb_shinfo(skb)->gso_size; if (unlikely(skb->len <= mss)) goto out; What is the gso_type, and does it include SKB_GSO_DODGY? 0 - not set. -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v3] virtio: Work around frames incorrectly marked as gso
From: Anton Ivanov
Some of the locally generated frames marked as GSO which
arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels. The frames are reported as
invalid, while they are in fact gso-less frames.
The easiest way to reproduce is to connect a User Mode
Linux instance to the host using the vector raw transport
and a vEth interface. Vector raw uses recvmmsg/sendmmsg
with virtio headers on af_packet sockets. When running iperf
between the UML and the host, UML regularly complains about
EINVAL return from recvmmsg.
This patch marks the vnet header as non-GSO instead of
reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..2c99c752cb20 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -98,10 +98,11 @@ static inline int virtio_net_hdr_from_skb(const struct
sk_buff *skb,
bool has_data_valid,
int vlan_hlen)
{
+ struct skb_shared_info *sinfo = skb_shinfo(skb);
+
memset(hdr, 0, sizeof(*hdr)); /* no info leak */
- if (skb_is_gso(skb)) {
- struct skb_shared_info *sinfo = skb_shinfo(skb);
+ if (skb_is_gso(skb) && sinfo->gso_type) {
/* This is a hint as to how much should be linear. */
hdr->hdr_len = __cpu_to_virtio16(little_endian,
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] virtio: Work around frames incorrectly marked as gso
On 24/02/2020 12:46, Michael S. Tsirkin wrote:
On Mon, Feb 24, 2020 at 10:19:12AM +, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the locally generated frames marked as GSO which
arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
A bit more info on how to reproduce couldn't hurt here.
Will do, a v3 will follow shortly.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
Acked-by: Michael S. Tsirkin
Eric - as you looked at this in the past, would you mind acking please?
---
include/linux/virtio_net.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..94fb78c3a2ab 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -100,8 +100,8 @@ static inline int virtio_net_hdr_from_skb(const struct
sk_buff *skb,
{
memset(hdr, 0, sizeof(*hdr)); /* no info leak */
- if (skb_is_gso(skb)) {
- struct skb_shared_info *sinfo = skb_shinfo(skb);
+ struct skb_shared_info *sinfo = skb_shinfo(skb);
I need to move this a few lines up - the kernel build robot is quite rightfully
complaining.
+ if (skb_is_gso(skb) && sinfo->gso_type) {
/* This is a hint as to how much should be linear. */
hdr->hdr_len = __cpu_to_virtio16(little_endian,
--
2.20.1
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2] virtio: Work around frames incorrectly marked as gso
From: Anton Ivanov
Some of the locally generated frames marked as GSO which
arrive at virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..94fb78c3a2ab 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -100,8 +100,8 @@ static inline int virtio_net_hdr_from_skb(const struct
sk_buff *skb,
{
memset(hdr, 0, sizeof(*hdr)); /* no info leak */
- if (skb_is_gso(skb)) {
- struct skb_shared_info *sinfo = skb_shinfo(skb);
+ struct skb_shared_info *sinfo = skb_shinfo(skb);
+ if (skb_is_gso(skb) && sinfo->gso_type) {
/* This is a hint as to how much should be linear. */
hdr->hdr_len = __cpu_to_virtio16(little_endian,
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 20/02/2020 07:58, Michael S. Tsirkin wrote:
On Thu, Feb 13, 2020 at 04:23:24PM +, Anton Ivanov wrote:
On 13/02/2020 15:53, Michael S. Tsirkin wrote:
On Thu, Feb 13, 2020 at 07:44:06AM -0800, Eric Dumazet wrote:
On 2/13/20 2:00 AM, Michael S. Tsirkin wrote:
On Wed, Feb 12, 2020 at 05:38:09PM +, Anton Ivanov wrote:
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
The printk in virtio_net_hdr_from_skb says NULL.
That is probably normal for a locally originated frame.
I cannot reproduce this with network traffic by the way - it happens only if
the traffic is locally originated on the host.
A,
OK so is it code in __tcp_transmit_skb that sets gso_size to non-null
when gso_type is 0?
Correct way to determine if a packet is a gso one is by looking at gso_size.
Then only it is legal looking at gso_type
static inline bool skb_is_gso(const struct sk_buff *skb)
{
return skb_shinfo(skb)->gso_size;
}
/* Note: Should be called only if skb_is_gso(skb) is true */
static inline bool skb_is_gso_v6(const struct sk_buff *skb)
...
There is absolutely no relation between GSO and skb->data_len, skb can be
linearized
for various orthogonal reasons.
The reported problem is that virtio gets a packet where gso_size
is !0 but gso_type is 0.
It currently drops these on the assumption that it's some type
of a gso packet it does not know how to handle.
So you are saying if skb_is_gso we can still have gso_type set to 0,
and that's an expected configuration?
So the patch should just be:
-if (skb_is_gso(skb)) {
+if (skb_is_gso(skb) && sinfo->gso_type) {
Yes, provided that skb_is_gso(skb) and sinfo->gso_type == 0 is a valid state.
I agree with Jason, there may be something wrong going on here and we need to
find the source which creates these packets.
A.
Want to submit a patch to address this for now?
I can do that on Monday - traveling till then.
A.
?
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridge Greys Limited, England and Wales company No 10273661
http://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 13/02/2020 15:53, Michael S. Tsirkin wrote:
On Thu, Feb 13, 2020 at 07:44:06AM -0800, Eric Dumazet wrote:
On 2/13/20 2:00 AM, Michael S. Tsirkin wrote:
On Wed, Feb 12, 2020 at 05:38:09PM +, Anton Ivanov wrote:
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
The printk in virtio_net_hdr_from_skb says NULL.
That is probably normal for a locally originated frame.
I cannot reproduce this with network traffic by the way - it happens only if
the traffic is locally originated on the host.
A,
OK so is it code in __tcp_transmit_skb that sets gso_size to non-null
when gso_type is 0?
Correct way to determine if a packet is a gso one is by looking at gso_size.
Then only it is legal looking at gso_type
static inline bool skb_is_gso(const struct sk_buff *skb)
{
return skb_shinfo(skb)->gso_size;
}
/* Note: Should be called only if skb_is_gso(skb) is true */
static inline bool skb_is_gso_v6(const struct sk_buff *skb)
...
There is absolutely no relation between GSO and skb->data_len, skb can be
linearized
for various orthogonal reasons.
The reported problem is that virtio gets a packet where gso_size
is !0 but gso_type is 0.
It currently drops these on the assumption that it's some type
of a gso packet it does not know how to handle.
So you are saying if skb_is_gso we can still have gso_type set to 0,
and that's an expected configuration?
So the patch should just be:
-if (skb_is_gso(skb)) {
+if (skb_is_gso(skb) && sinfo->gso_type) {
Yes, provided that skb_is_gso(skb) and sinfo->gso_type == 0 is a valid state.
I agree with Jason, there may be something wrong going on here and we need to
find the source which creates these packets.
A.
?
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 13/02/2020 10:00, Michael S. Tsirkin wrote:
On Wed, Feb 12, 2020 at 05:38:09PM +, Anton Ivanov wrote:
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
The printk in virtio_net_hdr_from_skb says NULL.
That is probably normal for a locally originated frame.
I cannot reproduce this with network traffic by the way - it happens only if
the traffic is locally originated on the host.
A,
OK so is it code in __tcp_transmit_skb that sets gso_size to non-null
when gso_type is 0?
It does look like that, but I cannot see it when reading it :(
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 13/02/2020 03:31, Jason Wang wrote:
On 2020/2/13 上午1:38, Anton Ivanov wrote:
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
The printk in virtio_net_hdr_from_skb says NULL.
That is probably normal for a locally originated frame.
I cannot reproduce this with network traffic by the way - it happens only if
the traffic is locally originated on the host.
A,
Or maybe you can try add dump_stack() there.
That unfortunately is not very informative. At that point it shows me the
invocation chain from recvmmsg:
[ 2334.180854] Call Trace:
[ 2334.181947] dump_stack+0x5c/0x80
[ 2334.183021] packet_recvmsg.cold+0x23/0x49
[ 2334.184063] ___sys_recvmsg+0xe1/0x1f0
[ 2334.185034] ? packet_poll+0xca/0x130
[ 2334.186014] ? sock_poll+0x77/0xb0
[ 2334.186977] ? ep_item_poll.isra.0+0x3f/0xb0
[ 2334.187936] ? ep_send_events_proc+0xf1/0x240
[ 2334.188901] ? dequeue_signal+0xdb/0x180
[ 2334.189848] do_recvmmsg+0xc8/0x2d0
[ 2334.190728] ? ep_poll+0x8c/0x470
[ 2334.191581] __sys_recvmmsg+0x108/0x150
[ 2334.192441] __x64_sys_recvmmsg+0x25/0x30
[ 2334.193346] do_syscall_64+0x53/0x140
[ 2334.194262] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Thanks
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
The printk in virtio_net_hdr_from_skb says NULL.
That is probably normal for a locally originated frame.
I cannot reproduce this with network traffic by the way - it happens only if
the traffic is locally originated on the host.
A,
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 12/02/2020 10:19, Michael S. Tsirkin wrote:
On Wed, Feb 12, 2020 at 10:03:31AM +, Anton Ivanov wrote:
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
I will rebuild my rig and retest (it's been a while since I worked on this bug).
In theory, it should be veth - the test is over a vEth pair and all frames are
locally originated by iperf.
In practice - I will retest and post the results sometimes later today.
Brgds,
ok if it's veth then you need to add a similar printk patch to veth
and re-run to see where does it come from originally.
Most likely - an iperf running on localhost :) It is generating the traffic.
Thanks, I will add both printks and re-test ASAP.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 11/02/2020 10:37, Michael S. Tsirkin wrote:
On Tue, Feb 11, 2020 at 07:42:37AM +, Anton Ivanov wrote:
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int
virtio_net_hdr_from_skb(const struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
I think we need to find the culprit and fix it there, lots of other things
can break otherwise.
Just printing out skb->dev->name should do the trick, no?
I will rebuild my rig and retest (it's been a while since I worked on this bug).
In theory, it should be veth - the test is over a vEth pair and all frames are
locally originated by iperf.
In practice - I will retest and post the results sometimes later today.
Brgds,
>
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 11/02/2020 02:51, Jason Wang wrote:
On 2020/2/11 上午12:55, Anton Ivanov wrote:
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const
struct sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
Do you mean gso_size is set but gso_type is not? Looks like a bug
elsewhere.
Thanks
Yes.
I could not trace it where it is coming from.
I see it when doing recvmmsg on raw sockets in the UML vector network
drivers.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio: Work around frames incorrectly marked as gso
On 09/12/2019 10:48, anton.iva...@cambridgegreys.com wrote:
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const struct
sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
ping.
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH] virtio: Work around frames incorrectly marked as gso
From: Anton Ivanov
Some of the frames marked as GSO which arrive at
virtio_net_hdr_from_skb() have no GSO_TYPE, no
fragments (data_len = 0) and length significantly shorter
than the MTU (752 in my experiments).
This is observed on raw sockets reading off vEth interfaces
in all 4.x and 5.x kernels I tested.
These frames are reported as invalid while they are in fact
gso-less frames.
This patch marks the vnet header as no-GSO for them instead
of reporting it as invalid.
Signed-off-by: Anton Ivanov
---
include/linux/virtio_net.h | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
index 0d1fe9297ac6..d90d5cff1b9a 100644
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -112,8 +112,12 @@ static inline int virtio_net_hdr_from_skb(const struct
sk_buff *skb,
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
else if (sinfo->gso_type & SKB_GSO_TCPV6)
hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
- else
- return -EINVAL;
+ else {
+ if (skb->data_len == 0)
+ hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
+ else
+ return -EINVAL;
+ }
if (sinfo->gso_type & SKB_GSO_TCP_ECN)
hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
} else
--
2.20.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: custom virt-io support (in user-mode-linux)
On 22/05/2019 14:02, Johannes Berg wrote: Hi, While my main interest is mostly in UML right now [1] I've CC'ed the qemu and virtualization lists because something similar might actually apply to other types of virtualization. I'm thinking about adding virt-io support to UML, but the tricky part is that while I want to use the virt-io basics (because it's a nice interface from the 'inside'), I don't actually want the stock drivers that are part of the kernel now (like virtio-net etc.) but rather something that integrates with wifi (probably building on hwsim). The 'inside' interfaces aren't really a problem - just have a specific device ID for this, and then write a normal virtio kernel driver for it. The 'outside' interfaces are where my thinking breaks down right now. Looking at lkl, the outside is just all implemented in lkl as code that gets linked to the library, so in UML terms it'd just be extra 'outside' code like the timer handling or other netdev stuff we have today. Looking at qemu, it's of course also implemented there, and then interfaces with the real network, console abstraction, etc. However, like I said above, I really need something very custom and not likely to make it upstream to any project (because what point is that if you cannot connect to the rest of the environment I'm building), so I'm thinking that perhaps it should be possible to write an abstract 'outside' that lets you interact with it really from out-of-process? Perhaps through some kind of shared memory segment? I think that gets tricky with virt-io doing DMA (I think it does?) though, so that part would have to be implemented directly and not out-of-process? But really that's why I'm asking - is there a better way than to just link the device-side virt-io code into the same binary (be it lkl lib, uml binary, qemu binary)? Thanks, johannes [1] Actually, I've considered using qemu, but it doesn't have virtualized time and doesn't seem to support TSC virtualization. I guess I could remove TSC from the guest CPU and add a virtualized HPET, but I've yet to convince myself this works - on UML I made virtual time as a prototype already: https://patchwork.ozlabs.org/patch/1095814/ (though my real goal isn't to just skip time forward when the host goes idle, it's to sync with other simulated components) ___ linux-um mailing list linux...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um I have looked at using virtio semantics in UML in the past around the point when I wanted to make the recvmmsg/sendmmsg vector drivers common in UML and QEMU. It is certainly possible, I went for the native approach at the end though. -- Anton R. Ivanov https://www.kot-begemot.co.uk/ ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: custom virt-io support (in user-mode-linux)
On 22/05/2019 14:46, Johannes Berg wrote:
Hi Anton,
I'm thinking about adding virt-io support to UML, but the tricky part is
that while I want to use the virt-io basics (because it's a nice
interface from the 'inside'), I don't actually want the stock drivers
that are part of the kernel now (like virtio-net etc.) but rather
something that integrates with wifi (probably building on hwsim).
I have looked at using virtio semantics in UML in the past around the
point when I wanted to make the recvmmsg/sendmmsg vector drivers common
in UML and QEMU. It is certainly possible,
I went for the native approach at the end though.
Hmm. I'm not sure what you mean by either :-)
Is there any commonality between the vector drivers?
I was looking purely from a network driver perspective.
I had two options - either do a direct read/write as it does today or
implement the ring/king semantics and read/write from that.
I decided to not bother with the latter and read/write directly from/to
skbs.
I can't see how
that'd work without a bus abstraction (like virtio) in qemu? I mean, the
kernel driver just calls uml_vector_sendmmsg(), which I'd say belongs
more to the 'outside world', but that can't really be done in qemu?
Ok, I guess then I see what you mean by 'native' though.
Similarly, of course, I can implement arbitrary virt-io devices - just
the kernel side doesn't call a function like uml_vector_sendmmsg()
directly, but instead the virt-io model, and the model calls the
function, which essentially is the same just with a (convenient)
abstraction layer.
But this leaves the fundamental fact the model code ("vector_user.c" or
a similar "virtio_user.c") is still part of the build.
I guess what I'm thinking is have something like "virtio_user_rpc.c"
that uses some appropriate RPC to interact with the real model. IOW,
rather than having all the model-specific logic actually be here (like
vector_user.c actually knows how to send network packets over a real
socket fd), try to call out to some RPC that contains the real model.
Now that I thought about it further, I guess my question boils down to
"did anyone ever think about doing RPC for Virt-IO instead of putting
the entire device model into the hypervisor/emulator/...".
Virtio in general no. UML specifically - yes. I have thought of mapping
out all key device calls to RPCs for a few applications. The issue is
that it is fairly difficult to make all of this function cleanly without
blocking in strange places.
You may probably want to look at the UML UBD driver. That is an example
of moving out all processing to an external thread and talking to it via
a request/response API. While it still expects shared memory and needs
access to UML address space the model should be more amenable to
replacing various calls with RPCs as you have now left the rest of the
kernel to run while you are processing the RPC. It also provides you
with RPC completion interrupts, etc as a side effect.
So you basically have UML -> Thread -> RPCs -> Model?
johannes
___
linux-um mailing list
linux...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um
--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
