svn commit: r335967 - head/sys/dev/mxge

[Andrew Gallatin]

On 7/4/18 9:20 PM, Rodney W. Grimes wrote:
>> On 07/04/18 15:46, Rodney W. Grimes wrote:
>>>> Author: gallatin
>>>> Date: Wed Jul  4 19:29:06 2018
>>>> New Revision: 335967
>>>> URL: https://urldefense.proofpoint.com/v2/url?u=https-3A__svnweb.freebsd.org_changeset_base_335967&d=DwICAg&c=imBPVzF25OnBgGmVOlcsiEgHoG1i6YHLR0Sj_gZ4adc&r=Ed-falealxPeqc22ehgAUCLh8zlZbibZLSMWJeZro4A&m=2rIiw5AUJ2ishkBkygGMa_9kr0LJOaonX8ni3BF2BHk&s=MwCt6_IgNah0XklsYThsXFcwZD54Xl78TRlnFXJ4zWs&e=
>>>>
>>>> Log:
>>>>     mxge: choose appropriate values for hw tso
>>>>
>>>> Modified:
>>>>     head/sys/dev/mxge/if_mxge.c
>>>>
>>>> Modified: head/sys/dev/mxge/if_mxge.c
>>>> ==============================================================================
>>>> --- head/sys/dev/mxge/if_mxge.c	Wed Jul  4 18:54:44 2018	(r335966)
>>>> +++ head/sys/dev/mxge/if_mxge.c	Wed Jul  4 19:29:06 2018	(r335967)
>>>> @@ -4984,6 +4984,9 @@ mxge_attach(device_t dev)
>>>>    	ifp->if_ioctl = mxge_ioctl;
>>>>    	ifp->if_start = mxge_start;
>>>>    	ifp->if_get_counter = mxge_get_counter;
>>>> +	ifp->if_hw_tsomax = 65536 - (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN);
>>>
>>> Would not this be more accurate (need to reorder assigns):
>>> 	ifp->if_hw_tsomax = ifp->if_hw_tsomaxsegsize - (ETHER_HDR_LEN + ETHER_VLAN_ENCAP_LEN);
>>>
>>>> +	ifp->if_hw_tsomaxsegcount = sc->ss[0].tx.max_desc;
>>>> +	ifp->if_hw_tsomaxsegsize = 65536;
>>
>>
>> It seems simpler as-is to me.
> 
> It is using a magic constant twice, where one has a
> derived value that is dependent on the value of the other.
> That is bad and error prone and does not document that
> one depends on the other.  Please fix this.  Or at least
> make 65536 a #define so that it only needs changed one
> place and clearly shows the interdependence of these
> values.

To me, 65536 is one of the few cases where the magic number is
more meaningful than a name.  But fine, if you feel that
strongly about it, I'll change it for you.