Discussion:
mbuf initialization macros
(too old to reply)
Kengo NAKAHARA
2016-04-15 02:42:22 UTC
Permalink
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.


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Elz
2016-04-15 08:44:39 UTC
Permalink
Date: Fri, 15 Apr 2016 11:42:22 +0900
From: Kengo NAKAHARA <k-***@iij.ad.jp>
Message-ID: <***@iij.ad.jp>

| Could you comment this patch?

I would make them static inline functions, rather than macros, so you
don't need to use "implementation" var names (not that it matters as
a macro arg) and avoid problems should someone, sometime later, use a
non-constant arg (m++ or something).

I would also (after an audit of the uses) add any fields that are
commonly set to something other than NULL/0/etc in the init, and make
those params to the function - if the example you showed is typical,
then for M_HDR_INIT() (which as a func would probably be m_hdr_init())
I'd add a type param (and move the mowner_init() call into the func,)
as I would expect that frequently code that does things this way is
likely to want a type that is not MT_DATA.

But only for those values are frequently set to something specific in the
current places where you would (eventually) replace inline code by a
macro/static_inline_func call. (Doesn't need to be all cases, but a param
that everyone has to set, just because one place wants the field different
than default, would be annoying).

For M_PKTHDR_INIT() I'd add data & flags params to set m_data and m_flags.
That is: make the macro/func turn the mbuf into a pkhhdr, rather than just
init the pkthdr fields after that has already happened.

If any of the other m_pkthdr fields are often set to different than
the default values (most likely would probably be rcvif) make that a
param too - just like (probably) "type" for M_HDR_INIT().

kre


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-18 04:34:07 UTC
Permalink
Hi,

Thank you for comments.
Post by Robert Elz
Date: Fri, 15 Apr 2016 11:42:22 +0900
| Could you comment this patch?
I would make them static inline functions, rather than macros, so you
don't need to use "implementation" var names (not that it matters as
a macro arg) and avoid problems should someone, sometime later, use a
non-constant arg (m++ or something).
Fair enough. I use static inline function.
Post by Robert Elz
I would also (after an audit of the uses) add any fields that are
commonly set to something other than NULL/0/etc in the init, and make
those params to the function - if the example you showed is typical,
then for M_HDR_INIT() (which as a func would probably be m_hdr_init())
I'd add a type param (and move the mowner_init() call into the func,)
as I would expect that frequently code that does things this way is
likely to want a type that is not MT_DATA.
I see. I add a type param and move the monwer_init().
Post by Robert Elz
But only for those values are frequently set to something specific in the
current places where you would (eventually) replace inline code by a
macro/static_inline_func call. (Doesn't need to be all cases, but a param
that everyone has to set, just because one place wants the field different
than default, would be annoying).
As all of mbuf stack allocating functions set the following fields,
I add these fields to params.
- m_next
- m_data
- m_len

Some of the functions set m_flags and m_nextpkt, however m_flags is
almost set 0 and m_nextpkt is always set NULL. So I don't add these
fields.
Post by Robert Elz
For M_PKTHDR_INIT() I'd add data & flags params to set m_data and m_flags.
That is: make the macro/func turn the mbuf into a pkhhdr, rather than just
init the pkthdr fields after that has already happened.
I see. I set m_data and m_flags in m_pkthdr_init().
Post by Robert Elz
If any of the other m_pkthdr fields are often set to different than
the default values (most likely would probably be rcvif) make that a
param too - just like (probably) "type" for M_HDR_INIT().
Hmm, in the mbuf stack allocating functions, only dp8390_ipkdb_send()
@sys/dev/ic/dp8390.c sets M_PKTHDR to m_flags. The function doesn't
set m_pkthdr fields explicitly. Furthermore, other M_PKTHDR mbuf using
functions use MGETDHR() or m_gethdr(). So, I think m_pkthdr_init()
don't need to add params except for mbuf.

I reflect above comments. Here is updated patch.
====================
diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index f2056da..b48c599 100644
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -583,14 +583,8 @@ m_get(int nowait, int type)
return NULL;

mbstat_type_add(type, 1);
- 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;
+
+ m_hdr_init(m, type, NULL, 0, m->m_dat);

return m;
}
@@ -604,13 +598,7 @@ m_gethdr(int nowait, int type)
if (m == NULL)
return NULL;

- 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..127933c 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -867,6 +867,10 @@ int m_append(struct mbuf *, int, const void *);
/* Inline routines. */
static __inline u_int m_length(const struct mbuf *) __unused;

+static __inline void m_hdr_init(struct mbuf *, short, struct mbuf *,
+ char *, int);
+static __inline void m_pkthdr_init(struct mbuf *);
+
/* Statistics */
void mbstat_type_add(int, int);

@@ -934,6 +938,38 @@ m_length(const struct mbuf *m)
return pktlen;
}

+static __inline void
+m_hdr_init(struct mbuf *m, short type, struct mbuf *next, char *data, int len)
+{
+
+ KASSERT(m != NULL);
+
+ mowner_init(m, type);
+ m->m_ext_ref = m; /* default */
+ m->m_type = type;
+ m->m_len = len;
+ m->m_next = next;
+ m->m_nextpkt = NULL; /* default */
+ m->m_data = data;
+ m->m_flags = 0; /* default */
+}
+
+static __inline void
+m_pkthdr_init(struct mbuf *m)
+{
+
+ KASSERT(m != NULL);
+
+ 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);
+}
+
void m_print(const struct mbuf *, const char *, void (*)(const char *, ...)
__printflike(1, 2));

====================

Could you comment this patch?


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2016-04-18 15:51:13 UTC
Permalink
Post by Kengo NAKAHARA
@@ -867,6 +867,10 @@ int m_append(struct mbuf *, int, const void *);
/* Inline routines. */
static __inline u_int m_length(const struct mbuf *) __unused;
+static __inline void m_hdr_init(struct mbuf *, short, struct mbuf *,
+ char *, int);
+static __inline void m_pkthdr_init(struct mbuf *);
+
Looks good, but you should not need the above... Perhaps remove all
the static __inline decls. Is the __unused needed? Can it be moved in
the definition?

christos


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Elz
2016-04-18 23:51:50 UTC
Permalink
Date: Mon, 18 Apr 2016 13:34:07 +0900
From: Kengo NAKAHARA <k-***@iij.ad.jp>
Message-ID: <***@iij.ad.jp>

| Some of the functions set m_flags and m_nextpkt, however m_flags is
| almost set 0 and m_nextpkt is always set NULL. So I don't add these
| fields.

That's fine.

| Hmm, in the mbuf stack allocating functions, only dp8390_ipkdb_send()
| @sys/dev/ic/dp8390.c sets M_PKTHDR to m_flags. The function doesn't
| set m_pkthdr fields explicitly. Furthermore, other M_PKTHDR mbuf using
| functions use MGETDHR() or m_gethdr(). So, I think m_pkthdr_init()
| don't need to add params except for mbuf.

Also fine.

| I reflect above comments. Here is updated patch.
[...]
| Could you comment this patch?

As Christos said, you don't need prototypes for inline functions declared
in a header file (the declaration is its own prototype) unless they are going
to refer to each other (in which case they couldn't really be inline).

Aside from that, the changes you're proposing look fine to me. The __unused
that Christos mentioned, and the prototype for m_length, is not one of your
changes, and should probably be considered as a separate issue.

kre


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-19 04:29:23 UTC
Permalink
Hi,
Post by Christos Zoulas
Post by Kengo NAKAHARA
@@ -867,6 +867,10 @@ int m_append(struct mbuf *, int, const void *);
/* Inline routines. */
static __inline u_int m_length(const struct mbuf *) __unused;
+static __inline void m_hdr_init(struct mbuf *, short, struct mbuf *,
+ char *, int);
+static __inline void m_pkthdr_init(struct mbuf *);
+
Looks good, but you should not need the above... Perhaps remove all
the static __inline decls. Is the __unused needed? Can it be moved in
the definition?
Oh, sorry, I was confused by the existing m_length() declaration.
I remove my static __inline declarations.

Hmm, the static __inline m_length() declaration is added by mbuf.h r1.92
commit and __unused attribute is added by mbuf.h r1.97 commit. It seems
the declaration is not needed as long as I read the commit message.
# of course, GENERIC kernel can be built without the declaration

Can I remove the m_length() declaration before adding mbuf initialization
functions?


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-19 04:49:09 UTC
Permalink
Hi,
Post by Robert Elz
Date: Mon, 18 Apr 2016 13:34:07 +0900
| Some of the functions set m_flags and m_nextpkt, however m_flags is
| almost set 0 and m_nextpkt is always set NULL. So I don't add these
| fields.
That's fine.
| Hmm, in the mbuf stack allocating functions, only dp8390_ipkdb_send()
| set m_pkthdr fields explicitly. Furthermore, other M_PKTHDR mbuf using
| functions use MGETDHR() or m_gethdr(). So, I think m_pkthdr_init()
| don't need to add params except for mbuf.
Also fine.
| I reflect above comments. Here is updated patch.
[...]
| Could you comment this patch?
As Christos said, you don't need prototypes for inline functions declared
in a header file (the declaration is its own prototype) unless they are going
to refer to each other (in which case they couldn't really be inline).
Sorry, I was confused. I remove the declarations.
Post by Robert Elz
Aside from that, the changes you're proposing look fine to me. The __unused
that Christos mentioned, and the prototype for m_length, is not one of your
changes, and should probably be considered as a separate issue.
Yes, indeed.
Although, I think m_length() declaration may be removed (by separated
commit) on this occasion, since the declaration is not needed as long as
I read mbuf.h commit log.


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-19 09:29:47 UTC
Permalink
Hi,

Thank you for your detailed comment!
Post by Robert Elz
Date: Tue, 19 Apr 2016 13:49:09 +0900
| Sorry, I was confused. I remove the declarations.
There is no need to apologise for any of this - the prototype declarations
are certainly not wrong, just unnecessary (probably ... upon further thought
I am wondering if perhaps some compiler somewhere might complain about the
existence of a static func definition (inline or not) which is included in
a source file that does not use the func - and that might be what the __unused
is for ... The gcc in current (5.3.0) and the one in NetBSD 7 (4.8.4) do
not need it, but perhaps some earlier version? (Or clang?)
Maybe hold off on deleting the prototypes for another day - just in case...
(either that, or be prepared to put them in later if someone complains
about a build failure.)
| Although, I think m_length() declaration may be removed (by separated
| commit) on this occasion, since the declaration is not needed as long as
| I read mbuf.h commit log.
I do not generally recommend the use of commit logs as a way to discover
the truth about the universe -- they sometimes give some info as to what was
in a developer's mind when they made some change - but here I do not see
that.
Nevertheless, I believe that that prototype is also not needed (again,
probably). It might (and I stress, I am guessing) have been added
initially because mbuf.h contained references to m_length that preceded its
definition - however, they were all in macros, which means only really in
the source files that call the macros (macro definitions can contain any
random junk - it only matters where they are called) so the prototype was
never really needed for that (and in any case, no such macro remains now) -
or it may just be that the prototype was just converted from when m_length()
was a real (external) function and so needed the prototype, when it could
have just been deleted.
This one has been there over a decade now, and doing no apparent harm, so
leaving it another few days cannot hurt!
I see. I leave the declaration, and I leave it to someone(tm) who knows
the circumstances well.


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-20 09:06:47 UTC
Permalink
Hi,
Post by Kengo NAKAHARA
Thank you for your detailed comment!
snip
Post by Kengo NAKAHARA
Post by Robert Elz
This one has been there over a decade now, and doing no apparent harm, so
leaving it another few days cannot hurt!
I see. I leave the declaration, and I leave it to someone(tm) who knows
the circumstances well.
I commit it.
# and leave the declaration...

Thank you for reviews and advises!


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Joerg Sonnenberger
2016-04-19 11:49:24 UTC
Permalink
Post by Robert Elz
There is no need to apologise for any of this - the prototype declarations
are certainly not wrong, just unnecessary (probably ... upon further thought
I am wondering if perhaps some compiler somewhere might complain about the
existence of a static func definition (inline or not) which is included in
a source file that does not use the func - and that might be what the __unused
is for ... The gcc in current (5.3.0) and the one in NetBSD 7 (4.8.4) do
not need it, but perhaps some earlier version? (Or clang?)
Clang certainly has a warning under -Wall (IIRC) for unused static
functions, but it is also smart enough to filter out headers. I don't
see why __unused is helpful here.

Joerg

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Elz
2016-04-19 14:33:22 UTC
Permalink
Date: Tue, 19 Apr 2016 13:49:24 +0200
From: Joerg Sonnenberger <***@bec.de>
Message-ID: <***@britannica.bec.de>

| Clang certainly has a warning under -Wall (IIRC) for unused static
| functions, but it is also smart enough to filter out headers. I don't
| see why __unused is helpful here

I expect it isn't and can be deleted, I just was not confident enough
(nor know enough about the behaviour of the various compilers) to
recommend removing it (it has been there for a decade or so...)

But it certainly sounds as if removing it would be worth trying,
and should cause no problems.

kre


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Elz
2016-04-19 07:53:49 UTC
Permalink
Date: Tue, 19 Apr 2016 13:49:09 +0900
From: Kengo NAKAHARA <k-***@iij.ad.jp>
Message-ID: <***@iij.ad.jp>

| Sorry, I was confused. I remove the declarations.

There is no need to apologise for any of this - the prototype declarations
are certainly not wrong, just unnecessary (probably ... upon further thought
I am wondering if perhaps some compiler somewhere might complain about the
existence of a static func definition (inline or not) which is included in
a source file that does not use the func - and that might be what the __unused
is for ... The gcc in current (5.3.0) and the one in NetBSD 7 (4.8.4) do
not need it, but perhaps some earlier version? (Or clang?)

Maybe hold off on deleting the prototypes for another day - just in case...
(either that, or be prepared to put them in later if someone complains
about a build failure.)

| Although, I think m_length() declaration may be removed (by separated
| commit) on this occasion, since the declaration is not needed as long as
| I read mbuf.h commit log.

I do not generally recommend the use of commit logs as a way to discover
the truth about the universe -- they sometimes give some info as to what was
in a developer's mind when they made some change - but here I do not see
that.

Nevertheless, I believe that that prototype is also not needed (again,
probably). It might (and I stress, I am guessing) have been added
initially because mbuf.h contained references to m_length that preceded its
definition - however, they were all in macros, which means only really in
the source files that call the macros (macro definitions can contain any
random junk - it only matters where they are called) so the prototype was
never really needed for that (and in any case, no such macro remains now) -
or it may just be that the prototype was just converted from when m_length()
was a real (external) function and so needed the prototype, when it could
have just been deleted.

This one has been there over a decade now, and doing no apparent harm, so
leaving it another few days cannot hurt!

kre


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-19 09:39:46 UTC
Permalink
Hi,
Post by Kengo NAKAHARA
Post by Christos Zoulas
Post by Kengo NAKAHARA
@@ -867,6 +867,10 @@ int m_append(struct mbuf *, int, const void *);
/* Inline routines. */
static __inline u_int m_length(const struct mbuf *) __unused;
+static __inline void m_hdr_init(struct mbuf *, short, struct mbuf *,
+ char *, int);
+static __inline void m_pkthdr_init(struct mbuf *);
+
Looks good, but you should not need the above... Perhaps remove all
the static __inline decls. Is the __unused needed? Can it be moved in
the definition?
Oh, sorry, I was confused by the existing m_length() declaration.
I remove my static __inline declarations.
Hmm, the static __inline m_length() declaration is added by mbuf.h r1.92
commit and __unused attribute is added by mbuf.h r1.97 commit. It seems
the declaration is not needed as long as I read the commit message.
# of course, GENERIC kernel can be built without the declaration
Can I remove the m_length() declaration before adding mbuf initialization
functions?
I change my policy according to ***@n.o's advice. I leave the declaration.
I leave it to someone(tm)...


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2016-04-15 19:47:27 UTC
Permalink
Post by Kengo NAKAHARA
Hi,
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


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Kengo NAKAHARA
2016-04-18 04:38:50 UTC
Permalink
Hi,
snip
Post by Christos Zoulas
Post by Kengo NAKAHARA
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?
There is no reason. I missed it, sorry.

I update the patch. Of course, it use inline functions.
http://mail-index.netbsd.org/tech-net/2016/04/18/msg005780.html

Could you comment this patch?


Thanks,
--
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-***@iij.ad.jp>

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...