Discussion:
mbuf flags
(too old to reply)
Robert Swindells
2015-03-01 00:16:51 UTC
Permalink
I'm going to add an extra mbuf flag for SCTP, is there a preferred style
to use for it ?

Right now we have M_LINK[0-7] with the final one having a value of
0x80000, should I add the new flag before them and move them all up by
one bit or add it after ?

Also, keeping everything aligned would require adding an extra hex digit
to every one which would make it less clear what had really changed.

Nothing seems to be using M_LINK7 right now.

Robert Swindells


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Elz
2015-03-01 04:34:16 UTC
Permalink
Date: Sun, 1 Mar 2015 00:16:51 +0000 (GMT)
From: Robert Swindells <***@fdy2.co.uk>
Message-ID: <***@ren.fdy2.co.uk>

| Also, keeping everything aligned would require adding an extra hex digit
| to every one which would make it less clear what had really changed.

Add the extra digits in a separate (change nothing) commit first,
then your real change will be smooth, and still look nice.

kre


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2015-03-01 14:20:49 UTC
Permalink
Post by Robert Elz
Date: Sun, 1 Mar 2015 00:16:51 +0000 (GMT)
| Also, keeping everything aligned would require adding an extra hex digit
| to every one which would make it less clear what had really changed.
Add the extra digits in a separate (change nothing) commit first,
then your real change will be smooth, and still look nice.
I have this patch to add the extra link bits for FreeBSD compat needed
for net80122...

christos

Index: mbuf.h
===================================================================
RCS file: /cvsroot/src/sys/sys/mbuf.h,v
retrieving revision 1.156
diff -u -u -r1.156 mbuf.h
--- mbuf.h 5 Sep 2014 05:48:59 -0000 1.156
+++ mbuf.h 1 Mar 2015 14:18:52 -0000
@@ -322,29 +322,38 @@
MBUF_DEFINE(mbuf, MHLEN, MLEN);

/* mbuf flags */
-#define M_EXT 0x00001 /* has associated external storage */
-#define M_PKTHDR 0x00002 /* start of record */
-#define M_EOR 0x00004 /* end of record */
-#define M_PROTO1 0x00008 /* protocol-specific */
+#define M_EXT 0x00000001 /* has associated external storage */
+#define M_PKTHDR 0x00000002 /* start of record */
+#define M_EOR 0x00000004 /* end of record */
+#define M_PROTO1 0x00000008 /* protocol-specific */

/* mbuf pkthdr flags, also in m_flags */
-#define M_AUTHIPHDR 0x00010 /* data origin authentication for IP header */
-#define M_DECRYPTED 0x00020 /* confidentiality */
-#define M_LOOP 0x00040 /* for Mbuf statistics */
-#define M_AUTHIPDGM 0x00080 /* data origin authentication */
-#define M_BCAST 0x00100 /* send/received as link-level broadcast */
-#define M_MCAST 0x00200 /* send/received as link-level multicast */
-#define M_CANFASTFWD 0x00400 /* used by filters to indicate packet can
- be fast-forwarded */
-#define M_ANYCAST6 0x00800 /* received as IPv6 anycast */
-#define M_LINK0 0x01000 /* link layer specific flag */
-#define M_LINK1 0x02000 /* link layer specific flag */
-#define M_LINK2 0x04000 /* link layer specific flag */
-#define M_LINK3 0x08000 /* link layer specific flag */
-#define M_LINK4 0x10000 /* link layer specific flag */
-#define M_LINK5 0x20000 /* link layer specific flag */
-#define M_LINK6 0x40000 /* link layer specific flag */
-#define M_LINK7 0x80000 /* link layer specific flag */
+#define M_AUTHIPHDR 0x00000010 /* data origin authentication for
+ * IP header */
+#define M_DECRYPTED 0x00000020 /* confidentiality */
+#define M_LOOP 0x00000040 /* for Mbuf statistics */
+#define M_AUTHIPDGM 0x00000080 /* data origin authentication */
+#define M_BCAST 0x00000100 /* send/received as link-level
+ * broadcast */
+#define M_MCAST 0x00000200 /* send/received as link-level
+ * multicast */
+#define M_CANFASTFWD 0x00000400 /* used by filters to indicate
+ * packet can be fast-forwarded */
+#define M_ANYCAST6 0x00000800 /* received as IPv6 anycast */
+
+#define M_LINK0 0x00001000 /* link layer specific flag */
+#define M_LINK1 0x00002000 /* link layer specific flag */
+#define M_LINK2 0x00004000 /* link layer specific flag */
+
+#define M_LINK3 0x00008000 /* link layer specific flag */
+#define M_LINK4 0x00010000 /* link layer specific flag */
+#define M_LINK5 0x00020000 /* link layer specific flag */
+#define M_LINK6 0x00040000 /* link layer specific flag */
+#define M_LINK7 0x00080000 /* link layer specific flag */
+#define M_LINK8 0x00100000 /* link layer specific flag */
+#define M_LINK9 0x00200000 /* link layer specific flag */
+#define M_LINK10 0x00400000 /* link layer specific flag */
+#define M_LINK11 0x00800000 /* link layer specific flag */

/* additional flags for M_EXT mbufs */
#define M_EXT_FLAGS 0xff000000
@@ -359,7 +368,7 @@
#define M_FLAGS_BITS \
"\20\1EXT\2PKTHDR\3EOR\4PROTO1\5AUTHIPHDR\6DECRYPTED\7LOOP\10AUTHIPDGM" \
"\11BCAST\12MCAST\13CANFASTFWD\14ANYCAST6\15LINK0\16LINK1\17LINK2\20LINK3" \
- "\21LINK4\22LINK5\23LINK6\24LINK7" \
+ "\21LINK4\22LINK5\23LINK6\24LINK7\25LINK8\26LINK9\27LINK10\30LINK11" \
"\31EXT_CLUSTER\32EXT_PAGES\33EXT_ROMAP\34EXT_RW"

/* flags copied when copying m_pkthdr */


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Swindells
2015-03-01 22:40:43 UTC
Permalink
Post by Christos Zoulas
Post by Robert Elz
Date: Sun, 1 Mar 2015 00:16:51 +0000 (GMT)
| Also, keeping everything aligned would require adding an extra hex digit
| to every one which would make it less clear what had really changed.
Add the extra digits in a separate (change nothing) commit first,
then your real change will be smooth, and still look nice.
I have this patch to add the extra link bits for FreeBSD compat needed
for net80122...
[snip]

Ok, that still doesn't tell me which bit I should use though.

In Kame, the flag I am adding came before the M_LINK ones, it seems
better to me to follow this pattern and move up the M_LINK[0-7] flags
by one bit. This would mess up your patch though.

I presume we would move the M_EXT_ flags up by 4 bits at some point too.

I have modified mbuf.h to just contain the formatting changes:

Index: mbuf.h
===================================================================
RCS file: /cvsroot/src/sys/sys/mbuf.h,v
retrieving revision 1.156
diff -u -r1.156 mbuf.h
--- mbuf.h 5 Sep 2014 05:48:59 -0000 1.156
+++ mbuf.h 1 Mar 2015 22:25:37 -0000
@@ -322,29 +322,32 @@
MBUF_DEFINE(mbuf, MHLEN, MLEN);

/* mbuf flags */
-#define M_EXT 0x00001 /* has associated external storage */
-#define M_PKTHDR 0x00002 /* start of record */
-#define M_EOR 0x00004 /* end of record */
-#define M_PROTO1 0x00008 /* protocol-specific */
+#define M_EXT 0x00000001 /* has associated external storage */
+#define M_PKTHDR 0x00000002 /* start of record */
+#define M_EOR 0x00000004 /* end of record */
+#define M_PROTO1 0x00000008 /* protocol-specific */

/* mbuf pkthdr flags, also in m_flags */
-#define M_AUTHIPHDR 0x00010 /* data origin authentication for IP header */
-#define M_DECRYPTED 0x00020 /* confidentiality */
-#define M_LOOP 0x00040 /* for Mbuf statistics */
-#define M_AUTHIPDGM 0x00080 /* data origin authentication */
-#define M_BCAST 0x00100 /* send/received as link-level broadcast */
-#define M_MCAST 0x00200 /* send/received as link-level multicast */
-#define M_CANFASTFWD 0x00400 /* used by filters to indicate packet can
- be fast-forwarded */
-#define M_ANYCAST6 0x00800 /* received as IPv6 anycast */
-#define M_LINK0 0x01000 /* link layer specific flag */
-#define M_LINK1 0x02000 /* link layer specific flag */
-#define M_LINK2 0x04000 /* link layer specific flag */
-#define M_LINK3 0x08000 /* link layer specific flag */
-#define M_LINK4 0x10000 /* link layer specific flag */
-#define M_LINK5 0x20000 /* link layer specific flag */
-#define M_LINK6 0x40000 /* link layer specific flag */
-#define M_LINK7 0x80000 /* link layer specific flag */
+#define M_AUTHIPHDR 0x00000010 /* data origin authentication for
+ * IP header */
+#define M_DECRYPTED 0x00000020 /* confidentiality */
+#define M_LOOP 0x00000040 /* for Mbuf statistics */
+#define M_AUTHIPDGM 0x00000080 /* data origin authentication */
+#define M_BCAST 0x00000100 /* send/received as link-level
+ * broadcast */
+#define M_MCAST 0x00000200 /* send/received as link-level
+ * multicast */
+#define M_CANFASTFWD 0x00000400 /* used by filters to indicate
+ * packet can be fast-forwarded */
+#define M_ANYCAST6 0x00000800 /* received as IPv6 anycast */
+#define M_LINK0 0x00001000 /* link layer specific flag */
+#define M_LINK1 0x00002000 /* link layer specific flag */
+#define M_LINK2 0x00004000 /* link layer specific flag */
+#define M_LINK3 0x00008000 /* link layer specific flag */
+#define M_LINK4 0x00010000 /* link layer specific flag */
+#define M_LINK5 0x00020000 /* link layer specific flag */
+#define M_LINK6 0x00040000 /* link layer specific flag */
+#define M_LINK7 0x00080000 /* link layer specific flag */

/* additional flags for M_EXT mbufs */
#define M_EXT_FLAGS 0xff000000

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2015-03-01 23:23:04 UTC
Permalink
Post by Robert Swindells
Post by Christos Zoulas
Post by Robert Elz
Date: Sun, 1 Mar 2015 00:16:51 +0000 (GMT)
| Also, keeping everything aligned would require adding an extra hex digit
| to every one which would make it less clear what had really changed.
Add the extra digits in a separate (change nothing) commit first,
then your real change will be smooth, and still look nice.
I have this patch to add the extra link bits for FreeBSD compat needed
for net80122...
[snip]
Ok, that still doesn't tell me which bit I should use though.
In Kame, the flag I am adding came before the M_LINK ones, it seems
better to me to follow this pattern and move up the M_LINK[0-7] flags
by one bit. This would mess up your patch though.
If you do that, you'll break binary compatibility...
What's that flag?
Post by Robert Swindells
I presume we would move the M_EXT_ flags up by 4 bits at some point too.
Fine, lets do the just the formatting changes first.
FreeBSD uses M_PROTO1 -> M_PROTO11 in net80211 (which is M_LINK1-M_LINK11)...

christos


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Robert Swindells
2015-03-02 01:03:36 UTC
Permalink
Post by Christos Zoulas
Post by Robert Swindells
Post by Christos Zoulas
Post by Robert Elz
Date: Sun, 1 Mar 2015 00:16:51 +0000 (GMT)
| Also, keeping everything aligned would require adding an extra hex digit
| to every one which would make it less clear what had really changed.
Add the extra digits in a separate (change nothing) commit first,
then your real change will be smooth, and still look nice.
I have this patch to add the extra link bits for FreeBSD compat needed
for net80122...
[snip]
Ok, that still doesn't tell me which bit I should use though.
In Kame, the flag I am adding came before the M_LINK ones, it seems
better to me to follow this pattern and move up the M_LINK[0-7] flags
by one bit. This would mess up your patch though.
If you do that, you'll break binary compatibility...
I had thought that mbufs were kernel only. I guess I had forgotten
about rump.
Post by Christos Zoulas
What's that flag?
The flag is M_NOTIFICATION, if it is set then a matching MSG_NOTIFICATION
flag is passed to userspace by recv*().

FreeBSD maps it to M_PROTO1 in their source but they don't seem to use it
in mbufs.

I hadn't spotted until now that we have M_PROTO1 as well as the
M_LINK? flags. We only seem to use M_PROTO1 in sys/net/if_bridge.c and
sys/netbt/hci_link.c.
Post by Christos Zoulas
Post by Robert Swindells
I presume we would move the M_EXT_ flags up by 4 bits at some point too.
Fine, lets do the just the formatting changes first.
FreeBSD uses M_PROTO1 -> M_PROTO11 in net80211 (which is M_LINK1-M_LINK11)...
I can wait.

I had been thinking of committing SCTP in two stages anyway, the core
stuff in netinet and netinet6 first then the userland and an extra
syscall later.

Robert Swindells

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