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 DB34A44088; Tue, 21 May 2024 17:46:53 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5D62D40608; Tue, 21 May 2024 17:46:53 +0200 (CEST) Received: from mail-pg1-f174.google.com (mail-pg1-f174.google.com [209.85.215.174]) by mails.dpdk.org (Postfix) with ESMTP id E58734025C for ; Tue, 21 May 2024 17:46:51 +0200 (CEST) Received: by mail-pg1-f174.google.com with SMTP id 41be03b00d2f7-5d3907ff128so506385a12.3 for ; Tue, 21 May 2024 08:46:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1716306411; x=1716911211; 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=YL6x+uhesSr9efbokHnizPII9w67htk9QRu44Plzis0=; b=O9wmSutXUEnGzz5A5GTueYSjez3U6eyo4SUytPpN6mNWf62K/qsy0FJIgumlHvWJoT lxKHeCwr44JYDoUP9GKk/4cbK/JEENfpXDsYbAf8qOwvCOVQgXO1F99vbngn0h85gUvq TZut5oEBC46jDEdsrW4ix9k1ewbFPHu9zI1SOCT03TOa8olb2VV332Xa/UFzrdWkDZ6D Y3xPE4EAZbScHZCQEm1qPMu8DY/hupLFFg62UGzv3MAs0q89MtFNARZxFs08psO2ih4R ybZ0drqUCsMS1mxgxBMh+V33leIXBb1jN4Ay6DmbPEOaJoAGAqubeAMSy3Ff0m4TZlFy 7uCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1716306411; x=1716911211; 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=YL6x+uhesSr9efbokHnizPII9w67htk9QRu44Plzis0=; b=Oq8ihS3588/rKsfdZhk77RMaeias8v6OvSyH3fnFSbxchdAP/SL42JOOVJbKlC/TQW T5LTbMhhyHqRIaHZqrbXjmllZQrnhVKCCeEX08ziuFxelWvwnovFgpYrGQuUf3dXdlto nA518TtebBG9UXDWqOYnMXvwFuT3aGM/K4NCEzwV5v66kZZx3FGTT77hMqjKAiOFkliP VfxgR+JnpsGDJxgxSjmY/T8chO/dM8MbP/6TWr+KtPCo+YO6uhz5xKuhjYnfQPLQJNOm fV+0y5VYSiREg49gF/erR5/+FFraWO+1DQ4tsNPEUOYza/MGu1gmCeUsQxnkK6aTerGa U/rg== X-Gm-Message-State: AOJu0YzTMXirrNq+zwYcq6ihqivyxPZApt/Vg7eu0zxsrtxCOSlXNkWB vydRY7gcUESFa6bnx2rInrflitJy702RbrXB/IiXz6H7UkGw4DXHLBjDF+xFIuYCYlY251TVPWj Y X-Google-Smtp-Source: AGHT+IEvqL+ZTWRSGByBr/Dh2yFrYiDmq4lLDGwHZG9awmZM5gNsG2SSbrvKm8o8cG2C1wE9732iFQ== X-Received: by 2002:a17:902:ced0:b0:1f3:87a:312 with SMTP id d9443c01a7336-1f3087a052dmr72622215ad.16.1716306411040; Tue, 21 May 2024 08:46:51 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-1ef0c136074sm223225365ad.246.2024.05.21.08.46.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 May 2024 08:46:50 -0700 (PDT) Date: Tue, 21 May 2024 08:46:48 -0700 From: Stephen Hemminger To: Ferruh Yigit Cc: dev@dpdk.org Subject: Re: [PATCH v12 11/12] net/tap: do not mark queue full as error Message-ID: <20240521084648.2ef78531@hermes.local> In-Reply-To: <96d648dc-94ac-45bc-8398-0759083d4ca1@amd.com> References: <20240130034925.44869-1-stephen@networkplumber.org> <20240502213618.11391-1-stephen@networkplumber.org> <20240502213618.11391-12-stephen@networkplumber.org> <96d648dc-94ac-45bc-8398-0759083d4ca1@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Mon, 20 May 2024 18:52:27 +0100 Ferruh Yigit wrote: > On 5/2/2024 10:31 PM, Stephen Hemminger wrote: > > If tap device in kernel returns EAGAIN that means it is full. > > That is not an error. > > > > Signed-off-by: Stephen Hemminger > > --- > > drivers/net/tap/rte_eth_tap.c | 13 ++++++++----- > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c > > index 2484a82ccb..485bd35912 100644 > > --- a/drivers/net/tap/rte_eth_tap.c > > +++ b/drivers/net/tap/rte_eth_tap.c > > @@ -542,7 +542,6 @@ tap_write_mbufs(struct tx_queue *txq, uint16_t num_mbufs, > > struct rte_mbuf *seg = mbuf; > > uint64_t l4_ol_flags; > > int proto; > > - int n; > > int j; > > int k; /* current index in iovecs for copying segments */ > > > > @@ -647,14 +646,18 @@ tap_write_mbufs(struct tx_queue *txq, uint16_t num_mbufs, > > } > > > > /* copy the tx frame data */ > > - n = writev(process_private->fds[txq->queue_id], iovecs, k); > > - if (n <= 0) > > - return -1; > > + if (unlikely(writev(process_private->fds[txq->queue_id], iovecs, k) < 0)) { > > + TAP_LOG(DEBUG, "writev (qid=%u fd=%d) %s", > > + txq->queue_id, process_private->fds[txq->queue_id], > > + strerror(errno)); > > > > Do we really want logging in datapath? > > > + return (errno == EAGAIN) ? 0 : -1; > > > > Nack. > > Returning '0' will cause 'pmd_tx_burst()' to continue and increase > 'num_tx', this will mislead as packet sent successfully. > > We should have three return values from 'tap_write_mbufs()': > <0 -> Error, 'pmd_tx_burst()' should break, stats.errors updated. > 0 -> 'pmd_tx_burst()' should break, valid num_tx returned > >0 -> 'pmd_tx_burst()' work as it is. > > > + } > > > > (*num_packets)++; > > (*num_tx_bytes) += rte_pktmbuf_pkt_len(mbuf); > > } > > - return 0; > > + > > + return 1; > > > > Why '1', wouldn't it be better to return number of packets written > successfully. > Decided to drop this patch. Doing GSO inside the TAP device is the wrong place to do it. Kernel already has API to do GSO and checksum offload and it will be simpler and faster to use that. When TAP device is updated not to do the GSO here, this code goes away and handling the full condition properly gets much easier. When TAP is doing GSO there are lots of corner cases where a expanded GSO packet could get only partially sent.