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 2B06745656; Fri, 19 Jul 2024 19:07:07 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1693240EF1; Fri, 19 Jul 2024 19:07:07 +0200 (CEST) Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) by mails.dpdk.org (Postfix) with ESMTP id B291240E11 for ; Fri, 19 Jul 2024 19:07:05 +0200 (CEST) Received: by mail-pf1-f171.google.com with SMTP id d2e1a72fcca58-70b12572bd8so837177b3a.2 for ; Fri, 19 Jul 2024 10:07:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1721408825; x=1722013625; 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=kf7lWTMyyqm6rsf9043hafrFCVJucuQJ+yPXmOtzTb4=; b=AMjCbD9S8q304rFgANTFx2XWuhBm1eI9+ed8LiPBfofdwtpQQMClFr7svnjgC04QgS PB7e/m1AnUx1/QZLFhh8OUcNKFaNYNyBRtmvs0yVdRAVzYZFL+G+3fFve2VGYLajVdib fbmtrfnWUfydLVihrHycAIgH416ADxgazB3HocJcIGMhFcsw356ZukuVJrNJpNPDwPNr J0nKC4vF0V1s0IhjccKDIoX4a7CoChWcnOO4nkTGf6c173GZvgX0hgiZW+d24bremNmF +pc2AzSx2e7Emzb6iortMkWVsr1XiV/RzitMtKFsBS4yuoQ1N7oW4YvkAYA8jhx440ZV KzCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1721408825; x=1722013625; 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=kf7lWTMyyqm6rsf9043hafrFCVJucuQJ+yPXmOtzTb4=; b=JHZW5jCD8lpN8IoU++8RV7KOEerZ1yRw2Co1Z6ySMx+C0YmduSLhJnqEH8wD3OzyKb 72mXDmF365zocV7tMnz7zy7t+CrVvRFiL85ZHOzgNh+Wm3TZ1cpVYeTUO0HVxd1a6SPp R+TUbGkMZ48s3qnVdZ4eML7aunrALRMLFB+n7eXil6cjCz1VB5Qy8PmcRE3UAi74p/T1 c89qOo0FqmE4jeED9VHofS1tuhwGO6mJEVReABsmy9Z92xe/QbE7Qb51qjgPYGSIvHsv 1kPDNtItB2Q+mWxIQ9OIJtspXtOTGF0wKHA9Ya5iU1kCNOTs7yvWBKa/W0AAeOadjheB f3ng== X-Forwarded-Encrypted: i=1; AJvYcCWbV6huCYEDejg0GCitzIYCA8gWlVPaQ1sAl5lJ1y57IaDmZq4ccROPa9tsLeAAg/Sx2C77FNRdU1FBYQI= X-Gm-Message-State: AOJu0YxdSdoeGK0yFLpI5OlN1C/sA0vKlqUuVJq6NM85voF7U+mUYKsZ qXgH4rAQ6NU5tC22AAGChn7t+a9P9b8STPP0/MQiC7EWxX2vCrYIueuDaPxACVU= X-Google-Smtp-Source: AGHT+IFNYI8onrX5Q8ZvYgfAgZYvmcglxyAigCUnulGh/ghlj0qEReA8G5+qS/bDaX6Y3PGyv8OTCQ== X-Received: by 2002:a05:6a21:788c:b0:1c0:f1cb:c4b1 with SMTP id adf61e73a8af0-1c3fdc95368mr10436341637.13.1721408824689; Fri, 19 Jul 2024 10:07:04 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-70cff59d55bsm1406719b3a.164.2024.07.19.10.07.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Jul 2024 10:07:04 -0700 (PDT) Date: Fri, 19 Jul 2024 10:07:01 -0700 From: Stephen Hemminger To: Morten =?UTF-8?B?QnLDuHJ1cA==?= Cc: "Robin Jarry" , "Vladimir Medvedkin" , , "Konstantin Ananyev" , , "Sunil Kumar Kori" , "Rakesh Kudurumalla" , "Wisam Jaddo" , "Cristian Dumitrescu" , "Konstantin Ananyev" , "Akhil Goyal" , "Fan Zhang" , "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" , "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: <20240719100701.7e7dfc7f@hermes.local> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35E9F5AF@smartserver.smartshare.dk> References: <98CBD80474FA8B44BF855DF32C47DC35E9F5AB@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35E9F5AC@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35E9F5AE@smartserver.smartshare.dk> <98CBD80474FA8B44BF855DF32C47DC35E9F5AF@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 Fri, 19 Jul 2024 17:47:47 +0200 Morten Br=C3=B8rup wrote: > > From: Robin Jarry [mailto:rjarry@redhat.com] > >=20 > > Morten Br=C3=B8rup, Jul 19, 2024 at 12:46: =20 > > > When passing an IPv4 address as a value, alignment does matter; it > > > must be 4 byte aligned. =20 > >=20 > > I was expecting the compiler to do what is necessary to copy the data to > > an aligned register before jumping to the function. =20 >=20 > Yes, and hereby you have achieved 4-byte alignment of the parameter. >=20 > What I meant was: If the parameter's type makes the parameter explicitly = unaligned, e.g. an unaligned array of 4 bytes or an unaligned_uint32_t, the= code inside the function must also treat the parameter as unaligned, and c= annot assume it has magically become 4-byte aligned. >=20 > Our functions taking an IPv4 address parameter (by value) passes the valu= e as aligned. > Functions taking an IPv6 address parameter (by value) should behave exact= ly the same way: The compiler should do what is necessary to copy the data = to an aligned register *before* jumping to the function. (Note: In 64 bit a= rchitectures, 128 bits requires two 64 bit registers.) The point remains: I= f conversion from unaligned to aligned is required, it is the responsibilit= y of the code calling the function, not the function itself. >=20 > > =20 > > > On a CPU with 128 bit registers, I would probably also pass an IPv6 > > > address as a value. With such a CPU, the parameter type should be > > > uint128_t or rte_be128_t, depending on byte order. =20 > >=20 > > I don't think there is a portable/standard uint128_t yet. Everything > > I could find is either GCC or linux specific. =20 >=20 > Agree. I am using uint128_t conceptually. >=20 > > =20 > > > There's a 3rd option: > > > Have an IPv6 type that is simply an array of 16 bytes with no =20 > > explicitly specified alignment: =20 > > > > > > struct rte_ipv6_addr { > > > unsigned char addr_bytes[RTE_IPV6_ADDR_LEN]; > > > }; > > > > > > Or: > > > > > > typedef struct rte_ipv6_addr { > > > unsigned char addr_bytes[RTE_IPV6_ADDR_LEN]; > > > } rte_ipv6_addr_t; > > > > > > If used as is, it will be unaligned. > > > > > > And if alignment offers improved performance for some use cases, > > > explicit alignment attributes can be added to the type in those use > > > cases. > > > > > > Not using an uint128_t type (or a union of other types than unsigned > > > char) will also avoid byte order issues. > > > > > > I guess Stephen was right to begin with. :-) =20 > >=20 > > Having the type as a union (as is the POSIX type) makes casting to > > integers a lot less tedious and makes the structure overall more > > flexible. =20 >=20 > Maybe (probably?). However, if you explicitly make the type unaligned, ho= w can the same type be used in an optimized way where the developer knows t= hat it is 16 byte aligned? >=20 > NB: There's something in the C standard about type casting from char (and= unsigned char) being less restricted than typecasting from uint8_t, so per= haps using unsigned char instead of uint8_t could solve the recasting issue= your union is trying to solve. (Unfortunately, I cannot remember the sourc= e of this information.) >=20 > Generally I don't think that we should introduce complex types/structures= /unions only to simplify type casting, if it is at the expense of performan= ce or code readability. >=20 > >=20 > > We could completely add an unaligned be128 member to the union by the > > way. I don't see what is wrong with having sub union members. =20 >=20 > (Not that I agree to using a union, but..) > Agree. If it's a union, and alignment is explicitly set, adding 64 bit an= d 128 bit sub union members should be perfectly acceptable, as it does not = modify the alignment or anything else. >=20 > >=20 > > About your concern with byte order, since the union members have > > explicit rte_be*_t types, I don't think confusion can happen. I have > > also renamed the members, replacing the "u" prefix with "a" so that it > > does not indicate that it should be used as a host integer. > >=20 > > struct __rte_aligned(1) rte_ipv6_addr { > > union { > > unsigned char a[16]; > > unaligned_be16_t a16[8]; > > unaligned_be32_t a32[4]; > > unaligned_be64_t a64[2]; > > unaligned_be128_t a128[1]; > > }; > > } __rte_packed; =20 Don't use packed, it makes the compiler generate very slow access since it has to assume worst case alignment. The intermediate forms are not actually big endian. Don't do that. It also would cause checkers to think swaps are needed. >=20 > (Again, not that I'm accepting the structure, but...) > Yes, this would solve the byte order concern. >=20 > How do you write efficient code with this forcefully unaligned type? > Let's say an application has some structure with an IPv6 address field, w= hich the developer has designed to be 16 byte aligned in the structure. > The compiler would need to always access this 16 byte aligned field as un= aligned, because the rte_ipv6_addr type makes it explicitly unaligned. The force align is a bad idea.