From: Paolo Abeni <pabeni@redhat.com>
To: Bobby Eshleman <bobby.eshleman@bytedance.com>
Cc: Bobby Eshleman <bobbyeshleman@gmail.com>,
Cong Wang <cong.wang@bytedance.com>,
Stefano Garzarella <sgarzare@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Jason Wang <jasowang@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next v9] virtio/vsock: replace virtio_vsock_pkt with sk_buff
Date: Tue, 10 Jan 2023 15:45:32 +0100 [thread overview]
Message-ID: <5042e5c6e57a3f99895616c891512e482bf6ed28.camel@redhat.com> (raw)
In-Reply-To: <91593e9c8a475a26a465369f6caff86ac5d662e3.camel@redhat.com>
On Tue, 2023-01-10 at 09:36 +0100, Paolo Abeni wrote:
> On Sat, 2023-01-07 at 00:29 +0000, Bobby Eshleman wrote:
> > This commit changes virtio/vsock to use sk_buff instead of
> > virtio_vsock_pkt. Beyond better conforming to other net code, using
> > sk_buff allows vsock to use sk_buff-dependent features in the future
> > (such as sockmap) and improves throughput.
> >
> > This patch introduces the following performance changes:
> >
> > Tool/Config: uperf w/ 64 threads, SOCK_STREAM
> > Test Runs: 5, mean of results
> > Before: commit 95ec6bce2a0b ("Merge branch 'net-ipa-more-endpoints'")
> >
> > Test: 64KB, g2h
> > Before: 21.63 Gb/s
> > After: 25.59 Gb/s (+18%)
> >
> > Test: 16B, g2h
> > Before: 11.86 Mb/s
> > After: 17.41 Mb/s (+46%)
> >
> > Test: 64KB, h2g
> > Before: 2.15 Gb/s
> > After: 3.6 Gb/s (+67%)
> >
> > Test: 16B, h2g
> > Before: 14.38 Mb/s
> > After: 18.43 Mb/s (+28%)
> >
> > Signed-off-by: Bobby Eshleman <bobby.eshleman@bytedance.com>
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> > Acked-by: Paolo Abeni <pabeni@redhat.com>
> > ---
> >
> > Tested using vsock_test g2h and h2g. I'm not sure if it is standard
> > practice here to carry Acks and Reviews forward to future versions, but
> > I'm doing that here to hopefully make life easier for maintainers.
> > Please let me know if it is not standard practice.
>
> As Jakub noted, there is no clear rule for tag passing across different
> patch revisions.
>
> Here, given the complexity of the patch and the not trivial list of
> changes, I would have preferred you would have dropped my tag.
>
> > Changes in v9:
> > - check length in rx header
> > - guard alloactor from small requests
> > - squashed fix for v8 bug reported by syzbot:
> > syzbot+30b72abaa17c07fe39dd@syzkaller.appspotmail.com
>
> It's not clear to me what/where is the fix exactly, could you please
> clarify?
Reading the syzkaller report, it looks like iov_length() in
vhost_vsock_alloc_pkt() can not be trusted to carry a reasonable value.
As such, don't you additionally need to ensure/check that iov_length()
is greater or equal to sizeof(virtio_vsock_hdr) ?
Thanks.
Paolo
next prev parent reply other threads:[~2023-01-10 14:46 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-07 0:29 [PATCH net-next v9] virtio/vsock: replace virtio_vsock_pkt with sk_buff Bobby Eshleman
2023-01-10 8:36 ` Paolo Abeni
2023-01-10 14:45 ` Paolo Abeni [this message]
2022-12-22 6:12 ` Bobby Eshleman
2023-01-10 9:39 ` Michael S. Tsirkin
2023-01-10 16:04 ` Stefano Garzarella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5042e5c6e57a3f99895616c891512e482bf6ed28.camel@redhat.com \
--to=pabeni@redhat.com \
--cc=bobby.eshleman@bytedance.com \
--cc=bobbyeshleman@gmail.com \
--cc=cong.wang@bytedance.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jasowang@redhat.com \
--cc=kuba@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).