Discussion:
WiFi Refresh -- Report 1
(too old to reply)
Phil Nelson
2018-07-06 16:10:27 UTC
Permalink
Hello tech-net,

As promised, this is the first report on the progress of the WiFi refresh.

The FreeBSD net80211/* code has been committed to a "phil-wifi" branch.
The files ieee80211_netbsd.c ieee80211_netbsd.h started with the _freebsd*
versions. There 33 .h files and 41 .c files. Currently, only 11 of those files
are not compiling. (none of the drivers have been touched yet and there are
55 files not compiling due to the 80211 code changing.) No changes have
been committed yet, but should be done soon.

To get those 30 ieee80211*.c files compiling, a number of issues had to
be delayed. Currently, the sysctl code is mostly ifdefed out.
Any changes to the ieee80211*.c files will be at the final time as whole line
changes where the code looks similar to
#if __FreeBSD__
.... the original FreeBSD code
#elif __NetBSD__
.... the NetBSD specific code
#endif

In some places, it might still compile if neither is defined and in those
cases, I'm thinking of adding a
#else
#error
so it won't compile elsewhere.

The following is a list of issues that will need to be addressed: (discussion
on any/all of the following is encouraged.)

#1: sbuf routines. A number of the files use the sbuf routines. These
appear to be relatively contained to two files, sys/sbuf.h and kern/subr_sbuf.c.
I am using the sbuf.h to get the files to compile, buf have not yet tried compiling
subr_sbuf.c. I am planning on including those two files to support the ieee80211
code instead of rewriting that code.

#2: KASSERT has different arguments on each platform. In ieee80211_netbsd.h
I have defined a FBSDKASSERT that matches and in any ieee8021*.c file that
uses a KASSERT, I redefine KASSERT to be FBSDKASSERT. By not adding that
redefinition to the ieee80211_netbsd.h file, it doesn't redefine KASSERT for the
NetBSD code.

#3: FreeBSD directly manipulates the mbuf headers, specifically m->m_pkthdr.rcvif.
NetBSD uses a set of m_set_rcvif, m_get_rcvif, ... and those changes had to be made.

#4: The 80211 code uses 12 proto specific flags with their mbufs. Our mbufs don't
have that many defined and instead of using M_PROTOn (where n is 0 .. 11), our
mbufs have a number of M_LINKn flags. It appeared better to me to add more M_LINKn
flags to our mbufs. The following are the diffs I am using to get the files compiling.

$ cvs diff -u mbuf.h
X11 forwarding request failed on channel 0
Index: mbuf.h
===================================================================
RCS file: /cvsroot/src/sys/sys/mbuf.h,v
retrieving revision 1.207
diff -u -r1.207 mbuf.h
--- mbuf.h 1 Jun 2018 08:56:00 -0000 1.207
+++ mbuf.h 6 Jul 2018 15:45:06 -0000
@@ -352,15 +352,19 @@
#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 */

-#define M_VLANTAG 0x00100000 /* ether_vtag is valid */
+#define M_VLANTAG 0x01000000 /* ether_vtag is valid */

/* additional flags for M_EXT mbufs */
-#define M_EXT_FLAGS 0xff000000
-#define M_EXT_CLUSTER 0x01000000 /* ext is a cluster */
-#define M_EXT_PAGES 0x02000000 /* ext_pgs is valid */
-#define M_EXT_ROMAP 0x04000000 /* ext mapping is r-o at MMU */
-#define M_EXT_RW 0x08000000 /* ext storage is writable */
+#define M_EXT_FLAGS 0xfc000000
+#define M_EXT_CLUSTER 0x04000000 /* ext is a cluster */
+#define M_EXT_PAGES 0x08000000 /* ext_pgs is valid */
+#define M_EXT_ROMAP 0x10000000 /* ext mapping is r-o at MMU */
+#define M_EXT_RW 0x20000000 /* ext storage is writable */

/* for source-level compatibility */
#define M_NOTIFICATION M_PROTO1
@@ -368,14 +372,15 @@
#define M_FLAGS_BITS \
"\20\1EXT\2PKTHDR\3EOR\4PROTO1\5AUTHIPHDR\6DECRYPTED\7LOOP\10NONE" \
"\11BCAST\12MCAST\13CANFASTFWD\14ANYCAST6\15LINK0\16LINK1\17LINK2\20LINK3" \
- "\21LINK4\22LINK5\23LINK6\24LINK7" \
- "\25VLANTAG" \
- "\31EXT_CLUSTER\32EXT_PAGES\33EXT_ROMAP\34EXT_RW"
+ "\21LINK4\22LINK5\23LINK6\24LINK7\25LINK8\26LINK9\27LINK10\30LINK11" \
+ "\31VLANTAG" \
+ "\33EXT_CLUSTER\34EXT_PAGES\35EXT_ROMAP\36EXT_RW"

/* flags copied when copying m_pkthdr */
-#define M_COPYFLAGS (M_PKTHDR|M_EOR|M_BCAST|M_MCAST|M_CANFASTFWD| \
- M_ANYCAST6|M_LINK0|M_LINK1|M_LINK2|M_AUTHIPHDR|M_DECRYPTED|M_LOOP| \
- M_VLANTAG)
+#define M_COPYFLAGS (M_PKTHDR|M_EOR|M_BCAST|M_MCAST|M_CANFASTFWD| \
+ M_ANYCAST6|M_LINK0|M_LINK1|M_LINK2|M_LINK3|M_LINK4|M_LINK5|M_LINK6| \
+ M_LINK7|M_LINK8|M_LINK9|M_LINK10|M_LINK11|M_AUTHIPHDR|M_DECRYPTED| \
+ M_LOOP|M_VLANTAG)

/* flag copied when shallow-copying external storage */
#define M_EXTCOPYFLAGS (M_EXT|M_EXT_FLAGS)

The M_EXT flags have gone from using 8 bits to using 6 bits, but only 4 bits in use.

#5 FreeBSD module code is been disabled and due to that a number of __unused
modifiers have been added ... but this will not be the final fix. This will all be reviewed
as I continue.

#6 VLAN support is disabled as FreeBSD and NetBSD vlan support is completely different.
Once we have working code and more time, I'll review the VLAN support.

#7 taskqueue (FreeBSD) vs workqueue (NetBSD) ... I'm looking at adding a taskqueue
API front end to workqueue. I have not attempted it yet. taskqueue code has been
ifdeffed out with a "#ifdef notyet" which will be removed once the final solution is complete.

#8 the "struct ifnet" has some different fields between the two OSes and I haven't
worked on that issue yet. That will be required to get the final 11 files compiling.

#9 the struct ieee80211com is different in the new code and most drivers fail to compile
due to missing fields. Nothing has been done on this yet.

#10 ioctls and sysctls will need work. Almost nothing to date has been done.

Now .... back to getting those 11 final files to compile.

--Phil

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Jason Thorpe
2018-07-06 16:53:18 UTC
Permalink
Sounds like this is another compelling reason for Taylor to get his threadpool / task stuff finalized and integrated.

-- thorpej
Sent from my iPhone.
Post by Phil Nelson
#7 taskqueue (FreeBSD) vs workqueue (NetBSD) ... I'm looking at adding a taskqueue
API front end to workqueue. I have not attempted it yet. taskqueue code has been
ifdeffed out with a "#ifdef notyet" which will be removed once the final solution is complete.
--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Phil Nelson
2018-07-06 17:03:04 UTC
Permalink
Post by Jason Thorpe
Sounds like this is another compelling reason for Taylor to get his threadpool / task stuff finalized and integrated.
I do have a URL of the current code for this. I'm looking at that also.

--Phil

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2018-07-06 17:10:32 UTC
Permalink
Post by Phil Nelson
Hello tech-net,
#1: sbuf routines. A number of the files use the sbuf routines. These
appear to be relatively contained to two files, sys/sbuf.h and
kern/subr_sbuf.c.
I am using the sbuf.h to get the files to compile, buf have not yet tried compiling
subr_sbuf.c. I am planning on including those two files to support the ieee80211
code instead of rewriting that code.
Fine, we can decide what to do later.
Post by Phil Nelson
#2: KASSERT has different arguments on each platform. In ieee80211_netbsd.h
I have defined a FBSDKASSERT that matches and in any ieee8021*.c file that
uses a KASSERT, I redefine KASSERT to be FBSDKASSERT. By not adding that
redefinition to the ieee80211_netbsd.h file, it doesn't redefine KASSERT for the
NetBSD code.
I would call it IEEE80211_KASSERT(), so that it can be fed back upstream.
Post by Phil Nelson
#3: FreeBSD directly manipulates the mbuf headers, specifically
m->m_pkthdr.rcvif.
NetBSD uses a set of m_set_rcvif, m_get_rcvif, ... and those changes had to be made.
Good. You can add macros for the FreeBSD case (so that we can feed the
changes upstream).
Post by Phil Nelson
#4: The 80211 code uses 12 proto specific flags with their mbufs. Our mbufs don't
have that many defined and instead of using M_PROTOn (where n is 0 .. 11), our
mbufs have a number of M_LINKn flags. It appeared better to me to add more M_LINKn
flags to our mbufs. The following are the diffs I am using to get the files compiling.
Yes, I did the same.
Post by Phil Nelson
#5 FreeBSD module code is been disabled and due to that a number of __unused
modifiers have been added ... but this will not be the final fix. This will all be reviewed
as I continue.
OK.
Post by Phil Nelson
#6 VLAN support is disabled as FreeBSD and NetBSD vlan support is completely different.
Once we have working code and more time, I'll review the VLAN support.
OL.
Post by Phil Nelson
#7 taskqueue (FreeBSD) vs workqueue (NetBSD) ... I'm looking at adding a taskqueue
API front end to workqueue. I have not attempted it yet. taskqueue code has been
ifdeffed out with a "#ifdef notyet" which will be removed once the final
solution is complete.
Taylor had an implementation I think.
Post by Phil Nelson
#8 the "struct ifnet" has some different fields between the two OSes and I haven't
worked on that issue yet. That will be required to get the final 11 files compiling.
Yes, there are slightly different...
Post by Phil Nelson
#9 the struct ieee80211com is different in the new code and most drivers fail to compile
due to missing fields. Nothing has been done on this yet.
Ok.
Post by Phil Nelson
#10 ioctls and sysctls will need work. Almost nothing to date has been done.
Now .... back to getting those 11 final files to compile.
Great progress.

christos


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