From owner-dev-commits-src-all@freebsd.org Wed Jun 23 10:05:57 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id D937D64C9AF; Wed, 23 Jun 2021 10:05:57 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: from mail-ej1-f47.google.com (mail-ej1-f47.google.com [209.85.218.47]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4G8zRn1Grxz3vyv; Wed, 23 Jun 2021 10:05:56 +0000 (UTC) (envelope-from arichardson.kde@gmail.com) Received: by mail-ej1-f47.google.com with SMTP id nb6so3095049ejc.10; Wed, 23 Jun 2021 03:05:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Swlsjn6KWVWCwoUlQkIbgaG7TpbEZZrbkSBE+z7ZSJw=; b=GexjGVC3fPNQNFmLsfAtLWZJWBETWO8VylzD5CdVr8Qss2z08NGCPB5zZeUr2qMd6t bQxDwnxMtO+QEvh540Q5fLZcINmpDb51wr5WXJSj3j9wWKg2nrW1eQk743nyD13xXiZk p6TOKU4jbBOrq/+Nuxy6gjDfSo4yK8Yxw34xnbJdwDAzmlZpCrvVVE9A14OiCAkQ1vdn I9bHPnTDAtJ7f2c/YQd7rYnThIjC05PtDQIXmPDgtEKEdYuQE1Y8y8Ccx7adRKw7XPff e2BudRhwrRDxaKeKj+L0qWOVA9YXwXSM2f7avMZoVJanj41GyV+KtjRSUaaVwbJbODGk cqnA== X-Gm-Message-State: AOAM532kxA/7s/Z7eX9ib0ws6/JLMqxvsnW36lAcCCPPDD1HM+3naEjx wADrQ8twXtqedA1tt/NKRZOyyI2IY/Jnpg== X-Google-Smtp-Source: ABdhPJx1g/oy37CdT55R9DAC4FbaxiqKBjKkxxCeqtiE57+zEF5CU0xNlaUxjdN6JevV478KDvWquw== X-Received: by 2002:a17:906:a019:: with SMTP id p25mr9051794ejy.483.1624442755594; Wed, 23 Jun 2021 03:05:55 -0700 (PDT) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com. [209.85.128.52]) by smtp.gmail.com with ESMTPSA id p19sm3568630edr.73.2021.06.23.03.05.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 23 Jun 2021 03:05:55 -0700 (PDT) Received: by mail-wm1-f52.google.com with SMTP id n23so1215971wms.2; Wed, 23 Jun 2021 03:05:55 -0700 (PDT) X-Received: by 2002:a7b:c187:: with SMTP id y7mr9600185wmi.13.1624442754990; Wed, 23 Jun 2021 03:05:54 -0700 (PDT) MIME-Version: 1.0 References: <202106191953.15JJrdpI039180@gitrepo.freebsd.org> In-Reply-To: <202106191953.15JJrdpI039180@gitrepo.freebsd.org> From: Alexander Richardson Date: Wed, 23 Jun 2021 11:05:44 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: git: 9efcad61d830 - main - libalias: Restructure - Use AliasRange instead of PORT_BASE To: Lutz Donnerhacke Cc: src-committers , "" , dev-commits-src-main@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4G8zRn1Grxz3vyv X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of arichardsonkde@gmail.com designates 209.85.218.47 as permitted sender) smtp.mailfrom=arichardsonkde@gmail.com X-Spamd-Result: default: False [-2.94 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; TO_DN_SOME(0.00)[]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17:c]; RCVD_COUNT_THREE(0.00)[4]; NEURAL_HAM_SHORT(-0.94)[-0.940]; FORGED_SENDER(0.30)[arichardson@freebsd.org,arichardsonkde@gmail.com]; MIME_TRACE(0.00)[0:+]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[arichardson@freebsd.org,arichardsonkde@gmail.com]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; TAGGED_FROM(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; TO_MATCH_ENVRCPT_ALL(0.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[freebsd.org]; RBL_DBL_DONT_QUERY_IPS(0.00)[209.85.218.47:from]; SPAMHAUS_ZRD(0.00)[209.85.218.47:from:127.0.2.255]; RCVD_IN_DNSWL_NONE(0.00)[209.85.218.47:from]; RWL_MAILSPIKE_POSSIBLE(0.00)[209.85.218.47:from]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[dev-commits-src-all,dev-commits-src-main] X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jun 2021 10:05:57 -0000 On Sat, 19 Jun 2021 at 20:53, Lutz Donnerhacke wrote: > > The branch main has been updated by donner: > > URL: https://cgit.FreeBSD.org/src/commit/?id=9efcad61d8309ecad3c15392b277fd329a1e45e4 > > commit 9efcad61d8309ecad3c15392b277fd329a1e45e4 > Author: Lutz Donnerhacke > AuthorDate: 2021-05-28 17:17:40 +0000 > Commit: Lutz Donnerhacke > CommitDate: 2021-06-19 19:40:09 +0000 > > libalias: Restructure - Use AliasRange instead of PORT_BASE > > Get rid of PORT_BASE, replace by AliasRange. Simplify code. > Factor out the search for a new port. Improves the perfomance a bit. > > Discussed with: Dimitry Luhtionov > MFC after: 1 week > Differential Revision: https://reviews.freebsd.org/D30581 > --- > sys/netinet/libalias/alias_db.c | 171 +++++++++++++++++----------------------- > 1 file changed, 74 insertions(+), 97 deletions(-) > > diff --git a/sys/netinet/libalias/alias_db.c b/sys/netinet/libalias/alias_db.c > index 5f199394eb99..6bfe19a9b2a9 100644 > --- a/sys/netinet/libalias/alias_db.c > +++ b/sys/netinet/libalias/alias_db.c > @@ -574,12 +574,20 @@ FindLinkOut(struct libalias *, struct in_addr, struct in_addr, u_short, u_short, > static struct alias_link * > FindLinkIn(struct libalias *, struct in_addr, struct in_addr, u_short, u_short, int, int); > > -#define ALIAS_PORT_BASE 0x08000 > -#define ALIAS_PORT_MASK 0x07fff > -#define ALIAS_PORT_MASK_EVEN 0x07ffe > +static u_short _RandomPort(struct libalias *la); > + > #define GET_NEW_PORT_MAX_ATTEMPTS 20 > > -#define FIND_EVEN_ALIAS_BASE 1 > +/* get random port in network byte order */ > +static u_short > +_RandomPort(struct libalias *la) { > + u_short port; > + > + port = la->aliasPortLower + > + arc4random_uniform(la->aliasPortLength); > + > + return ntohs(port); > +} > > /* GetNewPort() allocates port numbers. Note that if a port number > is already in use, that does not mean that it cannot be used by > @@ -591,8 +599,7 @@ GetNewPort(struct libalias *la, struct alias_link *lnk, int alias_port_param) > { > int i; > int max_trials; > - u_short port_sys; > - u_short port_net; > + u_short port; > > LIBALIAS_LOCK_ASSERT(la); > /* > @@ -600,41 +607,18 @@ GetNewPort(struct libalias *la, struct alias_link *lnk, int alias_port_param) > * this parameter is zero or positive, it precisely specifies > * the port number. GetNewPort() will return this number > * without check that it is in use. > - > + * > + * The aliasing port is automatically selected by one of > + * two methods below: > + * > * When this parameter is GET_ALIAS_PORT, it indicates to get > * a randomly selected port number. > */ > - if (alias_port_param == GET_ALIAS_PORT) { > - /* > - * The aliasing port is automatically selected by one of > - * two methods below: > - */ > - max_trials = GET_NEW_PORT_MAX_ATTEMPTS; > - > - if (la->packetAliasMode & PKT_ALIAS_SAME_PORTS) { > - /* > - * When the PKT_ALIAS_SAME_PORTS option is chosen, > - * the first try will be the actual source port. If > - * this is already in use, the remainder of the > - * trials will be random. > - */ > - port_net = lnk->src_port; > - port_sys = ntohs(port_net); > - } else if (la->aliasPortLower) { > - /* First trial is a random port in the aliasing range. */ > - port_sys = la->aliasPortLower + > - (arc4random() % la->aliasPortLength); > - port_net = htons(port_sys); > - } else { > - /* First trial and all subsequent are random. */ > - port_sys = arc4random() & ALIAS_PORT_MASK; > - port_sys += ALIAS_PORT_BASE; > - port_net = htons(port_sys); > - } > - } else if (alias_port_param >= 0 && alias_port_param < 0x10000) { > + if (alias_port_param >= 0 && alias_port_param < 0x10000) { > lnk->alias_port = (u_short) alias_port_param; > return (0); > - } else { > + } > + if (alias_port_param != GET_ALIAS_PORT) { > #ifdef LIBALIAS_DEBUG > fprintf(stderr, "PacketAlias/GetNewPort(): "); > fprintf(stderr, "input parameter error\n"); > @@ -642,58 +626,57 @@ GetNewPort(struct libalias *la, struct alias_link *lnk, int alias_port_param) > return (-1); > } > > + max_trials = GET_NEW_PORT_MAX_ATTEMPTS; > + > + /* > + * When the PKT_ALIAS_SAME_PORTS option is chosen, > + * the first try will be the actual source port. If > + * this is already in use, the remainder of the > + * trials will be random. > + */ > + port = (la->packetAliasMode & PKT_ALIAS_SAME_PORTS) > + ? lnk->src_port > + : _RandomPort(la); > + > /* Port number search */ > - for (i = 0; i < max_trials; i++) { > - int go_ahead; > + for (i = 0; i < max_trials; i++, port = _RandomPort(la)) { > + struct group_in *grp; > struct alias_link *search_result; > > - search_result = FindLinkIn(la, lnk->dst_addr, lnk->alias_addr, > - lnk->dst_port, port_net, > - lnk->link_type, 0); > + grp = StartPointIn(la, lnk->alias_addr, port, lnk->link_type, 0); > + if (grp == NULL) > + break; > > + LIST_FOREACH(search_result, &grp->full, all.in) { > + if (lnk->dst_addr.s_addr == search_result->dst_addr.s_addr && > + lnk->dst_port == search_result->dst_port) > + break; /* found match */ > + } > if (search_result == NULL) > - go_ahead = 1; > - else if (!(lnk->flags & LINK_PARTIALLY_SPECIFIED) > - && (search_result->flags & LINK_PARTIALLY_SPECIFIED)) > - go_ahead = 1; > - else > - go_ahead = 0; > + break; > + } > > - if (go_ahead) { > -#ifndef NO_USE_SOCKETS > - if ((la->packetAliasMode & PKT_ALIAS_USE_SOCKETS) > - && (lnk->flags & LINK_PARTIALLY_SPECIFIED) > - && ((lnk->link_type == LINK_TCP) || > - (lnk->link_type == LINK_UDP))) { > - if (GetSocket(la, port_net, &lnk->sockfd, lnk->link_type)) { > - lnk->alias_port = port_net; > - return (0); > - } > - } else { > + if (i >= max_trials) { > +#ifdef LIBALIAS_DEBUG > + fprintf(stderr, "PacketAlias/GetNewPort(): "); > + fprintf(stderr, "could not find free port\n"); > #endif > - lnk->alias_port = port_net; > - return (0); > + return (-1); > + } > + > #ifndef NO_USE_SOCKETS > - } > -#endif > - } > - if (la->aliasPortLower) { > - port_sys = la->aliasPortLower + > - (arc4random() % la->aliasPortLength); > - port_net = htons(port_sys); > - } else { > - port_sys = arc4random() & ALIAS_PORT_MASK; > - port_sys += ALIAS_PORT_BASE; > - port_net = htons(port_sys); > + if ((la->packetAliasMode & PKT_ALIAS_USE_SOCKETS) && > + (lnk->flags & LINK_PARTIALLY_SPECIFIED) && > + ((lnk->link_type == LINK_TCP) || > + (lnk->link_type == LINK_UDP))) { > + if (!GetSocket(la, port, &lnk->sockfd, lnk->link_type)) { > + return (-1); > } > } > - > -#ifdef LIBALIAS_DEBUG > - fprintf(stderr, "PacketAlias/GetNewPort(): "); > - fprintf(stderr, "could not find free port\n"); > #endif > + lnk->alias_port = port; > > - return (-1); > + return (0); > } > > #ifndef NO_USE_SOCKETS > @@ -760,7 +743,7 @@ FindNewPortGroup(struct libalias *la, > { > int i, j; > int max_trials; > - u_short port_sys; > + u_short port; > int link_type; > > LIBALIAS_LOCK_ASSERT(la); > @@ -792,39 +775,31 @@ FindNewPortGroup(struct libalias *la, > * try will be the actual source port. If this is already > * in use, the remainder of the trials will be random. > */ > - port_sys = ntohs(src_port); > + port = src_port; > > } else { > - /* First trial and all subsequent are random. */ > - if (align == FIND_EVEN_ALIAS_BASE) > - port_sys = arc4random() & ALIAS_PORT_MASK_EVEN; > - else > - port_sys = arc4random() & ALIAS_PORT_MASK; > - > - port_sys += ALIAS_PORT_BASE; > + port = _RandomPort(la); > } > > /* Port number search */ > - for (i = 0; i < max_trials; i++) { > + for (i = 0; i < max_trials; i++, port = _RandomPort(la)) { > struct alias_link *search_result; > > - for (j = 0; j < port_count; j++) > + if (align) > + port &= htons(0xfffe); > + > + for (j = 0; j < port_count; j++) { > + u_short port_j = ntohs(port) + j; > + > if ((search_result = FindLinkIn(la, dst_addr, > - alias_addr, dst_port, htons(port_sys + j), > + alias_addr, dst_port, htons(port_j), > link_type, 0)) != NULL) > break; > + } > > /* Found a good range, return base */ > if (j == port_count) > - return (htons(port_sys)); > - > - /* Find a new base to try */ > - if (align == FIND_EVEN_ALIAS_BASE) > - port_sys = arc4random() & ALIAS_PORT_MASK_EVEN; > - else > - port_sys = arc4random() & ALIAS_PORT_MASK; > - > - port_sys += ALIAS_PORT_BASE; > + return (port); > } > > #ifdef LIBALIAS_DEBUG > @@ -2555,6 +2530,8 @@ LibAliasInit(struct libalias *la) > > la->aliasAddress.s_addr = INADDR_ANY; > la->targetAddress.s_addr = INADDR_ANY; > + la->aliasPortLower = 0x8000; > + la->aliasPortLength = 0x8000; > > la->icmpLinkCount = 0; > la->udpLinkCount = 0; Hi, This commit or one of the previous ones appears to have broken three tests: sys.netpfil.common.nat.ipfw_basic sys.netpfil.common.nat.ipfw_cgn sys.netpfil.common.nat.ipfw_portalias See https://ci.freebsd.org/job/FreeBSD-main-amd64-test/18437/ Could you look into this? Thanks, Alex