Date: Mon, 24 Mar 2014 02:14:06 -0300 From: Christopher Forgeron <csforgeron@gmail.com> To: Rick Macklem <rmacklem@uoguelph.ca> Cc: FreeBSD Net <freebsd-net@freebsd.org>, Garrett Wollman <wollman@freebsd.org>, Jack Vogel <jfvogel@gmail.com>, Markus Gebert <markus.gebert@hostpoint.ch> Subject: Re: 9.2 ixgbe tx queue hang Message-ID: <CAB2_NwAbHzFqa8RM5pwV7Yy5t=96JwzaF%2BSdjJN9kK3uhKKn_w@mail.gmail.com> In-Reply-To: <1164414873.1690348.1395622026185.JavaMail.root@uoguelph.ca> References: <CAB2_NwAcDPM6YKNLQMC0=YSp%2Bn9nBpXGJQR9ajbgbfcQFoWYPw@mail.gmail.com> <1164414873.1690348.1395622026185.JavaMail.root@uoguelph.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi, I'll follow up more tomorrow, as it's late and I don't have time for detail. The basic TSO patch didn't work, as packets were were still going over 65535 by a fair amount. I thought I wrote that earlier, but I am dumping a lot of info into a few threads, so I apologize if I'm not as concise as I could be. However, setting IP_MAXPACKET did. 4 hours of continuous run-time, no issues. No lost pings, no issues. Of course this isn't a fix - but it helps isolate the problem. I used IP_MAXPACKET = 32k originally, and I'm currently on 65495 bytes now (40 bytes shorter than IP_MAXPACKET). Of course, it's still sometimes going over IP_MAXPACKET, but since there is plenty of space to the real 65535 limit, we're not having any bad effects. Looking at my logs, I currently see the debug printf's showing packets of 65506 (11 bytes larger than IP_MAXPACKET). That's the real error. I should dump these packets to see what's in them.. perhaps wireshark or tcpdump is in order. One item of note is that setting the if's TSO doesn't matter in all output cases. I'll quote TSO code tomorrow that is still set to IP_MAXPACKET, thus setting the if's TSO isn't effective. From looking deeper at the transmit code, I thike I understand that we shouldn't be setting TSO to a value that is subtracted of headers, as the output routine should cover all angles, including TCP Options which can add size. I also booted up a 9.2-RELEASE system to make sure it was clean, and it is. No netstat -m denied stats. I didn't have time to put it under heavy load. The src diff between 10.0 and 9.2 holds the key to this problem. The largest changes I see are the use of htons / ntohs, and I have to wonder if there isn't a problem in there someplace. 9.2 -> 10 seems to have a lot of changes with how we calculate packet length and react to it, and this problem is one of packet length. Either we're not storing packet length properly, or we're not detecting it and fragmenting the packet properly. Lastly, I really should compile a debug kernel of 10-STABLE and check the asserts and the like. I will resume tomorrow as I feel we're maddeningly close. It's been ages since I had to do this much TCP, but I'm starting to get the hang of it again. Will resume tomorrow... On Sun, Mar 23, 2014 at 9:47 PM, Rick Macklem <rmacklem@uoguelph.ca> wrote: > Christopher Forgeron wrote: > > > > > > > > > > > > > > > > > > Update: > > > > For giggles, I set IP_MAXPACKET = 32768. > > > Well, I'm pretty sure you don't want to do that, except for an experiment. > You can just set if_hw_tsomax to whatever you want to try, at the place > my ixgbe.patch put it (just before the call to ether_ifattach()). > > > Over a hour of runtime, and no issues. This is better than with the > > TSO patch and the 9.2 ixgbe, as that was just a drastic reduction in > > errors. > > > So now the question becomes "how much does if_hw_tsomax need to be > reduced from 65535 to get this?". If reducing it by the additional > 4bytes for a vlan header is sufficient, then I understand what is > going on. If it needs to be reduced by more than that, then there > is something going on that I still don't understand. > > > Still have an 'angry' netstat -m on boot, and I'm still incrementing > > denied netbuf calls, so something else is wrong. > > > > I'm going to modify Rick's prinft in ixgbe to also output when we're > > over 32768. I'm sure it's still happening, but with an extra 32k of > > space, we're not busting like we did before. > > > > > > I notice a few interesting ip->ip_len changes since 9.2 - Like here, > > at line 720 > > > > > http://fxr.watson.org/fxr/diff/netinet/ip_output.c?v=FREEBSD10;im=kwqeqdhhvovqn;diffval=FREEBSD92;diffvar=v > > > > Looks like older code didn't byteswap with ntohs - I see that often > > in tcp_output.c, and in tcp_options.c. > > > > > > I'm also curious about this:Line 524 > > > http://fxr.watson.org/fxr/diff/netinet/ip_options.c?v=FREEBSD10;diffval=FREEBSD92;diffvar=v > > > > > > New 10 code: > > > > ip ->ip_len = htons ( ntohs ( ip ->ip_len) + optlen); Old 9.2 Code: > > ip ->ip_len += optlen; > > > Well, TSO segments aren't generated when optlen > 0, so I doubt this > matters for our issue (and I would find it hard to believe that this > would have been broken?). You can always look at the svn commit logs > to see why/how something was changed. > > > > > > > I wonder if there are any unexpected consequences of these changes, > > or perhaps a line someplace that doesn't make the change. > > > > Is there a dtrace command I could use to watch these functions and > > compare the new ip_len with ip->ip_len or other variables? > > > > > > > > > > > > > > > > On Sun, Mar 23, 2014 at 12:25 PM, Christopher Forgeron < > > csforgeron@gmail.com > wrote: > > > > > > > > > > > > > > > > On Sat, Mar 22, 2014 at 11:58 PM, Rick Macklem < rmacklem@uoguelph.ca > > > wrote: > > > > > > > > > > Christopher Forgeron wrote: > > > > > > > > Also should we not also subtract ETHER_VLAN_ENCAP_LEN from tsomax > > > to > > > make sure VLANs fit? > > > > > I took a look and, yes, this does seem to be needed. It will only be > > needed for the case where a vlan is in use and hwtagging is disabled, > > if I read the code correctly. > > > > > > > > Yes, or in the rare care where you configure your switch to pass the > > v_lan header through to the NIC. > > > > > > > > Do you use vlans? > > > > > > (Answered in above email) > > > > > > > > > > > > I've attached an updated patch. > > > > It might be nice to have the printf() patch in the driver too, so > > we can see how big the ones that are too big are? > > > > > > > > Yes, I'm going to leave those in until I know we have this fixed.. > > will probably leave it in a while longer as it should only have a > > minor performance impact to iter-loop like that, and I'd like to see > > what the story is a few months down the road. > > > > > > Thanks for the patches, will have to start giving them code-names so > > we can keep them straight. :-) I guess we have printf, tsomax, and > > this one. > > > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAB2_NwAbHzFqa8RM5pwV7Yy5t=96JwzaF%2BSdjJN9kK3uhKKn_w>