On Wed, May 31, 2023 at 12:35:05AM +0000, Bobby Eshleman wrote:
> This commit drops the transport->dgram_dequeue callback and makes
> vsock_dgram_recvmsg() generic. It also adds additional transport
> callbacks for use by the generic vsock_dgram_recvmsg(), such as for
> parsing skbs for CID/port which vary in format per transport.
>
> Signed-off-by: Bobby Eshleman <bobby.eshleman@xxxxxxxxxxxxx>
...
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index b370070194fa..b6a51afb74b8 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1731,57 +1731,40 @@ static int vmci_transport_dgram_enqueue(
> return err - sizeof(*dg);
> }
>
> -static int vmci_transport_dgram_dequeue(struct vsock_sock *vsk,
> - struct msghdr *msg, size_t len,
> - int flags)
> +int vmci_transport_dgram_get_cid(struct sk_buff *skb, unsigned int *cid)
> {
> - int err;
> struct vmci_datagram *dg;
> - size_t payload_len;
> - struct sk_buff *skb;
>
> - if (flags & MSG_OOB || flags & MSG_ERRQUEUE)
> - return -EOPNOTSUPP;
> + dg = (struct vmci_datagram *)skb->data;
> + if (!dg)
> + return -EINVAL;
>
> - /* Retrieve the head sk_buff from the socket's receive queue. */
> - err = 0;
> - skb = skb_recv_datagram(&vsk->sk, flags, &err);
> - if (!skb)
> - return err;
> + *cid = dg->src.context;
> + return 0;
> +}
Hi Bobby,
clang-16 with W=1 seems a bit unhappy about this.
net/vmw_vsock/vmci_transport.c:1734:5: warning: no previous prototype for function 'vmci_transport_dgram_get_cid' [-Wmissing-prototypes]
int vmci_transport_dgram_get_cid(struct sk_buff *skb, unsigned int *cid)
^
net/vmw_vsock/vmci_transport.c:1734:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int vmci_transport_dgram_get_cid(struct sk_buff *skb, unsigned int *cid)
^
static
net/vmw_vsock/vmci_transport.c:1746:5: warning: no previous prototype for function 'vmci_transport_dgram_get_port' [-Wmissing-prototypes]
int vmci_transport_dgram_get_port(struct sk_buff *skb, unsigned int *port)
^
net/vmw_vsock/vmci_transport.c:1746:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int vmci_transport_dgram_get_port(struct sk_buff *skb, unsigned int *port)
^
static
net/vmw_vsock/vmci_transport.c:1758:5: warning: no previous prototype for function 'vmci_transport_dgram_get_length' [-Wmissing-prototypes]
int vmci_transport_dgram_get_length(struct sk_buff *skb, size_t *len)
^
net/vmw_vsock/vmci_transport.c:1758:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int vmci_transport_dgram_get_length(struct sk_buff *skb, size_t *len)
^
I see similar warnings for net/vmw_vsock/af_vsock.c in patch 4/8.
> +
> +int vmci_transport_dgram_get_port(struct sk_buff *skb, unsigned int *port)
> +{
> + struct vmci_datagram *dg;
>
> dg = (struct vmci_datagram *)skb->data;
> if (!dg)
> - /* err is 0, meaning we read zero bytes. */
> - goto out;
> -
> - payload_len = dg->payload_size;
> - /* Ensure the sk_buff matches the payload size claimed in the packet. */
> - if (payload_len != skb->len - sizeof(*dg)) {
> - err = -EINVAL;
> - goto out;
> - }
> + return -EINVAL;
>
> - if (payload_len > len) {
> - payload_len = len;
> - msg->msg_flags |= MSG_TRUNC;
> - }
> + *port = dg->src.resource;
> + return 0;
> +}
>
> - /* Place the datagram payload in the user's iovec. */
> - err = skb_copy_datagram_msg(skb, sizeof(*dg), msg, payload_len);
> - if (err)
> - goto out;
> +int vmci_transport_dgram_get_length(struct sk_buff *skb, size_t *len)
> +{
> + struct vmci_datagram *dg;
>
> - if (msg->msg_name) {
> - /* Provide the address of the sender. */
> - DECLARE_SOCKADDR(struct sockaddr_vm *, vm_addr, msg->msg_name);
> - vsock_addr_init(vm_addr, dg->src.context, dg->src.resource);
> - msg->msg_namelen = sizeof(*vm_addr);
> - }
> - err = payload_len;
> + dg = (struct vmci_datagram *)skb->data;
> + if (!dg)
> + return -EINVAL;
>
> -out:
> - skb_free_datagram(&vsk->sk, skb);
> - return err;
> + *len = dg->payload_size;
> + return 0;
> }
>
> static bool vmci_transport_dgram_allow(u32 cid, u32 port)
...
