> On 22 Aug 2015, at 14:59, Rick Macklem <rmack...@uoguelph.ca> wrote: > > Daniel Braniss wrote: >> >>> On Aug 22, 2015, at 12:46 AM, Rick Macklem <rmack...@uoguelph.ca> wrote: >>> >>> Yonghyeon PYUN wrote: >>>> On Wed, Aug 19, 2015 at 09:00:35AM -0400, Rick Macklem wrote: >>>>> Hans Petter Selasky wrote: >>>>>> On 08/19/15 09:42, Yonghyeon PYUN wrote: >>>>>>> On Wed, Aug 19, 2015 at 09:00:52AM +0200, Hans Petter Selasky wrote: >>>>>>>> On 08/18/15 23:54, Rick Macklem wrote: >>>>>>>>> Ouch! Yes, I now see that the code that counts the # of mbufs is >>>>>>>>> before >>>>>>>>> the >>>>>>>>> code that adds the tcp/ip header mbuf. >>>>>>>>> >>>>>>>>> In my opinion, this should be fixed by setting if_hw_tsomaxsegcount >>>>>>>>> to >>>>>>>>> whatever >>>>>>>>> the driver provides - 1. It is not the driver's responsibility to >>>>>>>>> know if >>>>>>>>> a tcp/ip >>>>>>>>> header mbuf will be added and is a lot less confusing that expecting >>>>>>>>> the >>>>>>>>> driver >>>>>>>>> author to know to subtract one. (I had mistakenly thought that >>>>>>>>> tcp_output() had >>>>>>>>> added the tc/ip header mbuf before the loop that counts mbufs in the >>>>>>>>> list. >>>>>>>>> Btw, >>>>>>>>> this tcp/ip header mbuf also has leading space for the MAC layer >>>>>>>>> header.) >>>>>>>>> >>>>>>>> >>>>>>>> Hi Rick, >>>>>>>> >>>>>>>> Your question is good. With the Mellanox hardware we have separate >>>>>>>> so-called inline data space for the TCP/IP headers, so if the TCP >>>>>>>> stack >>>>>>>> subtracts something, then we would need to add something to the limit, >>>>>>>> because then the scatter gather list is only used for the data part. >>>>>>>> >>>>>>> >>>>>>> I think all drivers in tree don't subtract 1 for >>>>>>> if_hw_tsomaxsegcount. Probably touching Mellanox driver would be >>>>>>> simpler than fixing all other drivers in tree. >>>>>>> >>>>>>>> Maybe it can be controlled by some kind of flag, if all the three TSO >>>>>>>> limits should include the TCP/IP/ethernet headers too. I'm pretty sure >>>>>>>> we want both versions. >>>>>>>> >>>>>>> >>>>>>> Hmm, I'm afraid it's already complex. Drivers have to tell almost >>>>>>> the same information to both bus_dma(9) and network stack. >>>>>> >>>>>> Don't forget that not all drivers in the tree set the TSO limits before >>>>>> if_attach(), so possibly the subtraction of one TSO fragment needs to go >>>>>> into ip_output() .... >>>>>> >>>>> Ok, I realized that some drivers may not know the answers before >>>>> ether_ifattach(), >>>>> due to the way they are configured/written (I saw the use of >>>>> if_hw_tsomax_update() >>>>> in the patch). >>>> >>>> I was not able to find an interface that configures TSO parameters >>>> after if_t conversion. I'm under the impression >>>> if_hw_tsomax_update() is not designed to use this way. Probably we >>>> need a better one?(CCed to Gleb). >>>> >>>>> >>>>> If it is subtracted as a part of the assignment to if_hw_tsomaxsegcount >>>>> in >>>>> tcp_output() >>>>> at line#791 in tcp_output() like the following, I don't think it should >>>>> matter if the >>>>> values are set before ether_ifattach()? >>>>> /* >>>>> * Subtract 1 for the tcp/ip header mbuf that >>>>> * will be prepended to the mbuf chain in this >>>>> * function in the code below this block. >>>>> */ >>>>> if_hw_tsomaxsegcount = tp->t_tsomaxsegcount - 1; >>>>> >>>>> I don't have a good solution for the case where a driver doesn't plan on >>>>> using the >>>>> tcp/ip header provided by tcp_output() except to say the driver can add >>>>> one >>>>> to the >>>>> setting to compensate for that (and if they fail to do so, it still >>>>> works, >>>>> although >>>>> somewhat suboptimally). When I now read the comment in sys/net/if_var.h >>>>> it >>>>> is clear >>>>> what it means, but for some reason I didn't read it that way before? (I >>>>> think it was >>>>> the part that said the driver didn't have to subtract for the headers >>>>> that >>>>> confused me?) >>>>> In any case, we need to try and come up with a clear definition of what >>>>> they need to >>>>> be set to. >>>>> >>>>> I can now think of two ways to deal with this: >>>>> 1 - Leave tcp_output() as is, but provide a macro for the device driver >>>>> authors to use >>>>> that sets if_hw_tsomaxsegcount with a flag for "driver uses tcp/ip >>>>> header mbuf", >>>>> documenting that this flag should normally be true. >>>>> OR >>>>> 2 - Change tcp_output() as above, noting that this is a workaround for >>>>> confusion w.r.t. >>>>> whether or not if_hw_tsomaxsegcount should include the tcp/ip header >>>>> mbuf and >>>>> update the comment in if_var.h to reflect this. Then drivers that >>>>> don't >>>>> use the >>>>> tcp/ip header mbuf can increase their value for if_hw_tsomaxsegcount >>>>> by >>>>> 1. >>>>> (The comment should also mention that a value of 35 or greater is much >>>>> preferred to >>>>> 32 if the hardware will support that.) >>>>> >>>> >>>> Both works for me. My preference is 2 just because it's very >>>> common for most drivers that use tcp/ip header mbuf. >>> Thanks for this comment. I tend to agree, both for the reason you state and >>> also >>> because the patch is simple enough that it might qualify as an errata for >>> 10.2. >>> >>> I am hoping Daniel Braniss will be able to test the patch and let us know >>> if it >>> improves performance with TSO enabled? >> >> send me the patch and I’ll test it ASAP. >> danny >> > Patch is attached. The one for head will also include an update to the comment > in sys/net/if_var.h, but that isn't needed for testing.
well, the plot thickens. Yesterday, before running the new kernel, I decided to re run my test, and to my surprise i was getting good numbers, about 300MGB/s with and without TSO. this morning, the numbers were again bad, around 70MGB/s,what the ^%$#@! so, after some coffee, I run some more tests, and some conclusions: using a netapp(*) as the nfs client: - doing ifconfig ix0 tso or -tso does some magic and numbers are back to normal - for a while using another Fbsd/zfs as client all is nifty, actually a bit faster than the netapp (not a fair comparison, since the zfs client is not heavily used) and I can’t see any degradation. btw, this is with the patch applied, but was seeing similar numbers before the patch. running with tso, initially I get around 300MGB/s, but after a while(sorry can’t be more scientific) it drops down to about half, and finally to a pathetic 70MGB/s *: while running the tests I monitored the Netapp, and nothing out of the ordinary there. cheers, danny _______________________________________________ freebsd-net@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"