ix(intel) vs mlxen(mellanox) 10Gb performance

Rick Macklem rmacklem at uoguelph.ca
Sat Aug 22 11:59:21 UTC 2015 

Daniel Braniss wrote:
> 
> > On Aug 22, 2015, at 12:46 AM, Rick Macklem <rmacklem at 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.

Thanks for testing this, rick

> > 
> > rick
> > 
> >> _______________________________________________
> >> freebsd-stable at freebsd.org mailing list
> >> https://lists.freebsd.org/mailman/listinfo/freebsd-stable
> >> To unsubscribe, send any mail to "freebsd-stable-unsubscribe at freebsd.org"
> >> 
> 
> _______________________________________________
> freebsd-stable at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-stable
> To unsubscribe, send any mail to "freebsd-stable-unsubscribe at freebsd.org"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tsooutby1.patch
Type: text/x-patch
Size: 599 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-net/attachments/20150822/69700572/attachment.bin>

More information about the freebsd-net mailing list