From owner-freebsd-net@FreeBSD.ORG Tue Apr 2 16:17:23 2013 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id C37A9C38 for ; Tue, 2 Apr 2013 16:17:23 +0000 (UTC) (envelope-from ncrogers@gmail.com) Received: from mail-vc0-f179.google.com (mail-vc0-f179.google.com [209.85.220.179]) by mx1.freebsd.org (Postfix) with ESMTP id 849C7DF8 for ; Tue, 2 Apr 2013 16:17:23 +0000 (UTC) Received: by mail-vc0-f179.google.com with SMTP id gf12so612429vcb.38 for ; Tue, 02 Apr 2013 09:17:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type; bh=P4q14PpLabTUm3JaMjxNyQGp5xz4i7skEYcUImgcsCM=; b=01V0g89ifk9245+pUmyiic8odZdfFIjTwfZh2hfvKDzkYGNmtrZA76NrNKHjJy9m2n E9KxmMdhNlgjsx3fLwg72kA8NzBHB+gcMR9UMr3lIDKmfEaoDVgqmlYj8JWSOEYN+zrS 5HtyuNuDX+rWImberBDusLSqR8aGKX2q0TMdsEL8IRe28QQdGAXP4K9MfMgbXwh6SYa8 VtATpmLcvjJb5Noqup4tGrbIH6t5OeLKgUIrfhBkWulUCo2ejVyw0dEeiz3o9NVyli/S +vmRPhV6nU8o7w9+Z8G6FHMSvzxo2inp9m7d4DiKe8X//GoRpQPfqgq+TMjV53LCk9ra lqJw== MIME-Version: 1.0 X-Received: by 10.220.140.18 with SMTP id g18mr13211717vcu.54.1364919437054; Tue, 02 Apr 2013 09:17:17 -0700 (PDT) Received: by 10.52.176.131 with HTTP; Tue, 2 Apr 2013 09:17:16 -0700 (PDT) In-Reply-To: <515AEF67.4070206@gmail.com> References: <1355177348.71087.YahooMailClassic@web121601.mail.ne1.yahoo.com> <50C6656E.3020601@gmail.com> <20121211075853.GU48639@FreeBSD.org> <5159AB72.1050202@gmail.com> <515ADACD.8010108@gmail.com> <515AEF67.4070206@gmail.com> Date: Tue, 2 Apr 2013 09:17:16 -0700 Message-ID: Subject: Re: igb and ALTQ in 9.1-rc3 From: Nick Rogers To: Karim Fodil-Lemelin Content-Type: text/plain; charset=ISO-8859-1 Cc: "freebsd-net@freebsd.org" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Apr 2013 16:17:23 -0000 On Tue, Apr 2, 2013 at 7:47 AM, Karim Fodil-Lemelin wrote: > Hi Nick, > > Unfortunately I do not have a FBSD 9 box around where I can compile and test > this so bear with me as this is pretty much straight out of looking at the > source code directly (i.e it might not even compile). > > But if your desperate please try the following (untested) patch (applied to > stable/9): Thanks for the attempt. The patch does not apply cleanly to stable/9 for me. Also I am trying to work with the latest commits Jack has made to HEAD that allow defining IGB_LEGACY_TX to disable the new multiqueue stack. It seems the gist of this is the same as what you have changed, unless I am missing something. > > diff --git a/sys/dev/e1000/if_igb.c b/sys/dev/e1000/if_igb.c > index 30bb052..3a6de2e 100644 > --- a/sys/dev/e1000/if_igb.c > +++ b/sys/dev/e1000/if_igb.c > @@ -179,13 +179,13 @@ static int igb_detach(device_t); > static int igb_shutdown(device_t); > static int igb_suspend(device_t); > static int igb_resume(device_t); > +static void igb_start(struct ifnet *); > #if __FreeBSD_version >= 800000 > static int igb_mq_start(struct ifnet *, struct mbuf *); > static int igb_mq_start_locked(struct ifnet *, struct tx_ring *); > static void igb_qflush(struct ifnet *); > static void igb_deferred_mq_start(void *, int); > #else > -static void igb_start(struct ifnet *); > static void igb_start_locked(struct tx_ring *, struct ifnet *ifp); > #endif > static int igb_ioctl(struct ifnet *, u_long, caddr_t); > @@ -377,7 +377,7 @@ SYSCTL_INT(_hw_igb, OID_AUTO, header_split, > CTLFLAG_RDTUN, &igb_header_split, 0, > ** This will autoconfigure based on > ** the number of CPUs if left at 0. > */ > -static int igb_num_queues = 0; > +static int igb_num_queues = 1; > TUNABLE_INT("hw.igb.num_queues", &igb_num_queues); > SYSCTL_INT(_hw_igb, OID_AUTO, num_queues, CTLFLAG_RDTUN, &igb_num_queues, > 0, > "Number of queues to configure, 0 indicates autoconfigure"); > @@ -926,6 +926,8 @@ igb_start_locked(struct tx_ring *txr, struct ifnet *ifp) > txr->queue_status |= IGB_QUEUE_WORKING; > } > } > + > +#endif /* __FreeBSD_version < 800000 */ > > /* > * Legacy TX driver routine, called from the > @@ -940,14 +942,17 @@ igb_start(struct ifnet *ifp) > > if (ifp->if_drv_flags & IFF_DRV_RUNNING) { > IGB_TX_LOCK(txr); > +#if __FreeBSD_version < 800000 > + igb_start_locked(txr, ifp); > +#else > + igb_mq_start_locked(ifp, txr, NULL); > +#endif > igb_start_locked(txr, ifp); > IGB_TX_UNLOCK(txr); > } > return; > } > > -#else /* __FreeBSD_version >= 800000 */ > - > /* > ** Multiqueue Transmit Entry: > ** quick turnaround to the stack > @@ -3089,12 +3094,11 @@ igb_setup_interface(device_t dev, struct adapter > *adapter) > #if __FreeBSD_version >= 800000 > ifp->if_transmit = igb_mq_start; > ifp->if_qflush = igb_qflush; > -#else > +#endif > > ifp->if_start = igb_start; > IFQ_SET_MAXLEN(&ifp->if_snd, adapter->num_tx_desc - 1); > ifp->if_snd.ifq_drv_maxlen = adapter->num_tx_desc - 1; > IFQ_SET_READY(&ifp->if_snd); > -#endif > > ether_ifattach(ifp, adapter->hw.mac.addr); > > > > > On 02/04/2013 9:58 AM, Nick Rogers wrote: >> >> On Tuesday, April 2, 2013, Karim Fodil-Lemelin wrote: >> >>> Hi Nick, >>> >>> Can you verify that you have at least one of those options in your kernel >>> config file: >>> >>> ALTQ_CBQ >>> ALTQ_PRIQ >>> ALTQ_HFSC >> >> >> Yes I have hfsc included in the kernel. I have other machines using altq >> with em(4) interfaces and similar pf configurations on the same kernel >> that >> work fine. >> >> >>> Regards, >>> >>> Karim. >>> >>> On 01/04/2013 8:22 PM, Nick Rogers wrote: >>> >>> On Mon, Apr 1, 2013 at 8:44 AM, Karim Fodil-Lemelin >>> wrote: >>> >>> Hi Jack, >>> >>> I think this would help M. Rogers case as we have done something similar >>> here to circumvent the issue and it seems to work well. I would also add >>> that when using ALTQ we found it much more stable to set the number of >>> queues to 1: >>> >>> static int igb_num_queues = 1; >>> >>> Our approach consisted in keeping igb_start() defined and using >>> igb_mq_start_locked() inside it instead of igb_start_locked(). >>> >>> Regards, >>> >>> Karim. >>> >>> Thanks for the pointer. >>> >>> I've been working with Jack to get a fix for this in that downgrades >>> the stack to the older if_start/non-multiqueue interface. See the >>> following two commits he made to HEAD. >>> >>> http://svnweb.freebsd.org/**base/head/sys/dev/e1000/if_** >>> >>> igb.c?revision=248906&view=**markup >>> http://svnweb.freebsd.org/**base/head/sys/dev/e1000/if_** >>> >>> igb.h?revision=248908&view=**markup >>> >>> >>> I've applied these changes to latest 9.1-STABLE src and included the >>> IGB_LEGACY_TX in the e1000 Makefile. So far I am not having any luck >>> getting pfctl to think igb is supported. >>> >>> i.e. I am still getting the following when loading my PF config: >>> pfctl -f /etc/pf.conf >>> pfctl: igb0: driver does not support altq >>> >>> Can anyone comment on if the above referenced changes that Jack made >>> should be sufficient to get ALTQ working with igb? >>> >>> The error is coming from src/contrib/pf/pfctl/pfctl.c. There seems to >>> be a function that checks if the driver is supported or not but I >>> cannot figure out why the ioctl is not returning a device. >>> >>> Here is the device check code for reference: >>> >>> int >>> pfctl_add_altq(struct pfctl *pf, struct pf_altq *a) >>> { >>> if (altqsupport && >>> (loadopt & PFCTL_FLAG_ALTQ) != 0) { >>> memcpy(&pf->paltq->altq, a, sizeof(struct pf_altq)); >>> if ((pf->opts & PF_OPT_NOACTION) == 0) { >>> if (ioctl(pf->dev, DIOCADDALTQ, pf->paltq)) { >>> if (errno == ENXIO) >>> errx(1, "qtype not >>> configured"); >>> else if (errno == ENODEV) >>> errx(1, "%s: driver does not >>> support " >>> "altq", a->ifname); >>> else >>> err(1, "DIOCADDALTQ"); >>> } >>> } >>> pfaltq_store(&pf->paltq->altq)**; >>> >>> } >>> return (0); >>> } >>> >>> >>> >>> >>> >>> On 28/03/2013 7:16 PM, Jack Vogel wrote: >>> >>> Have been kept fairly busy with other matters, one thing I could do short >>> term is >>> change the defines in igb the way I did in the em driver so you could >>> still >>> define >>> the older if_start entry. Right now those are based on OS version and so >>> you will >>> automatically get if_transmit, but I could change it to be IGB_LEGACY_TX >>> or >>> so, >>> and that could be defined in the Makefile. >>> >>> Would this help? >>> >>> Jack >>> >>> >>> On Thu, Mar 28, 2013 at 2:31 PM, Nick Rogers wrote: >>> >>> On Tue, Dec 11, 2012 at 1:09 AM, Jack Vogel wrote: >>> >>> On Mon, Dec 10, 2012 at 11:58 PM, Gleb Smirnoff >>> >>> wrote: >>> >>> On Mon, Dec 10, 2012 at 03:31:19PM -0800, Jack Vogel wrote: >>> J> UH, maybe asking the owner of the driver would help :) >>> J> >>> J> ... and no, I've never been aware of doing anything to stop >>> >>> supporting >>> >>> >> _______________________________________________ >> freebsd-net@freebsd.org mailing list >> http://lists.freebsd.org/mailman/listinfo/freebsd-net >> To unsubscribe, send any mail to "freebsd-net-unsubscribe@freebsd.org" > >