From owner-svn-src-head@freebsd.org Thu Jul 5 01:20:30 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8249E1032C07; Thu, 5 Jul 2018 01:20:30 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id DF72D7DEB8; Thu, 5 Jul 2018 01:20:29 +0000 (UTC) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: from pdx.rh.CN85.dnsmgr.net (localhost [127.0.0.1]) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3) with ESMTP id w651KPeY045634; Wed, 4 Jul 2018 18:20:25 -0700 (PDT) (envelope-from freebsd@pdx.rh.CN85.dnsmgr.net) Received: (from freebsd@localhost) by pdx.rh.CN85.dnsmgr.net (8.13.3/8.13.3/Submit) id w651KP5K045633; Wed, 4 Jul 2018 18:20:25 -0700 (PDT) (envelope-from freebsd) From: "Rodney W. Grimes" Message-Id: <201807050120.w651KP5K045633@pdx.rh.CN85.dnsmgr.net> Subject: Re: svn commit: r335967 - head/sys/dev/mxge In-Reply-To: <658fb418-5a84-dc91-2cc8-1a503a3d43a6@cs.duke.edu> To: Andrew Gallatin Date: Wed, 4 Jul 2018 18:20:25 -0700 (PDT) CC: rgrimes@freebsd.org, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Reply-To: rgrimes@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Jul 2018 01:20:30 -0000 > 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. > Looking around at other drivers, I see > at least one (cxgbe) which does the same thing. Bad examples are usually easy to find. > After doing the grep, I'm more concerned with drivers which may > be setting their tsomaxsegsize incorrectly to be too small and > hurting their performance by causing TCP to chop needlessly > at smaller boundaries which are already enforced by their > busdma tags. PAGE_SIZE, which seems to be the common mistaken > size, won't hurt too much I suppose. But the default of 2K > is probably not very good. > > Drew > > -- Rod Grimes rgrimes@freebsd.org