From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id CA3655A026D for ; Fri, 26 Apr 2024 05:31:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1714102258; bh=yVfzAu+9d1Pq15e+7gu6Kf9bKTu81uR8aYu7O2C1hpc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rvN1+/g+MZNNu5DCtJ88cZNyo5JDAxwHpcK4DOTrKNBJmOGGVR4TQ3DMzVbmM0EKz tvJkWFtSIuTE/6+esdkiQ7OC6RSixlYxyQqeCgwRvu0ewZiBxwjCPi25YK+VqrQrzT P5U3xhjkg2KR2jks3F32MsHiviE/GyiqBxyAB68UL1chToRTJUJUlqx+IeFPSkIqH7 LCFbQjj+gbXKZIdu4R7mP9MKvt14qcva3NjVuQzouYiLRTfA6iN8l0HqOdfgOLm2VZ NQR1ZSB8QbTCF7Q4pGeYFlT3CbTverd2LmyaTXk/75m2QGVhag6AWxskoiQzcp04MF lrRGC3BgkVhPA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4VQdXy4WTlz4wxx; Fri, 26 Apr 2024 13:30:58 +1000 (AEST) Date: Fri, 26 Apr 2024 13:29:22 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH 1/2] tcp: leverage support of SO_PEEK_OFF socket option when available Message-ID: References: <20240420191920.104876-1-jmaloy@redhat.com> <20240420191920.104876-2-jmaloy@redhat.com> <72322857-eb15-26c8-1713-d999acd05364@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="bd8ev6Aa2svxXRR+" Content-Disposition: inline In-Reply-To: <72322857-eb15-26c8-1713-d999acd05364@redhat.com> Message-ID-Hash: MJ22KDNQIIQ5XNZRNOZYOOQZCERI34IY X-Message-ID-Hash: MJ22KDNQIIQ5XNZRNOZYOOQZCERI34IY X-MailFrom: dgibson@gandalf.ozlabs.org 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, sbrivio@redhat.com, lvivier@redhat.com, dgibson@redhat.com 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: --bd8ev6Aa2svxXRR+ Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 25, 2024 at 07:23:28PM -0400, Jon Maloy wrote: >=20 >=20 > On 2024-04-23 20:44, David Gibson wrote: > > On Sat, Apr 20, 2024 at 03:19:19PM -0400, Jon Maloy wrote: > > > The kernel may support recvmsg(MSG_PEEK), starting reading data from a > > Not worth a respin on its own, but I think the comma above is > > misplaced, and for me makes the sentence much harder to read. > >=20 > > > given offset set by the SO_PEEK_OFF socket option. This makes it > > > possible to avoid repeated reading of already read initial bytes of a > > > received message, hence saving read cycles when forwarding TCP messag= es > > > in the host->name space direction. > > >=20 > > > In this commit, we add functionality to leverage this feature when av= ailable, > [...] > > > @@ -2174,6 +2183,15 @@ static int tcp_data_from_sock(struct ctx *c, s= truct tcp_tap_conn *conn) > > > if (iov_rem) > > > iov_sock[fill_bufs].iov_len =3D iov_rem; > > > + if (peek_offset_cap) { > > > + /* Don't use discard buffer */ > > > + mh_sock.msg_iov =3D &iov_sock[1]; > > > + mh_sock.msg_iovlen -=3D 1; > > > + > > > + /* Keep kernel sk_peek_off in synch */ > > > + set_peek_offset(s, already_sent); > > I thought we didn't need to set SO_PEEK_OFF here - that it would track > > on its own, and we only needed to change it for retransmits. I don't > > think we even need to calculate 'already_sent' when we have > > SO_PEEK_OFF. In fact - if we set already_sent to 0 here, it might > > make things a bit cleaner than having to have special cases for > > adjusting the iov and sendlen. > In theory yes. > I tried it for a while, using SEQ_GE(max_ack_seq, ack_seq) as criteria for > retransmission. > I observed some strange behavior, like retransmits that seemingly did not > come from fast retransmit or timer retransmit, and that the kernel > 'sk_peek_off' > didn=B4t always have the expected value when comparing with 'already_sent= =B4. Ouch, that sounds bad. I'm pretty sure that means there's a bug on one side or the other. > Since my focus was on the zero-window issue I decided to skip this for now > and take the safe option. > I may revisit this later. > >=20 > > > + } > > > + > > > /* Receive into buffers, don't dequeue until acknowledged by guest= =2E */ > > > do > > > len =3D recvmsg(s, &mh_sock, MSG_PEEK); > > > @@ -2195,7 +2213,9 @@ static int tcp_data_from_sock(struct ctx *c, st= ruct tcp_tap_conn *conn) > > > return 0; > > > } > [...] > > > + peek_offset_cap =3D true; > > > + } > > > + close(s); > > > + } > > > + printf("SO_PEEK_OFF%ssupported\n", peek_offset_cap ? " " : " not ") > > Should be an info(). > Made it a debug() as suggested by Stefano. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --bd8ev6Aa2svxXRR+ Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmYrH5EACgkQzQJF27ox 2GecQQ/8CJAo01GrApajkUoSxicZijZeXiIuVRFn1zn7s9wh/HPSiby3KrdDjlFM DDcbO0Z+husX7JHMb6xRbOMRSx384WoQ11D3ojLY8aZ9xfugvUF+hsAj3lrhUpE6 ncXVmpjoItxvZdfci5J9KmPHUWUh9fwL1Z0DCQytCv3jW2mtRjrmtgppSh9KPTdo mWwU/vVgMgbtCcwT/O0f2drh3umnoLm1j896NlrqHkgsUFA+cznPKyjXihFX6996 dnp97XjMRN34qOHzuwphHFXc01mzXc+yreAAsXZ1/km4/xu+uNe7UjRLfoEkdPDd ZH653yJuI0bHpPuavyCucFnEh8GXrIuNW4LT+hu1APUx6vjzh4OCXs/sCy/fNmmr /rfXiJhJoyBtw8gRPDbfr7tbOetMZ+Pf9kmAarO5L1th1B4H71GDfSePGXNHUm58 e6BKJ7+wXEynqSWWL/qlCaBEgcQn2KBrexloJF5nnEgEyjuRxmGaNUctPSj7uAQc HHHiCZCZmkl4neh3nPrh97Olo9rW7P+dWYu38cnqPGBUM6k69p6fzPs/3MgIEYIB 4Nc5zVW6MNr3LhjXK66sRYmioWHz4rk1iOxocwxYLIdAB3gMIY5mxW+KiuIo14ZO Pw/+zDMPb+2ZCK0O+JFKlLF8KjiF1LaNECvJ2IVcaGsh1Yvle+o= =E9Bb -----END PGP SIGNATURE----- --bd8ev6Aa2svxXRR+--