From owner-freebsd-net@freebsd.org Thu Mar 26 20:02:34 2020 Return-Path: Delivered-To: freebsd-net@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 9D8E826051F for ; Thu, 26 Mar 2020 20:02:34 +0000 (UTC) (envelope-from vmaffione@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48pG9j2YGKz3Q1c for ; Thu, 26 Mar 2020 20:02:33 +0000 (UTC) (envelope-from vmaffione@freebsd.org) Received: from mail-qk1-f172.google.com (mail-qk1-f172.google.com [209.85.222.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) (Authenticated sender: vmaffione) by smtp.freebsd.org (Postfix) with ESMTPSA id 64B642ACF3 for ; Thu, 26 Mar 2020 20:02:26 +0000 (UTC) (envelope-from vmaffione@freebsd.org) Received: by mail-qk1-f172.google.com with SMTP id d11so8300423qko.3 for ; Thu, 26 Mar 2020 13:02:26 -0700 (PDT) X-Gm-Message-State: ANhLgQ16rTJWJRMerSROLs12PPebXkhpV1B5QvT6KwlpuZd7WYmgIoRs Ei+vvNsKuru3j7oIdvIRTgJa+ZYY0ThJz3xnQDQ= X-Google-Smtp-Source: ADFU+vsX0dnX5L9rW7lwV1V07txYuzHWhwQbwsgSP2gwigMmcrB0DE3Hwmj02vCa9J2kpuqHiKfXmaPKejLFBy2yAw0= X-Received: by 2002:a37:a1cc:: with SMTP id k195mr10418899qke.169.1585252945605; Thu, 26 Mar 2020 13:02:25 -0700 (PDT) MIME-Version: 1.0 References: <20200324123721.GA26248@staff.retn.net> <20200324140839.GB26248@staff.retn.net> <20200326102008.GA54860@staff.retn.net> In-Reply-To: <20200326102008.GA54860@staff.retn.net> From: Vincenzo Maffione Date: Thu, 26 Mar 2020 21:02:13 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: netmap/ixl and crc addition.. To: Alexandre Snarskii Cc: FreeBSD Net Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Mar 2020 20:02:34 -0000 Il giorno gio 26 mar 2020 alle ore 11:39 Alexandre Snarskii < snar@snar.spb.ru> ha scritto: > On Wed, Mar 25, 2020 at 11:31:30PM +0100, Vincenzo Maffione wrote: > > Hi Alexandre, > > Thanks. Your patch looks good to me. I assume you have tested it? > > Sure, this patch is already handles production traffic. > > > I will commit that to stable/11. > > Thanks. > > > > > The issue you report on stable/12 is more worrisome. The 'no space in TX > ring' > > condition (head==cur==tail) is ok per-se: on a subsequent poll() wakeup > (e.g. > > TX interrupt) or explicit ioctl(NIOCTXSYNC) you should see tail moving > forward, > > therefore freeing some space to be used in the ring. > > Unfortunately, further NIOCTXSYNC did not freed queue. > > Let me explain my observations in details: this issue was reproduceable > even under really light load (say, hundreds pps per queue). As the load > was ligth, I was able to insert debugging output before and after any > calls > to ioctl and poll, code is like: > > while(1) { > if (nm_tx_pending(txring)) { > printf("%s: pre-txsync cur: %d, head: %d, tail: %d\n", > thread_name, txring->cur, txring->head, > txring->tail); > if (ioctl(pa->fd, NIOCTXSYNC) == -1) { > log event > }; > printf("%s: post-txsync cur: %d, haad: %d, tail: %d\n", > thread_name, txring->cur, txring->head, > txring->tail); > }; > // setup poll descriptors with POLLIN only > printf("%s: pre-poll cur: %d, head: %d, tail: %d\n", > thread_name, txring->cur, txring->head, txring->tail); > ret = poll(fds, 2, 1000); > printf("%s: post-poll cur: %d, head: %d, tail: %d\n", > thread_name, txring->cur, txring->head, txring->tail); > > Ok, but I have some preliminary questions and notes: - You show the code that syncs the tx ring to collect completed transmissions (and thus advance tail). However, I do not see the code that submits new packets to be transmitted (and thus advances head/tail). Where is it? If this is happening in a separate thread, that leads to undefined behaviour, e.g. ring resets (although you can't crash the kernel). > Logs in normal situation (only single thread shown): > > Notice:nz(ixl0:2): pre-txsync cur: 649, head: 649, tail: 647 > Notice:nz(ixl0:2): post-txsync cur: 649, head: 649, tail: 648 > Notice:nz(ixl0:2): pre-poll cur: 649, head: 649, tail: 648 > Notice:nz(ixl0:2): post-poll cur: 649, head: 649, tail: 648 > > in pre-txsync: cur = head = tail + 2 (sometimes tail + 3), queue > is nearly empty. > in post-txsync: cur = head = tail + 1, queue is flushed. > > however, after some time, txcsync call advanced tail not to head - 1, > but to head: > > Notice:nz(ixl0:2): pre-txsync cur: 654, head: 654, tail: 652 > Notice:nz(ixl0:2): post-txsync cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): pre-poll cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): post-poll cur: 654, head: 654, tail: 654 > I see. This is wrong and must not happen. Once you get here, the tx ring becomes unusable, so we may as well ignore what happens afterwards. > > and following txcsyncs was not able to free (already empty) queue: > > Notice:nz(ixl0:2): pre-txsync cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): post-txsync cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): pre-poll cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): post-poll cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): pre-txsync cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): post-txsync cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): pre-poll cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): post-poll cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): pre-txsync cur: 654, head: 654, tail: 654 > Notice:nz(ixl0:2): post-txsync cur: 654, head: 654, tail: 654 > > application entered busy wait.. (in my case most packets just zero-copied > from rxring to txring, so if there are no space in txring, packets are not > consumed from rx). > > > However, the ring_reinit means that something is going wrong: either your > > application is using the TX ring incorrectly, or there is a bug in the > netmap > > ring_reinit was indeed caused by my incorrect use of txring: I > tried to ignore situation head == tail and inject packet anyway.. > Yes, ring reinit actually resets the ring variables to a sane value. In case your issue is not caused by unsafe access to the tx ring from multiple threads (see above), it would help to know what value do you have in sysctl hw.ixl.enable_head_writeback, and also try if the problem is still there if you enable or disable that parameter (which is relevant for how tail is advanced). > > > iflib code. Since FreeBSD 12, netmap support is provided by iflib, while > before > > netmap support was provided directly by the ixl driver. > > In any case, it would probably help if you could provide some more > detailed > > info (how to reproduce the problem). > > > > Cheers, > > Vincenzo > > > >