> > > 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.
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. >OK I'm convinced, reset and re-negotiate seems cleaner. -- MST
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