Page MenuHomeFreeBSD

net80211 drivers without struct ifnet
AbandonedPublic

Authored by glebius on May 26 2015, 3:35 PM.
Tags
None
Referenced Files
F104946004: D2655.id5941.diff
Wed, Dec 11, 12:33 AM
Unknown Object (File)
Mon, Dec 9, 5:48 AM
Unknown Object (File)
Wed, Dec 4, 4:30 PM
Unknown Object (File)
Wed, Dec 4, 4:27 PM
Unknown Object (File)
Wed, Dec 4, 4:26 PM
Unknown Object (File)
Wed, Dec 4, 4:09 PM
Unknown Object (File)
Wed, Dec 4, 4:06 PM
Unknown Object (File)
Wed, Dec 4, 4:04 PM

Details

Reviewers
None
Group Reviewers
network
Summary

This is part of larger project for new interface driver KPI, and
it will cut away usage of superfluous struct ifnet from WiFi
drivers.

All WiFi drivers create a device(9), and an ifnet(9). Then interface
cloning operation creates another ifnet(9) of wlan(4), which acts
as actual interface. All userland applications issue ioctls, read
statistics and run packet capture on the wlan(4), not on the
underlying parent.

The patch cuts down the ifnet(9) from drivers. Right now only iwn(4)
is converted. I will seek for testers and convert other drivers.
But before I'd like to get reviewed the base concept.

Changes to the stack:

o Struct ieee80211com is the single gateway between stack and driver:

  • packets are sent via new ic_transmit method, which is very much like the previous if_transmit.
  • bringing parent up/down is done via new ic_parent method
  • device specific ioctls (if any) are received on new ic_ioctl method
  • struct ieee80211com lives in the driver softc

o ieee80211_ifattach() registers an ieee80211com on global list
o wlan cloner seeks for specified parent on the global list
o VAP interface mac address isn't a copy, but a pointer. It points either to original mac address in ieee80211com, or if wlanX address is changed, then to IF_LLADDR of own interface.
o Handling of promisc and allmulti is done by accounting number of VAPs in given state.

Changes to a driver (iwn(4) provided as tested example):
o Don't create interface: if_alloc, if_attach, if_free.
o Convert IFF_DRV_RUNNING into internal flag in softc. Use it under driver lock always.
o Convert SIOCSIFFLAGS code into ic_parent method.
o Leave if_ioctl method to ic_ioctl if there are any driver specific ioctls, otherwise delete it.
o Convert interface start/transmit method to ic_transmit method.

  • As option use mbufq.

o Go through all functions and change initialization of struct ieee80211com pointer. It used to be ifp->if_l2com, now it lives in softc.
o Statistics.

  • Properly update vap packets/errors statistics if vap is known.
  • If vap is unknown, increase ic_ierrors/ic_oerrors.
  • TX stats are done on TX completion, in ieee80211_tx_complete()

Changes to /etc:
o Although the startup sequence didn't change: ifconfig wlan0 wlandev iwn0, wpa_supplicant, etc..., the rc.d needs extra care. The problem is that rc.d explicitly checked presence of the parent in 'ifconfig -l'. XXX: The rc.d diff needs extra polishing.

Test Plan

Test each driver :(

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

glebius retitled this revision from to net80211 drivers without struct ifnet.
glebius updated this object.
glebius edited the test plan for this revision. (Show Details)
glebius added a reviewer: network.
glebius updated this object.
glebius updated this object.
avos added inline comments.
sys/net80211/ieee80211_ioctl.c
2533

Without this check net80211 enters SCAN state even when device is not initialized (for example, issue wlandebug scan+state and then turn device off with RF switch)

sys/net80211/ieee80211_ioctl.c
2533

The device should check its internal RUNNING flag in the handler.

glebius updated this object.

Add converted bwi(4).

Convert ath(4). Not tested.

sys/dev/ath/if_ath_rx.c
1208

Style(9) issue here: please use tabulator instead of space after #undef.

I running into compile error, and this patch should fix them:

op@opn src> git diff
diff --git a/sys/dev/ath/if_ath.c b/sys/dev/ath/if_ath.c
index 53763a6..b719ed4 100644
--- a/sys/dev/ath/if_ath.c
+++ b/sys/dev/ath/if_ath.c
@@ -5729,8 +5729,7 @@ ath_scan_end(struct ieee80211com *ic)
 static void
 ath_update_chw(struct ieee80211com *ic)
 {
-       struct ifnet *ifp = ic->ic_ifp;
-       struct ath_softc *sc = ifp->if_softc;
+       struct ath_softc *sc = ic->ic_softc;
 
        DPRINTF(sc, ATH_DEBUG_STATE, "%s: called\n", __func__);
        ath_set_channel(ic);
diff --git a/sys/dev/ath/if_ath_tdma.c b/sys/dev/ath/if_ath_tdma.c
index fd23db1..d4c9ccd 100644
--- a/sys/dev/ath/if_ath_tdma.c
+++ b/sys/dev/ath/if_ath_tdma.c
@@ -359,7 +359,7 @@ ath_tdma_update(struct ieee80211_node *ni,
 #define        TU_TO_TSF(_tu)  (((u_int64_t)(_tu)) << 10)
        struct ieee80211vap *vap = ni->ni_vap;
        struct ieee80211com *ic = ni->ni_ic;
-       struct ath_softc *sc = ic->ic_ifp->if_softc;
+       struct ath_softc *sc = ic->ic_softc;
        struct ath_hal *ah = sc->sc_ah;
        const HAL_RATE_TABLE *rt = sc->sc_currates;
        u_int64_t tsf, rstamp, nextslot, nexttbtt, nexttbtt_full;

Address Oliver's comments on ath(4).

Update to fresh head, where bug in iwi(4) is fixed.

Address new LOR reported by Oliver.

  • Fix to iwi(4) and probably others, to clean up ni/m pointers in the TX ring after call to ieee80211_tx_complete()
  • Use ieee80211_tx_complete() in bwi(4).
  • Merge with latest head.

Promiscuous flag does nothing - it looks like the prefix increment is more suitable for ic_promisc / ic_allmulti variables.

Couple of KASSERT fixes in ral(4).

Submitted by: kevlo

Convert wpi(4).

Submitted by: Andriy Voskoboinyk <s3erios gmail.com>

sys/net80211/ieee80211.c
677

here

682

here

699

here

704

and here

Use proper increments in promisc/allmulti handlers. We are interested in
the resulting values, not in the previous ones.

Submitted by: Andriy Voskoboinyk

glebius added inline comments.
sys/net80211/ieee80211.c
677

Yep, good catch. Thanks!

glebius marked an inline comment as done.

Convert run(4).

Fix statistics in run(4).

Fix lock leak in urtw_parent().

  • Converted rsu(4).
  • Merged iwn(4).
glebius removed rS FreeBSD src repository - subversion as the repository for this revision.
  • Merge head (affected: ath, iwn).
  • Convert malo(4).
  • Add missing locking to malo(4).

Reported by: kevlo

Convert rest of drivers: mwl, ipw, bwn, wi and wtap.

  • Provide net.wlan.devices sysctl.

Convert rest of drivers: mwl, ipw, bwn, wi and wtap.

​Hi,
I've just tried the latest revision of D2655 on an i386 kernel (with "device mwl" and "device mwlfw" in the kernel configuration file).

cc  -c -O -pipe  -g -nostdinc  -I. -I/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys -I/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/contrib/libfdt -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h  -mno-mmx -mno-sse -msoft-float -ffreestanding -fwrapv -fstack-protector -gdwarf-2 -Wall -Wredundant-decls -Wnested-externs -Wstrict-prototypes  -Wmissing-prototypes -Wpointer-arith -Winline -Wcast-qual  -Wundef -Wno-pointer-sign -D__printf__=__freebsd_kprintf__  -Wmissing-include-dirs -fdiagnostics-show-option  -Wno-unknown-pragmas  -Wno-error-tautological-compare -Wno-error-empty-body  -Wno-error-parentheses-equality -Wno-error-unused-function  -Wno-error-pointer-sign  -mno-aes -mno-avx  -std=iso9899:1999 -Werror  /usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:1519:20: error: unused variable 'sc' [-Werror,-Wunused-variable]
        struct mwl_softc *sc = vap->iv_ic->ic_softc;
                          ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:1547:20: error: unused variable 'sc' [-Werror,-Wunused-variable]
        struct mwl_softc *sc = vap->iv_ic->ic_softc;
                          ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:1614:20: error: unused variable 'sc' [-Werror,-Wunused-variable]
        struct mwl_softc *sc = vap->iv_ic->ic_softc;
                          ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:2258:20: error: unused variable 'sc' [-Werror,-Wunused-variable]
        struct mwl_softc *sc = ic->ic_softc;
                          ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:2813:7: error: expected expression
                if (IFF_DUMPPKTS_RECV(sc, wh)) {
                    ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:248:35: note: expanded from macro 'IFF_DUMPPKTS_RECV'
#define IFF_DUMPPKTS_RECV(sc, wh)       do {} while (0)
                                        ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:3320:6: error: expected expression
        if (IFF_DUMPPKTS_XMIT(sc))
            ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:249:32: note: expanded from macro 'IFF_DUMPPKTS_XMIT'
#define IFF_DUMPPKTS_XMIT(sc)           do {} while (0)
                                        ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:3880:20: error: unused variable 'sc' [-Werror,-Wunused-variable]
        struct mwl_softc *sc = ic->ic_softc;
                          ^
/usr/local/BSDRP/GLEBWIFI/FreeBSD/src/sys/dev/mwl/if_mwl.c:3888:20: error: unused variable 'sc' [-Werror,-Wunused-variable]
        struct mwl_softc *sc = ic->ic_softc;
                          ^
8 errors generated.
*** Error code 1

Close it since it is committed.