Date: Wed, 19 Aug 2015 16:57:27 -0400 (EDT) From: Rick Macklem <rmacklem@uoguelph.ca> To: Daniel Braniss <danny@cs.huji.ac.il> Cc: Hans Petter Selasky <hps@selasky.org>, pyunyh@gmail.com, FreeBSD stable <freebsd-stable@freebsd.org>, FreeBSD Net <freebsd-net@freebsd.org>, Slawa Olhovchenkov <slw@zxy.spb.ru>, Christopher Forgeron <csforgeron@gmail.com> Subject: Re: ix(intel) vs mlxen(mellanox) 10Gb performance Message-ID: <796827231.26478408.1440017847125.JavaMail.zimbra@uoguelph.ca> In-Reply-To: <2BF7FA92-2DDD-452C-822C-534C0DC0B49F@cs.huji.ac.il> References: <1D52028A-B39F-4F9B-BD38-CB1D73BF5D56@cs.huji.ac.il> <55D333D6.5040102@selasky.org> <1325951625.25292515.1439934848268.JavaMail.zimbra@uoguelph.ca> <55D429A4.3010407@selasky.org> <20150819074212.GB964@michelle.fasterthan.com> <55D43615.1030401@selasky.org> <2013503980.25726607.1439989235806.JavaMail.zimbra@uoguelph.ca> <2BF7FA92-2DDD-452C-822C-534C0DC0B49F@cs.huji.ac.il>
next in thread | previous in thread | raw e-mail | index | archive | help
Daniel Braniss wrote:
>
> > On 19 Aug 2015, at 16:00, Rick Macklem <rmacklem@uoguelph.ca> 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).
> >
> > 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;
> >
Well, you can replace the line in sys/netinet/tcp_output.c that looks like:
if_hw_tsomaxsegcount = tp->t_tsomaxsegcount;
with the above line (at line #797 in head).
Any other patch for this will have the same effect, rick
> > 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.)
> >
> > Also, I'd like to apologize for some of my emails getting a little "blunt".
> > I just find
> > it flustrating that this problem is still showing up and is even in 10.2.
> > This is partly
> > my fault for not making it clearer to driver authors what
> > if_hw_tsomaxsegcount should be
> > set to, because I had it incorrect.
> >
> > Hopefully we can come up with a solution that everyone is comfortable with,
> > rick
>
>
> ok guys,
> when you have some code for me to try just let me know.
>
> danny
>
> _______________________________________________
> freebsd-net@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?796827231.26478408.1440017847125.JavaMail.zimbra>