kern/128766: [ PATCH ] VGE's [rt]xcsum can't be switched off

[Dan Lukes]

yongari at FreeBSD.org napsal/wrote, On 11/11/08 02:27:
> It seems that the hardware does not require reinitialization when
> checksum offload configuration is changed.
> Would you try patch at the followng URL?

True, vge_init() is not necesarry. Your code ...

===================================================================
	if ((mask & IFCAP_TXCSUM) != 0 &&
	    (ifp->if_capabilities & IFCAP_TXCSUM) != 0) {
		ifp->if_capenable ^= IFCAP_TXCSUM;
		if ((ifp->if_capenable & IFCAP_TXCSUM) != 0)
			ifp->if_hwassist |= VGE_CSUM_FEATURES;
		else
			ifp->if_hwassist &= ~VGE_CSUM_FEATURES;
	}
	if ((mask & IFCAP_RXCSUM) != 0 &&
	    (ifp->if_capabilities & IFCAP_RXCSUM) != 0)
		ifp->if_capenable ^= IFCAP_RXCSUM;
	}
===================================================================

... will work, but is suboptimal.

The tests (ifp->if_capabilities & IFCAP_TXCSUM) and 
(ifp->if_capabilities & IFCAP_RXCSUM) is true everytime because the test 
is done already in upper layer (see net.c:ifhwioctl(), it return EINVAL 
when someone try to set capabilities not offered by driver)

Code with such tests removed ...

===================================================================
	if (mask & IFCAP_TXCSUM) != 0) {
		ifp->if_capenable ^= IFCAP_TXCSUM;
		if ((ifp->if_capenable & IFCAP_TXCSUM) != 0)
			ifp->if_hwassist |= VGE_CSUM_FEATURES;
		else
			ifp->if_hwassist &= ~VGE_CSUM_FEATURES;
	}
	if (mask & IFCAP_RXCSUM != 0)
		ifp->if_capenable ^= IFCAP_RXCSUM;
	}
===================================================================

... is OK but ~VGE_CSUM_FEATURES shall be ~VGE_CSUM_FEATURE instead 
(-VGE_CSUM_FEATURES may not be equal to ~VGE_CSUM_FEATURES althought it 
is on most platforms).

In advance, we shall catch attempt to modify capabilities that are 
supported by driver, but that driver doesn't support to be switched off.

If we shall do
mask ^= IFCAP_POLLING
on the end of IFCAP_POLLING handling and
mask ^= (IFCAP_TXCSUM | IFCAP_RXCSUM)
on the end of CSUM handling then we can do

if (mask != 0)
	return(ENOTSUPP)
on the end of SIOCSIFCAP.

The resulting patch is attached.

					Dan

-------------- next part --------------
--- /usr/src/sys/dev/vge/if_vgevar.h.orig	2008-11-10 23:53:04.000000000 +0100
+++ /usr/src/sys/dev/vge/if_vgevar.h	2008-11-11 02:19:20.000000000 +0100
@@ -2228,16 +2228,21 @@
 				VGE_UNLOCK(sc);
 			}
 		}
+		mask ^= IFCAP_POLLING;
 #endif /* DEVICE_POLLING */
-		if (mask & IFCAP_HWCSUM) {
-			ifp->if_capenable |= ifr->ifr_reqcap & (IFCAP_HWCSUM);
+		if (mask & IFCAP_TXCSUM) {
+			ifp->if_capenable ^= IFCAP_TXCSUM;
 			if (ifp->if_capenable & IFCAP_TXCSUM)
-				ifp->if_hwassist = VGE_CSUM_FEATURES;
+				ifp->if_hwassist |= VGE_CSUM_FEATURES;
 			else
-				ifp->if_hwassist = 0;
-			if (ifp->if_drv_flags & IFF_DRV_RUNNING)
-				vge_init(sc);
+				ifp->if_hwassist &= ~VGE_CSUM_FEATURES;
 		}
+		if (mask & IFCAP_RXCSUM) {
+			ifp->if_capenable ^= IFCAP_RXCSUM;
+		}
+		mask ^= (IFCAP_TXCSUM | IFCAP_RXCSUM);
+		if (mask != 0)
+			return(ENODEV);
 	    }
 		break;
 	default: