All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alvaro Karsz <alvaro.karsz@solid-run.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>,
	"edumazet@google.com" <edumazet@google.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>
Subject: Re: [PATCH net] virtio-net: reject small vring sizes
Date: Tue, 25 Apr 2023 04:34:58 -0400	[thread overview]
Message-ID: <20230425041352-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <AM0PR04MB47237D46ADE7954289025B66D4669@AM0PR04MB4723.eurprd04.prod.outlook.com>

On Sun, Apr 23, 2023 at 12:28:49PM +0000, Alvaro Karsz wrote:
> 
> > > > The rest of stuff can probably just be moved to after find_vqs without
> > > > much pain.
> > > >
> > > Actually, I think that with a little bit of pain :)
> > > If we use small vrings and a GRO feature bit is set, Linux will need to allocate 64KB of continuous memory for every receive descriptor..
> > 
> > Oh right. Hmm. Well this is same as big packets though, isn't it?
> > 
> 
> Well, when VIRTIO_NET_F_MRG_RXBUF is not negotiated and one of the GRO features is, the receive buffers are page size buffers chained together to form a 64K buffer.
> In this case, do all the chained descriptors actually point to a single block of continuous memory, or is it possible for the descriptors to point to pages spread all over?
> 
> > 
> > > Instead of failing probe if GRO/CVQ are set, can we just reset the device if we discover small vrings and start over?
> > > Can we remember that this device uses small vrings, and then just avoid negotiating the features that we cannot support?
> > 
> > 
> > We technically can of course. I am just not sure supporting CVQ with just 1 s/g entry will
> > ever be viable.
> 
> Even if we won't support 1 s/g entry, do we want to fail probe in such cases?
> We could just disable the CVQ feature (with reset, as suggested before).
> I'm not saying that we should, just raising the option.
> 

So, let's add some funky flags in virtio device to block out
features, have core compare these before and after,
detect change, reset and retry?


-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alvaro Karsz <alvaro.karsz@solid-run.com>
Cc: Jason Wang <jasowang@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@google.com" <edumazet@google.com>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"virtualization@lists.linux-foundation.org" 
	<virtualization@lists.linux-foundation.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] virtio-net: reject small vring sizes
Date: Tue, 25 Apr 2023 04:34:58 -0400	[thread overview]
Message-ID: <20230425041352-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <AM0PR04MB47237D46ADE7954289025B66D4669@AM0PR04MB4723.eurprd04.prod.outlook.com>

On Sun, Apr 23, 2023 at 12:28:49PM +0000, Alvaro Karsz wrote:
> 
> > > > The rest of stuff can probably just be moved to after find_vqs without
> > > > much pain.
> > > >
> > > Actually, I think that with a little bit of pain :)
> > > If we use small vrings and a GRO feature bit is set, Linux will need to allocate 64KB of continuous memory for every receive descriptor..
> > 
> > Oh right. Hmm. Well this is same as big packets though, isn't it?
> > 
> 
> Well, when VIRTIO_NET_F_MRG_RXBUF is not negotiated and one of the GRO features is, the receive buffers are page size buffers chained together to form a 64K buffer.
> In this case, do all the chained descriptors actually point to a single block of continuous memory, or is it possible for the descriptors to point to pages spread all over?
> 
> > 
> > > Instead of failing probe if GRO/CVQ are set, can we just reset the device if we discover small vrings and start over?
> > > Can we remember that this device uses small vrings, and then just avoid negotiating the features that we cannot support?
> > 
> > 
> > We technically can of course. I am just not sure supporting CVQ with just 1 s/g entry will
> > ever be viable.
> 
> Even if we won't support 1 s/g entry, do we want to fail probe in such cases?
> We could just disable the CVQ feature (with reset, as suggested before).
> I'm not saying that we should, just raising the option.
> 

So, let's add some funky flags in virtio device to block out
features, have core compare these before and after,
detect change, reset and retry?


-- 
MST


  parent reply	other threads:[~2023-04-25  8:35 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-16  7:46 [PATCH net] virtio-net: reject small vring sizes Alvaro Karsz
2023-04-16  7:46 ` Alvaro Karsz
2023-04-16 16:54 ` Alvaro Karsz
2023-04-16 16:54   ` Alvaro Karsz
2023-04-16 20:45   ` Michael S. Tsirkin
2023-04-16 20:45     ` Michael S. Tsirkin
2023-04-17  3:24     ` Jason Wang
2023-04-17  3:24       ` Jason Wang
2023-04-17  6:20       ` Michael S. Tsirkin
2023-04-17  6:20         ` Michael S. Tsirkin
2023-04-17  6:38         ` Alvaro Karsz
2023-04-17  6:38           ` Alvaro Karsz
2023-04-17  6:41           ` Michael S. Tsirkin
2023-04-17  6:41             ` Michael S. Tsirkin
2023-04-17  7:03             ` Alvaro Karsz
2023-04-17  7:03               ` Alvaro Karsz
2023-04-17  7:10               ` Michael S. Tsirkin
2023-04-17  7:10                 ` Michael S. Tsirkin
2023-04-17  7:33                 ` Alvaro Karsz
2023-04-17  7:33                   ` Alvaro Karsz
2023-04-17  9:20                   ` Michael S. Tsirkin
2023-04-17  9:20                     ` Michael S. Tsirkin
2023-04-17 10:04                     ` Alvaro Karsz
2023-04-17 10:04                       ` Alvaro Karsz
2023-04-17 11:40                       ` Michael S. Tsirkin
2023-04-17 11:40                         ` Michael S. Tsirkin
2023-04-17 11:51                         ` Alvaro Karsz
2023-04-17 11:51                           ` Alvaro Karsz
2023-04-17 11:57                           ` Michael S. Tsirkin
2023-04-17 11:57                             ` Michael S. Tsirkin
2023-04-23  6:51                             ` Alvaro Karsz
2023-04-23  6:51                               ` Alvaro Karsz
2023-04-23  7:19                               ` Michael S. Tsirkin
2023-04-23  7:19                                 ` Michael S. Tsirkin
2023-04-23  7:52                                 ` Alvaro Karsz
2023-04-23  7:52                                   ` Alvaro Karsz
2023-04-23 11:06                                   ` Michael S. Tsirkin
2023-04-23 11:06                                     ` Michael S. Tsirkin
2023-04-23 12:28                                     ` Alvaro Karsz
2023-04-23 12:28                                       ` Alvaro Karsz
2023-04-23 20:17                                       ` Michael S. Tsirkin
2023-04-23 20:17                                         ` Michael S. Tsirkin
2023-04-25  8:34                                       ` Michael S. Tsirkin [this message]
2023-04-25  8:34                                         ` Michael S. Tsirkin
2023-04-25  9:41                                         ` Alvaro Karsz
2023-04-25  9:41                                           ` Alvaro Karsz
2023-04-25 11:11                                           ` Alvaro Karsz
2023-04-25 11:11                                             ` Alvaro Karsz
2023-04-25 12:33                                             ` Michael S. Tsirkin
2023-04-25 12:33                                               ` Michael S. Tsirkin
2023-04-25 12:31                                           ` Michael S. Tsirkin
2023-04-25 12:31                                             ` Michael S. Tsirkin
2023-04-25 13:02                                             ` Alvaro Karsz
2023-04-25 13:02                                               ` Alvaro Karsz
2023-04-25 13:08                                               ` Michael S. Tsirkin
2023-04-25 13:08                                                 ` Michael S. Tsirkin
2023-04-23  8:01                                 ` Alvaro Karsz
2023-04-23  8:01                                   ` Alvaro Karsz
2023-04-23 11:08                                   ` Michael S. Tsirkin
2023-04-23 11:08                                     ` Michael S. Tsirkin
2023-04-17  6:44           ` Xuan Zhuo
2023-04-17  6:44             ` Xuan Zhuo
2023-04-17  7:07             ` Alvaro Karsz
2023-04-17  7:07               ` Alvaro Karsz
2023-04-17  7:11               ` Michael S. Tsirkin
2023-04-17  7:11                 ` Michael S. Tsirkin
2023-04-16 20:38 ` Michael S. Tsirkin
2023-04-16 20:38   ` Michael S. Tsirkin
2023-04-17  6:43   ` Alvaro Karsz
2023-04-17  6:43     ` Alvaro Karsz
2023-04-23 11:09     ` Michael S. Tsirkin
2023-04-23 11:09       ` Michael S. Tsirkin
2023-04-17  1:53 ` Xuan Zhuo
2023-04-17  1:53   ` Xuan Zhuo
2023-04-17  6:47   ` Alvaro Karsz
2023-04-17  6:47     ` Alvaro Karsz
2023-04-17  3:34 ` Xuan Zhuo
2023-04-17  3:34   ` Xuan Zhuo

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=20230425041352-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=alvaro.karsz@solid-run.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.