Re: [RFC] Patch to improve TSO limitation formula in general

Hans Petter Selasky wrote:
> Hi Rick,
> 
> Did you get a chance to look further at my patch?
> 
Ok, I just took a quick look. (I didn't try and figure out
if the code in tcp_output() looked correct.)

> Is this something we can commit?
> 
Well, I'd like to sound more positive, but here are a number
of problems I see with the patch:
1 - if_hw_tsomax is already in use by at least one driver,
    so I think it would be a POLA violation to replace it
    and make it very difficult to MFC.
    --> I'd leave if_hw_tsomax as is and add your new stuff
        as separate fields so things don't get broken for
        drivers that don't support your new fields.
2 - As above, most drivers don't even set if_hw_tsomax, so
    it will take some time for drivers to be converted.
    --> As such, the defaults need to be such that an NFS
        mbuf list of 35 mbufs totalling just over 64K works.
        (The drivers that handle 32 mbufs in a list need to
         get an mbuf list with no more that 64K minus ethernet
         headers, so they can squeeze the TSO segment into 32
         mclbyte mbuf clusters via m_defrag().)
3 - I don't think making the fields "log 2" is necessary or
    appropriate. Many current drivers support 35 mbufs in the
    fragment list. This is enough for NFS and is not a power of 2.
    I think each field should be just a u_int that can replace
    spares in "struct ifnet".
    --> I do not think that the size of ethernet headers needs
        to be specified if if_hw_tsomax still exists, since the
        driver can reduce the max size by that much. (It also
        varies depending on network type and whether or not VLAN
        headers are done by the hardware or driver.)

In summary, I'd add two u_ints to "struct ifnet":
1 - for max size of a fragment
2 - for max # of transmit fragments
then I'd set the defaults for these so that current code (ie. old
drivers) don't break. This would probably mean a large default for
both of these along with the current default value for if_hw_tsomax.

Then the fun part..Make tcp_output() generate TSO segments that
are limited by all three of the above.

rick
ps: Hopefully others will comment on this, since I'm not a networking
    guy. (I just ended up involved in this because NFS over TSO enabled
    net interfaces was badly broken and I got tired of telling people
    to disable TSO.)

> --HPS
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to
> "freebsd-current-unsubscribe_at_freebsd.org"
>