From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 7470B5A0272 for ; Wed, 13 Mar 2024 17:59:55 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710349194; 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=ozRU1wloDl2RDsOf+W8H9YdiuCf0Cei27YLwPDeLZs4=; b=Ot6ZznaiwYOJlR+pi/+FbH+/kkSTzhqtLRexe1WsWKqky6FRIr+bjgYf9KB/sXA/Nh00JI seV8oRzNsjtNWja3S0tVXGik7DPYTG2lTUo7XKKarLBP9wYXL0g1IJfTd788oE1HdmFARF 7/BkA4IHmQSbBXdgIA9UKWhDaaD19s0= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-316-aNgbcq_7NbSVBBE_rpU4XQ-1; Wed, 13 Mar 2024 12:59:53 -0400 X-MC-Unique: aNgbcq_7NbSVBBE_rpU4XQ-1 Received: by mail-ed1-f72.google.com with SMTP id 4fb4d7f45d1cf-5688bbfa971so16678a12.3 for ; Wed, 13 Mar 2024 09:59:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710349191; x=1710953991; 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=qkZflvxLp3Jhw134g0EE7A9iAUMv7SBOb0mb8qSuQA0=; b=cKzA2Jkhuh9gURlLFuGQHuj1Yi9YJe2bDkuKr3pGgw2rbtU5zVjaR2KD0eSdcfH7o7 8LZAbfd4moQrdzw9g/xNaMioPYZilxVQ+nHx2IaQF+94tKIxWCr5xbPXRsmLqF5uFPgM eQorlPbvTOQlyVfwVdqaZnFIn0kVHCpBcq6xjKcjxB4r3Az2/9pjtZYExlCzNUk85KCG 0y8A/9/IVndAJflRxyqz+57CaqxlfWUWPvdCVLPPZFou9bsi8wCUV+weOlrrdC/ZqSWY m/BRby3Gva3z4wN6xTUbUWzixFdlkdVZZKTmLLa0mqrf4StAhmz5bf2tbSyjhKVZKyOF tfWw== X-Gm-Message-State: AOJu0YxBKx3lncvV25IqxzGrFw1UFdva/zepE8qzTLIoN+Hv+mN5LH0N C5LoPiAYVM4AgReEU+lVebQ7+SVoR2oIcLv8hpeqCpJ0ppvhYIzkNTewIxCaHMovsG3ONnvXxGO Is1+RFRI8ELylZUFLhfpVBfk6jZPJ1KbdrSogP0D4PIBmJnVByzKvZbcp8NmFqCgzT/j1zGhD4g 39LLmX6TmcxvApY+hf3HMcnXNkhCnffgj8Zro= X-Received: by 2002:a50:c052:0:b0:566:5dcc:1c27 with SMTP id u18-20020a50c052000000b005665dcc1c27mr2162962edd.41.1710349190911; Wed, 13 Mar 2024 09:59:50 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEofXIbVMwDs76s5gb2oR6ZBGU8qduxxG2jMPwaJ9Yl3nq4dfUMo2GffoZKcLu1vscQhWaynw== X-Received: by 2002:a50:c052:0:b0:566:5dcc:1c27 with SMTP id u18-20020a50c052000000b005665dcc1c27mr2162944edd.41.1710349190355; Wed, 13 Mar 2024 09:59:50 -0700 (PDT) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id i10-20020aa7c70a000000b0056899fc9a94sm47018edq.12.2024.03.13.09.59.49 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 13 Mar 2024 09:59:49 -0700 (PDT) Date: Wed, 13 Mar 2024 17:58:55 +0100 From: Stefano Brivio To: Laurent Vivier Subject: Re: [RFC] tcp: Replace TCP buffer structure by an iovec array Message-ID: <20240313175855.79826aa8@elisabeth> In-Reply-To: <96b64a53-ad12-4fb3-9ff6-225e82df7c09@redhat.com> References: <20240311133356.1405001-1-lvivier@redhat.com> <20240313123725.7a37f311@elisabeth> <96b64a53-ad12-4fb3-9ff6-225e82df7c09@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.36; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: XCB6WROWZNR2A2H2VG2H6UWJFSS45K6F X-Message-ID-Hash: XCB6WROWZNR2A2H2VG2H6UWJFSS45K6F 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 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 Wed, 13 Mar 2024 16:20:12 +0100 Laurent Vivier wrote: > On 3/13/24 12:37, Stefano Brivio wrote: > > On Mon, 11 Mar 2024 14:33:56 +0100 > > Laurent Vivier wrote: > > =20 > ... > >> @@ -445,133 +413,78 @@ struct tcp_buf_seq_update { > >> }; > >> =20 > >> /* Static buffers */ > >> - > >> /** > >> - * tcp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections > >> - * @pad:=09Align TCP header to 32 bytes, for AVX2 checksum calculatio= n only > >> - * @taph:=09Tap-level headers (partially pre-filled) > >> - * @iph:=09Pre-filled IP header (except for tot_len and saddr) > >> - * @uh:=09=09Headroom for TCP header > >> - * @data:=09Storage for TCP payload > >> + * tcp_l2_flags_t - TCP header and data to send option flags =20 > >=20 > > 'struct tcp_l2_flags_t - ...', by the way (pre-existing, I see). > > =20 > >> + * @th:=09=09TCP header > >> + * @opts=09TCP option flags > >> */ > >> -static struct tcp4_l2_buf_t { > >> -#ifdef __AVX2__ > >> -=09uint8_t pad[26];=09/* 0, align th to 32 bytes */ > >> -#else > >> -=09uint8_t pad[2];=09=09/*=09align iph to 4 bytes=090 */ > >> -#endif > >> -=09struct tap_hdr taph;=09/* 26=09=09=09=092 */ > >> -=09struct iphdr iph;=09/* 44=09=09=09=0920 */ > >> -=09struct tcphdr th;=09/* 64=09=09=09=0940 */ > >> -=09uint8_t data[MSS4];=09/* 84=09=09=09=0960 */ > >> -=09=09=09=09/* 65536=09=09=0965532 */ > >> +struct tcp_l2_flags_t { > >> +=09struct tcphdr th; > >> +=09char opts[OPT_MSS_LEN + OPT_WS_LEN + 1]; > >> +}; =20 > >=20 > > This should still be aligned (when declared below) with: > >=20 > > #ifdef __AVX2__ > > } __attribute__ ((packed, aligned(32))) > > #else > > } __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > > #endif > >=20 > > because yes, now you get the unsigned int alignment for free, but the > > 32-byte boundary for AVX2 (checksums) not really, so that needs to be > > there, and then for clarity I would keep the explicit attribute for the > > non-AVX2 case. > >=20 > > By the way, I guess you could still combine the definition and > > declaration as it was done earlier. =20 >=20 > We can't combine the definition and declaration because the same definiti= on is used with=20 > IPv4 and IPv6 TCP arrays declaration. Ah, sorry, of course! By the way of that, I wonder: would it then be possible to have a single copy of payload buffers, that's shared for IPv4 and IPv6? We would probably need to introduce a separate counter, on top of tcp[46]_l2_buf_used (which could become tcp[46]_l2_hdr_used). Well, probably not in scope for this change anyway. > I'm not sure: the __attribute__ must be used on the structure definition = or on the array=20 > of structures declaration? __attribute__ can be used on definitions or declarations depending on the type of attribute itself -- here it would be on the definition, because it affects the definition of the data type itself. Strictly speaking, you can use it on the declaration too, but gcc will ignore it: $ cat attr.c struct a { int b; }; int main() { struct a c __attribute__ ((packed, aligned(32))); return 0; } $ gcc -o attr attr.c attr.c: In function =E2=80=98main=E2=80=99: attr.c:7:12: warning: =E2=80=98packed=E2=80=99 attribute ignored [-Wattribu= tes] 7 | struct a c __attribute__ ((packed, aligned(32))); | =20 --=20 Stefano