On Mon, May 10, 2021 at 7:52 AM Stefano Garzarella <sgarzare@xxxxxxxxxx> wrote: > > On Fri, May 07, 2021 at 09:53:19AM -0700, Jiang Wang . wrote: > >Hi guys, > > > >I have one question about adding two new virtqueues for dgram. One new > >thought is that we don't add two new virtqueues but keep using existing > >virtqueues for both stream and dgram. > > > >At the beginning when I first thought about supporting dgram, I thought > >adding two new virtqueues would be easier and have better performance. > >But now, after the prototype is done, I think that to keep using > >existing virtqueues is also not complicated and could in fact be simpler. > >The performance difference may not be very big. > > I honestly didn't think it was easier to use two new queues, quite the > opposite. Got it. > > > >Original code has about 3 places which have assumptions about the > >virtqueues are only used by the stream. But we can change those codes. > >One place is to check pkt len. We can check only for stream pkts. > >Another two are in tx and rx code path where if queued replies pkts are > >too much, the code will stop the rx queue and resume later. We can keep > >that same logic. The dgram will be affected a little bit but that should > >be fine I think. Are there any other places that we should fix? > > Did you take a look at Arseny's series? > I think he's already found the places where to check the type and it > seems to me they are the places you listed. Yes. I checked his patch. And that helps. > > > >In short, the virtqueues are in a lower level and can support multiple > >flows and socket types. Use existing virtqueues also make it more > >compatible with old versions. > > It's not clear to me how compatibility is improved. Can you elaborate on > this? I was thinking if we don't add two new virtqueues, then maybe we don't need to add new feature bit too? If the other end does not support dgram, then the packets will be just dropped. What do you think? Do we still need to add dgram feature bits? I can have a feature bit for mergeable buffer. > > > >What do you guys think? I remember Stefano mentioned that we should add > >two new virtqueues for dgram. Stefano, do you have some specific reasons > >for that? Could we just keep using existing virtqueues? Thanks. > > > > My biggest concern was about the credit mechanism for datagrams. I mean > avoiding datagrams from crowding the queue without limits, preventing > streams from communicating. > > If you've found a way to limit datagram traffic, then maybe it's doable. I see. I will add some limit to dgram packets. Also, when the virtqueues are shared between stream and dgram, both of them need to grab a lock before using the virtqueue, so one will not completely block another one. > Thanks, > Stefano > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
