svn commit: r271504 - in head/sys: dev/oce dev/vmware/vmxnet3 dev/xen/netfront net netinet ofed/drivers/net/mlx4

[Rick Macklem]

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.)

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?

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 65536bytes
(32 * MCLBYTES). If it does not, then NFS over these devices is busted because
even m_defrag() can`t make them fit.

rick

rick

> --HPS
> 
> 
>