From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine 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=BEicHBrJ; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id 8FF125A0008 for ; Tue, 15 Apr 2025 20:54:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744743258; 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=CV+wTMHQF1Gk12G5Dgt0B7zDwrU+S2S5hI/GeNKzNOQ=; b=BEicHBrJPzbrPpSnNx72jvUJbjwwp4sJ4ucpHR2ngkPRMv72CFzEJv4lPolMOkfr/VLRJo iUr/RfzlY6B63w8mDRAaT1LixfCSAfV3oTkNuQnw/LxPv3XrLxXB2k+XmnIa834vLq1j6A f9SO6Zb5Xrxr1epjuOjuscmnb2f/H2E= 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-563-Os9Ed9S8PcCUTpTk6UHlZg-1; Tue, 15 Apr 2025 14:54:16 -0400 X-MC-Unique: Os9Ed9S8PcCUTpTk6UHlZg-1 X-Mimecast-MFC-AGG-ID: Os9Ed9S8PcCUTpTk6UHlZg_1744743256 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-39c184b20a2so2824673f8f.1 for ; Tue, 15 Apr 2025 11:54:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1744743256; x=1745348056; 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=CV+wTMHQF1Gk12G5Dgt0B7zDwrU+S2S5hI/GeNKzNOQ=; b=PVtBKD/LyDC6WZMgHbLtWMhjQCXU1fR79uKdOMBYc1Mx2QkkR+3TALral4prSNSQOQ CGMuLSIlDMEPjc1M468Tbbd23vk7/LskRyap3QmXnX65Zv3GpxbA/Ae2J8dnASJyL5LT dDsW9vpgorUUADKjhcLO3HGgYheTXp+QPVujvxqvYZvcmDHajJsn0g/flCpYT0bIzw6C gh6aeNJnB5PmScDDbVbmFF36I9dwqbVC6w+8cGlZSdKhlOXkt1l56ZLF0Ex3/FipYaDS hF3VymWUmy+dT/pB6E9cpgMy67LCS3rWXb4x1NJHDFG8rbuG74Z+mAa7mEuvG/Xc5bvp XskA== X-Gm-Message-State: AOJu0YzpWUVBfthER9eYfLBQKYw0znTfmd9bt38xaX1tsBl21CzJZHAG CS0tdNwybI8+rHswdXYphYZRM5zQgoE707MlGY7ALhLYLjUaimEMHbnlkVJUEfQV2Ie9JTOe10x azSM8lKq0ljkq/SOSxGnvg1kzMfy0hRLU5zRjphdzxIGblEMOJw== X-Gm-Gg: ASbGncvlU8+szy9F5hHbr0g1HlXZqtviTyHj7Z1lF5v8GNNTFmoDXuKYFFMvDu4c4Z1 wekF3DmMkngQ/ReUmVdl57pEp/KjdMdD/wGxYtHWeoUzsMI8sbot50PNr2m+oQ/FOIU95Llub/m gk4Csu5dXsttOcRD00QmK2cwuiu99cMfAwv2F2IqtM9oPaoxPZnvBczPzmtWo92tZzFJE7xODtT bq4122DobekQbYAApp+cmCjJfW1Y9lqZvd5QBbgR0RkfBA1NGrtNIur2WjPzGHodq7kmypo+Gun KyRskfHu/FFURvQY9CDnIb/I3DtUg5uqBGH/9mYu X-Received: by 2002:a5d:6d87:0:b0:39c:13fa:80b with SMTP id ffacd0b85a97d-39ee272a100mr496929f8f.12.1744743255358; Tue, 15 Apr 2025 11:54:15 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHnCpEhIrItuPtfpt166FXE/G1VKydG6HZMdhruGxOBnBbZmVvq1lNzOWkvrwQy7Cb320N3wg== X-Received: by 2002:a5d:6d87:0:b0:39c:13fa:80b with SMTP id ffacd0b85a97d-39ee272a100mr496831f8f.12.1744743253649; Tue, 15 Apr 2025 11:54:13 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-39eaf447777sm14838701f8f.100.2025.04.15.11.54.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 15 Apr 2025 11:54:12 -0700 (PDT) Date: Tue, 15 Apr 2025 20:54:00 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 5/7] udp: Add udp_pktinfo() helper Message-ID: <20250415205400.68c7c1d2@elisabeth> In-Reply-To: <20250415071624.2618589-6-david@gibson.dropbear.id.au> References: <20250415071624.2618589-1-david@gibson.dropbear.id.au> <20250415071624.2618589-6-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: bXt5Z_d35Xb1b5Cl-GNwJNg796oYUwp5gky6LXq-IDg_1744743256 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: OWBG2LPGEGVO5B2YLZQKB2DGWOD7JTNI X-Message-ID-Hash: OWBG2LPGEGVO5B2YLZQKB2DGWOD7JTNI 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, Jon Maloy 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: I took the liberty of applying this with two minor changes: On Tue, 15 Apr 2025 17:16:22 +1000 David Gibson wrote: > Currently we open code parsing the control message for IP_PKTINFO in > udp_peek_addr(). We have an upcoming case where we want to parse PKTINFO > in another place, so split this out into a helper function. > > While we're there, make the parsing a bit more robust: scan all cmsgs to > look for the one we want, rather than assuming there's only one. > > Signed-off-by: David Gibson > --- > udp.c | 52 ++++++++++++++++++++++++++++++++++++---------------- > 1 file changed, 36 insertions(+), 16 deletions(-) > > diff --git a/udp.c b/udp.c > index 0bec499d..352ab83b 100644 > --- a/udp.c > +++ b/udp.c > @@ -464,6 +464,41 @@ static void udp_send_tap_icmp6(const struct ctx *c, > tap_icmp6_send(c, saddr, eaddr, &msg, msglen); > } > > +/** > + * udp_pktinfo() - Retreive packet destination address from cmsg Nit: "Retrieve", and: > + * @msg: msghdr into which message has been received > + * @dst: (Local) destination address of message in @mh (output) > + * > + * Return: 0 on success, -1 if the information was missing (@dst is set to > + * inany_any6). > + */ > +static int udp_pktinfo(struct msghdr *msg, union inany_addr *dst) > +{ > + struct cmsghdr *hdr; > + > + for (hdr = CMSG_FIRSTHDR(msg); hdr; hdr = CMSG_NXTHDR(msg, hdr)) { > + if (hdr->cmsg_level == IPPROTO_IP && > + hdr->cmsg_type == IP_PKTINFO) { > + const struct in_pktinfo *i4 = (void *)CMSG_DATA(hdr); > + > + *dst = inany_from_v4(i4->ipi_addr); > + return 0; > + } > + > + if (hdr->cmsg_level == IPPROTO_IPV6 && > + hdr->cmsg_type == IPV6_PKTINFO) { > + const struct in6_pktinfo *i6 = (void *)CMSG_DATA(hdr); > + > + dst->a6 = i6->ipi6_addr; > + return 0; > + } > + } > + > + err("Missing PKTINFO cmsg on datagram"); ...from a quick glance at the matching kernel path, this looks a bit too likely for err(), so I got a bit scared, and turned it to debug(). I think warn() is actually more correct than debug() (I think it shouldn't really be err() though because we can keep using this flow), but I would feel a lot more confident about it once we have rate-limited versions of those. Of course, I'll change this back to err() or warn() if you have a compelling reason. Similar reasoning in 7/7 where this is called. By the way, two messages (here and in caller) look redundant to me, regardless of their severity, but I'm not sure if you have further changes in mind where these separate prints would make sense. -- Stefano