Re: [PATCH v2 5/5] test: Add memory/passt test cases

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 1 Nov 2022 11:58:26 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Mon, Oct 31, 2022 at 12:20:59PM +0100, Stefano Brivio wrote:
> > These show a summary of memory usage in kernel and userspace with
> > different port forwarding configurations, details of userspace usage
> > using 'nm' (passt only uses statically allocated memory), and details
> > of kernel memory from slab reporting facilities.
> > 
> > This adds a new test image, mbuto.mem.img, with harcoded IPv4 and
> > IPv6 addresses and routes, and just the tools we need to start and
> > stop passt, to report from /proc/slabinfo, /proc/meminfo, and to
> > print and parse symbol sizes using nm(1).
> > 
> > passt can't pivot_root() for sandboxing purposes on ramfs, so we need
> > to create another filesystem and chroot into it, first.  
> 
> Huh.. weird.

Yeah, it also surprised me, but man 2 pivot_root says:

       The  rootfs  (initial ramfs) cannot be pivot_root()ed.  The recommended
       method of changing the root filesystem in this case is to delete every‐
       thing  in rootfs, overmount rootfs with the new root, attach stdin/std‐
       out/stderr to the new /dev/console, and exec the new  init(1).   Helper
       programs for this process exist; see switch_root(8).

(of course I got the documented case of EINVAL before reading that).

> > We don't want to use pane context functions, as we're checking memory
> > usage for sockets: resort to screen-scraping.  
> 
> So, we don't do it at the moment, but it should be pretty easy to make
> the ssh-over-vsock stuff use a persistent control socket
> (ControlPersist / ControlMaster).  That would mean we'd just establish
> one connection then all the subsequent guest bound commands would go
> over that one initial connection.
> 
> It wouldn't take zero socket memory of course, but it should be small
> and more importantly stable.  So, if we take a diff of usage as you
> already do, the results should still be good.

Ah, yes, I guess so. I would track this as a ticket instead of trying
to implement it now.

> This patch is not working for me, for reasons I'm still debugging.
> Some more minor comments in the interim.
> 
> > 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  test/Makefile        |   5 +-
> >  test/lib/layout_ugly |  39 +++++++++
> >  test/lib/setup_ugly  |  37 +++++++++
> >  test/memory/passt    | 187 +++++++++++++++++++++++++++++++++++++++++++
> >  test/passt.mem.mbuto |  42 ++++++++++
> >  test/run             |   4 +
> >  6 files changed, 313 insertions(+), 1 deletion(-)
> >  create mode 100644 test/memory/passt
> >  create mode 100755 test/passt.mem.mbuto
> > 
> > diff --git a/test/Makefile b/test/Makefile
> > index 91498ff..9f2bc42 100644
> > --- a/test/Makefile
> > +++ b/test/Makefile
> > @@ -55,7 +55,7 @@ UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS)
> >  DOWNLOAD_ASSETS = mbuto \
> >  	$(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS)
> >  TESTDATA_ASSETS = small.bin big.bin medium.bin
> > -LOCAL_ASSETS = mbuto.img QEMU_EFI.fd \
> > +LOCAL_ASSETS = mbuto.img mbuto.mem.img QEMU_EFI.fd \
> >  	$(DEBIAN_IMGS:%=prepared-%) $(FEDORA_IMGS:%=prepared-%) \
> >  	$(UBUNTU_NEW_IMGS:%=prepared-%) \
> >  	nsholder guest-key guest-key.pub \
> > @@ -76,6 +76,9 @@ guest-key guest-key.pub:
> >  mbuto.img: passt.mbuto mbuto guest-key.pub $(TESTDATA_ASSETS)
> >  	./mbuto/mbuto -p ./$< -c lz4 -f $@
> >  
> > +mbuto.mem.img: passt.mem.mbuto mbuto ../passt.avx2
> > +	./mbuto/mbuto -p ./$< -c lz4 -f $@
> > +
> >  nsholder: nsholder.c
> >  	$(CC) $(CFLAGS) -o $@ $^
> >  
> > diff --git a/test/lib/layout_ugly b/test/lib/layout_ugly
> > index 9397b7d..d62d337 100644
> > --- a/test/lib/layout_ugly
> > +++ b/test/lib/layout_ugly
> > @@ -81,3 +81,42 @@ layout_pasta_simple() {
> >  
> >  	sleep 1
> >  }
> > +
> > +# layout_memory() - Screen-scraped panes for memory usage tests, big guest pane
> > +layout_memory() {
> > +	sleep 3
> > +
> > +	tmux kill-pane -a -t 0
> > +	cmd_write 0 clear
> > +
> > +	tmux split-window -h -l '35%' -t passt_test
> > +	tmux split-window -v -l '15%' -t passt_test
> > +
> > +	PANE_PASST=2
> > +	PANE_GUEST=0
> > +	PANE_INFO=1
> > +
> > +	get_info_cols
> > +
> > +	tmux send-keys -l -t ${PANE_INFO} 'while cat '"$STATEBASE/log_pipe"'; do :; done'
> > +	tmux send-keys -t ${PANE_INFO} -N 100 C-m
> > +	tmux select-pane -t ${PANE_INFO} -T "test log"
> > +
> > +	if context_exists guest; then
> > +		pane_watch_contexts ${PANE_GUEST} guest guest
> > +	else
> > +		tmux pipe-pane -O -t ${PANE_GUEST} "cat >> ${LOGDIR}/pane_guest.log"
> > +		tmux select-pane -t ${PANE_GUEST} -T "guest"
> > +	fi
> > +
> > +	if context_exists passt; then  
> 
> AFAICT you're always using a context pane for passt, so you don't need
> this conditional.

Right, I'll drop this.

> > +		pane_watch_contexts ${PANE_PASST} passt passt
> > +	else
> > +		tmux pipe-pane -O -t ${PANE_PASST} "cat >> ${LOGDIR}/pane_passt.log"
> > +		tmux select-pane -t ${PANE_PASST} -T "passt"
> > +	fi
> > +
> > +	info_layout "memory usage"
> > +
> > +	sleep 1
> > +}
> > diff --git a/test/lib/setup_ugly b/test/lib/setup_ugly
> > index 764177e..f2e07ba 100755
> > --- a/test/lib/setup_ugly
> > +++ b/test/lib/setup_ugly
> > @@ -13,6 +13,8 @@
> >  # Copyright (c) 2022 Red Hat GmbH
> >  # Author: Stefano Brivio <sbrivio@redhat.com>
> >  
> > +INITRAMFS_MEM="${BASEPATH}/mbuto.mem.img"
> > +
> >  # setup_distro() - Set up pane layout for distro tests
> >  setup_distro() {
> >  	layout_host
> > @@ -25,6 +27,30 @@ setup_pasta_options() {
> >  	layout_pasta_simple
> >  }
> >  
> > +# setup_memory() - Start qemu in guest pane, and passt in passt context
> > +setup_memory() {
> > +	context_setup_host passt
> > +
> > +	layout_memory
> > +
> > +	context_run passt "./passt -P ${STATESETUP}/passt.pid"
> > +
> > +	# pidfile isn't created until passt is listening
> > +	wait_for [ -f "${STATESETUP}/passt.pid" ]
> > +
> > +	pane_or_context_run guest './qrap 5 qemu-system-$(uname -m)'	   \
> > +		' -machine accel=kvm'                                      \
> > +		' -m '${VMEM}' -cpu host -smp '${VCPUS}                    \
> > +		' -kernel ' "/boot/vmlinuz-$(uname -r)"			   \
> > +		' -initrd '${INITRAMFS_MEM}' -nographic -serial stdio'	   \
> > +		' -nodefaults'						   \
> > +		' -append "console=ttyS0 mitigations=off apparmor=0 '	   \
> > +		'virtio-net.napi_tx=1"'					   \
> > +		" -device virtio-net-pci,netdev=hostnet0,x-txburst=16384"  \
> > +		" -netdev socket,fd=5,id=hostnet0"			   \
> > +		" -pidfile ${STATESETUP}/qemu.pid"
> > +}
> > +
> >  # teardown_distro() - Nothing to do, yet
> >  teardown_distro() {
> >  	:
> > @@ -36,3 +62,14 @@ teardown_pasta_options() {
> >  	teardown_context_watch ${PANE_PASST} passt
> >  }
> >  
> > +# teardown_passt() - Kill qemu with ^C, remove passt PID file
> > +teardown_memory() {
> > +	kill $(cat "${STATESETUP}/qemu.pid")
> > +
> > +	rm "${STATESETUP}/passt.pid"
> > +
> > +	tmux send-keys -t ${PANE_PASST} "C-c"  
> 
> You shouldn't need this for the passt pane, since it is context based,
> the teardown_context_watch will do it.

Oops, yes, I added this in a previous version where it was also not
context based. I'll drop this,

> > +	teardown_context_watch ${PANE_PASST} passt
> > +	teardown_context_watch ${PANE_GUEST} qemu  
> 
> Likewise you don't want this for the guest pane since it is *not*
> context based.  

and this, of course.

-- 
Stefano