Date: Wed, 4 Jul 2018 22:33:09 -0400 From: Andrew Gallatin <gallatin@cs.duke.edu> To: rgrimes@freebsd.org Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r335967 - head/sys/dev/mxge Message-ID: <97ae3381-7c25-7b41-9670-84b825722f52@cs.duke.edu> In-Reply-To: <201807050120.w651KP5K045633@pdx.rh.CN85.dnsmgr.net> References: <201807050120.w651KP5K045633@pdx.rh.CN85.dnsmgr.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?97ae3381-7c25-7b41-9670-84b825722f52>