Re: mbuf initialization macros

[christos%astron.com@localhost (Christos Zoulas)]

In article <5710550E.4010406%iij.ad.jp@localhost>,
Kengo NAKAHARA  <k-nakahara%iij.ad.jp@localhost> wrote:
>Hi,
>
>I found some functions(e.g. _pf_mtap2()@sys/net/bpf.c) use mbuf allocated
>on stack instead of allocated by m_get(). Furthermore, the functions
>initialize mbuf incompletely. It might cause problems when struct mbuf
>fields is modified.
>
>I think mbuf initialization macros is required to prevent these potential
>problems. That is, the following patch is required.
>
>====================
>diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
>index f2056da..a8426b7 100644
>--- a/sys/kern/uipc_mbuf.c
>+++ b/sys/kern/uipc_mbuf.c
>@@ -583,14 +583,10 @@ m_get(int nowait, int type)
>                return NULL;
> 
>        mbstat_type_add(type, 1);
>+
>+       M_HDR_INIT(m);
>        mowner_init(m, type);
>-       m->m_ext_ref = m;
>        m->m_type = type;
>-       m->m_len = 0;
>-       m->m_next = NULL;
>-       m->m_nextpkt = NULL;
>-       m->m_data = m->m_dat;
>-       m->m_flags = 0;
> 
>        return m;
> }
>@@ -606,11 +602,8 @@ m_gethdr(int nowait, int type)
> 
>        m->m_data = m->m_pktdat;
>        m->m_flags = M_PKTHDR;
>-       m->m_pkthdr.rcvif = NULL;
>-       m->m_pkthdr.len = 0;
>-       m->m_pkthdr.csum_flags = 0;
>-       m->m_pkthdr.csum_data = 0;
>-       SLIST_INIT(&m->m_pkthdr.tags);
>+
>+       M_PKTHDR_INIT(m);
> 
>        return m;
> }
>diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
>index 75f4d64..6882241 100644
>--- a/sys/sys/mbuf.h
>+++ b/sys/sys/mbuf.h
>@@ -179,6 +179,24 @@ struct     pkthdr {
>        u_int           segsz;                  /* segment size */
> };
> 
>+#define M_HDR_INIT(_m) do {                                            \
>+               (_m)->m_ext_ref = (_m);                                 \
>+               (_m)->m_type = MT_DATA;                                 \
>+               (_m)->m_len = 0;                                        \
>+               (_m)->m_next = NULL;                                    \
>+               (_m)->m_nextpkt = NULL;                                 \
>+               (_m)->m_data = (_m)->m_dat;                             \
>+               (_m)->m_flags = 0;                                      \
>+       } while(0)
>+
>+#define M_PKTHDR_INIT(_m) do {                                         \
>+               (_m)->m_pkthdr.rcvif = NULL;                            \
>+               (_m)->m_pkthdr.len = 0;                                 \
>+               (_m)->m_pkthdr.csum_flags = 0;                          \
>+               (_m)->m_pkthdr.csum_data = 0;                           \
>+               SLIST_INIT(&(_m)->m_pkthdr.tags);                       \
>+       } while(0)
>+
> /*
>  * Note: These bits are carefully arrange so that the compiler can have
>  * a prayer of generating a jump table.
>====================
>
>Could you comment this patch?
>I will commit this patch after a few days or a few weeks if there is
>no objection.
># And then, I will apply the macros to mbufs allocated on stack bit by bit.

Sure, but why not inline functions instead?

christos