From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ZZUTBQ3f; dkim-atps=neutral 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 1C85B5A061F for ; Wed, 13 Nov 2024 02:14:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1731460458; 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=b5sF1HcdenT6u15ytJ89bj5K7zN1Ok5NJBysCm6qxAM=; b=ZZUTBQ3fwW2+67TmSLkWgYqTp2P8VAsthy0QDp7s9SZ0rO5O/Yiwh1iOawBghCHTEQ2jHU rPNOHc2o8Z2N/doG/sb0ERVE46x2ZXQGg0/2CwSM93Q8eTUDH48ePP5puoclz3vfsq5tEi pPaPkSwOubUQuKHgNHg1WoTSvS5RCIw= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-610-qRnryqH1O16la3QT9jiixQ-1; Tue, 12 Nov 2024 20:14:17 -0500 X-MC-Unique: qRnryqH1O16la3QT9jiixQ-1 X-Mimecast-MFC-AGG-ID: qRnryqH1O16la3QT9jiixQ Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-37d59ad50f3so3443330f8f.0 for ; Tue, 12 Nov 2024 17:14:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731460455; x=1732065255; 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=b5sF1HcdenT6u15ytJ89bj5K7zN1Ok5NJBysCm6qxAM=; b=QU4U1SpioA4vkGwKSt5HklWISmbevpa4IB2DrlIthdoCCTWBa+PdrOFzfl0HLJh6hx w7+NNdQ7DSa3or4YJCqo/99/HvM03ljMT3Pap7PNZRvkAqZmh1NEGog5vHmzjnKpAW9T 3BV6YIjyb235tXDfeojJyB61U0MsGoWwrzwOx867mi+84xQI/gojYShIjiTtHSciteSH 5p0mkfCcl+jET2uH161M/Do9tJoVPwO53j/eftHyVS6hbcwwGHqC5Q63adzXUgXH6PBx KLbaD+Ko7T1A6nz91K/AWB2FY3uql2uJWTrxq75AJqwXZs1KzkIWDdKwUqzizsmvOopw ifhw== X-Gm-Message-State: AOJu0YyV033C+h9Dn6oN332N6KOOHTdw9zj9maABzgliDdgC+ve7fNSv R5ooYkANT2Kbfd1MGWZWO9wUQGqPH0e0KFoOKUKpVedwJN/E9bhADlZ7i9yIJPEGhaegXHtvUXJ UrAmt0usUehBwUfIIHxyqqeiXwRIwdyaK8brHupa4+/cR1hXPHujmOZASNA== X-Received: by 2002:a05:6000:784:b0:382:5af:e993 with SMTP id ffacd0b85a97d-38205afeb31mr5348603f8f.54.1731460455534; Tue, 12 Nov 2024 17:14:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IGcLJsv3wavIN8GmU0y9U6c8qIXCTIfIZ9NCzRSCeo83hqVMMCBStXc/IemUa+nIrZDQ1L6Bw== X-Received: by 2002:a05:6000:784:b0:382:5af:e993 with SMTP id ffacd0b85a97d-38205afeb31mr5348589f8f.54.1731460455084; Tue, 12 Nov 2024 17:14:15 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-381ed9a9c2dsm16657211f8f.60.2024.11.12.17.14.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Nov 2024 17:14:14 -0800 (PST) Date: Wed, 13 Nov 2024 02:14:12 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 3/6] ndp: Split out helpers for sending specific NDP message types Message-ID: <20241113021412.2e678d26@elisabeth> In-Reply-To: <20241112040618.1804081-4-david@gibson.dropbear.id.au> References: <20241112040618.1804081-1-david@gibson.dropbear.id.au> <20241112040618.1804081-4-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-MFC-PROC-ID: zJoP9i8ucoVyJV-_MVMsOYMhPRU7dbvZFKSHopin2lg_1731460456 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 7GPV5ZDPY66DUXN5JHYQ7CBA6B35LEAT X-Message-ID-Hash: 7GPV5ZDPY66DUXN5JHYQ7CBA6B35LEAT 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 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: Nits only: On Tue, 12 Nov 2024 15:06:15 +1100 David Gibson wrote: > Currently the large ndp() function responds to all NDP messages we handle, > both parsing the message as necessary and sending the response. Split out > the code to construct and send specific message types into ndp_na() (to > send NA messages) and ndp_ra() (to send RA messages). > > As well as breaking up an excessively large function, this is a first step > to being able to send unsolicited NDP messages. > > While we're there, remove a slighty ugly goto. > > Signed-off-by: David Gibson > --- > ndp.c | 136 +++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 78 insertions(+), 58 deletions(-) > > diff --git a/ndp.c b/ndp.c > index fa1b67a..e876c34 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -186,16 +186,13 @@ static void ndp_send(const struct ctx *c, const struct in6_addr *dst, > } > > /** > - * ndp() - Check for NDP solicitations, reply as needed > + * ndp_na() - Send an NDP Neighbour Advertisement (NA) message > * @c: Execution context > - * @ih: ICMPv6 header > - * @saddr: Source IPv6 address > - * @p: Packet pool > - * > - * Return: 0 if not handled here, 1 if handled, -1 on failure > + * @dst: IPv6 address to send the NA to > + * @addr: IPv6 address to advertise > */ > -int ndp(const struct ctx *c, const struct icmp6hdr *ih, > - const struct in6_addr *saddr, const struct pool *p) > +static void ndp_na(const struct ctx *c, const struct in6_addr *dst, > + const void *addr) > { > struct ndp_na na = { > .ih = { > @@ -212,6 +209,20 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, > }, > } > }; > + > + memcpy(&na.target_addr, addr, sizeof(na.target_addr)); > + memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN); > + > + ndp_send(c, dst, &na, sizeof(na)); > +} > + > +/** > + * ndp_ra() - Send an NDP Router Advertisement (RA) message > + * @c: Execution context > + * @dst: IPv6 address to send the RA to > + */ > +static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) > +{ > struct ndp_ra ra = { > .ih = { > .icmp6_type = RA, > @@ -238,58 +249,28 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, > }, > }, > }; > + unsigned char *ptr = NULL; > > - if (ih->icmp6_type < RS || ih->icmp6_type > NA) > - return 0; > - > - if (c->no_ndp) > - return 1; > - > - if (ih->icmp6_type == NS) { > - const struct ndp_ns *ns = > - packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL); > + memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix)); > > - if (!ns) > - return -1; > + ptr = &ra.var[0]; > > - if (IN6_IS_ADDR_UNSPECIFIED(saddr)) > - return 1; > - > - info("NDP: received NS, sending NA"); > - > - memcpy(&na.target_addr, &ns->target_addr, > - sizeof(na.target_addr)); > - memcpy(na.target_l2_addr.mac, c->our_tap_mac, ETH_ALEN); > + if (c->mtu != -1) { > + struct opt_mtu *mtu = (struct opt_mtu *)ptr; > + *mtu = (struct opt_mtu) { > + .header = { > + .type = OPT_MTU, > + .len = 1, > + }, > + .value = htonl(c->mtu), > + }; > + ptr += sizeof(struct opt_mtu); > + } > > - ndp_send(c, saddr, &na, sizeof(struct ndp_na)); > - } else if (ih->icmp6_type == RS) { > - unsigned char *ptr = NULL; > + if (!c->no_dhcp_dns) { > size_t dns_s_len = 0; > int i, n; > > - if (c->no_ra) > - return 1; > - > - info("NDP: received RS, sending RA"); > - memcpy(&ra.prefix, &c->ip6.addr, sizeof(ra.prefix)); > - > - ptr = &ra.var[0]; > - > - if (c->mtu != -1) { > - struct opt_mtu *mtu = (struct opt_mtu *)ptr; > - *mtu = (struct opt_mtu) { > - .header = { > - .type = OPT_MTU, > - .len = 1, > - }, > - .value = htonl(c->mtu), > - }; > - ptr += sizeof(struct opt_mtu); > - } > - > - if (c->no_dhcp_dns) > - goto dns_done; > - > for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++); > if (n) { > struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr; > @@ -305,7 +286,7 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, > sizeof(rdnss->dns[i])); > } > ptr += offsetof(struct opt_rdnss, dns) + > - i * sizeof(rdnss->dns[0]); > + i * sizeof(rdnss->dns[0]); Unrelated change, less readable, probably from your new editor / clangd? > > for (n = 0; *c->dns_search[n].n; n++) > dns_s_len += strlen(c->dns_search[n].n) + 2; > @@ -329,7 +310,7 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, > *(ptr++) = '.'; > > len = sizeof(dnssl->domains) - > - (ptr - dnssl->domains); > + (ptr - dnssl->domains); Same here. > > strncpy((char *)ptr, c->dns_search[i].n, len); > for (dot = (char *)ptr - 1; *dot; dot++) { > @@ -343,11 +324,50 @@ int ndp(const struct ctx *c, const struct icmp6hdr *ih, > memset(ptr, 0, 8 - dns_s_len % 8); /* padding */ > ptr += 8 - dns_s_len % 8; > } > + } > > -dns_done: > - memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN); > + memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN); > > - ndp_send(c, saddr, &ra, ptr - (unsigned char *)&ra); > + ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra); > +} > + > +/** > + * ndp() - Check for NDP solicitations, reply as needed > + * @c: Execution context > + * @ih: ICMPv6 header > + * @saddr: Source IPv6 address > + * @p: Packet pool > + * > + * Return: 0 if not handled here, 1 if handled, -1 on failure > + */ > +int ndp(const struct ctx *c, const struct icmp6hdr *ih, > + const struct in6_addr *saddr, const struct pool *p) > +{ > + if (ih->icmp6_type < RS || ih->icmp6_type > NA) > + return 0; > + > + if (c->no_ndp) > + return 1; > + > + if (ih->icmp6_type == NS) { > + const struct ndp_ns *ns = > + packet_get(p, 0, 0, sizeof(struct ndp_ns), NULL); The assignment could go on its line here? We don't save any line this way. > + > + if (!ns) > + return -1; > + > + if (IN6_IS_ADDR_UNSPECIFIED(saddr)) > + return 1; > + > + info("NDP: received NS, sending NA"); > + > + ndp_na(c, saddr, &ns->target_addr); > + } else if (ih->icmp6_type == RS) { > + if (c->no_ra) > + return 1; > + > + info("NDP: received RS, sending RA"); > + ndp_ra(c, saddr); > } > > return 1; -- Stefano