On 09/13/14 22:28, Rick Macklem wrote:
Hans Petter Selasky wrote:
On 09/13/14 22:04, Rick Macklem wrote:
Hans Petter Selasky wrote:
On 09/13/14 18:54, Adrian Chadd wrote:
Hi,
Just for the record:
* I'm glad you're tackling the TSO config stuff;
* I'm not glad you're trying to pack it into a u_int rather than
creating a new structure and adding fields for it.
I appreciate that you're trying to rush this in before 10.1, but
this
is exactly why things shouldn't be rushed in before release
deadlines.
:)
I'd really like to see this be broken out as a structure and the
bit
shifting games for what really shouldn't be packed into a u_int
fixed.
Otherwise this is going to be deadweight that has to persist past
11.0.
Hi Adrian,
I can make that change for -current, making the new structure and
such.
This change was intended for 10 where there is only one u_int for
this
information. Or do you want me to change that in 10 too?
Well, there are spare fields (if_ispare[4]) in struct ifnet that I
believe can be used for new u_ints when MFC'ng a patch that adds
fields to struct ifnet in head. (If I have this wrong, someone
please
correct me.)
I'll admit I don't really see an advantage to defining a structure
vs
just defining a couple of additional u_ints, but so long as the
structure
doesn't cause alignment issues for any arch, I don't see a problem
with
a structure.
I tend to agree with Adrian that this shouldn't be rushed. (I,
personally,
think that if_hw_tsomax was poorly chosen, but that is already in
use, so
I think we need to add to that and not replace it.)
I also hope that your testing has included quite a bit of activity
on
an NFS mount using TSO and the default 64K rsize, wsize, since that
is
going to generate a bunch of 35 mbuf transmit fragment lists and
there
is an edge case where the total data length excluding ethernet
header
is just under 64K (by less than the ethernet header length) where
the
list must be split by tcp_output() to avoid disaster.
Hi,
The ethernet and VLAN headers are still subtracted.
Where? I couldn't see it when I glanced at the patch.
(hdrlen in tcp_output() does not include ethernet header length and
that is the only thing I see subtracted from the max_len.)
Hi Rick,
When the drivers setup the "if_hw_tsomax" field, they need to subtract
the ethernet and vlan headers from the maximum TSO payload.
For example here:
+ sc->ifp->if_hw_tsomax = IF_HW_TSOMAX_BUILD_VALUE(
+ 65535 - (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN) /* bytes */,
+ OCE_MAX_TX_ELEMENTS /* maximum frag count */,
+ 12 /* 4K frag size */);
I see the default set to (65536 - 4). I don't know why you subtracted 4
but I would have expected the max ethernet header length to be subtracted
here?
That is another technical point. If you have a bunch of data to transfer
you want the start and stop physical addresses to be aligned to some
boundary, like cache line or 32-bit or 64-bit, because then the hardware
doesn't have to do the byte shifting when it starts reading the data
payload - right?
Note that this must be subtracted before use by tcp_output() because there
are several network device drivers that support 32 transmit segments and this
means that the TSO segment including ethernet headers must fit in 65536 bytes
(32 * MCLBYTES). If it does not, then NFS over these devices is busted because
even m_defrag() can`t make them fit.
I only found a few network drivers which actually set the TSO limit, and
for the rest: The default limit is 255 frags of MAX 65536 bytes, which
should not be reached in the cases you are describing.
--HPS
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
