ALTQ

[Jeffrey Hsu]

Joerg Sonnenberger wrote:
On Thu, Feb 10, 2005 at 10:18:45AM -0800, Matthew Dillon wrote:

   It looks ok except for the various IFQ_*() macros.  Passing a variable
   name as an argument to a macro (e.g. err) which then assigns the 
   variable creates a lot of confusion.  The NFSM code did this (albeit 
   much more aggregiously) and it created a real mess.


It still makes the variable used for error handling explicit.
The macros are compatible with KAME, I'd prefer to keep the current form.
They are used in two cases which a normal network driver should not trigger.
re(4) is a bit special because it tries to test the card first.
I wouldn't worry about vendor compatability.  We can also try to get the
changes back to KAME or we could import the KAME code on a vendor branch
and deal with vendor changes using cvs.
   Could you make those IFQ_*() macros real procedures instead of macros?
   Or at least real inline procedures.


Real functions is not an option, because it would enforce the overhead
for all interfaces, independent of ALTQ being available or not. Inline
functions would work, but still need a macro since IFQ_ENQUEUE's pattr
attribute is not available in non-ALTQ code. I'm not sure if that makes
the code really any easier to read.
If you're worried about compile-time efficiency, I would use the following,
which uses the compiler to optimize out the non-ALTQ code, while allowing
full compile-checking even when ALTQ is not defined.  It consolidates the
ALTQ and non-ALTQ cases into 1 inline version.
From

#ifdef ALTQ

#define IFQ_ENQUEUE(ifq, m, pattr, err) do {                            \
       if (ALTQ_IS_ENABLED((ifq)))                                     \
               ALTQ_ENQUEUE((ifq), (m), (pattr), (err));               \
       else {                                                          \
               if (IF_QFULL((ifq))) {                                  \
                       m_freem((m));                                   \
                       (err) = ENOBUFS;                                \
               } else {                                                \
                       IF_ENQUEUE((ifq), (m));                         \
                       (err) = 0;                                      \
               }                                                       \
       }                                                               \
       if ((err))                                                      \
               (ifq)->ifq_drops++;                                     \
} while (0)
#else

#define IFQ_ENQUEUE(ifq, m, pattr, err) do {                            \
       if (IF_QFULL(ifq)) {                                            \
               m_freem(m);                                             \
               (err) = ENOBUFS;                                        \
       } else {                                                        \
               IF_ENQUEUE(ifq, m);                                     \
               (err) = 0;                                              \
       }                                                               \
       if (err)                                                        \
               (ifq)->ifq_drops++;                                     \
} while (0)
#endif

#define IFQ_HANDOFF(ifp, m, pattr, err) do {                            \
       int s;                                                          \
                                                                       \
       s = splimp();                                                   \
       IFQ_ENQUEUE(&(ifp)->if_snd, m, pattr, err);                     \
       if ((err) == 0) {                                               \
               (ifp)->if_obytes += (m)->m_pkthdr.len;                  \
               if ((m)->m_flags & M_MCAST)                             \
                       (ifp)->if_omcasts++;                            \
               if (((ifp)->if_flags & IFF_OACTIVE) == 0)               \
                       (*(ifp)->if_start)(ifp);                        \
       }                                                               \
       splx(s);                                                        \
} while (0)
to

#ifndef ALTQ
#define ALTQF_ENABLED 0
#endif
static int __inline
ifq_enqueue(struct ifqueue *ifq, struct mbuf *m, struct altq_pktattr *pattr)   
{
       int error;

       if (ifq->altq_flags & ALTQF_ENABLED) {                 
               error = (*ifq->altq_enqueue)(ifq, m, pattr);
       } else {                        /* non-altq case */
               if (if_qfull(ifq)) {             
                       m_freem(m);
                       error = ENOBUFS;
               } else {       
                       if_enqueue(ifq, m);
                       error = 0;
               }
       }
       if (error)                                                        
               ifq->ifq_drops++;                                      
       return error;
} 

static int __inline
ifq_handoff(struct ifnet *ifp, struct mbuf *m, struct altq_pktattr *packetattr)
{
       int error, s;
       s = splimp();
       error = ifq_enqueue(&ifp->if_snd, m, packetattr);
       if (error == 0) {
               ifp->if_obytes += m->m_pkthdr.len;
               if (m->m_flags & M_MCAST)
                       ifp->if_omcasts++;
               if (!(ifp->if_flags & IFF_OACTIVE))
                       (*ifp->if_start)(ifp);
       }
       return error;
}