From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=fail (p=quarantine dis=none) header.from=redhat.com 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 ADF355A0265 for ; Thu, 22 Oct 2026 00:34:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1792622043; 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=jtfoglgJDwd5rCHReH55oZ+NM6LkNcluNovTFXFUjnE=; b=elOcI/v2EMCsbc9vdjRfZu1e//cDOgSvK1MqZHMVQa+bxnD1FnhBDYfSGloiQEbl66hTze QVS9bkuBUHj27V9RvjUo7wJ9btdMPb31brfQhnQLqXgmtZiRvdtnnPmJkzvq7QIeTEseqs p3hLGcArUiV2M+/I9L9B2YiT12PDQic= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-591-kE7JhoF_Pw-VxsdZ5pRoMQ-1; Wed, 21 Oct 2026 22:33:55 +0000 X-MC-Unique: kE7JhoF_Pw-VxsdZ5pRoMQ-1 X-Mimecast-MFC-AGG-ID: kE7JhoF_Pw-VxsdZ5pRoMQ_1792622034 Received: by mail-wr1-f72.google.com with SMTP id ffacd0b85a97d-48e1ad3cb66so623613f8f.3 for ; Wed, 21 Oct 2026 15:33:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792622034; x=1793226834; h=date:content-transfer-encoding:content-type:mime-version :organization:references:in-reply-to:message-id:subject:cc:to:from :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=jtfoglgJDwd5rCHReH55oZ+NM6LkNcluNovTFXFUjnE=; b=BtDRbPzMOt+zckSdH8DTmYyD2bAfw8Xy3TjFKKWcQ7jZCBShoeuAaWTUesEa0JVwVZ xOFdZgvT5rpavUNcQmuQguya9hOJRXh3DKKva/ZjHh//Y+L6EHXdq6mp6xCy6j1El4Yj uxRwO5En4CBKeRa/mfWzj0/mUwKEwHXelAm8mcN8ocZ62cx8h1RNzeFcAA95usEVUGVL XFMVFyeLEKTjcYKjPFeITltjvw8XhpL11WrxxXRtVdyhb1e73oCp7c+oO7Go2XzJTSei mJNB4CoLcaQ76RMbrkyl8wQgj5+XdL4KSOMapxAKXgkFhTMaoX/sdkPxSS2bTCkVGChB hhpg== X-Forwarded-Encrypted: i=1; AKwUvBx4i4gmNSwxak2JFLDw/+QnWwEHjDw6GxzYqa83ILbTOji1/shd1nH2aRwiF38VTSe8p/WCv/nO39Y=@passt.top X-Gm-Message-State: AFq9FYKg0dKu7UaJ8pkJMW8Pbnswn74kz5uTcZYduLLlQaa9wPFfUsnW 1Smd3CGCThQJ5cnw4/xmO68RRR3xZl0LT7JqzrhUT7m3PR4SphiXoJxMqXJcWySYCdc6iowH6tK Ylw9ky0hLyFYW7AeQQkO95IspD8XIHKg5znidM72frI7PmxQ82qyHQw== X-Gm-Gg: AYBFou3qLVKb2W9J2zJk+SrgovqN2VghST8rU10acfkB9cnnZsrXkAskHTzCUA2qI/+ uWQOFnicVmJlpWz1on6Xz4EAYzQjoWIjwGe3Bj8HDZBHIBDcbICRLDi3rbSSdlfIBddSBkxSpX9 qAodcDvx5GrHTqcm5RbMaNAj875/4dR1pNYqFuJQiPcarNhcPSKol4bZ18N7+fORjhl5iY8ye4G e2gmc9mmSkVhZ1no9VCpIH5cG0pflakaOdZZB9gHrte24eynpNIiuBkpcSkBBjE7c0zi5ErPAas 8JaLTB2aosB/04x98Txh8cQAkKHq/FsWtf6dJk8jsHGJh2C6LCZcsz4n9ywdTnQjSUpv/hQnQ87 1WYKN9CDlU/v66Ay0wAJq23N4qGo= X-Received: by 2002:a05:6000:98a:b0:48d:f957:bca4 with SMTP id ffacd0b85a97d-48e264dd014mr6762625f8f.44.1792622033575; Wed, 21 Oct 2026 15:33:53 -0700 (PDT) X-Received: by 2002:a05:6000:98a:b0:48d:f957:bca4 with SMTP id ffacd0b85a97d-48e264dd014mr6762576f8f.44.1792622032572; Wed, 21 Oct 2026 15:33:52 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-48e2583523asm8192265f8f.37.2026.10.21.15.33.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 Oct 2026 15:33:51 -0700 (PDT) From: Stefano Brivio To: Jon Maloy Subject: Re: [PASST v10] passt, pasta: Introduce unified multi-address data structures Message-ID: <20261022003349.4dfcb6f1@elisabeth> In-Reply-To: <20261015015747.2453213-1-jmaloy@redhat.com> References: <20261015015747.2453213-1-jmaloy@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Thu, 22 Oct 2026 00:33:50 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: k9n02A8lteGpmHPiFrR8tspjXglRoAWF4gLN6rKi9l0_1792622034 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: LRUHOSJUYGBTTDUOWLXOWCCQZGCU77E2 X-Message-ID-Hash: LRUHOSJUYGBTTDUOWLXOWCCQZGCU77E2 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: david@gibson.dropbear.id.au, 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: Sorry for the delay. It mostly looks good to me with a few caveats. First off, Coverity Scan complains quite a bit now (but I haven't looked into it): --- /home/sbrivio/passt/conf.c:444:3: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/conf.c:438:2: Call to null-returning function 1. path: Condition "!a", taking false branch. /home/sbrivio/passt/conf.c:444:3: 2. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times). /home/sbrivio/passt/inany.h:102:7: Call to null-returning function 2.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 2.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 2.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch. /home/sbrivio/passt/inany.h:102:2: 2.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch. /home/sbrivio/passt/inany.h:103:3: 2.5. return_null: Explicitly returning null. /home/sbrivio/passt/conf.c:444:3: 3. dereference: Dereferencing "inany_v4(&a->addr)", which is known to be "NULL". /home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null 4. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)". /home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null 5. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)". /home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null 6. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)". /home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null 7. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)". /home/sbrivio/passt/pif.c:44:2: 8. example_checked: Example 4 (cont.): "v4" has its value checked in "v4". /home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null 9. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)". /home/sbrivio/passt/arp.c:60:2: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/arp.c:46:2: Call to null-returning function 1. path: Condition "ah->ar_hrd != __bswap_16(1)", taking false branch. /home/sbrivio/passt/arp.c:46:2: 2. path: Condition "ah->ar_pro != __bswap_16(2048)", taking false branch. /home/sbrivio/passt/arp.c:46:2: 3. path: Condition "ah->ar_hln != 6", taking false branch. /home/sbrivio/passt/arp.c:46:2: 4. path: Condition "ah->ar_pln != 4", taking false branch. /home/sbrivio/passt/arp.c:46:2: 5. path: Condition "ah->ar_op != __bswap_16(1)", taking false branch. /home/sbrivio/passt/arp.c:54:2: 6. path: Condition "memcmp(am->sip, &inany_any4.v4mapped.a4, 4UL /* sizeof (am->sip) */)", taking true branch. /home/sbrivio/passt/arp.c:54:2: 7. path: Condition "!memcmp(am->sip, am->tip, 4UL /* sizeof (am->sip) */)", taking false branch. /home/sbrivio/passt/arp.c:60:2: 8. path: Condition "a", taking true branch. /home/sbrivio/passt/arp.c:60:2: 9. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times). /home/sbrivio/passt/inany.h:102:7: Call to null-returning function 9.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 9.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 9.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch. /home/sbrivio/passt/inany.h:102:2: 9.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch. /home/sbrivio/passt/inany.h:103:3: 9.5. return_null: Explicitly returning null. /home/sbrivio/passt/arp.c:60:2: 10. dereference: Passing null pointer "inany_v4(&a->addr)" to "memcmp", which dereferences it. /home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null 11. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)". /home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null 12. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)". /home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null 13. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)". /home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null 14. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)". /home/sbrivio/passt/pif.c:44:2: 15. example_checked: Example 4 (cont.): "v4" has its value checked in "v4". /home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null 16. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)". /home/sbrivio/passt/fwd.c:876:2: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/fwd.c:861:2: Call to null-returning function 1. path: Condition "(__bswap_32(((struct in_addr *)addr)->s_addr) >> 24) == 127", taking false branch. /home/sbrivio/passt/fwd.c:869:2: 2. path: Condition "((struct in_addr *)addr)->s_addr == (0U /* (((((in_addr_t)0 & 0xff000000U) >> 24) | (((in_addr_t)0 & 0xff0000U) >> 8)) | (((in_addr_t)0 & 0xff00U) << 8)) | (((in_addr_t)0 & 0xffU) << 24) */)", taking false branch. /home/sbrivio/passt/fwd.c:876:2: 3. path: Condition "a", taking true branch. /home/sbrivio/passt/fwd.c:876:2: 4. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times). /home/sbrivio/passt/inany.h:102:7: Call to null-returning function 4.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 4.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 4.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch. /home/sbrivio/passt/inany.h:102:2: 4.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch. /home/sbrivio/passt/inany.h:103:3: 4.5. return_null: Explicitly returning null. /home/sbrivio/passt/fwd.c:876:2: 5. dereference: Dereferencing "inany_v4(&a->addr)", which is known to be "NULL". /home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null 6. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)". /home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null 7. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)". /home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null 8. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)". /home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null 9. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)". /home/sbrivio/passt/pif.c:44:2: 10. example_checked: Example 4 (cont.): "v4" has its value checked in "v4". /home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null 11. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)". /home/sbrivio/passt/arp.c:156:2: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/arp.c:137:2: Call to null-returning function 1. path: Condition "!a", taking false branch. /home/sbrivio/passt/arp.c:156:2: 2. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times). /home/sbrivio/passt/inany.h:102:7: Call to null-returning function 2.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 2.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 2.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch. /home/sbrivio/passt/inany.h:102:2: 2.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch. /home/sbrivio/passt/inany.h:103:3: 2.5. return_null: Explicitly returning null. /home/sbrivio/passt/arp.c:156:2: 3. dereference: Passing null pointer "inany_v4(&a->addr)" to "memcpy", which dereferences it. [Note: The source code implementation of the function has been overridden by a builtin model.] /home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null 4. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)". /home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null 5. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)". /home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null 6. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)". /home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null 7. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)". /home/sbrivio/passt/pif.c:44:2: 8. example_checked: Example 4 (cont.): "v4" has its value checked in "v4". /home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null 9. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)". /home/sbrivio/passt/conf.c:423:2: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/conf.c:385:2: Call to null-returning function 1. path: Condition "!ifi", taking false branch. /home/sbrivio/passt/conf.c:388:2: 2. path: Condition "!ifi", taking false branch. /home/sbrivio/passt/conf.c:393:2: 3. path: Condition "((struct in_addr *)&ip4->guest_gw)->s_addr == (0U /* (((((in_addr_t)0 & 0xff000000U) >> 24) | (((in_addr_t)0 & 0xff0000U) >> 8)) | (((in_addr_t)0 & 0xff00U) << 8)) | (((in_addr_t)0 & 0xffU) << 24) */)", taking false branch. /home/sbrivio/passt/conf.c:403:2: 4. path: Condition "!a", taking false branch. /home/sbrivio/passt/conf.c:422:2: 5. path: Condition "a", taking true branch. /home/sbrivio/passt/conf.c:423:2: 6. returned_null: "inany_v4" returns "NULL" (checked 51 out of 60 times). /home/sbrivio/passt/inany.h:102:7: Call to null-returning function 6.1. path: Condition "__a->__in6_u.__u6_addr32[0] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 6.2. path: Condition "__a->__in6_u.__u6_addr32[1] == 0", taking true branch. /home/sbrivio/passt/inany.h:102:7: 6.3. path: Condition "__a->__in6_u.__u6_addr32[2] == __bswap_32(65535)", taking false branch. /home/sbrivio/passt/inany.h:102:2: 6.4. path: Condition "!({...; __a->__in6_u.__u6_addr32[0] == 0 && __a->__in6_u.__u6_addr32[1] == 0 && __a->__in6_u.__u6_addr32[2] == __bswap_32(65535);})", taking true branch. /home/sbrivio/passt/inany.h:103:3: 6.5. return_null: Explicitly returning null. /home/sbrivio/passt/conf.c:423:2: 7. dereference: Dereferencing "inany_v4(&a->addr)", which is known to be "NULL". /home/sbrivio/passt/inany.h:132:2: Examples where return value was checked for null 8. example_checked: Example 1: "inany_v4(addr)" has its value checked in "inany_v4(addr)". /home/sbrivio/passt/flow.c:875:7: Examples where return value was checked for null 9. example_checked: Example 2: "inany_v4(&side.eaddr)" has its value checked in "inany_v4(&side.eaddr)". /home/sbrivio/passt/inany.c:68:2: Examples where return value was checked for null 10. example_checked: Example 3: "inany_v4(src)" has its value checked in "v4 = inany_v4(src)". /home/sbrivio/passt/pif.c:40:2: Examples where return value was checked for null 11. example_assign: Example 4: Assigning: "v4" = return value from "inany_v4(addr)". /home/sbrivio/passt/pif.c:44:2: 12. example_checked: Example 4 (cont.): "v4" has its value checked in "v4". /home/sbrivio/passt/tcp.c:3722:2: Examples where return value was checked for null 13. example_checked: Example 5: "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)" has its value checked in "inany_v4(&conn->f.side[conn->f.pif[1] == PIF_TAP].oaddr)". --- and a few comments inline: On Wed, 14 Oct 2026 21:57:47 -0400 Jon Maloy wrote: > As preparation for supporting multiple addresses per interface, > we replace the single addr/prefix_len fields with an array. The > array consists of a new struct inany_addr_entry containing an > address and prefix length, both in inany_addr format. > > Despite some code refactoring, there are only two real functional > changes: > - The indicated IPv6 prefix length is now properly stored, instead > of being ignored and overridden with the hardcoded value 64, as > has been the case until now. > - Since even IPv4 addresses now are stored in IPv6 format, we > also store the corresponding prefix length in that format, > i.e. using the range [96,128] instead of [0,32]. > > In conf_ip6(), the explicit IN6_IS_ADDR_UNSPECIFIED() check on the > address at the end of the function is no longer needed: for > host-discovered addresses, it is now checked inside the if (!a) block > before calling fwd_set_addr(); for user-provided addresses (via -a), > validation already rejects unspecified addresses at parse time. > > Signed-off-by: Jon Maloy > > --- > v2: -Using inany_addr instead of protocol specific addresses as > entry address field. > > v3: -Merging into one array, directly in struct ctx > -Changed prefix_len and flags fields in struct inany_addr_entry > to uint8_t, since that makes the struct directly migratable > > v4: -Updated according to changes in previous commits > -Updated according to feedback from David G. > -Squashed IP4_MASK macro commit into this one > > v6: -Renamed and moved some definitions > -Introduced fwd_set_addr() and fwd_get_addr() already in this commit > -Eliminated first_v4/v6() functions, replaced with fwd_get_addr() > -Some other changes as suggested by David G. > -I kept the flag CONF_ADDR_LINKLOCAL, since it will be > needed later in an address selection function. > > v7: -Introduced CONF_ADDR_GENERATED flag > -Other fixes based on feedback from David and Stefano. > -I changed signature of inany_prefix_len(), but I did not change > its semantics, since the premise of David's comment is wrong: the > caller does *not* explicitly know he is dealing with an IPv4 address. > In fact, there are examples later in this series where it may be an > IPv6 address, and the caller just trusts he gets the return value in > the appropriate format. > -Intoduced the inverse of inany_prefix_len(), called inany_prefix_len6() > which always returns the prefix in IPv6 or mapped IPv4 format. > The name of the function isn't great, but any alternative I came up > with became too long to be practical. > > v8: -Added inany_prefix_v4() (David G.) to extract IPv4 address and > prefix length simultaneously, replacing inany_prefix_len(). > -Removed both inany_prefix_len() and inany_prefix_len6() > auto-detecting functions. Prefix length attached to an inany is > now always stored in IPv6 format (96-128 for IPv4-mapped). > -fwd_set_addr() now requires prefix_len in IPv6 format; callers > updated to pass prefix_len + 96 for IPv4 addresses. > -Fixed -n option to store prefix_len in IPv6 format. > -Fixed addr_seen regression: when address was set via -a, > addr_seen was not initialised in conf_ip4(). > -Replaced assert() with graceful no-op in dhcp.c and ndp.c when > no address is configured (David G.). > -Note: addr_seen is still initialised both at conf_ip*() time > and at DHCP/DHCPv6 send time (David's concern). This > inconsistency will be addressed in a follow-up. > -Introduced a new CONF_ADDR_ANY macro tobe used for address > selection, as suggested by Stefano. > -Deferred introduction of an explicit scope field (replacing > CONF_ADDR_LINKLOCAL flag) to a later commit or series. > > v9: -Rebased on changes made by David Gibson. > -Some minor changes based on feedback from David. > -Defined a new macro CONF_ADDR_GLOBAL to supplant CONF_ADDR_ANY. > -I did not follow up David's suggestion to introduce three > functions to replace the current fwd_get_addr(), because I > don't think it would solve any problems. The 'flags' field > encode both provenance and type properties orthogonally, and > callers need different combinations of them. That is the very > point of that field, and I see nothing confusing in it. > It is correct that some of the calls to fwd_get_addr() go away > in later commits, but enough of them remain to justify its > existence. Defining three new functions to replace it would only > make them wrappers around fwd_get_addr(), and serve no purpose in > my view. > > v10: -Redefined and refactored function fwd_get_addr(). > --- > arp.c | 12 ++++- > conf.c | 152 ++++++++++++++++++++++++++++++++----------------------- > dhcp.c | 20 ++++++-- > dhcpv6.c | 15 ++++-- > fwd.c | 111 +++++++++++++++++++++++++++++++++------- > fwd.h | 4 ++ > inany.h | 42 +++++++++++++++ > ip.h | 2 + > ndp.c | 19 +++++-- > passt.h | 60 +++++++++++++++++++--- > pasta.c | 23 ++++++--- > tap.c | 8 ++- > 12 files changed, 358 insertions(+), 110 deletions(-) > > diff --git a/arp.c b/arp.c > index bb042e95..9ee6ae65 100644 > --- a/arp.c > +++ b/arp.c > @@ -41,6 +41,8 @@ > static bool ignore_arp(const struct ctx *c, > const struct arphdr *ah, const struct arpmsg *am) > { > + const struct guest_addr *a; > + > if (ah->ar_hrd != htons(ARPHRD_ETHER) || > ah->ar_pro != htons(ETH_P_IP) || > ah->ar_hln != ETH_ALEN || > @@ -54,7 +56,8 @@ static bool ignore_arp(const struct ctx *c, > return true; > > /* Don't resolve the guest's assigned address, either. */ > - if (!memcmp(am->tip, &c->ip4.addr, sizeof(am->tip))) > + a = fwd_get_addr(c, AF_INET, true, 0); > + if (a && !memcmp(am->tip, inany_v4(&a->addr), sizeof(am->tip))) > return true; > > return false; > @@ -123,12 +126,17 @@ int arp(const struct ctx *c, struct iov_tail *data) > */ > void arp_send_init_req(const struct ctx *c) > { > + const struct guest_addr *a; > struct { > struct ethhdr eh; > struct arphdr ah; > struct arpmsg am; > } __attribute__((__packed__)) req; > > + a = fwd_get_addr(c, AF_INET, true, 0); > + if (!a) > + return; > + > /* Ethernet header */ > req.eh.h_proto = htons(ETH_P_ARP); > memcpy(req.eh.h_dest, MAC_BROADCAST, sizeof(req.eh.h_dest)); > @@ -145,7 +153,7 @@ void arp_send_init_req(const struct ctx *c) > memcpy(req.am.sha, c->our_tap_mac, sizeof(req.am.sha)); > memcpy(req.am.sip, &c->ip4.our_tap_addr, sizeof(req.am.sip)); > memcpy(req.am.tha, MAC_BROADCAST, sizeof(req.am.tha)); > - memcpy(req.am.tip, &c->ip4.addr, sizeof(req.am.tip)); > + memcpy(req.am.tip, inany_v4(&a->addr), sizeof(req.am.tip)); > > debug("Sending initial ARP request for guest MAC address"); > tap_send_single(c, &req, sizeof(req)); > diff --git a/conf.c b/conf.c > index 149a4c79..6fbdc86d 100644 > --- a/conf.c > +++ b/conf.c > @@ -372,13 +372,16 @@ static uint8_t conf_ip4_prefix(const char *arg) > > /** > * conf_ip4() - Verify or detect IPv4 support, get relevant addresses > + * @c: Execution context > * @ifi: Host interface to attempt (0 to determine one) > - * @ip4: IPv4 context (will be written) > * > * Return: interface index for IPv4, or 0 on failure. > */ > -static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) > +static unsigned int conf_ip4(struct ctx *c, unsigned int ifi) > { > + const struct guest_addr *a = fwd_get_addr(c, AF_INET, true, 0); > + struct ip4_ctx *ip4 = &c->ip4; > + > if (!ifi) > ifi = nl_get_ext_if(nl_sock, AF_INET); > > @@ -397,67 +400,69 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) > } > } > > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) { > + if (!a) { > + struct in_addr addr; > + int prefix_len; > int rc = nl_addr_get(nl_sock, ifi, AF_INET, > - &ip4->addr, &ip4->prefix_len, NULL); > + &addr, &prefix_len, NULL); > if (rc < 0) { > debug("Couldn't discover IPv4 address: %s", > strerror_(-rc)); > return 0; > } > + if (IN4_IS_ADDR_UNSPECIFIED(&addr)) { > + warn("Host returned address 0.0.0.0"); > + return 0; > + } I don't understand exactly if this is the part breaking local mode but... something does. Before this patch: $ pasta -- pasta --config-net true || echo oops No interfaces with usable IPv4 routes No interfaces with usable IPv6 routes IPv4: no external interface as template, use local mode IPv6: no external interface as template, use local mode With this patch: $ ./pasta -- ./pasta --config-net true || echo oops No interfaces with usable IPv4 routes No interfaces with usable IPv6 routes IPv4: no external interface as template, use local mode IPv6: no external interface as template, use local mode Couldn't set IPv4 route(s) in guest: Network is unreachable oops In general we shouldn't fail if there's no address configured. > + fwd_set_addr(c, &inany_from_v4(addr), CONF_ADDR_HOST, > + prefix_len + 96); > } > > - if (!ip4->prefix_len) { > - in_addr_t addr = ntohl(ip4->addr.s_addr); > - if (IN_CLASSA(addr)) > - ip4->prefix_len = (32 - IN_CLASSA_NSHIFT); > - else if (IN_CLASSB(addr)) > - ip4->prefix_len = (32 - IN_CLASSB_NSHIFT); > - else if (IN_CLASSC(addr)) > - ip4->prefix_len = (32 - IN_CLASSC_NSHIFT); > - else > - ip4->prefix_len = 32; > - } If you get rid of this fallback calculation, my understanding is that any IPv4 address configured via -a without a specific prefix length is a /32, correct? That would be an unintended functional change. > - > - ip4->addr_seen = ip4->addr; > - > + a = fwd_get_addr(c, AF_INET, true, 0); > + assert(a); > + ip4->addr_seen = *inany_v4(&a->addr); > ip4->our_tap_addr = ip4->guest_gw; > > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) > - return 0; > - > return ifi; > } > > /** > * conf_ip4_local() - Configure IPv4 addresses and attributes for local mode > - * @ip4: IPv4 context (will be written) > + * @c: Execution context (will be written) > */ > -static void conf_ip4_local(struct ip4_ctx *ip4) > +static void conf_ip4_local(struct ctx *c) > { > - if (IN4_IS_ADDR_UNSPECIFIED(&ip4->addr)) > - ip4->addr = IP4_LL_GUEST_ADDR; > - ip4->addr_seen = ip4->addr; > + const struct guest_addr *a = fwd_get_addr(c, AF_INET, true, 0); > + struct ip4_ctx *ip4 = &c->ip4; > + > + if (!a) { > + fwd_set_addr(c, &inany_from_v4(IP4_LL_GUEST_ADDR), > + CONF_ADDR_GENERATED, > + IP4_LL_PREFIX_LEN + 96); > + ip4->addr_seen = IP4_LL_GUEST_ADDR; > + } else { > + ip4->addr_seen = *inany_v4(&a->addr); > + } > > if (IN4_IS_ADDR_UNSPECIFIED(&ip4->guest_gw)) > ip4->guest_gw = IP4_LL_GUEST_GW; > ip4->our_tap_addr = ip4->guest_gw; > > - if (!ip4->prefix_len) > - ip4->prefix_len = IP4_LL_PREFIX_LEN; > - > ip4->no_copy_addrs = ip4->no_copy_routes = true; > } > > /** > * conf_ip6() - Verify or detect IPv6 support, get relevant addresses > + * @c: Execution context > * @ifi: Host interface to attempt (0 to determine one) > - * @ip6: IPv6 context (will be written) > * > * Return: interface index for IPv6, or 0 on failure. > */ > -static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) > +static unsigned int conf_ip6(struct ctx *c, unsigned int ifi) > { > + struct ip6_ctx *ip6 = &c->ip6; > + const struct guest_addr *a; > + union inany_addr addr; > int prefix_len = 0; > int rc; > > @@ -478,23 +483,30 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) > } > } > > - rc = nl_addr_get(nl_sock, ifi, AF_INET6, > - IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL, > + rc = nl_addr_get(nl_sock, ifi, AF_INET6, &addr.a6, > &prefix_len, &ip6->our_tap_ll); > if (rc < 0) { > debug("Couldn't discover IPv6 address: %s", strerror_(-rc)); > return 0; > } > > - ip6->addr_seen = ip6->addr; > + a = fwd_get_addr(c, AF_INET6, true, 0); > + if (!a) { > + if (IN6_IS_ADDR_UNSPECIFIED(&addr)) > + return 0; > + > + fwd_set_addr(c, &addr, CONF_ADDR_HOST, prefix_len); > + ip6->addr_seen = addr.a6; > + } else { > + ip6->addr_seen = a->addr.a6; > + } > > if (IN6_IS_ADDR_LINKLOCAL(&ip6->guest_gw)) > ip6->our_tap_ll = ip6->guest_gw; > else > ip6->our_tap_addr = ip6->guest_gw; > > - if (IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) || > - IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) > + if (IN6_IS_ADDR_UNSPECIFIED(&ip6->our_tap_ll)) > return 0; > > return ifi; > @@ -502,10 +514,12 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) > > /** > * conf_ip6_local() - Configure IPv6 addresses and attributes for local mode > - * @ip6: IPv6 context (will be written) > + * @c: Execution context (will be written) > */ > -static void conf_ip6_local(struct ip6_ctx *ip6) > +static void conf_ip6_local(struct ctx *c) > { > + struct ip6_ctx *ip6 = &c->ip6; > + > if (IN6_IS_ADDR_UNSPECIFIED(&ip6->guest_gw)) > ip6->guest_gw = IP6_LL_GUEST_GW; > > @@ -801,6 +815,7 @@ enum passt_modes conf_mode(int argc, char *argv[]) > static void conf_print(const struct ctx *c) > { > char buf[INANY_ADDRSTRLEN]; > + const struct guest_addr *a; > int i; > > if (c->fd_control_listen >= 0) > @@ -848,16 +863,19 @@ static void conf_print(const struct ctx *c) > inet_ntop(AF_INET, &c->ip4.map_host_loopback, > buf, sizeof(buf))); > > - if (!c->no_dhcp) { > + a = fwd_get_addr(c, AF_INET, true, 0); > + if (a && !c->no_dhcp) { > uint32_t mask; > + int plen = 0; > > - mask = htonl(0xffffffff << (32 - c->ip4.prefix_len)); > + inany_prefix_v4(&a->addr, a->prefix_len, &plen); > + mask = IN4_MASK(plen); > > info("DHCP:"); > info(" assign: %s", > - inet_ntop(AF_INET, &c->ip4.addr, buf, sizeof(buf))); > + inany_ntop(&a->addr, buf, sizeof(buf))); > info(" mask: %s", > - inet_ntop(AF_INET, &mask, buf, sizeof(buf))); > + inet_ntop(AF_INET, &mask, buf, sizeof(buf))); > info(" router: %s", > inet_ntop(AF_INET, &c->ip4.guest_gw, > buf, sizeof(buf))); > @@ -868,8 +886,8 @@ static void conf_print(const struct ctx *c) > break; > if (!i) > info("DNS:"); > - inet_ntop(AF_INET, &c->ip4.dns[i], buf, sizeof(buf)); > - info(" %s", buf); > + info(" %s", inet_ntop(AF_INET, &c->ip4.dns[i], > + buf, sizeof(buf))); Unrelated change? > } > > for (i = 0; *c->dns_search[i].n; i++) { > @@ -894,13 +912,14 @@ static void conf_print(const struct ctx *c) > else > goto dns6; > > - info(" assign: %s", > - inet_ntop(AF_INET6, &c->ip6.addr, buf, sizeof(buf))); > + a = fwd_get_addr(c, AF_INET6, true, 0); > + if (a) > + info(" assign: %s", > + inany_ntop(&a->addr, buf, sizeof(buf))); > info(" router: %s", > inet_ntop(AF_INET6, &c->ip6.guest_gw, buf, sizeof(buf))); > info(" our link-local: %s", > - inet_ntop(AF_INET6, &c->ip6.our_tap_ll, > - buf, sizeof(buf))); > + inet_ntop(AF_INET6, &c->ip6.our_tap_ll, buf, sizeof(buf))); > > dns6: > for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) { > @@ -908,8 +927,10 @@ dns6: > break; > if (!i) > info("DNS:"); > - inet_ntop(AF_INET6, &c->ip6.dns[i], buf, sizeof(buf)); > - info(" %s", buf); > + info(" %s", > + inet_ntop(AF_INET6, &c->ip6.dns[i], > + buf, sizeof(buf))); Same here. > + > } > > for (i = 0; *c->dns_search[i].n; i++) { > @@ -1213,15 +1234,13 @@ static bool conf_addr(struct ctx *c, char *arg, uint8_t opt_n) > inany_is_loopback(&addr)) > goto bad; > > + fwd_set_addr(c, &addr, CONF_ADDR_USER, prefix_len); > if (a4) { > - c->ip4.addr = *a4; > - c->ip4.prefix_len = prefix_len - 96; > - c->ip4.addr_fixed = true; > c->ip4.no_copy_addrs = true; > + c->ip4.addr_fixed = true; > } else { > - c->ip6.addr = addr.a6; > - c->ip6.addr_fixed = true; > c->ip6.no_copy_addrs = true; > + c->ip6.addr_fixed = true; > } > > return is_prefix; > @@ -1648,13 +1667,20 @@ void conf(struct ctx *c, int argc, char **argv) > case 'a': > opt_a_is_prefix = conf_addr(c, optarg, opt_n); > break; > - case 'n': > + case 'n': { See https://archives.passt.top/passt-dev/20261008000617.0f8f22a9@elisabeth/ for some thoughts on this. In general I think we should avoid introducing extra blocks unless strictly necessary (there are a few cases where we need that). Otherwise we should do it consistently... and that would look pretty bad, I reckon. > + struct guest_addr *a; > + > if (opt_a_is_prefix) > die("Redundant prefix length specification"); > > - c->ip4.prefix_len = conf_ip4_prefix(optarg); > - opt_n = c->ip4.prefix_len + 96; > + opt_n = conf_ip4_prefix(optarg) + 96; > + > + for_each_addr(a, c->addrs, c->addr_count, AF_INET) { > + a->prefix_len = opt_n; > + break; > + } > break; > + } > case 'M': > p = optarg; > if (!parse_mac(&p, c->our_tap_mac) || > @@ -1853,9 +1879,9 @@ void conf(struct ctx *c, int argc, char **argv) > > nl_sock_init(c, false); > if (!v6_only && !c->splice_only) > - c->ifi4 = conf_ip4(ifi4, &c->ip4); > + c->ifi4 = conf_ip4(c, ifi4); > if (!v4_only && !c->splice_only) > - c->ifi6 = conf_ip6(ifi6, &c->ip6); > + c->ifi6 = conf_ip6(c, ifi6); > > if (c->ifi4 && c->mtu < IPV4_MIN_MTU) { > warn("MTU %"PRIu16" is too small for IPv4 (minimum %u)", > @@ -1878,7 +1904,7 @@ void conf(struct ctx *c, int argc, char **argv) > if (!c->ifi4 && !v6_only) { > if (!c->splice_only) { > info("IPv4: no external interface as template, use local mode"); > - conf_ip4_local(&c->ip4); > + conf_ip4_local(c); > } > c->ifi4 = -1; > } > @@ -1886,7 +1912,7 @@ void conf(struct ctx *c, int argc, char **argv) > if (!c->ifi6 && !v4_only) { > if (!c->splice_only) { > info("IPv6: no external interface as template, use local mode"); > - conf_ip6_local(&c->ip6); > + conf_ip6_local(c); > } > c->ifi6 = -1; > } > @@ -1959,7 +1985,7 @@ void conf(struct ctx *c, int argc, char **argv) > if (!c->ifi6) { > c->no_ndp = 1; > c->no_dhcpv6 = 1; > - } else if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) { > + } else if (!fwd_get_addr(c, AF_INET6, true, 0)) { > c->no_dhcpv6 = 1; > } > > diff --git a/dhcp.c b/dhcp.c > index c3c74224..17eebc4a 100644 > --- a/dhcp.c > +++ b/dhcp.c > @@ -303,7 +303,9 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len) > int dhcp(const struct ctx *c, struct iov_tail *data) > { > char macstr[ETH_ADDRSTRLEN]; > + const struct guest_addr *a; > size_t mlen, dlen, opt_len; > + const struct in_addr *addr; > struct in_addr mask, dst; > struct ethhdr eh_storage; > struct iphdr iph_storage; > @@ -315,6 +317,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > struct msg const *m; > struct msg reply; > unsigned int i; > + int plen; > > eh = IOV_REMOVE_HEADER(data, eh_storage); > iph = IOV_PEEK_HEADER(data, iph_storage); > @@ -344,6 +347,15 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > m->op != BOOTREQUEST) > return -1; > > + a = fwd_get_addr(c, AF_INET, true, CONF_ADDR_USER); > + if (!a) > + a = fwd_get_addr(c, AF_INET, true, 0); > + if (!a) > + return -1; > + > + addr = inany_prefix_v4(&a->addr, a->prefix_len, &plen); > + assert(addr); > + > reply.op = BOOTREPLY; > reply.htype = m->htype; > reply.hlen = m->hlen; > @@ -352,7 +364,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > reply.secs = 0; > reply.flags = m->flags; > reply.ciaddr = m->ciaddr; > - reply.yiaddr = c->ip4.addr; > + reply.yiaddr = *addr; > reply.siaddr = 0; > reply.giaddr = m->giaddr; > memcpy(&reply.chaddr, m->chaddr, sizeof(reply.chaddr)); > @@ -408,7 +420,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > > info(" from %s", eth_ntop(m->chaddr, macstr, sizeof(macstr))); > > - mask.s_addr = htonl(0xffffffff << (32 - c->ip4.prefix_len)); > + mask.s_addr = IN4_MASK(plen); > memcpy(opts[1].s, &mask, sizeof(mask)); > memcpy(opts[3].s, &c->ip4.guest_gw, sizeof(c->ip4.guest_gw)); > memcpy(opts[54].s, &c->ip4.our_tap_addr, sizeof(c->ip4.our_tap_addr)); > @@ -416,7 +428,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > /* If the gateway is not on the assigned subnet, send an option 121 > * (Classless Static Routing) adding a dummy route to it. > */ > - if ((c->ip4.addr.s_addr & mask.s_addr) > + if ((addr->s_addr & mask.s_addr) > != (c->ip4.guest_gw.s_addr & mask.s_addr)) { > /* a.b.c.d/32:0.0.0.0, 0:a.b.c.d */ > opts[121].slen = 14; > @@ -475,7 +487,7 @@ int dhcp(const struct ctx *c, struct iov_tail *data) > if (m->flags & FLAG_BROADCAST) > dst = in4addr_broadcast; > else > - dst = c->ip4.addr; > + dst = *addr; > > tap_udp4_send(c, c->ip4.our_tap_addr, 67, dst, 68, &reply, dlen); > > diff --git a/dhcpv6.c b/dhcpv6.c > index 29c7e320..5fbba7cf 100644 > --- a/dhcpv6.c > +++ b/dhcpv6.c > @@ -318,7 +318,7 @@ static bool dhcpv6_opt(struct iov_tail *data, uint16_t type) > * false otherwise and @data is unmodified > */ > static bool dhcpv6_ia_notonlink(struct iov_tail *data, > - struct in6_addr *la) > + const struct in6_addr *la) > { > int ia_types[2] = { OPT_IA_NA, OPT_IA_TA }; > struct opt_ia_addr opt_addr_storage; > @@ -567,6 +567,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > struct opt_hdr client_id_storage; > /* cppcheck-suppress [variableScope,unmatchedSuppression] */ > struct opt_ia_na ia_storage; > + const struct guest_addr *a; > const struct in6_addr *src; > struct msg_hdr mh_storage; > const struct msg_hdr *mh; > @@ -574,6 +575,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > const struct udphdr *uh; > size_t mlen, n; > > + a = fwd_get_addr(c, AF_INET6, true, 0); > + > uh = IOV_REMOVE_HEADER(data, uh_storage); > if (!uh) > return -1; > @@ -627,7 +630,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > if (mh->type == TYPE_CONFIRM && server_id) > return -1; > > - if (dhcpv6_ia_notonlink(data, &c->ip6.addr)) { > + if (a && dhcpv6_ia_notonlink(data, &a->addr.a6)) { > > dhcpv6_send_ia_notonlink(c, saddr, data, > &client_id_base, > @@ -682,7 +685,8 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > resp.hdr.xid = mh->xid; > > tap_udp6_send(c, src, 547, saddr, 546, mh->xid, &resp, n); > - c->ip6.addr_seen = c->ip6.addr; > + if (a) > + c->ip6.addr_seen = a->addr.a6; > > return 1; > } > @@ -694,6 +698,7 @@ int dhcpv6(struct ctx *c, struct iov_tail *data, > void dhcpv6_init(const struct ctx *c) > { > time_t y2k = 946684800; /* Epoch to 2000-01-01T00:00:00Z, no mktime() */ > + const struct guest_addr *a; > uint32_t duid_time; > > duid_time = htonl(difftime(time(NULL), y2k)); > @@ -706,5 +711,7 @@ void dhcpv6_init(const struct ctx *c) > memcpy(resp_not_on_link.server_id.duid_lladdr, > c->our_tap_mac, sizeof(c->our_tap_mac)); > > - resp.ia_addr.addr = c->ip6.addr; > + a = fwd_get_addr(c, AF_INET6, true, 0); > + if (a) > + resp.ia_addr.addr = a->addr.a6; > } > diff --git a/fwd.c b/fwd.c > index 5679a3d6..574db1b3 100644 > --- a/fwd.c > +++ b/fwd.c > @@ -243,6 +243,61 @@ void fwd_neigh_table_init(const struct ctx *c) > fwd_neigh_table_update(c, &mga, c->our_tap_mac, true); > } > > +/** > + * fwd_set_addr() - Add or update an address in the unified address array > + * @c: Execution context > + * @addr: Address to add (IPv4-mapped or IPv6) > + * @flags: CONF_ADDR_* flags for this address > + * @prefix_len: Prefix length in IPv6 format (96-128 for IPv4-mapped) > + * > + * Find the first existing entry of the same address family and > + * overwrite it, or create a new one if none exists > + */ > +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, > + uint8_t flags, int prefix_len) > +{ > + struct guest_addr *a; > + > + for_each_addr(a, c->addrs, c->addr_count, inany_af(addr)) { No need for curly brackets. > + goto found; > + } > + > + if (c->addr_count >= MAX_GUEST_ADDRS) > + return; > + > + a = &c->addrs[c->addr_count++]; > + > +found: > + a->addr = *addr; > + a->prefix_len = prefix_len; > + a->flags = flags; > +} > + > +/** > + * fwd_get_addr() - Get guest address entry matching criteria > + * @c: Execution context > + * @af: Address family (AF_INET, AF_INET6, or AF_UNSPEC for any) > + * @global: If true, match global only; if false, match link-local only > + * @flags: Required provenance flags (any-match), 0 for any provenance > + * > + * Return: first address entry matching criteria, or NULL > + */ > +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af, > + bool global, uint8_t flags) I think I suggested this in an early revision but I didn't get a reply: wouldn't it be convenient to have '0' defined as some kind of CONF_ADDR_ANY value? Callers mostly pass '0' there, and having a specific define for that would make things clearer. > +{ > + const struct guest_addr *a; > + > + for_each_addr(a, c->addrs, c->addr_count, af) { > + if (inany_is_linklocal(&a->addr) == global) > + continue; > + if (flags && !(a->flags & flags)) > + continue; > + return a; > + } > + > + return NULL; > +} > + > /* Forwarding table storage, generally accessed via pointers in struct ctx */ > static struct fwd_table fwd_in; > static struct fwd_table fwd_out; > @@ -799,8 +854,10 @@ static bool is_dns_flow(uint8_t proto, const struct flowside *ini) > * translation, false otherwise > */ > static bool fwd_guest_accessible4(const struct ctx *c, > - const struct in_addr *addr) > + const struct in_addr *addr) > { > + const struct guest_addr *a; > + > if (IN4_IS_ADDR_LOOPBACK(addr)) > return false; > > @@ -815,7 +872,8 @@ static bool fwd_guest_accessible4(const struct ctx *c, > /* For IPv4, addr_seen is initialised to addr, so is always a valid > * address > */ > - if (IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr) || > + a = fwd_get_addr(c, AF_INET, true, 0); > + if ((a && IN4_ARE_ADDR_EQUAL(addr, inany_v4(&a->addr))) || > IN4_ARE_ADDR_EQUAL(addr, &c->ip4.addr_seen)) > return false; > > @@ -833,10 +891,13 @@ static bool fwd_guest_accessible4(const struct ctx *c, > static bool fwd_guest_accessible6(const struct ctx *c, > const struct in6_addr *addr) > { > + const struct guest_addr *a; > + > if (IN6_IS_ADDR_LOOPBACK(addr)) > return false; > > - if (IN6_ARE_ADDR_EQUAL(addr, &c->ip6.addr)) > + a = fwd_get_addr(c, AF_INET6, true, 0); > + if (a && IN6_ARE_ADDR_EQUAL(addr, &a->addr.a6)) > return false; > > /* For IPv6, addr_seen starts unspecified, because we don't know what LL > @@ -881,16 +942,21 @@ static bool fwd_guest_accessible(const struct ctx *c, > static void nat_outbound(const struct ctx *c, const union inany_addr *addr, > union inany_addr *translated) > { > - if (inany_equals4(addr, &c->ip4.map_host_loopback)) > + const struct guest_addr *ga; > + > + if (inany_equals4(addr, &c->ip4.map_host_loopback)) { > *translated = inany_loopback4; > - else if (inany_equals6(addr, &c->ip6.map_host_loopback)) > + } else if (inany_equals6(addr, &c->ip6.map_host_loopback)) { > *translated = inany_loopback6; > - else if (inany_equals4(addr, &c->ip4.map_guest_addr)) > - *translated = inany_from_v4(c->ip4.addr); > - else if (inany_equals6(addr, &c->ip6.map_guest_addr)) > - translated->a6 = c->ip6.addr; > - else > + } else if (inany_equals4(addr, &c->ip4.map_guest_addr)) { > + ga = fwd_get_addr(c, AF_INET, true, 0); > + *translated = ga ? ga->addr : inany_any4; > + } else if (inany_equals6(addr, &c->ip6.map_guest_addr)) { > + ga = fwd_get_addr(c, AF_INET6, true, 0); > + translated->a6 = ga ? ga->addr.a6 : in6addr_any; > + } else { > *translated = *addr; > + } > } > > /** > @@ -995,16 +1061,23 @@ bool nat_inbound(const struct ctx *c, const union inany_addr *addr, > } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback) && > inany_equals6(addr, &in6addr_loopback)) { > translated->a6 = c->ip6.map_host_loopback; > - } else if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && > - inany_equals4(addr, &c->ip4.addr)) { > - *translated = inany_from_v4(c->ip4.map_guest_addr); > - } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) && > - inany_equals6(addr, &c->ip6.addr)) { > - translated->a6 = c->ip6.map_guest_addr; > - } else if (fwd_guest_accessible(c, addr)) { > - *translated = *addr; > } else { > - return false; > + const struct guest_addr *ga4, *ga6; > + > + ga4 = fwd_get_addr(c, AF_INET, true, 0); > + ga6 = fwd_get_addr(c, AF_INET6, true, 0); > + > + if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_guest_addr) && > + ga4 && inany_equals(addr, &ga4->addr)) { > + *translated = inany_from_v4(c->ip4.map_guest_addr); > + } else if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_guest_addr) && > + ga6 && inany_equals(addr, &ga6->addr)) { > + translated->a6 = c->ip6.map_guest_addr; > + } else if (fwd_guest_accessible(c, addr)) { > + *translated = *addr; > + } else { > + return false; > + } > } > > return true; > diff --git a/fwd.h b/fwd.h > index b60697d9..d001d8ae 100644 > --- a/fwd.h > +++ b/fwd.h > @@ -79,5 +79,9 @@ void fwd_neigh_table_free(const struct ctx *c, > void fwd_neigh_mac_get(const struct ctx *c, const union inany_addr *addr, > uint8_t *mac); > void fwd_neigh_table_init(const struct ctx *c); > +void fwd_set_addr(struct ctx *c, const union inany_addr *addr, > + uint8_t flags, int prefix_len); > +const struct guest_addr *fwd_get_addr(const struct ctx *c, sa_family_t af, > + bool global, uint8_t flags); > > #endif /* FWD_H */ > diff --git a/inany.h b/inany.h > index d0befb15..4f2331ee 100644 > --- a/inany.h > +++ b/inany.h > @@ -104,6 +104,34 @@ static inline struct in_addr *inany_v4(const union inany_addr *addr) > return (struct in_addr *)&addr->v4mapped.a4; > } > > +/** inany_prefix_v4() - Extract IPv4 address and prefix length from inany > + * @addr: IPv4 (as mapped) or IPv6 address > + * @prefix_len: Prefix length in IPv6 format (96-128 for IPv4) > + * @v4_plen: Set to IPv4 prefix length (0-32) on success > + * > + * Return: IPv4 address if @addr is IPv4, NULL otherwise > + */ > +static inline struct in_addr *inany_prefix_v4(const union inany_addr *addr, > + int prefix_len, int *v4_plen) > +{ > + struct in_addr *v4 = inany_v4(addr); > + > + if (!v4) > + return NULL; We usually try to make early returns and returns in general a bit more apparent, that is, we would usually have an extra newline here... > + *v4_plen = prefix_len - 96; ...and here too (but this isn't done consistently). > + return v4; > +} > + > +/** inany_af - Get address family of IPv[46] address inany_af() (it's a function) > + * @addr: IPv4 or IPv6 address > + * > + * Return: AF_INET for IPv4, AF_INET6 for IPv6 > + */ > +static inline sa_family_t inany_af(const union inany_addr *addr) > +{ > + return inany_v4(addr) ? AF_INET : AF_INET6; > +} > + > /** inany_default_prefix_len() - Get default prefix length for address > * @addr: IPv4 or iPv6 address > * > @@ -230,6 +258,20 @@ static inline bool inany_is_linklocal6(const union inany_addr *a) > return IN6_IS_ADDR_LINKLOCAL(&a->a6); > } > > +/** inany_is_linklocal() - Check if address is link-local > + * @a: IPv[46] address > + * > + * Return: true if @a is in 169.254.0.0/16 or fe80::/10 > + */ > +static inline bool inany_is_linklocal(const union inany_addr *a) > +{ > + const struct in_addr *v4 = inany_v4(a); > + > + if (v4) > + return IN4_IS_ADDR_LINKLOCAL(v4); Same here, extra newline for consistency. > + return IN6_IS_ADDR_LINKLOCAL(&a->a6); > +} > + > /** inany_is_multicast() - Check if address is multicast or broadcast > * @a: IPv[46] address > * > diff --git a/ip.h b/ip.h > index aab9b86a..ac93c543 100644 > --- a/ip.h > +++ b/ip.h > @@ -19,6 +19,8 @@ > (ntohl(((struct in_addr *)(a))->s_addr) >> IN_CLASSA_NSHIFT == IN_LOOPBACKNET) > #define IN4_IS_ADDR_MULTICAST(a) \ > (IN_MULTICAST(ntohl(((struct in_addr *)(a))->s_addr))) > +#define IN4_MASK(prefix) \ > + ((prefix) <= 0 ? 0 : htonl(0xffffffff << (32 - (prefix)))) > #define IN4_ARE_ADDR_EQUAL(a, b) \ > (((struct in_addr *)(a))->s_addr == ((struct in_addr *)b)->s_addr) > #define IN4ADDR_LOOPBACK_INIT \ > diff --git a/ndp.c b/ndp.c > index 1f2bcb0c..ac1325e1 100644 > --- a/ndp.c > +++ b/ndp.c > @@ -257,7 +257,6 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) > .valid_lifetime = ~0U, > .pref_lifetime = ~0U, > }, > - .prefix = c->ip6.addr, > .source_ll = { > .header = { > .type = OPT_SRC_L2_ADDR, > @@ -265,8 +264,15 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) > }, > }, > }; > + const struct guest_addr *a; > unsigned char *ptr = NULL; > > + a = fwd_get_addr(c, AF_INET6, true, 0); > + if (!a) > + return; > + > + ra.prefix = a->addr.a6; > + > ptr = &ra.var[0]; > > if (c->mtu) { > @@ -460,6 +466,7 @@ first: > */ > void ndp_send_init_req(const struct ctx *c) > { > + const struct guest_addr *a; > struct ndp_ns ns = { > .ih = { > .icmp6_type = NS, > @@ -468,8 +475,14 @@ void ndp_send_init_req(const struct ctx *c) > .icmp6_solicited = 0, /* Reserved */ > .icmp6_override = 0, /* Reserved */ > }, > - .target_addr = c->ip6.addr > + .target_addr = IN6ADDR_ANY_INIT > }; > + > + a = fwd_get_addr(c, AF_INET6, true, 0); > + if (!a) > + return; > + > + ns.target_addr = a->addr.a6; > debug("Sending initial NDP NS request for guest MAC address"); > - ndp_send(c, &c->ip6.addr, &ns, sizeof(ns)); > + ndp_send(c, &a->addr.a6, &ns, sizeof(ns)); > } > diff --git a/passt.h b/passt.h > index afd8e9f0..47e25d5c 100644 > --- a/passt.h > +++ b/passt.h > @@ -64,11 +64,27 @@ enum passt_modes { > MODE_VU, > }; > > +/* Maximum number of addresses in context address array */ > +#define MAX_GUEST_ADDRS 32 > + > +/** > + * struct guest_addr - Unified IPv4/IPv6 address entry > + * @addr: IPv4 (as mapped) or IPv6 address > + * @prefix_len: Prefix length in IPv6/IPv4-mapped [0,128]/[96,128] format > + * @flags: CONF_ADDR_* flags > + */ > +struct guest_addr { > + union inany_addr addr; > + uint8_t prefix_len; > + uint8_t flags; > +#define CONF_ADDR_USER BIT(0) /* User set via -a */ > +#define CONF_ADDR_HOST BIT(1) /* From host interface */ > +#define CONF_ADDR_GENERATED BIT(2) /* Generated by PASST/PASTA */ > +}; > + > /** > * struct ip4_ctx - IPv4 execution context > - * @addr: IPv4 address assigned to guest > * @addr_seen: Latest IPv4 address seen as source from tap > - * @prefixlen: IPv4 prefix length (netmask) > * @guest_gw: IPv4 gateway as seen by the guest > * @map_host_loopback: Outbound connections to this address are NATted to the > * host's 127.0.0.1 > @@ -87,9 +103,7 @@ enum passt_modes { > */ > struct ip4_ctx { > /* PIF_TAP addresses */ > - struct in_addr addr; > struct in_addr addr_seen; > - int prefix_len; > struct in_addr guest_gw; > struct in_addr map_host_loopback; > struct in_addr map_guest_addr; > @@ -110,7 +124,6 @@ struct ip4_ctx { > > /** > * struct ip6_ctx - IPv6 execution context > - * @addr: IPv6 address assigned to guest > * @addr_seen: Latest IPv6 global/site address seen as source from tap > * @addr_ll_seen: Latest IPv6 link-local address seen as source from tap > * @guest_gw: IPv6 gateway as seen by the guest > @@ -132,7 +145,6 @@ struct ip4_ctx { > */ > struct ip6_ctx { > /* PIF_TAP addresses */ > - struct in6_addr addr; > struct in6_addr addr_seen; > struct in6_addr addr_ll_seen; > struct in6_addr guest_gw; > @@ -192,6 +204,8 @@ struct ip6_ctx { > * @fqdn: Guest FQDN > * @ifi6: Template interface for IPv6, -1: none, 0: IPv6 disabled > * @ip6: IPv6 configuration > + * @addrs: Unified address array for both IPv4 (mapped) and IPv6 > + * @addr_count: Number of active entries in @addrs array > * @pasta_ifn: Name of namespace interface for pasta > * @pasta_ifi: Index of namespace interface for pasta > * @pasta_conf_ns: Configure namespace after creating it > @@ -276,6 +290,9 @@ struct ctx { > int ifi6; > struct ip6_ctx ip6; > > + struct guest_addr addrs[MAX_GUEST_ADDRS]; > + int addr_count; > + > char pasta_ifn[IF_NAMESIZE]; > unsigned int pasta_ifi; > int pasta_conf_ns; > @@ -321,6 +338,37 @@ struct ctx { > > extern struct ctx passt_ctx; > > +/** > + * next_addr_idx_() - Find next address index matching family filter > + * @addrs: Array of guest addresses > + * @count: Number of addresses in array > + * @i: Starting index > + * @af: Address family filter: AF_INET, AF_INET6, or AF_UNSPEC for all > + * > + * Return: next matching index, or count if none found > + */ > +static inline int next_addr_idx_(const struct guest_addr *addrs, int count, > + int i, sa_family_t af) > +{ > + for (; i < count; i++) { > + if (af == AF_UNSPEC || af == inany_af(&addrs[i].addr)) > + return i; > + } > + return i; > +} > + > +/** > + * for_each_addr() - Iterate over addresses in array > + * @a: Pointer variable for current entry (struct guest_addr *) > + * @addrs: Array of guest addresses (e.g., c->addrs) > + * @count: Number of addresses (e.g., c->addr_count) > + * @af: Address family filter: AF_INET, AF_INET6, or 0 for all > + */ > +#define for_each_addr(a, addrs, count, af) \ > + for (int i_ = next_addr_idx_((addrs), (count), 0, (af)); \ > + i_ < (count) && ((a) = &(addrs)[i_], true); \ > + i_ = next_addr_idx_((addrs), (count), i_ + 1, (af))) If I understand correctly, next_addr_idx_() is a trick to avoid an explicit if clause with a 'continue' statement here. I think it would be more readable like that (assuming it's doable), though, and we consistently do that in iterator macros. > + > void proto_update_l2_buf(const unsigned char *eth_d); > > #endif /* PASST_H */ > diff --git a/pasta.c b/pasta.c > index 10f3ec8d..24427ddb 100644 > --- a/pasta.c > +++ b/pasta.c > @@ -299,23 +299,32 @@ void pasta_start_ns(struct ctx *c, int argc, char *argv[]) > * > * Return: 0 on success, negative error code on failure > */ > -static int pasta_conf_addrs(struct ctx *c, sa_family_t af, int ifi, > +static int pasta_conf_addrs(const struct ctx *c, sa_family_t af, int ifi, > bool no_copy) > { > + const struct guest_addr *a; > + > if (!ifi) > return 0; > > if (!no_copy) > return nl_addr_dup(nl_sock, ifi, nl_sock_ns, c->pasta_ifi, af); > > - if (af == AF_INET && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr)) > + a = fwd_get_addr(c, af, true, 0); > + if (!a) > + return 0; > + > + if (af == AF_INET) { > + const struct in_addr *v4; > + int plen = 0; > + > + v4 = inany_prefix_v4(&a->addr, a->prefix_len, &plen); > return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET, > - &c->ip4.addr, c->ip4.prefix_len); > - if (af == AF_INET6 && !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) > - return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET6, > - &c->ip6.addr, 64); > + v4, plen); > + } > > - return 0; > + return nl_addr_set(nl_sock_ns, c->pasta_ifi, AF_INET6, > + &a->addr.a6, a->prefix_len); > } > > /** > diff --git a/tap.c b/tap.c > index dfa66c71..604b54ca 100644 > --- a/tap.c > +++ b/tap.c > @@ -1005,8 +1005,12 @@ resume: > c->ip6.addr_seen = *saddr; > } > > - if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr)) > - c->ip6.addr = *saddr; > + if (!fwd_get_addr(c, AF_INET6, true, 0) && > + !fwd_get_addr(c, AF_INET6, false, 0)) { > + union inany_addr addr = { .a6 = *saddr }; > + > + fwd_set_addr(c, &addr, CONF_ADDR_GENERATED, 64); > + } > } else if (!c->ip6.addr_fixed && > !IN6_IS_ADDR_UNSPECIFIED(saddr)) { > c->ip6.addr_seen = *saddr; -- Stefano