From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 30FFA4563A; Thu, 18 Jul 2024 23:15:09 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C2BA40662; Thu, 18 Jul 2024 23:15:09 +0200 (CEST) Received: from mail-pf1-f182.google.com (mail-pf1-f182.google.com [209.85.210.182]) by mails.dpdk.org (Postfix) with ESMTP id 1BCEC40616 for ; Thu, 18 Jul 2024 23:15:06 +0200 (CEST) Received: by mail-pf1-f182.google.com with SMTP id d2e1a72fcca58-70b04cb28acso217352b3a.0 for ; Thu, 18 Jul 2024 14:15:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1721337306; x=1721942106; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=ns3pQ29oZy9N3Ai6tv+J6VqveGz0rsu4oMXGek5/DkI=; b=a7og8zpQHHYjRypX+diTFokTj72fzfgBZNqL+FRHqk6zvEYB3ToxAqCa8nRlvIsQDn CHuIRg5l4B6Ey5jmvW1PFJrQSlzWwgMSZ7atbs7aXAd/uefpufANyHsto9sAynXgoiET ujEXK9iFkIMDG+pC8KKov4cg5gCWO7PJE8s1jjV7JsJugfjKUkZw18kfGBKkMhDbgfN0 hwZjlNllgdf0QWvjFinWo9LaeBgEzjI3BgewQ1JufdSd/PQqPulXyEExguWCQ56ctDYc j3jIjSfpi/GExhdI+nkyaiW1B0j+z0FkOru1+6yS9TxDLB6AMjbJKSZAHDHP6GH9qvCD Np6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721337306; x=1721942106; h=content-transfer-encoding:mime-version: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=ns3pQ29oZy9N3Ai6tv+J6VqveGz0rsu4oMXGek5/DkI=; b=RrGGlkIFow1wj5Hj9sQsDQ3SXkGlY/qdsa/4BuM1YS2idszUqFB7dGNHclEHTx4eGO 6qx1VKQNOAuHkPhnaOFJwABInjl8T127gRMV3VjwdcRmFfIpivUJTfMLcMY1+QdcmhtG 95XeIFdR3rU/KKMAD+8hRMLn/7EYJkEVfY47nVMWBtAgHQZT5+VDFwFav47dsjo7FC4C 26kinRMYEUaXQ9eNw64AxeUJ33UxsSHigcATCu/LRr+yUID2afByICOB9Xw2BUfJY6+0 kb5YSTT+J0DSel2fxbh3npnBKcAwmK7lkyRtQUu9ZjNSgtpl5/SEEYa6jJSSUFXpOaEN RNGQ== X-Forwarded-Encrypted: i=1; AJvYcCUEVA0L/P4m2IquNValwXPUPAVao9X2++3wTFiqB+YY1EZOGyNbpdQGnmbjyJqqS1NvRLTAnjBMSOk1nGI= X-Gm-Message-State: AOJu0Yzj7EfgL2V432B9YK+vtEUFpnlouJ/ul5ibAoycZBeTc1UFdQ5s se3mIf+rh3Z4F4xDh+sV3cgJaE9UVMNn7sqDTfXY95uNL5zWtHHeF7/YgTLmtiM= X-Google-Smtp-Source: AGHT+IEcHaSY+EmjPPr9GsjVU3iaa/UKEQflW0socxn3G2dCY71x9i4ew4/qtrl5lQqIlBluv9x/VQ== X-Received: by 2002:a05:6a00:14c5:b0:705:9669:af1f with SMTP id d2e1a72fcca58-70cfd5a77d1mr846747b3a.10.1721337305884; Thu, 18 Jul 2024 14:15:05 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-70cfc8015c1sm262667b3a.106.2024.07.18.14.15.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 18 Jul 2024 14:15:05 -0700 (PDT) Date: Thu, 18 Jul 2024 14:15:01 -0700 From: Stephen Hemminger To: Morten =?UTF-8?B?QnLDuHJ1cA==?= Cc: "Robin Jarry" , , "Sunil Kumar Kori" , "Rakesh Kudurumalla" , "Vladimir Medvedkin" , "Wisam Jaddo" , "Cristian Dumitrescu" , "Konstantin Ananyev" , "Akhil Goyal" , "Fan Zhang" , "Bruce Richardson" , "Yipeng Wang" , "Sameh Gobriel" , "Nithin Dabilpuram" , "Kiran Kumar K" , "Satha Rao" , "Harman Kalra" , "Ankur Dwivedi" , "Anoob Joseph" , "Tejasree Kondoj" , "Gagandeep Singh" , "Hemant Agrawal" , "Ajit Khaparde" , "Somnath Kotur" , "Chas Williams" , "Min Hu (Connor)" , "Potnuri Bharat Teja" , "Sachin Saxena" , "Ziyang Xuan" , "Xiaoyun Wang" , "Jie Hai" , "Yisen Zhuang" , "Jingjing Wu" , "Dariusz Sosnowski" , "Viacheslav Ovsiienko" , "Bing Zhao" , "Ori Kam" , "Suanming Mou" , "Matan Azrad" , "Chaoyong He" , "Devendra Singh Rawat" , "Alok Prasad" , "Andrew Rybchenko" , "Jiawen Wu" , "Jian Wang" , "Thomas Monjalon" , "Ferruh Yigit" , "Jiayu Hu" , "Pavan Nikhilesh" , "Maxime Coquelin" , "Chenbo Xia" Subject: Re: IPv6 APIs rework Message-ID: <20240718141501.24a7c79c@hermes.local> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F5AB@smartserver.smartshare.dk> References: <98CBD80474FA8B44BF855DF32C47DC35E9F5AB@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Thu, 18 Jul 2024 22:27:03 +0200 Morten Br=C3=B8rup wrote: > > From: Robin Jarry [mailto:rjarry@redhat.com] > >=20 > > Hi folks, > >=20 > > while working on IPv6 support for grout [1], I noticed that all DPDK > > IPv6 APIs used fixed sized arrays in the route lookup functions [2]. > >=20 > > int rte_fib6_lookup_bulk(struct rte_fib6 *fib, > > uint8_t ips[][RTE_FIB6_IPV6_ADDR_SIZE], > > uint64_t *next_hops, > > int n); > >=20 > > If I'm not mistaken, using sized arrays in function signatures is only > > for documentation purposes and does not result in any specific compiler > > checks. In the above example, the ips parameter is considered as a plain > > old `uint8_t **` pointer. > >=20 > > Also, not having a dedicated type for IPv6 addresses requires obscure > > pointer arithmetic [3] and casting [4]. > >=20 > > I'd like to introduce a real IPv6 address structure that has the same > > alignment than a dumb `uint8_t *` pointer but has an union to ease > > casting and most importantly presents the whole thing as an explicit > > typed structure: > >=20 > > #define RTE_IPV6_ADDR_SIZE 16 > >=20 > > struct rte_ipv6_addr { > > union { > > uint8_t u8[RTE_IPV6_ADDR_SIZE]; > > uint16_t u16[RTE_IPV6_ADDR_SIZE / sizeof(uint16_t)]; > > uint32_t u32[RTE_IPV6_ADDR_SIZE / sizeof(uint32_t)]; > > uint64_t u64[RTE_IPV6_ADDR_SIZE / sizeof(uint64_t)]; > > }; > > } __rte_packed __rte_aligned(1); > >=20 > > This would require some breakage of the APIs but I think it would > > benefit code readability and maintainability in the long term. =20 >=20 > In short: Although I like the idea of a unified IPv6 address type very mu= ch, I'm not sure consensus can be reached about the optimal alignment of su= ch a type. >=20 > The long version: >=20 > Please consider this proposal in a broader perspective. >=20 > The IPv4 FIB lookup takes an uint32_t array, so the IPv4 address type her= e is 4 byte aligned: uint32_t *ips > Generally, uint32_t or rte_be32_t is used for IPv4 addresses, and both th= ese types are 4 byte aligned. In other words: IPv4 addresses are considered= 4 byte aligned by DPDK. >=20 > I don't think it is similarly simple for IPv6 addresses. >=20 > The alignment of IPv6 addresses may depend on how/where they are used, e.= g.: > 1. For the FIB library, it might be good for vector implementations to ha= ve the IPv6 addresses naturally aligned (i.e. 16 byte aligned), like the ui= nt128_t/__int128/__m128i type (or the rte_xmm_t type [XMM]). Furthermore, a= simple integer type (uint128_t equivalent) might be preferable in this API. > 2. In the IPv6 packet header, the IPv6 addresses are not 16 byte aligned,= they are 8 byte aligned. So we cannot make the IPv6 address type 16 byte a= ligned. >=20 > I fear that broadly dumbing down the IPv6 address type to always use 1 by= te alignment could potentially introduce unwanted performance penalties (no= w or in the future). We didn't do it for IPv4 addresses, so let's not do it= for IPv6 addresses. >=20 > Perhaps we could use the lowest "non-exotic" (considering the use of IPv6= addresses) alignment, which I would guess is 8 byte - as in the IPv6 packe= t header. > For reference, Ethernet addresses are defined as 2 byte aligned [ETH]. >=20 > [XMM]: https://elixir.bootlin.com/dpdk/v24.03/source/lib/eal/x86/include/= rte_vect.h#L42 > [ETH]: https://elixir.bootlin.com/dpdk/v24.07-rc2/source/lib/net/rte_ethe= r.h#L74 >=20 > >=20 > > int rte_fib6_lookup_bulk(struct rte_fib6 *fib, > > const struct rte_ipv6_addr *ips, > > uint64_t *next_hops, > > int n); > >=20 > > I already have a semi-working draft and am in the process of splitting > > the changes into small chunks to make them easier to review. > >=20 > > https://github.com/DPDK/dpdk/compare/main...rjarry:dpdk:ipv6-address- > > rework > >=20 > > Is that something that would be of interest? If yes, I would like to > > announce API breakage before the release of 24.07 so that the changes > > can be integrated into 24.11. > >=20 > > Cheers! > >=20 > > [1] https://github.com/rjarry/grout > > [2] > > https://doc.dpdk.org/api/rte__fib6_8h.html#a924678410ccb9551cda3e75d742a > > 11e3 > > [3] https://git.dpdk.org/dpdk/tree/lib/fib/trie_avx512.c?h=3Dv24.07- > > rc2#n340 > > [4] https://git.dpdk.org/dpdk/tree/lib/hash/rte_thash.h?h=3Dv24.07- > > rc2#n156 > >=20 > > -- > > Robin =20 >=20 If you look at the standard netinet/in.h the storage of IPv6 addresses is in in6_addr. DPDK has always wanted to do its own thing... The in6_addr is a union with no explicit alignment. struct in6_addr { union { uint8_t __u6_addr8[16]; uint16_t __u6_addr16[8]; uint32_t __u6_addr32[4]; } __in6_u; Better to not have explicit alignment and not have 64 bit value.