Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 2 Dec 2020 09:53:45 +0100 | From | Stefano Garzarella <> | Subject | Re: [PATCH net-next v1 2/3] virtio_transport_common: Set sibling VMs flag on the receive path |
| |
On Tue, Dec 01, 2020 at 09:01:05PM +0200, Paraschiv, Andra-Irina wrote: > > >On 01/12/2020 18:22, Stefano Garzarella wrote: >> >>On Tue, Dec 01, 2020 at 05:25:04PM +0200, Andra Paraschiv wrote: >>>The vsock flag can be set during the connect() setup logic, when >>>initializing the vsock address data structure variable. Then the vsock >>>transport is assigned, also considering this flag. >>> >>>The vsock transport is also assigned on the (listen) receive path. The >>>flag needs to be set considering the use case. >>> >>>Set the vsock flag of the remote address to the one targeted for sibling >>>VMs communication if the following conditions are met: >>> >>>* The source CID of the packet is higher than VMADDR_CID_HOST. >>>* The destination CID of the packet is higher than VMADDR_CID_HOST. >>> >>>Signed-off-by: Andra Paraschiv <andraprs@amazon.com> >>>--- >>>net/vmw_vsock/virtio_transport_common.c | 8 ++++++++ >>>1 file changed, 8 insertions(+) >>> >>>diff --git a/net/vmw_vsock/virtio_transport_common.c >>>b/net/vmw_vsock/virtio_transport_common.c >>>index 5956939eebb78..871c84e0916b1 100644 >>>--- a/net/vmw_vsock/virtio_transport_common.c >>>+++ b/net/vmw_vsock/virtio_transport_common.c >>>@@ -1062,6 +1062,14 @@ virtio_transport_recv_listen(struct sock >>>*sk, struct virtio_vsock_pkt *pkt, >>> vsock_addr_init(&vchild->remote_addr, >>>le64_to_cpu(pkt->hdr.src_cid), >>> le32_to_cpu(pkt->hdr.src_port)); >>> >> >>Maybe is better to create an helper function that other transports can >>use for the same purpose or we can put this code in the >>vsock_assign_transport() and set this flag only when the 'psk' argument >>is not NULL (this is the case when it's called by the transports when we >>receive a new connection request and 'psk' is the listener socket). >> >>The second way should allow us to support all the transports without >>touching them. > >Ack, I was wondering about the other transports such as vmci or hyperv. > >I can move the logic below in the codebase that assigns the transport, >after checking 'psk'. > >> >>>+ /* If the packet is coming with the source and destination >>>CIDs higher >>>+ * than VMADDR_CID_HOST, then a vsock channel should be >>>established for >>>+ * sibling VMs communication. >>>+ */ >>>+ if (vchild->local_addr.svm_cid > VMADDR_CID_HOST && >>>+ vchild->remote_addr.svm_cid > VMADDR_CID_HOST) >>>+ vchild->remote_addr.svm_flag = >>>VMADDR_FLAG_SIBLING_VMS_COMMUNICATION; >> >>svm_flag is always initialized to 0 in vsock_addr_init(), so this >>assignment is the first one and it's okay, but to avoid future issues >>I'd use |= here to set the flag. > >Fair point. I was thinking more towards exclusive flags values >(purposes), but that's fine with the bitwise operator if we would get >a set of flag values together. I will also update the field name to >'svm_flags', let me know if we should keep the previous one or there >is a better option.
Yeah, maybe in the future we will add some new flags and we'll only need to add them without touching this code.
Agree with the new 'svm_flags' field name.
Thanks, Stefano
| ![\](/images/icornerr.gif) |