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=R0lHQ/oi; 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 3659A5A0278 for ; Fri, 08 Aug 2025 11:33:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1754645612; 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:autocrypt:autocrypt; bh=IyDUy9YH9+d49oS7RfrRsjd6hquUvZIahiZm5CCOUKY=; b=R0lHQ/oik+YJGlyhhZ9M0w31SOa2r+sQpd/JdfeTqzAkLEm+v3Z7j9i9mOSXVdGiT/YWLI wD/+mIKnGuLLx0GSZTVTqbQ7zsXdxGbvrc58NH5OfiZLu2FCRTDjVG8HSrdBQUMO9NCiMl z0jhlMcw4T5DeK6eHKc/DCk7t+Hqqkk= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-361-DFtgEiI1MsKs3VmL9YBZ2Q-1; Fri, 08 Aug 2025 05:33:28 -0400 X-MC-Unique: DFtgEiI1MsKs3VmL9YBZ2Q-1 X-Mimecast-MFC-AGG-ID: DFtgEiI1MsKs3VmL9YBZ2Q_1754645607 Received: by mail-qk1-f199.google.com with SMTP id af79cd13be357-7e696444d0cso441737385a.1 for ; Fri, 08 Aug 2025 02:33:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1754645607; x=1755250407; h=content-transfer-encoding:in-reply-to:autocrypt:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=IyDUy9YH9+d49oS7RfrRsjd6hquUvZIahiZm5CCOUKY=; b=sVDsVLwui0JRJ1KHlRstLv7dnOeIXTySsCxgm+pomfQk5n2kCyhokmFzGAqgJXSoxI RznPe2yqow3/NCPxFJpv5qhxrko4FgJ+UYi4HO8k/YNGJsStICRJp8WqoBgLTaJEOkBJ UO5lzKbCxKXLbJf8nCUgqzt6ysK3TgkYmK2P5HAiCWg4+51TnKbQyo+BzHYcjHmpta3E oesAHAcyka8SeacT0gTu1ukQ61GY5m7Nev8/Ed3H7sk9KXuZaMf8nYteK/nykXOdVFWP u/x9Nbp4iWoJqzex5qnAStxfqouJsH/5ABCM3BdsQmIyD/C/jUGNUs6RWpIfsM7EEhhu gF2Q== X-Gm-Message-State: AOJu0YwkHL3E0k0S+HkpWwEiOpFbu7J70iGt1jsVHHRfIYLtNJw6JIPV 5A2yziKHCPm4stNUBP1OeBcAO/SFHi0vPTcKykE4yDvXtt1SDSurt++7f7jebhZsjZz/7rHoIqP YxU7GC/nqLaYbpJlQF5F7hx6uBn8gaUY4ZoCa5qhFfZaN3DMj4V7uow== X-Gm-Gg: ASbGnctDPFf1w6aqqBmOZpJF5Az2QQUo2HIdZGNFvs9CE/ZexL6OvNvh0EdMW6FRfnj dit8rCFUDiAAfwL45LGq9p0kVywV6k8SyHizcpivgG0LILCaWi+cBMWGWlUzQh/zdgEIvsPu4aj uzgclk29UqAZyUvRzI7Aon6MPLrIb7wnhQN19FrvrLU3MkM0MIopUGwZN4JUUcFUQWSeVDqYsHk UoPFRroIYUp0RI4rJxLLprZveyto+PbMB77rYejAEFD5ix2Sc/97FFP8Xjj35bERjoKv0k30DUP fbZFdwYFEu/RPPpYzKLVB+N8qzZHs/RaETG2fqxIzcRf/4FalUL+FgVDIwnOgoJi+Y5RhQCOADa SaP9r4rOZ X-Received: by 2002:a05:620a:a35a:b0:7e8:74d:fdf0 with SMTP id af79cd13be357-7e82c731cc3mr245300585a.41.1754645607194; Fri, 08 Aug 2025 02:33:27 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHZIiGrS7Eygc1WjRqLNvwG3FevkeB3QfRtd7fgw3rj1u/+vOVc3OKDElFUTPthcaFDWvmdgQ== X-Received: by 2002:a05:620a:a35a:b0:7e8:74d:fdf0 with SMTP id af79cd13be357-7e82c731cc3mr245296985a.41.1754645606405; Fri, 08 Aug 2025 02:33:26 -0700 (PDT) Received: from ?IPV6:2a01:e0a:e10:ef90:343a:68f:2e91:95c? ([2a01:e0a:e10:ef90:343a:68f:2e91:95c]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-4b069ca56d6sm63614481cf.36.2025.08.08.02.33.25 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 08 Aug 2025 02:33:26 -0700 (PDT) Message-ID: Date: Fri, 8 Aug 2025 11:33:24 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v8 17/30] dhcp: Convert to iov_tail To: David Gibson References: <20250805154628.301343-1-lvivier@redhat.com> <20250805154628.301343-18-lvivier@redhat.com> From: Laurent Vivier Autocrypt: addr=lvivier@redhat.com; keydata= xsFNBFYFJhkBEAC2me7w2+RizYOKZM+vZCx69GTewOwqzHrrHSG07MUAxJ6AY29/+HYf6EY2 WoeuLWDmXE7A3oJoIsRecD6BXHTb0OYS20lS608anr3B0xn5g0BX7es9Mw+hV/pL+63EOCVm SUVTEQwbGQN62guOKnJJJfphbbv82glIC/Ei4Ky8BwZkUuXd7d5NFJKC9/GDrbWdj75cDNQx UZ9XXbXEKY9MHX83Uy7JFoiFDMOVHn55HnncflUncO0zDzY7CxFeQFwYRbsCXOUL9yBtqLer Ky8/yjBskIlNrp0uQSt9LMoMsdSjYLYhvk1StsNPg74+s4u0Q6z45+l8RAsgLw5OLtTa+ePM JyS7OIGNYxAX6eZk1+91a6tnqfyPcMbduxyBaYXn94HUG162BeuyBkbNoIDkB7pCByed1A7q q9/FbuTDwgVGVLYthYSfTtN0Y60OgNkWCMtFwKxRaXt1WFA5ceqinN/XkgA+vf2Ch72zBkJL RBIhfOPFv5f2Hkkj0MvsUXpOWaOjatiu0fpPo6Hw14UEpywke1zN4NKubApQOlNKZZC4hu6/ 8pv2t4HRi7s0K88jQYBRPObjrN5+owtI51xMaYzvPitHQ2053LmgsOdN9EKOqZeHAYG2SmRW LOxYWKX14YkZI5j/TXfKlTpwSMvXho+efN4kgFvFmP6WT+tPnwARAQABzSNMYXVyZW50IFZp dmllciA8bHZpdmllckByZWRoYXQuY29tPsLBeAQTAQIAIgUCVgVQgAIbAwYLCQgHAwIGFQgC CQoLBBYCAwECHgECF4AACgkQ8ww4vT8vvjwpgg//fSGy0Rs/t8cPFuzoY1cex4limJQfReLr SJXCANg9NOWy/bFK5wunj+h/RCFxIFhZcyXveurkBwYikDPUrBoBRoOJY/BHK0iZo7/WQkur 6H5losVZtrotmKOGnP/lJYZ3H6OWvXzdz8LL5hb3TvGOP68K8Bn8UsIaZJoeiKhaNR0sOJyI YYbgFQPWMHfVwHD/U+/gqRhD7apVysxv5by/pKDln1I5v0cRRH6hd8M8oXgKhF2+rAOL7gvh jEHSSWKUlMjC7YwwjSZmUkL+TQyE18e2XBk85X8Da3FznrLiHZFHQ/NzETYxRjnOzD7/kOVy gKD/o7asyWQVU65mh/ECrtjfhtCBSYmIIVkopoLaVJ/kEbVJQegT2P6NgERC/31kmTF69vn8 uQyW11Hk8tyubicByL3/XVBrq4jZdJW3cePNJbTNaT0d/bjMg5zCWHbMErUib2Nellnbg6bc 2HLDe0NLVPuRZhHUHM9hO/JNnHfvgiRQDh6loNOUnm9Iw2YiVgZNnT4soUehMZ7au8PwSl4I KYE4ulJ8RRiydN7fES3IZWmOPlyskp1QMQBD/w16o+lEtY6HSFEzsK3o0vuBRBVp2WKnssVH qeeV01ZHw0bvWKjxVNOksP98eJfWLfV9l9e7s6TaAeySKRRubtJ+21PRuYAxKsaueBfUE7ZT 7zfOwU0EVgUmGQEQALxSQRbl/QOnmssVDxWhHM5TGxl7oLNJms2zmBpcmlrIsn8nNz0rRyxT 460k2niaTwowSRK8KWVDeAW6ZAaWiYjLlTunoKwvF8vP3JyWpBz0diTxL5o+xpvy/Q6YU3BN efdq8Vy3rFsxgW7mMSrI/CxJ667y8ot5DVugeS2NyHfmZlPGE0Nsy7hlebS4liisXOrN3jFz asKyUws3VXek4V65lHwB23BVzsnFMn/bw/rPliqXGcwl8CoJu8dSyrCcd1Ibs0/Inq9S9+t0 VmWiQWfQkz4rvEeTQkp/VfgZ6z98JRW7S6l6eophoWs0/ZyRfOm+QVSqRfFZdxdP2PlGeIFM C3fXJgygXJkFPyWkVElr76JTbtSHsGWbt6xUlYHKXWo+xf9WgtLeby3cfSkEchACrxDrQpj+ Jt/JFP+q997dybkyZ5IoHWuPkn7uZGBrKIHmBunTco1+cKSuRiSCYpBIXZMHCzPgVDjk4viP brV9NwRkmaOxVvye0vctJeWvJ6KA7NoAURplIGCqkCRwg0MmLrfoZnK/gRqVJ/f6adhU1oo6 z4p2/z3PemA0C0ANatgHgBb90cd16AUxpdEQmOCmdNnNJF/3Zt3inzF+NFzHoM5Vwq6rc1JP jfC3oqRLJzqAEHBDjQFlqNR3IFCIAo4SYQRBdAHBCzkM4rWyRhuVABEBAAHCwV8EGAECAAkF AlYFJhkCGwwACgkQ8ww4vT8vvjwg9w//VQrcnVg3TsjEybxDEUBm8dBmnKqcnTBFmxN5FFtI WlEuY8+YMiWRykd8Ln9RJ/98/ghABHz9TN8TRo2b6WimV64FmlVn17Ri6FgFU3xNt9TTEChq AcNg88eYryKsYpFwegGpwUlaUaaGh1m9OrTzcQy+klVfZWaVJ9Nw0keoGRGb8j4XjVpL8+2x OhXKrM1fzzb8JtAuSbuzZSQPDwQEI5CKKxp7zf76J21YeRrEW4WDznPyVcDTa+tz++q2S/Bp P4W98bXCBIuQgs2m+OflERv5c3Ojldp04/S4NEjXEYRWdiCxN7ca5iPml5gLtuvhJMSy36gl U6IW9kn30IWuSoBpTkgV7rLUEhh9Ms82VWW/h2TxL8enfx40PrfbDtWwqRID3WY8jLrjKfTd R3LW8BnUDNkG+c4FzvvGUs8AvuqxxyHbXAfDx9o/jXfPHVRmJVhSmd+hC3mcQ+4iX5bBPBPM oDqSoLt5w9GoQQ6gDVP2ZjTWqwSRMLzNr37rJjZ1pt0DCMMTbiYIUcrhX8eveCJtY7NGWNyx FCRkhxRuGcpwPmRVDwOl39MB3iTsRighiMnijkbLXiKoJ5CDVvX5yicNqYJPKh5MFXN1bvsB kmYiStMRbrD0HoY1kx5/VozBtc70OU0EB8Wrv9hZD+Ofp0T3KOr1RUHvCZoLURfFhSQ= In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: RHxAH9KN9DaH2NhtX-ggZWMDioPK4HyHz-wu5vE34V4_1754645607 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Message-ID-Hash: RFMUE6K5JLM7DREXI5EWA2GNUE3WUJBL X-Message-ID-Hash: RFMUE6K5JLM7DREXI5EWA2GNUE3WUJBL X-MailFrom: lvivier@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: On 06/08/2025 06:38, David Gibson wrote: > On Tue, Aug 05, 2025 at 05:46:15PM +0200, Laurent Vivier wrote: >> Use packet_data() and extract headers using IOV_REMOVE_HEADER() >> and IOV_PEEK_HEADER() rather than packet_get(). > > Unlike the previous patch, I think using iov_tail does work here, > because there's a single scan through the options, rather than > repeatedly scanning for options of specific types. > >> Signed-off-by: Laurent Vivier >> --- >> dhcp.c | 46 ++++++++++++++++++++++++++++------------------ >> 1 file changed, 28 insertions(+), 18 deletions(-) >> >> diff --git a/dhcp.c b/dhcp.c >> index b0de04be6f27..cf73d4b07767 100644 >> --- a/dhcp.c >> +++ b/dhcp.c >> @@ -302,27 +302,33 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) >> */ >> int dhcp(const struct ctx *c, const struct pool *p) >> { >> - size_t mlen, dlen, offset = 0, opt_len, opt_off = 0; >> char macstr[ETH_ADDRSTRLEN]; >> + size_t mlen, dlen, opt_len; >> struct in_addr mask, dst; >> + struct ethhdr eh_storage; >> + struct iphdr iph_storage; >> + struct udphdr uh_storage; >> const struct ethhdr *eh; >> const struct iphdr *iph; >> const struct udphdr *uh; >> + struct iov_tail data; >> struct msg const *m; > > Pre-existing, but I'm a bit baffled as to what the (const *) is doing here. > >> struct msg reply; >> unsigned int i; >> + struct msg m_storage; >> >> - eh = packet_get(p, 0, offset, sizeof(*eh), NULL); >> - offset += sizeof(*eh); >> + if (!packet_data(p, 0, &data)) >> + return -1; >> >> - iph = packet_get(p, 0, offset, sizeof(*iph), NULL); >> + eh = IOV_REMOVE_HEADER(&data, eh_storage); >> + iph = IOV_PEEK_HEADER(&data, iph_storage); >> if (!eh || !iph) >> return -1; >> >> - offset += iph->ihl * 4UL; >> - uh = packet_get(p, 0, offset, sizeof(*uh), &mlen); >> - offset += sizeof(*uh); >> + if (!iov_tail_drop(&data, iph->ihl * 4UL)) >> + return -1; >> >> + uh = IOV_REMOVE_HEADER(&data, uh_storage); >> if (!uh) >> return -1; >> >> @@ -332,7 +338,10 @@ int dhcp(const struct ctx *c, const struct pool *p) >> if (c->no_dhcp) >> return 1; >> >> - m = packet_get(p, 0, offset, offsetof(struct msg, o), &opt_len); >> + mlen = iov_tail_size(&data); >> + m = (struct msg const *)iov_remove_header_(&data, &m_storage, >> + offsetof(struct msg, o), >> + __alignof__(struct msg)); >> if (!m || >> mlen != ntohs(uh->len) - sizeof(*uh) || >> mlen < offsetof(struct msg, o) || >> @@ -355,27 +364,28 @@ int dhcp(const struct ctx *c, const struct pool *p) >> memset(&reply.file, 0, sizeof(reply.file)); >> reply.magic = m->magic; >> >> - offset += offsetof(struct msg, o); >> - >> for (i = 0; i < ARRAY_SIZE(opts); i++) >> opts[i].clen = -1; >> >> - while (opt_off + 2 < opt_len) { >> - const uint8_t *olen, *val; >> + opt_len = iov_tail_size(&data); >> + while (opt_len >= 2) { >> + uint8_t olen_storage, type_storage; >> + const uint8_t *olen; >> uint8_t *type; >> >> - type = packet_get(p, 0, offset + opt_off, 1, NULL); >> - olen = packet_get(p, 0, offset + opt_off + 1, 1, NULL); >> + type = IOV_REMOVE_HEADER(&data, type_storage); >> + olen = IOV_REMOVE_HEADER(&data, olen_storage); > > It seems a bit mad to access single bytes via 8-byte pointers, but > it's probably not worth the hassle of handling it differently in this > one case. > >> if (!type || !olen) >> return -1; >> >> - val = packet_get(p, 0, offset + opt_off + 2, *olen, NULL); >> - if (!val) >> + opt_len = iov_tail_size(&data); >> + if (opt_len < *olen) >> return -1; >> >> - memcpy(&opts[*type].c, val, *olen); >> + iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen); > > So, IIUC, if *olen is much too big, this is still safe.. > >> opts[*type].clen = *olen; > > .. but recording *olen unedited as the length of the option is > probably wrong in that case. I don't understand how to edit *olen. There is no change regarding the original code. > >> - opt_off += *olen + 2; >> + iov_tail_drop(&data, *olen); >> + opt_len -= *olen; > > Isn't the stanza above doing the equivalent of an > iov_remove_header_()? No, in fact iov_remove_header_() copy to the buffer only if the data are discontinuous, in this case we want to copy the data unconditionally to edit them later. We don't want to edit the data in the iovec buffer. > >> } >> >> opts[80].slen = -1; > Thanks, Laurent