From owner-freebsd-bugs@FreeBSD.ORG Wed Nov 12 02:15:13 2008 Return-Path: Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DB21A106567D for ; Wed, 12 Nov 2008 02:15:13 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from rv-out-0506.google.com (rv-out-0506.google.com [209.85.198.237]) by mx1.freebsd.org (Postfix) with ESMTP id AA4BB8FC17 for ; Wed, 12 Nov 2008 02:15:13 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: by rv-out-0506.google.com with SMTP id b25so171970rvf.43 for ; Tue, 11 Nov 2008 18:15:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:received:date:from :to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=pglZWIuR7SV1GMKjxIfN2qe1VOGuUgWfQPFg+K+A9gw=; b=hpqz+6baRMc8ll0RWTqid9Q2i+zYxopXwzlw5ZI0+YYx7/v8q/OjGF1Z+P4Btw5hOy WqIFWZl9lf1AmnHluEhxlS7w5wYqf8W1eaDXnS4pHnE6UjGRhaK20V90oH88Pj7YvV9d OdSjRnkZ0gPk+D9J5oTmrtTLjNu9OYH0jChZM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=SC9ixiJ51+fO9JmuYU4h8nWLE4yyjteJsnle8q9ophDR5np50Wn8xRd6KPVOx+iZn0 aXDMh9m8w/HLwN7gXr9Ws7l+54ZbJajoL+TcoGy4EWAnmJI0XI55dZvP9AwwAMKKlfD8 8en0Tkyj51eFwjYj30Ncl2XEitUG2qukyRwhU= Received: by 10.140.165.21 with SMTP id n21mr4615504rve.240.1226456113465; Tue, 11 Nov 2008 18:15:13 -0800 (PST) Received: from michelle.cdnetworks.co.kr ([211.53.35.84]) by mx.google.com with ESMTPS id g31sm27325548rvb.7.2008.11.11.18.15.10 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 11 Nov 2008 18:15:12 -0800 (PST) Received: from michelle.cdnetworks.co.kr (localhost.cdnetworks.co.kr [127.0.0.1]) by michelle.cdnetworks.co.kr (8.13.5/8.13.5) with ESMTP id mAC2D972030594 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 12 Nov 2008 11:13:09 +0900 (KST) (envelope-from pyunyh@gmail.com) Received: (from yongari@localhost) by michelle.cdnetworks.co.kr (8.13.5/8.13.5/Submit) id mAC2D7ck030593; Wed, 12 Nov 2008 11:13:07 +0900 (KST) (envelope-from pyunyh@gmail.com) Date: Wed, 12 Nov 2008 11:13:07 +0900 From: Pyun YongHyeon To: Dan Lukes Message-ID: <20081112021307.GA30203@cdnetworks.co.kr> References: <200811110127.mAB1RiQV095192@freefall.freebsd.org> <49194D6A.30807@obluda.cz> <20081111101606.GB26349@cdnetworks.co.kr> <49197DB7.5080702@obluda.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <49197DB7.5080702@obluda.cz> User-Agent: Mutt/1.4.2.1i Cc: freebsd-bugs@FreeBSD.org, yongari@FreeBSD.org Subject: Re: kern/128766: [ PATCH ] VGE's [rt]xcsum can't be switched off X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: pyunyh@gmail.com List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Nov 2008 02:15:14 -0000 On Tue, Nov 11, 2008 at 01:42:31PM +0100, Dan Lukes wrote: > Pyun YongHyeon napsal/wrote, On 11/11/08 11:16: > > > 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; > > > } > > > =================================================================== > > > > > > >Yeah, ATM your code is correct for userland side. But there is no > >guarantee that all vge(4) hardwares have the same capabilities. > > It check it against if_capabilites, eg against capabilites supported by > a instance of driver as if_capabilities are capabilities of such > particular hardware. > > >If > >Tx checksum offload was broken for specific revision, driver should > >check whether the capability is available against the hardware. > > If txcsum was broken for specific revision then driver shall not mark > the txcsum to be supported in if_capabilities. > Correct. That is one of reason why I check if_capabilities in ioctl() without relying on ifhwioctl().(as you said ifhwioctl() checks if_capabilities but I'd like more tight check due to other part of kernel. Adding an additional if_capabilities check wouldn't hurt and it's not in fast path.) This also separates chipset specific code from capability ioctl. Without that I would have embedded another variable into softc. > >There are still a couple of drivers in tree that misuses this kind > >of checking. > > >Also remember some subsystem still directly calls driver ioctls > >without relying on ifhwioctl() so it still needs the check > >(e.g. bridge(4)). > > True. Either drivers or callers shall do such test. It's questionable if > all drivers shall be corrected or bridge(4) code, but you are comitter, > it's your decision. > [...] > > > 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. > > > > > > >See above. I think you can't do that. Drivers should always check > >against its usable capabilities. > > I'm speaking about something diferent. if_capabilities are capabilities > for such instance of driver. But i'm not speaking about the request to > change of capability that are not supported by driver at all. I'm > speaking about the request to change the capability that is supported > but can't be turned off. > Ok. does the patch I posted work for you? (I hoped I finish vge(4) cleanups but half-broken VT6122 I have made it difficult to do that.) -- Regards, Pyun YongHyeon