From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 8E3155A004F for ; Wed, 10 Jul 2024 23:32:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1720647123; 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=rhdIFNCMzMIEdlJnHTF78YZrv8Xtkusary69TKDjWEg=; b=aXf+CnZHryq6b3FXLzQvjHIXgK7rIjrCuEjJTyRzsqwNttnSWjDKR0VgFlUMbwsOnBqRDW Fc7L+rb5D9v4N50j2eB6TeuMubsFkqM/RXoMudz3Cha+VynXU4d2rYoLXG0x65+zRe58M4 ArD/svxjIItzavI+sSlHiG2m8/JZLe4= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-520-xXgkvBqwMu-zbbkTBrvkAA-1; Wed, 10 Jul 2024 17:32:01 -0400 X-MC-Unique: xXgkvBqwMu-zbbkTBrvkAA-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-79f1bf8ad5eso26735685a.3 for ; Wed, 10 Jul 2024 14:32:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1720647120; x=1721251920; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=rhdIFNCMzMIEdlJnHTF78YZrv8Xtkusary69TKDjWEg=; b=fmEfGIE/H/WGTXGgaN5iH9y4quHdkg1gD8eesET/Gv+g6E4P4NVsASy4fzzMjp+hOJ 180x5QA1CYSRSROdG5dizpuZQCfHuGnANtLcFnAOfOYMmbdu1SYz4ZfjVm5w0omVNDIt B3dEPt1UilsDGUJ49UelheI/JDlDY1QzHefMhuV8x8didH+XTKnX8HLrmCFkQozFfXEN OBUDbvoeCovwtB80JkACnbGyjivWvOSCeLV2dhj6wkkLpM54QxzsM4ZUWHgvkdvtkiQk ywMA2PdqL/BbZp1REVqBdqGtyE9WRuU9qct/lXsPS2LFZm9J/YrDyCszVOECnbjVwv/z +5mA== X-Gm-Message-State: AOJu0YxH9GzM3GfYEURPCAIHuXL7eidemmyCLEuOiIGXCxtVyCyYOKac 07W7U788dUbAULJ5/oETBsqH9nMMnpvwTTGe+BTKj7fpholS/TA8odOnUXFK4VhHkLJ0ytGo0I/ ZDLZANbW7Ga3cLEL4o/PEUnclSJzxBfgo8N3ydJJkURhNHu17WvP0WyJR+9jU X-Received: by 2002:a05:620a:24cb:b0:79f:148:db04 with SMTP id af79cd13be357-79f19a79520mr953512385a.45.1720647120376; Wed, 10 Jul 2024 14:32:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEi+xTXn0abJSPzyUe4lLR+qIbxbib1IVHf5wEEGP/gwvEcyticJy6zwC5HumOE8XkTjTy/Eg== X-Received: by 2002:a05:620a:24cb:b0:79f:148:db04 with SMTP id af79cd13be357-79f19a79520mr953510185a.45.1720647119992; Wed, 10 Jul 2024 14:31:59 -0700 (PDT) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-447f9b26badsm23713821cf.17.2024.07.10.14.31.58 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 10 Jul 2024 14:31:59 -0700 (PDT) Date: Wed, 10 Jul 2024 23:30:38 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v7 02/27] flow: Common address information for target side Message-ID: <20240710233038.6275c284@elisabeth> In-Reply-To: <20240705020724.3447719-3-david@gibson.dropbear.id.au> References: <20240705020724.3447719-1-david@gibson.dropbear.id.au> <20240705020724.3447719-3-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: FPKLU4TVG64N4BIDN7OIGCQGDXDDHK77 X-Message-ID-Hash: FPKLU4TVG64N4BIDN7OIGCQGDXDDHK77 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, jmaloy@redhat.com 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: Two minor details: On Fri, 5 Jul 2024 12:06:59 +1000 David Gibson wrote: > Require the address and port information for the target (non > initiating) side to be populated when a flow enters TGT state. > Implement that for TCP and ICMP. For now this leaves some information > redundantly recorded in both generic and type specific fields. We'll > fix that in later patches. > > For TCP we now use the information from the flow to construct the > destination socket address in both tcp_conn_from_tap() and > tcp_splice_connect(). > > Signed-off-by: David Gibson > --- > flow.c | 38 ++++++++++++++++++------ > flow_table.h | 5 +++- > icmp.c | 3 +- > inany.h | 1 - > pif.c | 45 ++++++++++++++++++++++++++++ > pif.h | 17 +++++++++++ > tcp.c | 82 ++++++++++++++++++++++++++++------------------------ > tcp_splice.c | 45 +++++++++++----------------- > 8 files changed, 158 insertions(+), 78 deletions(-) > > diff --git a/flow.c b/flow.c > index 44e7b3b8..f064fad1 100644 > --- a/flow.c > +++ b/flow.c > @@ -165,8 +165,10 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...) > */ > static void flow_set_state(struct flow_common *f, enum flow_state state) > { > - char estr[INANY_ADDRSTRLEN], fstr[INANY_ADDRSTRLEN]; > + char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; > + char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; > const struct flowside *ini = &f->side[INISIDE]; > + const struct flowside *tgt = &f->side[TGTSIDE]; > uint8_t oldstate = f->state; > > ASSERT(state < FLOW_NUM_STATES); > @@ -177,19 +179,24 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) > FLOW_STATE(f)); > > if (MAX(state, oldstate) >= FLOW_STATE_TGT) > - flow_log_(f, LOG_DEBUG, "%s [%s]:%hu -> [%s]:%hu => %s", > + flow_log_(f, LOG_DEBUG, > + "%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", > pif_name(f->pif[INISIDE]), > - inany_ntop(&ini->eaddr, estr, sizeof(estr)), > + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), > ini->eport, > - inany_ntop(&ini->faddr, fstr, sizeof(fstr)), > + inany_ntop(&ini->faddr, fstr0, sizeof(fstr0)), > ini->fport, > - pif_name(f->pif[TGTSIDE])); > + pif_name(f->pif[TGTSIDE]), > + inany_ntop(&tgt->faddr, fstr1, sizeof(fstr1)), > + tgt->fport, > + inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), > + tgt->eport); > else if (MAX(state, oldstate) >= FLOW_STATE_INI) > flow_log_(f, LOG_DEBUG, "%s [%s]:%hu -> [%s]:%hu => ?", > pif_name(f->pif[INISIDE]), > - inany_ntop(&ini->eaddr, estr, sizeof(estr)), > + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), > ini->eport, > - inany_ntop(&ini->faddr, fstr, sizeof(fstr)), > + inany_ntop(&ini->faddr, fstr0, sizeof(fstr0)), > ini->fport); > } > > @@ -261,21 +268,34 @@ const struct flowside *flow_initiate_sa(union flow *flow, uint8_t pif, > } > > /** > - * flow_target() - Move flow to TGT, setting TGTSIDE details > + * flow_target_af() - Move flow to TGT, setting TGTSIDE details > * @flow: Flow to change state > * @pif: pif of the target side > + * @af: Address family for @eaddr and @faddr > + * @saddr: Source address (pointer to in_addr or in6_addr) > + * @sport: Endpoint port > + * @daddr: Destination address (pointer to in_addr or in6_addr) > + * @dport: Destination port > + * > + * Return: pointer to the target flowside information > */ > -void flow_target(union flow *flow, uint8_t pif) > +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) > { > struct flow_common *f = &flow->f; > + struct flowside *tgt = &f->side[TGTSIDE]; > > ASSERT(pif != PIF_NONE); > ASSERT(flow_new_entry == flow && f->state == FLOW_STATE_INI); > ASSERT(f->type == FLOW_TYPE_NONE); > ASSERT(f->pif[INISIDE] != PIF_NONE && f->pif[TGTSIDE] == PIF_NONE); > > + flowside_from_af(tgt, af, daddr, dport, saddr, sport); > f->pif[TGTSIDE] = pif; > flow_set_state(f, FLOW_STATE_TGT); > + return tgt; > } > > /** > diff --git a/flow_table.h b/flow_table.h > index ad1bc787..00dca4b2 100644 > --- a/flow_table.h > +++ b/flow_table.h > @@ -114,7 +114,10 @@ const struct flowside *flow_initiate_af(union flow *flow, uint8_t pif, > const struct flowside *flow_initiate_sa(union flow *flow, uint8_t pif, > const union sockaddr_inany *ssa, > in_port_t dport); > -void flow_target(union flow *flow, uint8_t pif); > +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); > > 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/icmp.c b/icmp.c > index cf88ac1f..fd92c7da 100644 > --- a/icmp.c > +++ b/icmp.c > @@ -167,7 +167,8 @@ static struct icmp_ping_flow *icmp_ping_new(const struct ctx *c, > return NULL; > > flow_initiate_af(flow, PIF_TAP, af, saddr, id, daddr, id); > - flow_target(flow, PIF_HOST); > + /* FIXME: Record outbound source address when known */ > + flow_target_af(flow, PIF_HOST, af, NULL, 0, daddr, 0); > pingf = FLOW_SET_TYPE(flow, flowtype, ping); > > pingf->seq = -1; > diff --git a/inany.h b/inany.h > index 47b66fa9..8eaf5335 100644 > --- a/inany.h > +++ b/inany.h > @@ -187,7 +187,6 @@ static inline bool inany_is_unspecified(const union inany_addr *a) > * > * Return: true if @a is in fe80::/10 (IPv6 link local unicast) > */ > -/* cppcheck-suppress unusedFunction */ > static inline bool inany_is_linklocal6(const union inany_addr *a) > { > return IN6_IS_ADDR_LINKLOCAL(&a->a6); > diff --git a/pif.c b/pif.c > index ebf01cc8..9f2d39cc 100644 > --- a/pif.c > +++ b/pif.c > @@ -7,9 +7,14 @@ > > #include > #include > +#include > > #include "util.h" > #include "pif.h" > +#include "siphash.h" > +#include "ip.h" > +#include "inany.h" > +#include "passt.h" > > const char *pif_type_str[] = { > [PIF_NONE] = "", > @@ -19,3 +24,43 @@ const char *pif_type_str[] = { > }; > static_assert(ARRAY_SIZE(pif_type_str) == PIF_NUM_TYPES, > "pif_type_str[] doesn't match enum pif_type"); > + > + > +/** pif_sockaddr() - Construct a socket address suitable for an interface > + * @c: Execution context > + * @sa: Pointer to sockaddr to fill in > + * @sl: Updated to relevant of length of initialised @sa to relevant length > + * @pif: Interface to create the socket address > + * @addr: IPv[46] address > + * @port: Port (host byte order) > + * > + * Return: true if resulting socket address is non-trivial (specified address or > + * non-zero port), false otherwise This is not really intuitive in the only caller using this, tcp_bind_outbound(). I wonder if it would make more sense to perform this check directly there, and have this returning void instead. > + */ > +bool pif_sockaddr(const struct ctx *c, union sockaddr_inany *sa, socklen_t *sl, > + uint8_t pif, const union inany_addr *addr, in_port_t port) > +{ > + const struct in_addr *v4 = inany_v4(addr); > + > + ASSERT(pif_is_socket(pif)); > + > + if (v4) { > + sa->sa_family = AF_INET; > + sa->sa4.sin_addr = *v4; > + sa->sa4.sin_port = htons(port); > + memset(&sa->sa4.sin_zero, 0, sizeof(sa->sa4.sin_zero)); > + *sl = sizeof(sa->sa4); > + return !IN4_IS_ADDR_UNSPECIFIED(v4) || port; > + } > + > + sa->sa_family = AF_INET6; > + sa->sa6.sin6_addr = addr->a6; > + sa->sa6.sin6_port = htons(port); > + if (pif == PIF_HOST && IN6_IS_ADDR_LINKLOCAL(&addr->a6)) > + sa->sa6.sin6_scope_id = c->ifi6; > + else > + sa->sa6.sin6_scope_id = 0; > + sa->sa6.sin6_flowinfo = 0; > + *sl = sizeof(sa->sa6); > + return !IN6_IS_ADDR_UNSPECIFIED(&addr->a6) || port; > +} -- Stefano