On Thu, 29 Sep 2022 03:04:03 -0400, "Michael S. Tsirkin" <m...@redhat.com>
wrote:
> On Thu, Sep 29, 2022 at 09:48:33AM +0800, Jason Wang wrote:
> > On Wed, Sep 28, 2022 at 9:39 PM Michael S. Tsirkin <m...@redhat.com> wrote:
> > >
> > > On Mon, Sep 26, 2022 at 04:06:17PM +0800, Jason Wang wrote:
> > > > > Jason I think the issue with previous proposals is that they conflict
> > > > > with VIRTIO_F_ANY_LAYOUT. We have repeatedly found that giving the
> > > > > driver flexibility in arranging the packet in memory is benefitial.
> > > >
> > > >
> > > > Yes, but I didn't found how it can conflict the any_layout. Device can
> > > > just
> > > > to not split the header when the layout doesn't fit for header
> > > > splitting.
> > > > (And this seems the case even if we're using buffers).
> > >
> > > Well spec says:
> > >
> > > indicates to both the device and the driver that no
> > > assumptions were made about framing.
> > >
> > > if device assumes that descriptor boundaries are where
> > > driver wants packet to be stored that is clearly
> > > an assumption.
> >
> > Yes but what I want to say is, the device can choose to not split the
> > packet if the framing doesn't fit. Does it still comply with the above
> > description?
> >
> > Thanks
>
> The point of ANY_LAYOUT is to give drivers maximum flexibility.
> For example, if driver wants to split the header at some specific
> offset this is already possible without extra functionality.
>
> Let's keep it that way.
>
> Now, let's formulate what are some of the problems with the current way.
>
>
>
> A- mergeable buffers is even more flexible, since a single packet
> is built up of multiple buffers.
If I understand correctly, this is our v8.
> And in theory device can
> choose arbitrary set of buffers to store a packet.
> So you could supply a small buffer for headers followed by a bigger
> one for payload, in theory even without any changes.
This is very interesting, I did not think of this point.
This is helpful to reduce the waste of memory.
> Problem 1: However since this is not how devices currently operate,
> a feature bit would be helpful.
>
> Problem 2: Also, in the past we found it useful to be able to figure out
> whether
> packet fits in a single buffer without looking at the header.
> For this reason, we have this text:
>
> If a receive packet is spread over multiple buffers, the device
> MUST use all buffers but the last (i.e. the first \field{num_buffers} -
> 1 buffers) completely up to the full length of each buffer
> supplied by the driver.
>
> if we want to keep this optimization and allow using a separate
> buffer for headers, then I think we could rely on the feature bit
> from Problem 1 and just make an exception for the first buffer.
> Also num_buffers is then always >= 2, maybe state this to avoid
> confusion.
>
Yes, I think this is feasible.
>
>
>
>
> B- without mergeable, there's no flexibility. In particular, there can
> not be uninitialized space between header and data. If we had flexibility
> here, this could be
> helpful for alignment, security, etc.
> Unfortunately, our hands are tied:
>
>
> \field{len} is particularly useful
> for drivers using untrusted buffers: if a driver does not know exactly
> how much has been written by the device, the driver would have to zero
> the buffer in advance to ensure no data leakage occurs.
>
> For example, a network driver may hand a received buffer directly to
> an unprivileged userspace application. If the network device has not
> overwritten the bytes which were in that buffer, this could leak the
> contents of freed memory from other processes to the application.
I don't think this is very troublesome, the device can memset the hole by 0.
>
>
> so all buffers have to be initialized completely up to the reported
> used length.
>
> It could be that the guarantee is not relevant in some use-cases.
> We would have to specify that this is an exception to the rule,
> explain that drivers must be careful about information leaks.
> Let's say there's a feature bit that adds uninitialized space
> somewhere. How much was added? We can find out by parsing the
> packet but once you start doing that just to assemble the skb
> you have already lost performance.
> So lots of spec work, some security risks, and unclear performance.
>
>
>
>
> Is above a fair summary?
>
>
>
> If yes I would say let's address A, but make sure we ask drivers
> to clear the new feature bit if there's no mergeable
> (as opposed to e.g. failing probe) so we can add
> support for !mergeable down the road.
>
>
>
>
>
> --
> MST
>
---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
