From: Cornelia Huck <cohuck@redhat.com>
To: Stefano Garzarella <sgarzare@redhat.com>,
mst@redhat.com, stefanha@redhat.com
Cc: virtio-comment@lists.oasis-open.org,
arseny.krasnov@kaspersky.com, jasowang@redhat.com
Subject: [virtio-comment] Re: [PATCH v11 0/3] virtio-vsock: SOCK_SEQPACKET description
Date: Thu, 13 Jan 2022 12:34:20 +0100 [thread overview]
Message-ID: <87czkvx41v.fsf@redhat.com> (raw)
In-Reply-To: <20220113112203.pgwfwtnqti2g4xap@steredhat>
On Thu, Jan 13 2022, Stefano Garzarella <sgarzare@redhat.com> wrote:
> On Thu, Jan 13, 2022 at 11:50:09AM +0100, Cornelia Huck wrote:
>>On Wed, Jan 12 2022, Stefano Garzarella <sgarzare@redhat.com> wrote:
>>
>>> v11:
>>> - reworked "Message and record boundaries" paragraph [stefanha]
>>>
>>> v10: https://markmail.org/message/aebu4zqp4kxdm4ej
>>>
>>> Linux kernel and QEMU already merged SOCK_SEQPACKET support,
>>> so I'm resending Arseny's patches to have consistent virtio-spec
>>> and implementation.
>>
>>It really should not have been merged without a spec :( But now that it
>
> Yeah, I agree but the linux-net maintainers merged it right away, so we
> rushed to merge QEMU too to use the feature.
>
>>has happened already, we need to get a proper spec added sooner rather
>>than later, and it would be nice if it could make virtio 1.2, which
>>would mean that voting needs to start this week.
>
> Having it in virtio 1.2 would be great.
>
>>
>>(I don't see an issue at https://github.com/oasis-tcs/virtio-spec/issues
>>yet; creating one would be an obvious pre-req.)
>
> Do I create it now or when we have all the patches ready?
If you already know that there will be a v12, I'd probably hold it off
until you post that; but it is easy to simply edit the issue to update
the link to the archive.
>
>>
>>>
>>> I added patch 2, following the discussion about F_STREAM feature bit:
>>> https://markmail.org/message/aoaspjy2jhidwbuo#query:+page:1+mid:obw54zzikgqimhjk+state:results
>>>
>>> About patch 2, the vhost-vsock device in the Linux kernel doesn't set bit 0
>>> (F_STREAM), so at this point I don't know if it's better to use a negative
>>> feature flag (e.g. F_NO_STREAM) or we go for F_STREAM and send a patch to
>>> linux-stable (and QEMU?) to solve this issue.
>>
>>I fear that even with a patch for stable, there can still be old setups
>>around for which "only F_SEQPACKET set" translates to "supports both
>>stream and seqpacket"...
>
> Which is usually true, since seqpacket is pretty much stream with the
> message/record boundaries.
Hm; so, does it make much sense to support seqpacket but not stream? If
not, maybe seqpacket can always imply stream, and we'd eliminate the
uncertainty.
>
>> so I guess it mostly becomes a question of
>>whether ignoring those broken setups is tolerable. (The old setups not
>>setting any feature bits are fine with the proposed patch.) Using a
>>negative feature bit is uglier, but also clearer. I suppose we want to
>>be able to make stream sockets optional?
>
> Yes, especially with other types, like datagram (not yet merged). We
> would like to be able to allow just datagram, without stream and
> seqpacket.
This would probably work well even if we have seqpacket imply stream.
>
>> If so, I think the negative
>>approach is the better option, unless we can live with some broken
>>setups.
>>
>
> Yes, it's probably the best thing.
>
> I'm thinking about QEMU and the mess to add a new kernel supported
> feature. (vsock is almost always used with vhost-vsock).
> We might be in the case where we migrate from a new QEMU that supports
> F_STREAM to an old one that doesn't set it but supports stream anyway.
>
> So I'd like to change patch 2 to F_NO_STREAM. For the spec, it's not the
> best, but for the current implementation, I think it's the cleanest
> solution.
> Otherwise maybe we should write in the spec that if F_SEQPACKET is set
> this means that stream is supported even if F_STREAM is not set.
Yes, if that works, it would probably be the less ugly option.
>
>
> Michael, Stefan, what do you think?
>
>
> Thanks,
> Stefano
This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.
In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.
Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/
next prev parent reply other threads:[~2022-01-13 11:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-12 17:09 [PATCH v11 0/3] virtio-vsock: SOCK_SEQPACKET description Stefano Garzarella
2022-01-12 17:09 ` [PATCH v11 1/3] virtio-vsock: use C style defines for constants Stefano Garzarella
2022-01-12 17:09 ` [PATCH v11 2/3] virtio-vsock: add VIRTIO_VSOCK_F_STREAM feature bit Stefano Garzarella
2022-01-12 17:09 ` [PATCH v11 3/3] virtio-vsock: SOCK_SEQPACKET description Stefano Garzarella
2022-01-13 10:50 ` [virtio-comment] Re: [PATCH v11 0/3] " Cornelia Huck
2022-01-13 11:22 ` Stefano Garzarella
2022-01-13 11:34 ` Cornelia Huck [this message]
2022-01-13 13:57 ` Stefano Garzarella
2022-01-13 14:06 ` [virtio-comment] " Cornelia Huck
2022-01-13 14:16 ` Stefano Garzarella
2022-01-13 15:11 ` Michael S. Tsirkin
2022-01-13 16:33 ` Stefano Garzarella
2022-01-13 13:19 ` Michael S. Tsirkin
2022-01-13 13:52 ` 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=87czkvx41v.fsf@redhat.com \
--to=cohuck@redhat.com \
--cc=arseny.krasnov@kaspersky.com \
--cc=jasowang@redhat.com \
--cc=mst@redhat.com \
--cc=sgarzare@redhat.com \
--cc=stefanha@redhat.com \
--cc=virtio-comment@lists.oasis-open.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