On Sun, Jul 08, 2012 at 01:45:45AM +0300, Lazaros Koromilas wrote:
> Hello all,
>
> I'm resending a diff that enables network cards running with
> the wpi driver to enter promiscuous mode. I have changed
> WPI_CMD_ASSOCIATE to WPI_CMD_ASSOCIATED to better designate its
You forgot to update a reference to this constant in a comment.
Personally I'd prefer to leave the name alone to make the diff smaller.
> purpose: alter options while in associated state. I'm running
> with this for some time now without problems on a Thinkpad X60s.
>
> Can anyone test? Comments?
I can test, but already have some questions after review, see below.
>
> Thanx!
> Lazaros.
>
>
> Index: if_wpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_wpi.c,v
> retrieving revision 1.110
> diff -u -p -r1.110 if_wpi.c
> --- if_wpi.c 2 Jun 2011 18:36:53 -0000 1.110
> +++ if_wpi.c 7 Jul 2012 18:01:54 -0000
> @@ -120,6 +120,7 @@ int wpi_ioctl(struct ifnet *, u_long, c
> int wpi_cmd(struct wpi_softc *, int, const void *, int, int);
> int wpi_mrr_setup(struct wpi_softc *);
> void wpi_updateedca(struct ieee80211com *);
> +void wpi_set_promisc(struct wpi_softc *, int);
> void wpi_set_led(struct wpi_softc *, uint8_t, uint8_t, uint8_t);
> int wpi_set_timing(struct wpi_softc *, struct ieee80211_node *);
> void wpi_power_calibration(struct wpi_softc *);
> @@ -2002,12 +2003,21 @@ wpi_ioctl(struct ifnet *ifp, u_long cmd,
> /* FALLTHROUGH */
> case SIOCSIFFLAGS:
> if (ifp->if_flags & IFF_UP) {
> - if (!(ifp->if_flags & IFF_RUNNING))
> + if (ifp->if_flags & IFF_RUNNING) {
> + if (ifp->if_flags & IFF_PROMISC &&
> + !(sc->sc_if_flags & IFF_PROMISC)) {
> + wpi_set_promisc(sc, 1);
> + } else if (!(ifp->if_flags & IFF_PROMISC) &&
> + sc->sc_if_flags & IFF_PROMISC) {
> + wpi_set_promisc(sc, 0);
> + }
> + } else
> error = wpi_init(ifp);
> } else {
> if (ifp->if_flags & IFF_RUNNING)
> wpi_stop(ifp, 1);
> }
> + sc->sc_if_flags = ifp->if_flags;
> break;
>
> case SIOCADDMULTI:
> @@ -2206,6 +2216,26 @@ wpi_updateedca(struct ieee80211com *ic)
> }
>
> void
> +wpi_set_promisc(struct wpi_softc *sc, int turnon)
> +{
> + struct wpi_assoc cmd;
> +
> + if (turnon)
> + sc->rxon.filter |= htole32(WPI_FILTER_PROMISC |
> + WPI_FILTER_CTL);
> + else
> + sc->rxon.filter &= ~htole32(WPI_FILTER_PROMISC |
> + WPI_FILTER_CTL);
> +
> + memset(&cmd, 0, sizeof cmd);
> + cmd.flags = sc->rxon.flags;
> + cmd.filter = sc->rxon.filter;
> + cmd.ofdm_mask = sc->rxon.ofdm_mask;
> + cmd.cck_mask = sc->rxon.cck_mask;
> + (void)wpi_cmd(sc, WPI_CMD_ASSOCIATED, &cmd, sizeof cmd, 1);
The linux driver ("iwlegacy") doesn't run this command in async mode.
Is there a reason why you're passing 1 for the last param, i.e. not
waiting for a command-complete interrupt when sending WPI_CMD_ASSOCIATE?
> +}
> +
> +void
> wpi_set_led(struct wpi_softc *sc, uint8_t which, uint8_t off, uint8_t on)
> {
> struct wpi_cmd_led led;
> @@ -3327,6 +3357,7 @@ wpi_init(struct ifnet *ifp)
>
> ifp->if_flags &= ~IFF_OACTIVE;
> ifp->if_flags |= IFF_RUNNING;
> + sc->sc_if_flags = ifp->if_flags;
You don't need all of if_flags, just the IFF_PROMISC bit.
Why not add a new flag to sc->sc_flags and use that instead?
>
> if (ic->ic_opmode != IEEE80211_M_MONITOR)
> ieee80211_begin_scan(ifp);
> Index: if_wpireg.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_wpireg.h,v
> retrieving revision 1.27
> diff -u -p -r1.27 if_wpireg.h
> --- if_wpireg.h 24 Oct 2009 20:17:17 -0000 1.27
> +++ if_wpireg.h 7 Jul 2012 18:01:54 -0000
> @@ -252,7 +252,7 @@ struct wpi_rx_desc {
> struct wpi_tx_cmd {
> uint8_t code;
> #define WPI_CMD_RXON 16
> -#define WPI_CMD_ASSOCIATE 17
> +#define WPI_CMD_ASSOCIATED 17
> #define WPI_CMD_EDCA_PARAMS 19
> #define WPI_CMD_TIMING 20
> #define WPI_CMD_ADD_NODE 24
> Index: if_wpivar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_wpivar.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 if_wpivar.h
> --- if_wpivar.h 7 Sep 2010 16:21:45 -0000 1.23
> +++ if_wpivar.h 7 Jul 2012 18:01:54 -0000
> @@ -144,6 +144,8 @@ struct wpi_softc {
> #define WPI_FLAG_HAS_5GHZ (1 << 0)
> #define WPI_FLAG_BUSY (1 << 1)
>
> + int sc_if_flags;
> +
> /* Shared area. */
> struct wpi_dma_info shared_dma;
> struct wpi_shared *shared;
