From nobody Mon Aug 02 18:53:46 2021 X-Original-To: freebsd-net@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id B159D12B43CE for ; Mon, 2 Aug 2021 18:53:48 +0000 (UTC) (envelope-from jmg@gold.funkthat.com) Received: from gold.funkthat.com (gate2.funkthat.com [208.87.223.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "gate2.funkthat.com", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GdnGN3SzNz4pvJ; Mon, 2 Aug 2021 18:53:48 +0000 (UTC) (envelope-from jmg@gold.funkthat.com) Received: from gold.funkthat.com (localhost [127.0.0.1]) by gold.funkthat.com (8.15.2/8.15.2) with ESMTPS id 172IrkHC062217 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 2 Aug 2021 11:53:46 -0700 (PDT) (envelope-from jmg@gold.funkthat.com) Received: (from jmg@localhost) by gold.funkthat.com (8.15.2/8.15.2/Submit) id 172IrkJ0062216; Mon, 2 Aug 2021 11:53:46 -0700 (PDT) (envelope-from jmg) Date: Mon, 2 Aug 2021 11:53:46 -0700 From: John-Mark Gurney To: Eric Joyner Cc: freebsd-net@freebsd.org Subject: Re: igb fc (aka flowcontrol) issue... Message-ID: <20210802185346.GG41029@funkthat.com> Mail-Followup-To: Eric Joyner , freebsd-net@freebsd.org References: <20210802015852.GF41029@funkthat.com> List-Id: Networking and TCP/IP with FreeBSD List-Archive: https://lists.freebsd.org/archives/freebsd-net List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-net@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: FreeBSD 11.3-STABLE amd64 X-PGP-Fingerprint: D87A 235F FB71 1F3F 55B7 ED9B D5FF 5A51 C0AC 3D65 X-Files: The truth is out there X-URL: https://www.funkthat.com/ X-Resume: https://www.funkthat.com/~jmg/resume.html X-TipJar: bitcoin:13Qmb6AeTgQecazTWph4XasEsP7nGRbAPE X-to-the-FBI-CIA-and-NSA: HI! HOW YA DOIN? can i haz chizburger? User-Agent: Mutt/1.6.1 (2016-04-27) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (gold.funkthat.com [127.0.0.1]); Mon, 02 Aug 2021 11:53:46 -0700 (PDT) X-Rspamd-Queue-Id: 4GdnGN3SzNz4pvJ X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-ThisMailContainsUnwantedMimeParts: N Eric Joyner wrote this message on Sun, Aug 01, 2021 at 21:10 -0700: > On Sun, Aug 1, 2021 at 6:59 PM John-Mark Gurney wrote: > > > I have a dual port igb card: > > igb0: port 0x2020-0x203f mem > > 0xd1020000-0xd103ffff,0xd0c00000-0xd0ffffff,0xd1044000-0xd1047fff irq 17 at > > device 0.0 on pci3 > > igb0: Using 1024 TX descriptors and 1024 RX descriptors > > igb0: Using 4 RX queues 4 TX queues > > igb0: Using MSI-X interrupts with 5 vectors > > igb0: Ethernet address: xxxx > > igb0: netmap queues/slots: TX 4/1024, RX 4/1024 > > igb1: port 0x2000-0x201f mem > > 0xd1000000-0xd101ffff,0xd0800000-0xd0bfffff,0xd1040000-0xd1043fff irq 18 at > > device 0.1 on pci3 > > igb1: Using 1024 TX descriptors and 1024 RX descriptors > > igb1: Using 4 RX queues 4 TX queues > > igb1: Using MSI-X interrupts with 5 vectors > > igb1: Ethernet address: xxxx > > igb1: netmap queues/slots: TX 4/1024, RX 4/1024 > > > > And I have observed an issue with it when adjusting flow control, it > > seems to adjust both ports at the same time: > > # sysctl dev.igb.0.fc > > dev.igb.0.fc: 3 > > # sysctl dev.igb.1.fc > > dev.igb.1.fc: 3 > > # sysctl dev.igb.0.fc=0 > > dev.igb.0.fc: 3 -> 0 > > # sysctl dev.igb.1.fc > > dev.igb.1.fc: 0 > > > > Is this correct behavior? > > > > Also, the fc sysctl is not documented, I propose the following: > > diff --git a/share/man/man4/em.4 b/share/man/man4/em.4 > > index a1fa22c..bad82e9 100644 > > --- a/share/man/man4/em.4 > > +++ b/share/man/man4/em.4 > > @@ -265,6 +265,26 @@ If > > is non-zero, this tunable limits the maximum delay in which a transmit > > interrupt is generated. > > .El > > +.Sh SYSCTL VARIABLES > > +The following variable is availabel: > > +.Bl -tag -width "dev.em.X.fc" > > +.It Va dev.em.X.fc > > +This sysctl sets the flow control for the card (this means both ports > > +on a dual port card). The values are as follows: > > +.Bl -tag -width "XX" -offset "XXXX" -compact > > +.It 0 > > +Disabled > > +.It 1 > > +Process received pause frames, do not transmit them. > > +.It 2 > > +Send pause frames, but to not process received frames. > > +.It 3 > > +Process and send pause frames. > > +.It 4 > > +No software override, use EEPROM configuration. > > +.El > > +.El > > +Note: That the variable is available for igb as well. > > .Sh FILES > > .Bl -tag -width /dev/led/em* > > .It Pa /dev/led/em* > > I was looking at the hardware specs to see a mention of it, but I didn't > find anything. But looking at the code, I do see that in if_em.c's > em_set_flowcntl(), the value "input" used in the output of the sysctl is a > "static int". So it seems like if you set the value to 0, you're going to > read 0 from it, even on the second port. That looks like a bug that needs > to be fixed. Yeah, it looks like reading the status of the flowcontrol doesn't work. It just reads whatever was last set. change intput to: int input; input = adapter->fc; does fix the sysctl problem of reading. But, it does appear that adapter->fc does not get initalized anywhere, which means reading it won't be correct until it's set the first time. And even then, it's initalized to 0, so if you first set it to zero, it won't change anything, because it doesn't have the correct state in the var, and only does the set if the value changed... So w/o additional work, you have to turn it on first, before you can turn if off... > And for the man page, that sysctl doesn't accept "4" as a valid value > (unless you're going to add that?). I wouldn't put that flow control sets > the value for both ports on the card, either. But otherwise, that seems > reasonable to me. Well, I was just going by the comment in e1000_phy.c: * other: No software override. The flow control configuration * in the EEPROM is used. and I picked 4 for other. But now that I look at the code, the code doesn't implement that case. I'll drop that case then. I added that note because that was the observed behavior... I'll drop it as it's been tracked down to a sysctl implemention bug. (I wanted the patch to be what I was going to commit so there wasn't confusion.) -- John-Mark Gurney Voice: +1 415 225 5579 "All that I will do, has been done, All that I have, has not."