[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] Re: [RFC PATCH v1 0/1] virtio-vsock: introduce SEQPACKET description
On Tue, Mar 16, 2021 at 09:50:48AM -0400, Michael S. Tsirkin wrote:
On Mon, Feb 22, 2021 at 11:16:54AM +0100, Stefano Garzarella wrote:On Thu, Feb 18, 2021 at 09:07:12AM +0300, Arseny Krasnov wrote: > This patchset adds description of SOCK_SEQPACKET socket's type > of virtio vsock. > > Here is implementation: > https://lkml.org/lkml/2021/2/18/24 > > Arseny Krasnov(1): > virtio-vsock: add SOCK_SEQPACKET description > > virtio-vsock.tex | 40 +++++++++++++++++++++++++++++++++++++--- > 1 files changed, 37 insertions(+), 3 deletions(-) > > TODO: > - for messages identification I use header's field called 'msg_cnt'. > May be it is better to call it 'msg_id' or 'msg_num', because it > is used as ID, but implemented as counter. If we use it only as an identifier, I think 'msg_id' is preferable and we shouldn't mention in the specs that it's a counter, since it's just a possible implementation. Instead if we use the counter for some control, for example if we lost a packet, then maybe msg_cnt is better. But since the channel shouldn't lose packets, I don't think this is the case. > > - in current version of specification, some values of headers' fields > still unnamed, for example type of socket (stream or seqpacket), then > shutdown flags. Spec says that for stream socket value of 'type' > must be 1. For receive shutdown bit 0 is set in 'flags', for send > shutdown bit 1 is set in 'flags'. But in Linux these unnamed ones and > zeroes are implemented as enums, so may be it will be ok to add such > enums in specification (as 'enum virtio_vsock_event_id'). Since we have an enumerate for VIRTIO_VSOCK_OP_* values, I think we can add enums for socket type and maybe 'flags'.We really need to switch enums to defines, for consistency.
I fully agree, indeed at least in the virtio-vsock part, we use the enumerate as they were many defines, always redefining the assigned value. Using defines, we are forced to always define the value and I think that's fair!
Thanks, Stefano
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]