Re: [PATCH v3 18/20] tcp: Get our socket port using getsockname() when connecting from guest

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4039 bytes --]

On Fri, Jan 31, 2025 at 08:39:51PM +0100, Stefano Brivio wrote:
> For migration only: we need to store 'oport', our socket-side port,
> as we establish a connection from the guest, so that we can bind the
> same oport as source port in the migration target.
> 
> Use getsockname() to fetch that.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  flow.c       |  4 ++--
>  flow_table.h |  4 ++--
>  tcp.c        | 24 +++++++++++++++++++++++-
>  3 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/flow.c b/flow.c
> index 5638ff1..506cbac 100644
> --- a/flow.c
> +++ b/flow.c
> @@ -411,8 +411,8 @@ const struct flowside *flow_initiate_sa(union flow *flow, uint8_t pif,
>   *
>   * Return: pointer to the target flowside information
>   */
> -const struct flowside *flow_target(const struct ctx *c, union flow *flow,
> -				   uint8_t proto)
> +struct flowside *flow_target(const struct ctx *c, union flow *flow,
> +			     uint8_t proto)
>  {
>  	char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN];
>  	struct flow_common *f = &flow->f;
> diff --git a/flow_table.h b/flow_table.h
> index 633805d..b107107 100644
> --- a/flow_table.h
> +++ b/flow_table.h
> @@ -178,8 +178,8 @@ const struct flowside *flow_target_af(union flow *flow, uint8_t pif,
>  				      sa_family_t af,
>  				      const void *saddr, in_port_t sport,
>  				      const void *daddr, in_port_t dport);
> -const struct flowside *flow_target(const struct ctx *c, union flow *flow,
> -				   uint8_t proto);
> +struct flowside *flow_target(const struct ctx *c, union flow *flow,
> +			     uint8_t proto);
>  
>  union flow *flow_set_type(union flow *flow, enum flow_type type);
>  #define FLOW_SET_TYPE(flow_, t_, var_)	(&flow_set_type((flow_), (t_))->var_)
> diff --git a/tcp.c b/tcp.c
> index 0bd2a02..4fd405b 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1471,6 +1471,8 @@ static void tcp_bind_outbound(const struct ctx *c,
>   * @opts:	Pointer to start of options
>   * @optlen:	Bytes in options: caller MUST ensure available length
>   * @now:	Current timestamp
> + *
> + * #syscalls:vu getsockname
>   */
>  static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
>  			      const void *saddr, const void *daddr,
> @@ -1479,9 +1481,10 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
>  {
>  	in_port_t srcport = ntohs(th->source);
>  	in_port_t dstport = ntohs(th->dest);
> -	const struct flowside *ini, *tgt;
> +	const struct flowside *ini;
>  	struct tcp_tap_conn *conn;
>  	union sockaddr_inany sa;
> +	struct flowside *tgt;
>  	union flow *flow;
>  	int s = -1, mss;
>  	uint64_t hash;
> @@ -1586,6 +1589,25 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af,
>  	}
>  
>  	tcp_epoll_ctl(c, conn);
> +
> +	if (c->mode == MODE_VU) { /* To rebind to same oport after migration */

I suspect we'll want this local side information in more places in
future, but this is fine for now.

> +		if (af == AF_INET) {
> +			struct sockaddr_in s_in;
> +			socklen_t sl;
> +
> +			sl = sizeof(s_in);
> +			getsockname(s, (struct sockaddr *)&s_in, &sl);
> +			tgt->oport = ntohs(s_in.sin_port);

Since we're already doing the getsockname() we should also update
tgt->oaddr, and that might matter in cases where the host has multiple
local addresses.

> +		} else {
> +			struct sockaddr_in6 s_in6;
> +			socklen_t sl;
> +
> +			sl = sizeof(s_in6);
> +			getsockname(s, (struct sockaddr *)&s_in6, &sl);
> +			tgt->oport = ntohs(s_in6.sin6_port);
> +		}

We should add an inany_getsockname() or something helper for this.  In
fact I'm pretty sure I wrote one at some point, but it was lost in the
shuffles of various flow table iterations.

> +	}
> +
>  	FLOW_ACTIVATE(conn);
>  	return;
>  

-- 
David Gibson (he or they)	| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]