From owner-freebsd-sparc64@FreeBSD.ORG Fri Jun 18 01:21:11 2004 Return-Path: Delivered-To: freebsd-sparc64@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 83B1E16A4CE for ; Fri, 18 Jun 2004 01:21:11 +0000 (GMT) Received: from mail.gmx.net (mail.gmx.net [213.165.64.20]) by mx1.FreeBSD.org (Postfix) with SMTP id A6AA243D53 for ; Fri, 18 Jun 2004 01:21:10 +0000 (GMT) (envelope-from tmoestl@gmx.net) Received: (qmail 5806 invoked by uid 65534); 18 Jun 2004 01:20:38 -0000 Received: from p50907E21.dip.t-dialin.net (EHLO timesink.dyndns.org) (80.144.126.33) by mail.gmx.net (mp006) with SMTP; 18 Jun 2004 03:20:38 +0200 X-Authenticated: #5374206 Received: by abel (Postfix, from userid 1001) id CC93F6E8; Fri, 18 Jun 2004 03:20:45 +0200 (CEST) Date: Fri, 18 Jun 2004 03:20:45 +0200 From: Thomas Moestl To: Pyun YongHyeon Message-ID: <20040618012045.GF747@timesink.dyndns.org> References: <20040616034520.GB7887@kt-is.co.kr> <20040617024759.GA5610@timesink.dyndns.org> <20040617063022.GA11797@kt-is.co.kr> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040617063022.GA11797@kt-is.co.kr> User-Agent: Mutt/1.5.6i cc: freebsd-sparc64@freebsd.org Subject: Re: TCP/UDP cksum offload on hme(4) X-BeenThere: freebsd-sparc64@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Porting FreeBSD to the Sparc List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jun 2004 01:21:11 -0000 On Thu, 2004/06/17 at 15:30:22 +0900, Pyun YongHyeon wrote: > On Thu, Jun 17, 2004 at 04:48:00AM +0200, Thomas Moestl wrote: > > On Wed, 2004/06/16 at 12:45:20 +0900, Pyun YongHyeon wrote: > > > + > > > + for(; m && m->m_len == 0; m = m->m_next) > > > + ; > > > + if (m == NULL || m->m_len < ETHER_HDR_LEN) { > > > + printf("hme_txcksum: m_len < ETHER_HDR_LEN\n"); > > > + return; /* cksum will be corrupted */ > > > + } > > > > It would probably be best to try as hard as possible to not fail > > here. Since the conditions in which we would fail because of mbuf > > sizes should rarely be true, maybe a good option would be to just > > sacrifice some CPU time and do an m_pullup() then to rectify the mbuf > > chain according to our needs (hme_txcksum would need to return a > > struct mbuf * for that)? > > > Because we will have ALTQ feature in near future, the driver > can't call IF_PREPEND anymore. So if m_pullup() failed it would > be dropped. I thought, we may not encounter such mbuf fragmentation > in real environments.(I didn't see any of these fragment condition > on my setup.) > If we use m_pullup() it will make ALTQ-enabled hme driver useless. As far as I know, the latest consensus was that there would be an equivalent function/macro even with ALTQ; otherwise, the driver would require some massive rework, as we depend on the prepend functionality right now (because we cannot know if we have a sufficient amount of free descriptors in advance). On second thought, though, I agree that adding more complexity to handle this case may be overkill, and that handling m_pullup() failures could get a bit yucky. > > > > + > > > + hlen = ip->ip_hl << 2; > > > + pktlen -= sizeof(struct ether_header); > > > + if (hlen < sizeof(struct ip)) > > > + return; > > > + if (ntohs(ip->ip_len) < hlen) > > > + return; > > > > This test is a bit redundant with the TCP/UDP ones below. > > > When we have a packet that contains IP header with options plus some > corrupted TCP/UDP header(i.e. less than the size of the header), > should the check be performed? Hmmm, in that case we could not use the checksumming anyway, so it would not be necessary. But that is, of course, nitpicking :) > New patch is available at: > http://www.kr.freebsd.org/~yongari/hme.freebsd.diff2 Just one further remark: > + if ((ifp->if_flags & IFF_LINK0) != 0) > + hme_csum_features |= CSUM_UDP; > + else > + hme_csum_features &= ~CSUM_UDP; Maybe if_hwassist should be changed accordingly in this case, too. It might be a bit counter-intuitive to need to do an extra SIOCSIFCAP before this takes effect. Thanks for your changes! Since people have reported that this patch works well on PCI hmes, I am planning to commit it soon. - Thomas -- Thomas Moestl http://www.tu-bs.de/~y0015675/ http://people.FreeBSD.org/~tmm/ "I couldn't read it because my parents forgot to pay the gravity bill." -- Calvin and Hobbes