From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=HZ8wVY87; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 294665A0262 for ; Fri, 20 Mar 2026 21:58:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1774040308; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=TANIOPMJMk4uZ1OEdShId4jZBXBGMzBNN3p7XnaEZ8Y=; b=HZ8wVY87oTUaNc9ehuy54tuEQI/+CrCuujj4blGmNCWLa7HYyxBePwLYaELCanga76t6Xe nyi6DuRB67CD60CfMJP5qvPtBlz2bD5mgmYRGKnIP5MfcLNZsM+4klXqKMwKeszm+CN3as Xcp8Q+MsFkYCewFZjIdj6szGi7Nd+Ws= Received: from mail-wr1-f69.google.com (mail-wr1-f69.google.com [209.85.221.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-364-hwCB7Yj6MhywnKJxZ02vEw-1; Fri, 20 Mar 2026 16:58:27 -0400 X-MC-Unique: hwCB7Yj6MhywnKJxZ02vEw-1 X-Mimecast-MFC-AGG-ID: hwCB7Yj6MhywnKJxZ02vEw_1774040306 Received: by mail-wr1-f69.google.com with SMTP id ffacd0b85a97d-43b3e6eb998so914698f8f.3 for ; Fri, 20 Mar 2026 13:58:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774040305; x=1774645105; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=TANIOPMJMk4uZ1OEdShId4jZBXBGMzBNN3p7XnaEZ8Y=; b=Y0F21Me27pS45T4pqCU5MWvOVKUB2jaY2jxf84dhMalI6BidnfPWRwdFR6jjhCXHt+ z/do8jz9qB5JR3WWGAM638D7Xl6XECvhXM2m7WJ5meD2NAqwkYHCYqCLRjpt9Zuyjx+b irp+pdX2wB9ongYukN1T2Qr5IqDrFrRRLKCBrgwsJvHu12bw6CrDbim5+NVJwbGhGWzq pbAN/3cp+oI4eVYAWAILPpZ8OW0DArXAkTmAeKwq0X84Alav0c6rPb085SgYVwOJ+0sA 0E7dfS/euSk04L8P32Ees304kybR0mv481vHBNnxAz/PY9HLfUS9q+f7dHWUMZbzyws/ S4gA== X-Gm-Message-State: AOJu0Yy5uH8UDKIornvcyTANhHZyfjN6a2WZme7vSDwkjFuSh9H2gPz1 ZA+cm1fr/iQzKCBuycivUcyeCBNx7VHsdWjQinuUDIWz+bEO4AHMGI9qg/96mEApxDmXJ2alHnY 93kI0MqjHspiLe/ZpOxbdeHB5xoWmAsyB5or2rj6ljA6AH1okbpaTR5MxTsI72g== X-Gm-Gg: ATEYQzy+naa35drLdSZ4WLwl1xsJ6cLcT5qeTJUARxV8qOonOccR5zfRaB2CKHVLgsE cPnx8fCnPAmgJn+9WcvqWwf5oj23bEzR+uFkBREQnQ63n5raiIxPK8Ae+1CBHJpbkas/sf5Hvux y9gZtq6a/mfF6DgKR9n44B7Z6CMg7DB6snxnXhHqnB0Rl/J6rg0UfxQwmMtXox8M/S7lrS6W2UJ xBwuhzH0Lsv6NFZUP2TbCuHxuZvaZdzZvCgs3jb19x/GvPXJUXAFLqaCNa90PBiucNMUck+xhUD 71kn8dBTH6Py1zWgdM27LNkeT+lvnT3gQ5x1wirWndyDpRRepr0I/6G+rLlC1H7A1oVqdqOQ1nS qbtetKlDkJnnnbM0b2r1jdV3CrliLpam8/PgQG5oVjz0bI40o7A== X-Received: by 2002:a05:6000:608:b0:43b:42af:75e with SMTP id ffacd0b85a97d-43b64271ce0mr7209505f8f.44.1774040305138; Fri, 20 Mar 2026 13:58:25 -0700 (PDT) X-Received: by 2002:a05:6000:608:b0:43b:42af:75e with SMTP id ffacd0b85a97d-43b64271ce0mr7209478f8f.44.1774040304578; Fri, 20 Mar 2026 13:58:24 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43b644bd923sm10602187f8f.12.2026.03.20.13.58.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 20 Mar 2026 13:58:24 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 02/15] serialise: Split functions user for serialisation from util.c Message-ID: <20260320215823.7bc63e6c@elisabeth> In-Reply-To: <20260319061157.1983818-3-david@gibson.dropbear.id.au> References: <20260319061157.1983818-1-david@gibson.dropbear.id.au> <20260319061157.1983818-3-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Fri, 20 Mar 2026 21:58:23 +0100 (CET) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: SIt7bHSuj88nsh-sgExDt3Yo_P2vlRc_bpGA4xP7GUY_1774040306 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: TR4WPRX2I6TMK5L6YM64F52JU3TGTZII X-Message-ID-Hash: TR4WPRX2I6TMK5L6YM64F52JU3TGTZII X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, 19 Mar 2026 17:11:44 +1100 David Gibson wrote: > The read_all_buf() and write_all_buf() functions in util.c are > primarily used for serialising data structures to a stream during > migraiton. We're going to have further use for such serialisation > when we add dynamic configuration updates, where we'll want to share > the code with the client program. > > To make that easier move the functions into a new serialisation.c This is (thankfully! :)) serialise.c. > file, and rename thematically. While we're there add some macros for > the common idiom of sending all of a given variable using sizeof(). > > Signed-off-by: David Gibson > --- > Makefile | 11 ++++--- > flow.c | 5 +-- > migrate.c | 9 +++--- > pcap.c | 3 +- > serialise.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > serialise.h | 17 +++++++++++ > tcp.c | 19 ++++++------ > util.c | 78 +++-------------------------------------------- > util.h | 2 -- > 9 files changed, 136 insertions(+), 96 deletions(-) > create mode 100644 serialise.c > create mode 100644 serialise.h > > diff --git a/Makefile b/Makefile > index 513dc6c6..5b6891d7 100644 > --- a/Makefile > +++ b/Makefile > @@ -40,8 +40,8 @@ FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) > PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c epoll_ctl.c \ > flow.c fwd.c icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c \ > log.c mld.c ndp.c netlink.c migrate.c packet.c passt.c pasta.c pcap.c \ > - pif.c repair.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c udp.c \ > - udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c > + pif.c repair.c serialise.c tap.c tcp.c tcp_buf.c tcp_splice.c tcp_vu.c \ > + udp.c udp_flow.c udp_vu.c util.c vhost_user.c virtio.c vu_common.c > QRAP_SRCS = qrap.c > PASST_REPAIR_SRCS = passt-repair.c > SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) > @@ -51,9 +51,10 @@ MANPAGES = passt.1 pasta.1 qrap.1 passt-repair.1 > PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h epoll_ctl.h \ > flow.h fwd.h flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h \ > isolation.h lineread.h log.h migrate.h ndp.h netlink.h packet.h \ > - passt.h pasta.h pcap.h pif.h repair.h siphash.h tap.h tcp.h tcp_buf.h \ > - tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h udp_flow.h \ > - udp_internal.h udp_vu.h util.h vhost_user.h virtio.h vu_common.h > + passt.h pasta.h pcap.h pif.h repair.h serialise.h siphash.h tap.h tcp.h \ > + tcp_buf.h tcp_conn.h tcp_internal.h tcp_splice.h tcp_vu.h udp.h \ > + udp_flow.h udp_internal.h udp_vu.h util.h vhost_user.h virtio.h \ > + vu_common.h > HEADERS = $(PASST_HEADERS) seccomp.h > > C := \#include \nint main(){int a=getrandom(0, 0, 0);} > diff --git a/flow.c b/flow.c > index 4282da2e..7d3c5ae6 100644 > --- a/flow.c > +++ b/flow.c > @@ -21,6 +21,7 @@ > #include "flow_table.h" > #include "repair.h" > #include "epoll_ctl.h" > +#include "serialise.h" > > const char *flow_state_str[] = { > [FLOW_STATE_FREE] = "FREE", > @@ -1135,7 +1136,7 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage *stage, > } > > count = htonl(count); > - if (write_all_buf(fd, &count, sizeof(count))) { > + if (sewrite_var(fd, &count)) { I find these names much less descriptive ("write the whole buffer" vs. "serialised write of... variable?") and kind of confusing ("that must be where SELinux labels are written"). I'm not sure if dropping the third argument is actually a nice thing to do to the reader. Sure, it's shorter and it looks like an easy win, but it actually makes it a bit obscure I think, because the size is not specified, and it doesn't look like a macro, so one might naturally think that "var" refers to a standard "variable" data type (int?) that we use when writing to particular file descriptors. And instead no, it's a macro, so it will write exactly sizeof(count) bytes. But at this point shouldn't we just say that explicitly? I don't think the brevity saves us any line either. About the seread/sewrite part: I don't have great ideas right now but a two possible alternatives could be: - just keep write_all_buf() / read_all_buf() and who cares if the filename doesn't match, we already have that pattern in many places, it doesn't strictly need to match (if it did, it would be redundant) - rename the file to buf.c and call them buf_write_all() / buf_read_all() which look like the same thing ...then in the next patch some of those could become write_u32() or write_u32_buf() or buf_write_u32() etc. Other than that this patch looks good to me. -- Stefano