Re: [PATCH 03/10] tcp: Partially unify IPv4 and IPv6 paths in tcp_hash_match()

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

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

On Mon, Nov 07, 2022 at 07:08:27PM +0100, Stefano Brivio wrote:
> On Fri,  4 Nov 2022 19:43:26 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > When given an IPv4 address tcp_hash_match() checks if the connection
> > stores an IPv4-mapped IPv6 address, and if so compares the mapped part of
> > that address to the given address.  This is equivalent to converting a
> > given IPv4 address to an IPv4-mapped IPv6 address then comparing it to the
> > connection address using the existing IPv6 logic.
> > 
> > Convert to this slightly simpler form, which will also allow some further
> > simplifications in future.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  tcp.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tcp.c b/tcp.c
> > index 26dd268..508d6e9 100644
> > --- a/tcp.c
> > +++ b/tcp.c
> > @@ -1285,13 +1285,14 @@ static int tcp_opt_get(const char *opts, size_t len, uint8_t type_find,
> >  static int tcp_hash_match(const struct tcp_conn *conn, int af, const void *addr,
> >  			  in_port_t tap_port, in_port_t sock_port)
> >  {
> > -	if (af == AF_INET && CONN_V4(conn)			&&
> > -	    !memcmp(&conn->a.a4.a, addr, sizeof(conn->a.a4.a))	&&
> > -	    conn->tap_port == tap_port && conn->sock_port == sock_port)
> > -		return 1;
> > +	struct in6_addr v4mapped;
> > +
> > +	if (af == AF_INET) {
> > +		encode_ip4mapped_ip6(&v4mapped, addr);
> > +		addr = &v4mapped;
> > +	}
> 
> It's probably worth the simplification, just mind that this replaces a
> 32-bit comparison with 128 bits of writes.

I'm aware, however this is mitigated later in the series.  Once I make
tcp_hash_match() take a (possibly v4mapped) v6 address only it's just
a 128-bit comparison, which we already effectively do split between
CONN_V4() and the v4 specific path.  Obviously that means constructing
the v4mapped address in the caller, but we were already doing that in
at least one of the cases.

> 
> >  
> > -	if (af == AF_INET6					&&
> > -	    IN6_ARE_ADDR_EQUAL(&conn->a.a6, addr)		&&
> > +	if (IN6_ARE_ADDR_EQUAL(&conn->a.a6, addr)		&&
> >  	    conn->tap_port == tap_port && conn->sock_port == sock_port)
> >  		return 1;
> >  
> 

-- 
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

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