From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 651CF5A026D for ; Tue, 16 Jan 2024 07:27:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1705386425; bh=3eBOVQ+VOKIJoDYRn2ev5m81frWi9tXcPuRj5BrI6kc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=C5o2Q0wOBl2kGzkzTQBxWe34oSPEddvd4kn687jiXHE3aa/D6wlyOATcy9rVkUp// evMnzc+etbwSL63prAzfIKu8PwZjibhoNnWAqYrUyRveGDyVczUKM51AAEaq9zD6cL 1gKwIsG6lRlOk8gpbbLOZjQNHYDph5p8Du0g9oabLBB85goCbYL6bMUiqPeCYxgrSL wh+3cwXPKZ+24gytf+GXKN+j3k/lZ+ZzTZwVZx/nudE6MKXdU+SxhhfZ0orbFHIhaE F0b3KObFrFbVz0fcBzUVtfsQ5Yn64iqG/BQeuRfcZY/MGC5UmBQ9GeIDgkeLBDcFGM EQ7oHEXq563Yg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TDfDn6kc8z4xFj; Tue, 16 Jan 2024 17:27:05 +1100 (AEDT) Date: Tue, 16 Jan 2024 17:14:01 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 01/15] flow: Common data structures for tracking flow addresses Message-ID: References: <20231221070237.1422557-1-david@gibson.dropbear.id.au> <20231221070237.1422557-2-david@gibson.dropbear.id.au> <20240113235040.60be469f@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="DGNr76daePw9pIvh" Content-Disposition: inline In-Reply-To: <20240113235040.60be469f@elisabeth> Message-ID-Hash: VZGPQ7QBM4VGIS5VOUAZFJLN6WKSGIAN X-Message-ID-Hash: VZGPQ7QBM4VGIS5VOUAZFJLN6WKSGIAN X-MailFrom: dgibson@gandalf.ozlabs.org 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: --DGNr76daePw9pIvh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jan 13, 2024 at 11:50:40PM +0100, Stefano Brivio wrote: > On Thu, 21 Dec 2023 18:02:23 +1100 > David Gibson wrote: >=20 > > Handling of each protocol needs some degree of tracking of the addresses > > and ports at the end of each connection or flow. Sometimes that's expl= icit > > (as in the guest visible addresses for TCP connections), sometimes impl= icit > > (the bound and connected addresses of sockets). > >=20 > > To allow more general and robust handling, and more consistency across > > protocols we want to uniformly track the address and port at each end of > > the connection. Furthermore, because we allow port remapping, and we > > sometimes need to apply NAT, the addresses and ports can be different as > > seen by the guest/namespace and as by the host. > >=20 > > Introduce 'struct flowside' to keep track of common information related= to > > one side of each flow. For now that's the addresses, ports and the pif= id. > > Store two of these in the common fields of a flow to track that informa= tion > > for both sides. > >=20 > > For now we just introduce the structure and fields themselves, along wi= th > > a simple helper. Later patches will actually use these to store useful > > information. > >=20 > > Signed-off-by: David Gibson > > --- > > flow.h | 33 +++++++++++++++++++++++++++++++++ > > passt.h | 2 ++ > > tcp_conn.h | 1 - > > 3 files changed, 35 insertions(+), 1 deletion(-) > >=20 > > diff --git a/flow.h b/flow.h > > index 48a0ab4..e090ba0 100644 > > --- a/flow.h > > +++ b/flow.h > > @@ -27,11 +27,44 @@ extern const char *flow_type_str[]; > > #define FLOW_TYPE(f) \ > > ((f)->type < FLOW_NUM_TYPES ? flow_type_str[(f)->type] : "?") > > =20 > > +/** > > + * struct flowside - Common information for one side of a flow > > + * @eaddr: Endpoint address (remote address from passt's PoV) > > + * @faddr: Forwarding address (local address from passt's PoV) > > + * @eport: Endpoint port > > + * @fport: Forwarding port > > + * @pif: pif ID on which this side of the flow exists > > + */ > > +struct flowside { > > + union inany_addr faddr; > > + union inany_addr eaddr; > > + in_port_t fport; > > + in_port_t eport; > > + uint8_t pif; > > +}; > > +static_assert(_Alignof(struct flowside) =3D=3D _Alignof(uint32_t), > > + "Unexpected alignment for struct flowside"); > > + >=20 > Nits: >=20 > > +/** flowside_complete - Check if flowside is fully initialized >=20 > flowside_complete(). By the way, shouldn't we call it something more > descriptive such as flowside_is_complete()? It's not obvious this is > a check otherwise. >=20 > > + * @fside: flowside to check >=20 > * Return: true if pif, addresses and ports are set >=20 > ...or something of that sort. >=20 > > + */ > > +static inline bool flowside_complete(const struct flowside *fside) > > +{ > > + return fside->pif !=3D PIF_NONE && > > + !IN6_IS_ADDR_UNSPECIFIED(&fside->faddr) && > > + !IN6_IS_ADDR_UNSPECIFIED(&fside->eaddr) && > > + fside->fport !=3D 0 && fside->eport !=3D 0; >=20 > I would align everything after 'return ', that is: >=20 > return fside->pif !=3D PIF_NONE && > !IN6_IS_ADDR_UNSPECIFIED(&fside->faddr) && > !IN6_IS_ADDR_UNSPECIFIED(&fside->eaddr) && > fside->fport !=3D 0 && fside->eport !=3D 0; >=20 > mostly for consistency -- not a strong preference. All seems reasonable, updated accordingly. I've also updated 'flow_complete' to 'flow_is_complete' later in the series. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --DGNr76daePw9pIvh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWmHqgACgkQzQJF27ox 2GdDIQ//QEdfObtHJkcS4+Xf/om6NlMNfZdrR8efpkCLFhLsiw6v+0bV+NnP7Y19 pGPIkle392pJVWoMJuiS2Jl56xBxbNKyJf12rsR4ohyoDzcFRoZHVx5YfFfEVclJ lBWTjHnRtV7RvKy5uF6dPbyKekeiQLGURUGUJjqSUbvw6yTREzS16VtlCtsX6P1b hOz4/h1l7F2EQRUWteiY5YY5cFpyFuxsAJ2QnVsx3U3Zeam0k767Ca6Jr1ELNfmG OuPRjFhlSwfHXXkgNyvxT8cjq/OBH22GzyEnsBp707JCcHfOIEvvFOUf+4N/8nw8 R5KRi14mjlo40MWU02n1xX+MIFSCrFyEsgl220Y5qgCveryfSwEZZv7oAsbPLvtg /+tDyYsIqc+sGrKiwFucb5Q36KXRlzvbuLJVqJW4BeLQGMzSivOhhS59nT0lTFOP e/3LJP9cPY4XRjoURgQhWuedFDdToF9F7WKlCah+PWRNFqgq0bGj6GNZ6ceYI3Am zkpDZx3oUlOnvNg7zZeXZ/rUcj8/9QwtoB3vaflHjDZ+cHJmN3YT3UuNQB6LOsgK 9MciYLG0+ZmmQomvQT9H7wmOvuSOrZkKEAZA90I0PskbrXiEhccU5tGsNqc4qluv 1Ksgrf2M+Grqsfxe1C2gB7Kpz780AYPorcAB80DtFWje8JY8mfc= =S5Bo -----END PGP SIGNATURE----- --DGNr76daePw9pIvh--