On Saturday, November 05, 2016 04:30:43 PM Sean Bruno wrote:
> Author: sbruno
> Date: Sat Nov 5 16:30:42 2016
> New Revision: 308345
> URL: https://svnweb.freebsd.org/changeset/base/308345
>
> Log:
> r295133 attempted to deactivate TSO in the 100Mbit link case with this
> adapter to work around bugs in TSO handling at this speed.
>
> em_init_locked is called during first boot of the adapter and will
> see that link_speed is unitialized, effectively turning off tso for
> all cards at all speeds, which I believe was *not* the intent.
>
> Move the handling of TSO deactivation to the link handler where we can
> more effectively make the decision about what to do. In addition,
> completely purge the TSO capabilities instead of disabling just CSUM_TSO.
>
> Thanks to jhb for explanation of the hw capabilites api.
>
> Thanks to royger and cognet for testing the 100Mbit failure case to
> ensure that their adapters do indeed still work.
>
> MFC after: 1 week
> Sponsored by: Limelight Networks
>
> Modified:
> head/sys/dev/e1000/if_em.c
>
> Modified: head/sys/dev/e1000/if_em.c
> ==============================================================================
> --- head/sys/dev/e1000/if_em.c Sat Nov 5 16:23:33 2016
> (r308344)
> +++ head/sys/dev/e1000/if_em.c Sat Nov 5 16:30:42 2016
> (r308345)
> @@ -369,11 +369,6 @@ MODULE_DEPEND(em, netmap, 1, 1, 1);
> #define MAX_INTS_PER_SEC 8000
> #define DEFAULT_ITR (1000000000/(MAX_INTS_PER_SEC * 256))
>
> -/* Allow common code without TSO */
> -#ifndef CSUM_TSO
> -#define CSUM_TSO 0
> -#endif
> -
> #define TSO_WORKAROUND 4
>
> static SYSCTL_NODE(_hw, OID_AUTO, em, CTLFLAG_RD, 0, "EM driver parameters");
> @@ -1396,15 +1391,9 @@ em_init_locked(struct adapter *adapter)
> if_clearhwassist(ifp);
> if (if_getcapenable(ifp) & IFCAP_TXCSUM)
> if_sethwassistbits(ifp, CSUM_TCP | CSUM_UDP, 0);
> - /*
> - ** There have proven to be problems with TSO when not
> - ** at full gigabit speed, so disable the assist automatically
> - ** when at lower speeds. -jfv
> - */
> - if (if_getcapenable(ifp) & IFCAP_TSO4) {
> - if (adapter->link_speed == SPEED_1000)
> - if_sethwassistbits(ifp, CSUM_TSO, 0);
> - }
> +
> + if (if_getcapenable(ifp) & IFCAP_TSO4)
> + if_sethwassistbits(ifp, CSUM_TSO, 0);
Does this always disable TSO? Should this part be removed entirely?
(That is, it seems like this would disable TSO even on Gigabit links).
> /* Configure for OS presence */
> em_init_manageability(adapter);
> @@ -2412,6 +2401,18 @@ em_update_link_status(struct adapter *ad
> if (link_check && (adapter->link_active == 0)) {
> e1000_get_speed_and_duplex(hw, &adapter->link_speed,
> &adapter->link_duplex);
> + /*
> + ** There have proven to be problems with TSO when not
> + ** at full gigabit speed, so disable the assist automatically
> + ** when at lower speeds. -jfv
> + */
> + if (adapter->link_speed != SPEED_1000) {
> + if_sethwassistbits(ifp, 0, CSUM_TSO);
> + if_setcapenablebit(ifp, 0, IFCAP_TSO4);
> + if_setcapabilitiesbit(ifp, 0, IFCAP_TSO4);
> +
> + }
Even though I suggested it, I wonder if it wouldn't be better to only
modify if_capenable and not if_capabilities, that way the admin can
decide to use 'ifconfig em0 tso' to force it back on (e.g. if moving
an adapter from 100 to 1G).
--
John Baldwin
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"
