On Sun, Jul 08, 2012 at 10:59:09AM +0200, Stefan Sperling wrote:
> 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.
Thanks for looking at it. Reverted the naming change.
Sending new diff.
> > + (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?
Not really, no. Fixed that. I added printing because all sync
command calls are handled this way, but can be removed if it's
not acceptable.
> > @@ -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?
You are right, I originally added the extra sc_if_flags in order to XOR
with if_flags and detect the promisc status change. Does this logic
seem simpler/better? Also removed the initialization above.
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 8 Jul 2012 16:45:14 -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;
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 8 Jul 2012 16:45:15 -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 *);
+int wpi_set_promisc(struct wpi_softc *);
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,17 @@ 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 ^ sc->sc_if_flags) &
+ IFF_PROMISC)
+ error = wpi_set_promisc(sc);
+ } 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:
@@ -2203,6 +2209,34 @@ wpi_updateedca(struct ieee80211com *ic)
}
(void)wpi_cmd(sc, WPI_CMD_EDCA_PARAMS, &cmd, sizeof cmd, 1);
#undef WPI_EXP2
+}
+
+int
+wpi_set_promisc(struct wpi_softc *sc)
+{
+ struct ieee80211com *ic = &sc->sc_ic;
+ struct ifnet *ifp = &ic->ic_if;
+ struct wpi_assoc cmd;
+ int error;
+
+ if (ifp->if_flags & IFF_PROMISC)
+ 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;
+ error = wpi_cmd(sc, WPI_CMD_ASSOCIATE, &cmd, sizeof cmd, 0);
+ if (error != 0) {
+ printf("%s: could not set filter\n", sc->sc_dev.dv_xname);
+ return error;
+ }
+ return 0;
}
void
