Skip to contentSkip to conversation

Removes FDs from VhostUserMessage structure

PATCH Superseded Christophe Fontaine 3 participants 9 messages Jan 23, 2022 – Feb 8, 2022 Network drivers

lore
Share & tools
lore
#0 lore raw

cover Removes FDs from VhostUserMessage structure

This patch removes the file descriptors from the struct VhostUserMessage as it is not part of the spec.
These FDs are moved to a englobing context structure.

Christophe Fontaine (1):
vhost: Move fds outside of VhostUserMessage

 drivers/net/virtio/virtio_user/vhost_user.c |   1 -
 lib/vhost/vhost_crypto.c                    |  10 +-
 lib/vhost/vhost_user.c                      | 529 ++++++++++----------
 lib/vhost/vhost_user.h                      |   7 +-
 4 files changed, 288 insertions(+), 259 deletions(-)
Signature · 1 line
2.34.1
#1 lore raw

PATCH 1/1 vhost: Move fds outside of VhostUserMessage

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

---
 drivers/net/virtio/virtio_user/vhost_user.c |   1 -
 lib/vhost/vhost_crypto.c                    |  10 +-
 lib/vhost/vhost_user.c                      | 529 ++++++++++----------
 lib/vhost/vhost_user.h                      |   7 +-
 4 files changed, 288 insertions(+), 259 deletions(-)
drivers/net/virtio/virtio_user/vhost_user.c +0 -1
index cc830a660f..c9e445f6c8 100644
@@ -97,7 +97,6 @@ struct vhost_user_msg {
 		struct vhost_vring_addr addr;
 		struct vhost_memory memory;
 	} payload;
-	int fds[VHOST_MEMORY_MAX_NREGIONS];
 } __rte_packed;
 
 #define VHOST_USER_HDR_SIZE offsetof(struct vhost_user_msg, payload.u64)
lib/vhost/vhost_crypto.c +5 -5
index 926b5c0bd9..afaf19ec80 100644
@@ -453,7 +453,7 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 {
 	struct virtio_net *dev = get_device(vid);
 	struct vhost_crypto *vcrypto;
-	VhostUserMsg *vmsg = msg;
+	struct vhu_msg_context *ctx = msg;
 	enum rte_vhost_msg_result ret = RTE_VHOST_MSG_RESULT_OK;
 
 	if (dev == NULL) {
@@ -467,15 +467,15 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 		return RTE_VHOST_MSG_RESULT_ERR;
 	}
 
-	switch (vmsg->request.master) {
+	switch (ctx->msg.request.master) {
 	case VHOST_USER_CRYPTO_CREATE_SESS:
 		vhost_crypto_create_sess(vcrypto,
-				&vmsg->payload.crypto_session);
-		vmsg->fd_num = 0;
+				&ctx->msg.payload.crypto_session);
+		ctx->fd_num = 0;
 		ret = RTE_VHOST_MSG_RESULT_REPLY;
 		break;
 	case VHOST_USER_CRYPTO_CLOSE_SESS:
-		if (vhost_crypto_close_sess(vcrypto, vmsg->payload.u64))
+		if (vhost_crypto_close_sess(vcrypto, ctx->msg.payload.u64))
 			ret = RTE_VHOST_MSG_RESULT_ERR;
 		break;
 	default:
lib/vhost/vhost_user.c +277 -252

1,279 lines not on this page · raw message

lib/vhost/vhost_user.h +6 -1
index 16fe03f889..9314fd3d07 100644
@@ -149,9 +149,14 @@ typedef struct VhostUserMsg {
 		VhostUserVringArea area;
 		VhostUserInflight inflight;
 	} payload;
+	/* Nothing should be added after the payload */
+} __rte_packed VhostUserMsg;
+
+struct vhu_msg_context {
 	int fds[VHOST_MEMORY_MAX_NREGIONS];
 	int fd_num;
-} __rte_packed VhostUserMsg;
+	VhostUserMsg msg;
+};
 
 #define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64)
 
Signature · 1 line
2.34.1
Reviewed-by on v1 1/1
#2 lore raw

Re: [PATCH 1/1] vhost: Move fds outside of VhostUserMessage

Hi Christophe,

Quoted 6 lines · Christophe Fontaine

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

Quoted 8 lines
---
  drivers/net/virtio/virtio_user/vhost_user.c |   1 -
  lib/vhost/vhost_crypto.c                    |  10 +-
  lib/vhost/vhost_user.c                      | 529 ++++++++++----------
  lib/vhost/vhost_user.h                      |   7 +-
  4 files changed, 288 insertions(+), 259 deletions(-)

Thanks for working on this, I agree the FDs should be moved outside the Vhost-user message structure.

Thanks,
Maxime

#3 lore raw

Re: [PATCH 1/1] vhost: Move fds outside of VhostUserMessage

In reply to Christophe Fontaine

Hi Christophe,

Quoted 6 lines · Christophe Fontaine

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

Quoted 7 lines
---
  drivers/net/virtio/virtio_user/vhost_user.c |   1 -
  lib/vhost/vhost_crypto.c                    |  10 +-
  lib/vhost/vhost_user.c                      | 529 ++++++++++----------
  lib/vhost/vhost_user.h                      |   7 +-
  4 files changed, 288 insertions(+), 259 deletions(-)

I was about to apply your patch, but it conflicts with previous patches applied.

Could you please rebase it and post a new revision?

Thanks in advance,
Maxime

#4 lore raw

v2 cover Removes FDs from VhostUserMessage structure

In reply to Christophe Fontaine

This patch removes the file descriptors from the struct VhostUserMessage as it is not part of the spec.
These FDs are moved to a englobing context structure.

v2: rebased on top of dpdk-next-virtio

Christophe Fontaine (1):
vhost: move fds outside of VhostUserMessage

 drivers/net/virtio/virtio_user/vhost_user.c |   1 -
 lib/vhost/vhost_crypto.c                    |  10 +-
 lib/vhost/vhost_user.c                      | 534 ++++++++++----------
 lib/vhost/vhost_user.h                      |   7 +-
 4 files changed, 290 insertions(+), 262 deletions(-)
Signature · 1 line
2.34.1
#5 lore raw

PATCH v2 1/1 vhost: move fds outside of VhostUserMessage

From: Christophe Fontaine <[email protected]>

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

---
 drivers/net/virtio/virtio_user/vhost_user.c |   1 -
 lib/vhost/vhost_crypto.c                    |  10 +-
 lib/vhost/vhost_user.c                      | 534 ++++++++++----------
 lib/vhost/vhost_user.h                      |   7 +-
 4 files changed, 290 insertions(+), 262 deletions(-)
drivers/net/virtio/virtio_user/vhost_user.c +0 -1
index 0a39393c45..00d0dcaa74 100644
@@ -97,7 +97,6 @@ struct vhost_user_msg {
 		struct vhost_vring_addr addr;
 		struct vhost_memory memory;
 	} payload;
-	int fds[VHOST_MEMORY_MAX_NREGIONS];
 } __rte_packed;
 
 #define VHOST_USER_HDR_SIZE offsetof(struct vhost_user_msg, payload.u64)
lib/vhost/vhost_crypto.c +5 -5
index 926b5c0bd9..afaf19ec80 100644
@@ -453,7 +453,7 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 {
 	struct virtio_net *dev = get_device(vid);
 	struct vhost_crypto *vcrypto;
-	VhostUserMsg *vmsg = msg;
+	struct vhu_msg_context *ctx = msg;
 	enum rte_vhost_msg_result ret = RTE_VHOST_MSG_RESULT_OK;
 
 	if (dev == NULL) {
@@ -467,15 +467,15 @@ vhost_crypto_msg_post_handler(int vid, void *msg)
 		return RTE_VHOST_MSG_RESULT_ERR;
 	}
 
-	switch (vmsg->request.master) {
+	switch (ctx->msg.request.master) {
 	case VHOST_USER_CRYPTO_CREATE_SESS:
 		vhost_crypto_create_sess(vcrypto,
-				&vmsg->payload.crypto_session);
-		vmsg->fd_num = 0;
+				&ctx->msg.payload.crypto_session);
+		ctx->fd_num = 0;
 		ret = RTE_VHOST_MSG_RESULT_REPLY;
 		break;
 	case VHOST_USER_CRYPTO_CLOSE_SESS:
-		if (vhost_crypto_close_sess(vcrypto, vmsg->payload.u64))
+		if (vhost_crypto_close_sess(vcrypto, ctx->msg.payload.u64))
 			ret = RTE_VHOST_MSG_RESULT_ERR;
 		break;
 	default:
lib/vhost/vhost_user.c +279 -255

1,291 lines not on this page · raw message

lib/vhost/vhost_user.h +6 -1
index c8e299e30c..be53669f3b 100644
@@ -149,9 +149,14 @@ typedef struct VhostUserMsg {
 		VhostUserVringArea area;
 		VhostUserInflight inflight;
 	} payload;
+	/* Nothing should be added after the payload */
+} __rte_packed VhostUserMsg;
+
+struct vhu_msg_context {
 	int fds[VHOST_MEMORY_MAX_NREGIONS];
 	int fd_num;
-} __rte_packed VhostUserMsg;
+	VhostUserMsg msg;
+};
 
 #define VHOST_USER_HDR_SIZE offsetof(VhostUserMsg, payload.u64)
 
Signature · 1 line
2.34.1
Reviewed-by on v2 1/1
#6 lore raw

Re: [PATCH v2 1/1] vhost: move fds outside of VhostUserMessage

Quoted 8 lines · Christophe Fontaine

From: Christophe Fontaine <[email protected]>

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

Quoted 8 lines
---
  drivers/net/virtio/virtio_user/vhost_user.c |   1 -
  lib/vhost/vhost_crypto.c                    |  10 +-
  lib/vhost/vhost_user.c                      | 534 ++++++++++----------
  lib/vhost/vhost_user.h                      |   7 +-
  4 files changed, 290 insertions(+), 262 deletions(-)

Thanks,
Maxime

Reviewed-by on v2 1/1
#7 lore raw

Re: [PATCH v2 1/1] vhost: move fds outside of VhostUserMessage

In reply to Christophe Fontaine
Quoted 10 lines · Christophe Fontaine

From: Christophe Fontaine <[email protected]>

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

Signature · 1 line
David Marchand
#8 lore raw

Re: [PATCH v2 1/1] vhost: move fds outside of VhostUserMessage

In reply to Christophe Fontaine
Quoted 8 lines · Christophe Fontaine

From: Christophe Fontaine <[email protected]>

FDs at the end of the VhostUserMessage structure limits the size of the payload. Move them to an other englobing structure, before the header & payload of a VhostUserMessage.
Also removes a reference to fds in the VHUMsg structure defined in drivers/net/virtio/virtio_user/vhost_user.c

Quoted 8 lines
---
  drivers/net/virtio/virtio_user/vhost_user.c |   1 -
  lib/vhost/vhost_crypto.c                    |  10 +-
  lib/vhost/vhost_user.c                      | 534 ++++++++++----------
  lib/vhost/vhost_user.h                      |   7 +-
  4 files changed, 290 insertions(+), 262 deletions(-)

Applied to dpdk-next-virtio/main.

Thanks,
Maxime