Guenter Roeck
2022-Aug-15 20:34 UTC
[PATCH] virtio_net: Revert "virtio_net: set the default max ring size by find_vqs()"
On Mon, Aug 15, 2022 at 05:16:50AM -0400, Michael S. Tsirkin wrote:> This reverts commit 762faee5a2678559d3dc09d95f8f2c54cd0466a7. > > This has been reported to trip up guests on GCP (Google Cloud). Why is > not yet clear - to be debugged, but the patch itself has several other > issues: > > - It treats unknown speed as < 10G > - It leaves userspace no way to find out the ring size set by hypervisor > - It tests speed when link is down > - It ignores the virtio spec advice: > Both \field{speed} and \field{duplex} can change, thus the driver > is expected to re-read these values after receiving a > configuration change notification. > - It is not clear the performance impact has been tested properly > > Revert the patch for now. > > Link: https://lore.kernel.org/r/20220814212610.GA3690074%40roeck-us.net > Link: https://lore.kernel.org/r/20220815070203.plwjx7b3cyugpdt7%40awork3.anarazel.de > Link: https://lore.kernel.org/r/3df6bb82-1951-455d-a768-e9e1513eb667%40www.fastmail.com > Link: https://lore.kernel.org/r/FCDC5DDE-3CDD-4B8A-916F-CA7D87B547CE%40anarazel.de > Fixes: 762faee5a267 ("virtio_net: set the default max ring size by find_vqs()") > Cc: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > Cc: Jason Wang <jasowang at redhat.com> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > Tested-by: Andres Freund <andres at anarazel.de>I ran this patch through a total of 14 syskaller tests, 2 test runs each on 7 different crashes reported by syzkaller (as reported to the linux-kernel mailing list). No problems were reported. I also ran a single cross-check with one of the syzkaller runs on top of v6.0-rc1, without this patch. That test run failed. Overall, I think we can call this fixed. Guenter --- syskaller reports: Reported-and-tested-by: syzbot+2984d1b7aef6b51353f0 at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=3b9175e0879a7749 dashboard link: https://syzkaller.appspot.com/bug?extid=2984d1b7aef6b51353f0 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 userspace arch: i386 patch: https://syzkaller.appspot.com/x/patch.diff?x=11949fc3080000 Reported-and-tested-by: syzbot+2c35c4d66094ddfe198e at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=3cb39b084894e9a5 dashboard link: https://syzkaller.appspot.com/bug?extid=2c35c4d66094ddfe198e compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=163e20f3080000 Reported-and-tested-by: syzbot+97f830ad641de86d08c0 at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=f267ed4fb258122a dashboard link: https://syzkaller.appspot.com/bug?extid=97f830ad641de86d08c0 compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=146c8e5b080000 Reported-and-tested-by: syzbot+005efde5e97744047fe4 at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=3cb39b084894e9a5 dashboard link: https://syzkaller.appspot.com/bug?extid=005efde5e97744047fe4 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=106c8e5b080000 Reported-and-tested-by: syzbot+9ada839c852179f13999 at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=3b9175e0879a7749 dashboard link: https://syzkaller.appspot.com/bug?extid=9ada839c852179f13999 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=118756f3080000 Reported-and-tested-by: syzbot+382af021ce115a936b1f at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=e656d8727a25e83b dashboard link: https://syzkaller.appspot.com/bug?extid=382af021ce115a936b1f compiler: Debian clang version 13.0.1-++20220126092033+75e33f71c2da-1~exp1~20220126212112.63, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=135f650d080000 Reported-and-tested-by: syzbot+24df94a8d05d5a3e68f0 at syzkaller.appspotmail.com Tested on: commit: 568035b0 Linux 6.0-rc1 git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v6.0-rc1 kernel config: https://syzkaller.appspot.com/x/.config?x=3b9175e0879a7749 dashboard link: https://syzkaller.appspot.com/bug?extid=24df94a8d05d5a3e68f0 compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2 patch: https://syzkaller.appspot.com/x/patch.diff?x=12758a47080000
Michael S. Tsirkin
2022-Aug-15 20:42 UTC
[PATCH] virtio_net: Revert "virtio_net: set the default max ring size by find_vqs()"
On Mon, Aug 15, 2022 at 01:34:26PM -0700, Guenter Roeck wrote:> On Mon, Aug 15, 2022 at 05:16:50AM -0400, Michael S. Tsirkin wrote: > > This reverts commit 762faee5a2678559d3dc09d95f8f2c54cd0466a7. > > > > This has been reported to trip up guests on GCP (Google Cloud). Why is > > not yet clear - to be debugged, but the patch itself has several other > > issues: > > > > - It treats unknown speed as < 10G > > - It leaves userspace no way to find out the ring size set by hypervisor > > - It tests speed when link is down > > - It ignores the virtio spec advice: > > Both \field{speed} and \field{duplex} can change, thus the driver > > is expected to re-read these values after receiving a > > configuration change notification. > > - It is not clear the performance impact has been tested properly > > > > Revert the patch for now. > > > > Link: https://lore.kernel.org/r/20220814212610.GA3690074%40roeck-us.net > > Link: https://lore.kernel.org/r/20220815070203.plwjx7b3cyugpdt7%40awork3.anarazel.de > > Link: https://lore.kernel.org/r/3df6bb82-1951-455d-a768-e9e1513eb667%40www.fastmail.com > > Link: https://lore.kernel.org/r/FCDC5DDE-3CDD-4B8A-916F-CA7D87B547CE%40anarazel.de > > Fixes: 762faee5a267 ("virtio_net: set the default max ring size by find_vqs()") > > Cc: Xuan Zhuo <xuanzhuo at linux.alibaba.com> > > Cc: Jason Wang <jasowang at redhat.com> > > Signed-off-by: Michael S. Tsirkin <mst at redhat.com> > > Tested-by: Andres Freund <andres at anarazel.de> > > I ran this patch through a total of 14 syskaller tests, 2 test runs each on > 7 different crashes reported by syzkaller (as reported to the linux-kernel > mailing list). No problems were reported. I also ran a single cross-check > with one of the syzkaller runs on top of v6.0-rc1, without this patch. > That test run failed. > > Overall, I think we can call this fixed. > > GuenterIt's more of a work around though since we don't yet have the root cause for this. I suspect a GCP hypervisor bug at the moment. This is excercising a path we previously only took on GFP_KERNEL allocation failures during probe, I don't think that happens a lot. -- MST