Re: ALTQ
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;
}
討論串 (同標題文章)