From owner-svn-src-head@freebsd.org Thu Jul 5 02:33:11 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 B008E103AFB2; Thu, 5 Jul 2018 02:33:11 +0000 (UTC) (envelope-from gallatin@cs.duke.edu) Received: from duke.cs.duke.edu (duke.cs.duke.edu [152.3.140.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 6EB6581B54; Thu, 5 Jul 2018 02:33:11 +0000 (UTC) (envelope-from gallatin@cs.duke.edu) Received: from [192.168.200.210] (c-73-216-227-39.hsd1.va.comcast.net [73.216.227.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: gallatin) by duke.cs.duke.edu (Postfix) with ESMTPSA id 5F5922700341; Wed, 4 Jul 2018 22:33:10 -0400 (EDT) DMARC-Filter: OpenDMARC Filter v1.3.1 duke.cs.duke.edu 5F5922700341 Authentication-Results: duke.cs.duke.edu; dmarc=none header.from=cs.duke.edu DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=cs.duke.edu; s=mail0816; t=1530757990; bh=/14FZyGAfCM4YC/ClGl69iMd2FWNGvDRUyHUEo9mLL4=; h=Subject:To:From:Date:From; b=XrQ+JIxcHJwEJRfjEyIJ/Y7YEyft/PWQsKEgB+qXy+M6FtoEJMrwoZZ2J1azWH8iz MaqvkDP0htYf6O9i61hg7J0FzabPZPUopLoegW5aM+5myc/mMfwDp0f4nca0I6V+hR j9YWnD8HmbvPTGzMGvsh6ae4N9qKTIs0jWfRH5gW+7aWia06SEO4K7LMvlYACZnUKb ZCMwWo80239yi42rFognb3ootcRGHJPHxhh720fsmPSqr+rnokFwAY3YmZFt+UADoy pUG1HjaYtyrtMZoaSKimgi4SWLnISY6kOzVZmaiishN8yJC09y7ZBu8sDaU6AYxbUm 6uyCkA+2tCRHA== Subject: Re: svn commit: r335967 - head/sys/dev/mxge To: rgrimes@freebsd.org Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201807050120.w651KP5K045633@pdx.rh.CN85.dnsmgr.net> From: Andrew Gallatin Message-ID: <97ae3381-7c25-7b41-9670-84b825722f52@cs.duke.edu> Date: Wed, 4 Jul 2018 22:33:09 -0400 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <201807050120.w651KP5K045633@pdx.rh.CN85.dnsmgr.net> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 02:33:11 -0000 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.