On 12/02/2015 04:31 PM, Yuanhan Liu wrote:

On Wed, Dec 02, 2015 at 03:53:45PM +0200, Panu Matilainen wrote:

quoted

On 12/02/2015 05:43 AM, Yuanhan Liu wrote:

quoted

VHOST_USER_SET_LOG_BASE request is used to tell the backend (dpdk
vhost-user) where we should log dirty pages, and how big the log
buffer is.

This request introduces a new payload:

	typedef struct VhostUserLog {
		uint64_t mmap_size;
		uint64_t mmap_offset;
	} VhostUserLog;

Also, a fd is delivered from QEMU by ancillary data.

With those info given, an area of memory is mmaped, assigned
to dev->log_base, for logging dirty pages.

Signed-off-by: Yuanhan Liu <redacted>
---
  lib/librte_vhost/rte_virtio_net.h             |  2 ++
  lib/librte_vhost/vhost_user/vhost-net-user.c  |  7 ++++-
  lib/librte_vhost/vhost_user/vhost-net-user.h  |  6 ++++
  lib/librte_vhost/vhost_user/virtio-net-user.c | 44 +++++++++++++++++++++++++++
  lib/librte_vhost/vhost_user/virtio-net-user.h |  1 +
  5 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 5687452..416dac2 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h

@@ -127,6 +127,8 @@ struct virtio_net {
  #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ)
  	char			ifname[IF_NAME_SZ];	/**< Name of the tap device or socket path. */
  	uint32_t		virt_qp_nb;	/**< number of queue pair we have allocated */
+	uint64_t		log_size;	/**< Size of log area */
+	uint8_t			*log_base;	/**< Where dirty pages are logged */
  	void			*priv;		/**< private context */
  	struct vhost_virtqueue	*virtqueue[VHOST_MAX_QUEUE_PAIRS * 2];	/**< Contains all virtqueue information. */
  } __rte_cache_aligned;

This (and other changes in patch 2 breaks the librte_vhost ABI
again, so you'd need to at least add a deprecation note to 2.2 to be
able to do it in 2.3 at all according to the ABI policy.

I was thinking that adding a new field (instead of renaming it or
removing it) isn't an ABI break. So, I was wrong?

Adding or removing a field in the middle of a public struct is always an 
ABI break. Adding to the end often is too, but not always.
Renaming a field is an API break but not an ABI break - the compiler 
cares but the cpu does not.

quoted

Perhaps a better option would be adding some padding to the structs
now for 2.2 since the vhost ABI is broken there anyway. That would
at least give a chance to keep it compatible from 2.2 to 2.3.

It will not be compatible, unless we add exact same fields (not
something like uint8_t pad[xx]). Otherwise, the pad field renaming
is also an ABI break, right?

There's no ABI (or API) break in changing reserved unused fields to 
something else, as long as care is taken with sizes and alignment. In 
any case padding is best added to the end of a struct to minimize risks 
and keep things simple.

	- Panu -

Thomas, should I write an ABI deprecation note? Can I make it for
v2.2 release If I make one tomorrow? (Sorry that I'm not awared
of that it would be an ABI break).

	--yliu