igb and ALTQ in 9.1-rc3

Karim Fodil-Lemelin fodillemlinkarim at gmail.com
Tue Apr 2 14:47:07 UTC 2013


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):

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
>> <fodillemlinkarim at gmail.com> 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.c?revision=248906&view=markup>
>> http://svnweb.freebsd.org/**base/head/sys/dev/e1000/if_**
>> igb.h?revision=248908&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 <ncrogers at gmail.com> wrote:
>>
>>   On Tue, Dec 11, 2012 at 1:09 AM, Jack Vogel <jfvogel at gmail.com> wrote:
>>
>> On Mon, Dec 10, 2012 at 11:58 PM, Gleb Smirnoff <glebius at freebsd.org>
>>
>> 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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscribe at freebsd.org"



More information about the freebsd-net mailing list