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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 05756184 for ; Tue, 23 Sep 2025 14:48:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758638882; cv=none; b=CbKcOBCrLB6G9dblLM1ZnUaxfED2kkUcI/kBDw4wwDSbWDxF2ix9Tf1q6wFg8obLfQr1wefIEHEGy/mhA2lwftFLrPJvQaMG4olrKskZw/7U4UM8wbi2/FXxqLZl1Xfy38FUEMxBDD/sQxFcBI3OV8rZMLFDErBHClz0DW3bl/0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758638882; c=relaxed/simple; bh=KeqE1HEyB8QFnK2pN1rXqm6q+uT9Q0dbCDWcjv2Nc1M=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=n34JkwFGUND1FLybu/D+8KZVVg0ySbAdnVFZxik4Zgo0cw7TsBaxKE+jeaEPF7snymKFm1SbBHTX5bnvDjaIK7HcslJX6fe2iCfVsnHYa2TalkWDGRj8A5bf4kHqo4l58rXzWJuvJhaP/iqakIsqKbxBQVDV/n5boWem7RjtfBA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=KExiN7Eh; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="KExiN7Eh" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1758638879; 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: in-reply-to:in-reply-to:references:references; bh=lGsm6H7P2q74l6MeTkSvpQNEFLpHSBIxi1h0StSUMNY=; b=KExiN7EhHDLXvmkQESqgzF+qF2DsZFU9FYqmKwe8NkA8H4tr5VM4ZHtxUEdIuwib1gTVk/ 8I55vkw4t6obB1pbZqvFVJh/xFLYUkmqAWO05LI/1/9Q9Gned4EwT2B/x6zbeAzwifIAo0 bGZ5G9QI38Xrok73DxskVHuSwsj6rxc= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-396-TV0IPWRLNOufwNtt391z8w-1; Tue, 23 Sep 2025 10:47:57 -0400 X-MC-Unique: TV0IPWRLNOufwNtt391z8w-1 X-Mimecast-MFC-AGG-ID: TV0IPWRLNOufwNtt391z8w_1758638876 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-3f924ae2a89so2325631f8f.3 for ; Tue, 23 Sep 2025 07:47:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1758638876; x=1759243676; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=lGsm6H7P2q74l6MeTkSvpQNEFLpHSBIxi1h0StSUMNY=; b=NKrVgGPrpY2R4nh7HOYdNg1QuRUlfYN7fv5AEvT6ZV2ZTjMDmjK3uZlXZCM3qiN3tH SFDVFitVVMr925BVH7cjU/Ba5rPqH3cehpRyHlIwPkwKtLb/QcOEn2cc+gBJXTo/USAX R59Zo+QyIg6cO79Fbf757K5Sn9tzk5sFoJQ1vM/wxqskFn6tZAiFaVu2N+Hl72PvYw/C 2q/28rEEExW1RsanFa5x64dcmaqEGwQoEzzo0kZYR7NriEM7CNdHC4VlqI5s+sWwzgWF Z10yorUxWPdsZiJMxDe3QIhdsGImR/MXKzv8Bt2jpRMFBSxtp3V/+7aeEV0aoiWK35Hq IDyQ== X-Forwarded-Encrypted: i=1; AJvYcCUDzBX3/7n0TvhrS3A6jY6Dkyp9FqWk3Fg++RiHN99h2sQRWDfKfc4KJn6IAMm+Ayu1aQRzFj2g0eulyZ0=@vger.kernel.org X-Gm-Message-State: AOJu0Yxf6KMbjh5WTzGl9tjN72cuswNE0tYJ+dfuKmQ8TqEFIlx0vGWx EShvtm0VoPrRMDskyvroygp97aWHHXuQ4huDeoLM7DHCj5+2iUSNtnSrS5+s1jgfj9O5DtLwz54 3AlzvUZoxM8TeiJVRBDAa+DJncyD4ZRU+4hLuexyjAmzC1V+ppGYOLl1eTc3GYf0MPQ== X-Gm-Gg: ASbGncs6AJllpSbuqumEjaLnx3ZbA/7Rcxdf6UZpyjXSL9HMEXaMZVyUXru4tJ0NhkJ MLu0Wt5v1s3FRdts7Txymhqo3T/EqUhthJqHzkB4f2Nxi35fdzEUo8imPYVTXLNUEi+F/DEzcAq U/F7CpWhpm7CW7GyravtQNDSMF/0jrH0rVawGkgUzM66EWqvikIMAqU4RxdowD2rogza9fYKSAw AqRXKuQ4I+U4XewZFQLKXa80NZh6Th2AndVI+PAKP32I21hBe1jkwgAMMgBcMSn872u+YqSw2uR s6kr2I1j2gmpo6zS7NNkEARmRR0OoyznTY4= X-Received: by 2002:a05:6000:220c:b0:3f3:88e1:9e30 with SMTP id ffacd0b85a97d-405c5bd85e9mr2624051f8f.15.1758638875920; Tue, 23 Sep 2025 07:47:55 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFVQW7wtomtpR1qDlFcvwbCRqGqsve/Ts8yCwjgDlTYhRUQTsERWyfPd2452H8Z8mCijph+Xg== X-Received: by 2002:a05:6000:220c:b0:3f3:88e1:9e30 with SMTP id ffacd0b85a97d-405c5bd85e9mr2624025f8f.15.1758638875505; Tue, 23 Sep 2025 07:47:55 -0700 (PDT) Received: from redhat.com ([2a06:c701:73ea:f900:52ee:df2b:4811:77e0]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3f61703b206sm14087074f8f.6.2025.09.23.07.47.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 23 Sep 2025 07:47:55 -0700 (PDT) Date: Tue, 23 Sep 2025 10:47:52 -0400 From: "Michael S. Tsirkin" To: Simon Schippers Cc: willemdebruijn.kernel@gmail.com, jasowang@redhat.com, eperezma@redhat.com, stephen@networkplumber.org, leiyang@redhat.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux.dev, kvm@vger.kernel.org, Tim Gebauer Subject: Re: [PATCH net-next v5 3/8] TUN, TAP & vhost_net: Stop netdev queue before reaching a full ptr_ring Message-ID: <20250923104348-mutt-send-email-mst@kernel.org> References: <20250922221553.47802-1-simon.schippers@tu-dortmund.de> <20250922221553.47802-4-simon.schippers@tu-dortmund.de> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250922221553.47802-4-simon.schippers@tu-dortmund.de> On Tue, Sep 23, 2025 at 12:15:48AM +0200, Simon Schippers wrote: > Stop the netdev queue ahead of __ptr_ring_produce when > __ptr_ring_full_next signals the ring is about to fill. Due to the > smp_wmb() of __ptr_ring_produce the consumer is guaranteed to be able to > notice the stopped netdev queue after seeing the new ptr_ring entry. As > both __ptr_ring_full_next and __ptr_ring_produce need the producer_lock, > the lock is held during the execution of both methods. > > dev->lltx is disabled to ensure that tun_net_xmit is not called even > though the netdev queue is stopped (which happened in my testing, > resulting in rare packet drops). Consequently, the update of trans_start > in tun_net_xmit is also removed. > > Co-developed-by: Tim Gebauer > Signed-off-by: Tim Gebauer > Signed-off-by: Simon Schippers > --- > drivers/net/tun.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index 86a9e927d0ff..c6b22af9bae8 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -931,7 +931,7 @@ static int tun_net_init(struct net_device *dev) > dev->vlan_features = dev->features & > ~(NETIF_F_HW_VLAN_CTAG_TX | > NETIF_F_HW_VLAN_STAG_TX); > - dev->lltx = true; > + dev->lltx = false; > > tun->flags = (tun->flags & ~TUN_FEATURES) | > (ifr->ifr_flags & TUN_FEATURES); > @@ -1060,14 +1060,18 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev) > > nf_reset_ct(skb); > > - if (ptr_ring_produce(&tfile->tx_ring, skb)) { > + queue = netdev_get_tx_queue(dev, txq); > + > + spin_lock(&tfile->tx_ring.producer_lock); > + if (__ptr_ring_full_next(&tfile->tx_ring)) > + netif_tx_stop_queue(queue); > + > + if (unlikely(__ptr_ring_produce(&tfile->tx_ring, skb))) { > + spin_unlock(&tfile->tx_ring.producer_lock); > drop_reason = SKB_DROP_REASON_FULL_RING; > goto drop; > } The comment makes it sound like you always keep one slot free in the queue but that is not the case - you just check before calling __ptr_ring_produce. But it is racy isn't it? So first of all I suspect you are missing an mb before netif_tx_stop_queue. Second it's racy because more entries can get freed afterwards. Which maybe is ok in this instance? But it really should be explained in more detail, if so. Now - why not just check ring full *after* __ptr_ring_produce? Why do we need all these new APIs, and we can use existing ones which at least are not so hard to understand. > - > - /* dev->lltx requires to do our own update of trans_start */ > - queue = netdev_get_tx_queue(dev, txq); > - txq_trans_cond_update(queue); > + spin_unlock(&tfile->tx_ring.producer_lock); > > /* Notify and wake up reader process */ > if (tfile->flags & TUN_FASYNC) > -- > 2.43.0