rt2x00: Add Multicast/Broadcast filtering

PATCH In review Ivo van Doorn 3 participants 9 messages Jul 31, 2007 – Aug 3, 2007 Networking drivers (wireless)

lore

Share & tools

lore

Ivo van Doorn

author

2007-07-31 18:34 UTC #0 lore raw

PATCH 18/24 rt2x00: Add Multicast/Broadcast filtering

Quoted 1 line

From ed0b806ae27bbd5f8b9b5a92f7420e3c6670e64e Mon Sep 17 00:00:00 2001

From: Ivo van Doorn <[email protected]>
Date: Mon, 30 Jul 2007 16:59:00 +0200
Subject: [PATCH 18/24] rt2x00: Add Multicast/Broadcast filtering

Expand support for Promisc mode filtering, and also support Multicast and Broadcast filtering using the same method.
This requires renaming some variables and methods. As usual, monitor mode filters nothing and accept as much frames as possible.

Signed-off-by Ivo van Doorn <[email protected]>

---
 drivers/net/wireless/rt2400pci.c    |    7 +++--
 drivers/net/wireless/rt2500pci.c    |   14 +++++++-----
 drivers/net/wireless/rt2500usb.c    |   15 ++++++++----
 drivers/net/wireless/rt2500usb.h    |    4 +-
 drivers/net/wireless/rt2x00.h       |   25 +++++++++++----------
 drivers/net/wireless/rt2x00config.c |   25 ++++++++++++---------
 drivers/net/wireless/rt2x00lib.h    |    2 +-
 drivers/net/wireless/rt2x00mac.c    |   40 +++++++++++++++++++++-------------
 drivers/net/wireless/rt61pci.c      |   14 +++++++-----
 drivers/net/wireless/rt73usb.c      |   18 ++++++++-------
 10 files changed, 95 insertions(+), 69 deletions(-)

drivers/net/wireless/rt2400pci.c +4 -3

index 49dc1aa..e9bf565 100644
@@ -276,9 +276,10 @@ static void rt2400pci_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid)
 	rt2x00pci_register_multiwrite(rt2x00dev, CSR5, &reg, sizeof(reg));
 }
 
-static void rt2400pci_config_promisc(struct rt2x00_dev *rt2x00dev,
-				     const int promisc)
+static void rt2400pci_config_packet_filter(struct rt2x00_dev *rt2x00dev,
+					   const int filter)
 {
+	int promisc = !!(filter & IFF_PROMISC);
 	u32 reg;
 
 	rt2x00pci_register_read(rt2x00dev, RXCSR0, &reg);

@@ -1633,7 +1634,7 @@ static const struct rt2x00lib_ops rt2400pci_rt2x00_ops = {
 	.fill_rxdone		= rt2400pci_fill_rxdone,
 	.config_mac_addr	= rt2400pci_config_mac_addr,
 	.config_bssid		= rt2400pci_config_bssid,
-	.config_promisc		= rt2400pci_config_promisc,
+	.config_packet_filter	= rt2400pci_config_packet_filter,
 	.config_type		= rt2400pci_config_type,
 	.config			= rt2400pci_config,
 };

drivers/net/wireless/rt2500pci.c +8 -6

index f493a9c..7661a82 100644
@@ -276,13 +276,18 @@ static void rt2500pci_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid)
 	rt2x00pci_register_multiwrite(rt2x00dev, CSR5, &reg, sizeof(reg));
 }
 
-static void rt2500pci_config_promisc(struct rt2x00_dev *rt2x00dev,
-				     const int promisc)
+static void rt2500pci_config_packet_filter(struct rt2x00_dev *rt2x00dev,
+					   const int filter)
 {
+	int promisc = !!(filter & IFF_PROMISC);
+	int multicast = !!(filter & IFF_MULTICAST);
+	int broadcast = !!(filter & IFF_BROADCAST);
 	u32 reg;
 
 	rt2x00pci_register_read(rt2x00dev, RXCSR0, &reg);
 	rt2x00_set_field32(&reg, RXCSR0_DROP_NOT_TO_ME, !promisc);
+	rt2x00_set_field32(&reg, RXCSR0_DROP_MCAST, !multicast);
+	rt2x00_set_field32(&reg, RXCSR0_DROP_BCAST, !broadcast);
 	rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);
 }

@@ -314,9 +319,6 @@ static void rt2500pci_config_type(struct rt2x00_dev *rt2x00dev, const int type)
 		rt2x00_set_field32(&reg, RXCSR0_DROP_VERSION_ERROR, 1);
 	}
 
-	rt2x00_set_field32(&reg, RXCSR0_DROP_MCAST, 0);
-	rt2x00_set_field32(&reg, RXCSR0_DROP_BCAST, 0);
-
 	rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);
 
 	/*

@@ -1859,7 +1861,7 @@ static const struct rt2x00lib_ops rt2500pci_rt2x00_ops = {
 	.fill_rxdone		= rt2500pci_fill_rxdone,
 	.config_mac_addr	= rt2500pci_config_mac_addr,
 	.config_bssid		= rt2500pci_config_bssid,
-	.config_promisc		= rt2500pci_config_promisc,
+	.config_packet_filter	= rt2500pci_config_packet_filter,
 	.config_type		= rt2500pci_config_type,
 	.config			= rt2500pci_config,
 };

drivers/net/wireless/rt2500usb.c +10 -5

index 8626e55..2e19634 100644
@@ -281,13 +281,18 @@ static void rt2500usb_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid)
 	rt2500usb_register_multiwrite(rt2x00dev, MAC_CSR5, &reg, sizeof(reg));
 }
 
-static void rt2500usb_config_promisc(struct rt2x00_dev *rt2x00dev,
-				     const int promisc)
+static void rt2500usb_config_packet_filter(struct rt2x00_dev *rt2x00dev,
+					   const int filter)
 {
+	int promisc = !!(filter & IFF_PROMISC);
+	int multicast = !!(filter & IFF_MULTICAST);
+	int broadcast = !!(filter & IFF_BROADCAST);
 	u16 reg;
 
 	rt2500usb_register_read(rt2x00dev, TXRX_CSR2, &reg);
 	rt2x00_set_field16(&reg, TXRX_CSR2_DROP_NOT_TO_ME, !promisc);
+	rt2x00_set_field16(&reg, TXRX_CSR2_DROP_MULTICAST, !multicast);
+	rt2x00_set_field16(&reg, TXRX_CSR2_DROP_BROADCAST, !broadcast);
 	rt2500usb_register_write(rt2x00dev, TXRX_CSR2, reg);
 }

@@ -1469,10 +1474,10 @@ static int rt2500usb_probe_hw(struct rt2x00_dev *rt2x00dev)
 	rt2500usb_probe_hw_mode(rt2x00dev);
 
 	/*
-	 * USB devices require scheduled promisc mode toggling
+	 * USB devices require scheduled packet filter toggling
 	 * This device supports ATIM
 	 */
-	__set_bit(REQUIRE_SCHEDULE_PROMISC, &rt2x00dev->flags);
+	__set_bit(PACKET_FILTER_SCHEDULED, &rt2x00dev->flags);
 	__set_bit(DEVICE_SUPPORT_ATIM, &rt2x00dev->flags);
 
 	/*

@@ -1533,7 +1538,7 @@ static const struct rt2x00lib_ops rt2500usb_rt2x00_ops = {
 	.fill_rxdone		= rt2500usb_fill_rxdone,
 	.config_mac_addr	= rt2500usb_config_mac_addr,
 	.config_bssid		= rt2500usb_config_bssid,
-	.config_promisc		= rt2500usb_config_promisc,
+	.config_packet_filter	= rt2500usb_config_packet_filter,
 	.config_type		= rt2500usb_config_type,
 	.config			= rt2500usb_config,
 };

drivers/net/wireless/rt2500usb.h +2 -2

index 5feb34e..9df67fb 100644
@@ -247,8 +247,8 @@
 #define TXRX_CSR2_DROP_NOT_TO_ME	FIELD16(0x0010)
 #define TXRX_CSR2_DROP_TODS		FIELD16(0x0020)
 #define TXRX_CSR2_DROP_VERSION_ERROR	FIELD16(0x0040)
-#define TXRX_CSR2_DROP_MCAST		FIELD16(0x0200)
-#define TXRX_CSR2_DROP_BCAST		FIELD16(0x0400)
+#define TXRX_CSR2_DROP_MULTICAST	FIELD16(0x0200)
+#define TXRX_CSR2_DROP_BROADCAST	FIELD16(0x0400)
 
 /*
  * RX BBP ID registers

drivers/net/wireless/rt2x00.h +13 ~~-12~~

index 7f3febc..9a52251 100644
@@ -276,11 +276,13 @@ struct interface {
 	u8 bssid[ETH_ALEN];
 
 	/*
-	 * Store the promisc mode for the current interface.
-	 * monitor mode always forces promisc mode to be enabled,
-	 * so we need to store the promisc mode seperately.
+	 * Store the packet filter mode for the current interface.
+	 * monitor mode always disabled filtering. But in such
+	 * cases we still need to store the value here in case
+	 * the monitor mode interfaces are removed, while a
+	 * non-monitor mode interface remains.
 	 */
-	char promisc;
+	char filter;
 
 	/*
 	 * Monitor mode count, the number of interfaces

@@ -384,10 +386,9 @@ struct rt2x00lib_ops {
 	 */
 	void (*config_mac_addr) (struct rt2x00_dev *rt2x00dev, u8 *mac);
 	void (*config_bssid) (struct rt2x00_dev *rt2x00dev, u8 *bssid);
-	void (*config_promisc) (struct rt2x00_dev *rt2x00dev,
-				const int promisc);
+	void (*config_packet_filter) (struct rt2x00_dev *rt2x00dev,
+				      const int filter);
 	void (*config_type) (struct rt2x00_dev *rt2x00dev, const int type);
-
 	void (*config) (struct rt2x00_dev *rt2x00dev, const int flags,
 			struct ieee80211_conf *conf);
 #define CONFIG_UPDATE_PHYMODE		( 1 << 1 )

@@ -473,11 +474,11 @@ struct rt2x00_dev {
 #define DEVICE_INITIALIZED		3
 #define DEVICE_INITIALIZED_HW		4
 #define REQUIRE_FIRMWARE		5
-#define REQUIRE_SCHEDULE_PROMISC	6
-#define INTERFACE_RESET			7
-#define INTERFACE_ENABLED		8
-#define INTERFACE_ENABLED_MONITOR	9
-#define INTERFACE_ENABLED_PROMISC	10
+#define PACKET_FILTER_SCHEDULED		6
+#define PACKET_FILTER_PENDING		7
+#define INTERFACE_RESET			8
+#define INTERFACE_ENABLED		9
+#define INTERFACE_ENABLED_MONITOR	10
 #define DEVICE_SUPPORT_ATIM		11
 #define DEVICE_SUPPORT_HW_BUTTON	12
 #define CONFIG_FRAME_TYPE		13

drivers/net/wireless/rt2x00config.c +14 ~~-11~~

index 22e13d0..3733a84 100644
@@ -46,16 +46,19 @@ void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid)
 		rt2x00dev->ops->lib->config_mac_addr(rt2x00dev, bssid);
 }
 
-void rt2x00lib_config_promisc(struct rt2x00_dev *rt2x00dev, int promisc)
+void rt2x00lib_config_packet_filter(struct rt2x00_dev *rt2x00dev, int filter)
 {
 	struct interface *intf = &rt2x00dev->interface;
 
 	/*
 	 * When a monitor interface is present,
-	 * we should force promisc mode.
+	 * we should force enable all modes.
 	 */
-	if (is_monitor_present(intf))
-		promisc = 1;
+	if (is_monitor_present(intf)) {
+		filter = IFF_PROMISC | IFF_MULTICAST | IFF_BROADCAST;
+		if (intf->filter != filter)
+			__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
+	}
 
 	/*
 	 * Only configure the device when something has changed,

@@ -63,14 +66,14 @@ void rt2x00lib_config_promisc(struct rt2x00_dev *rt2x00dev, int promisc)
 	 * will be forced upon the device.
 	 */
 	if (!test_bit(INTERFACE_RESET, &rt2x00dev->flags) &&
-	    test_bit(INTERFACE_ENABLED_PROMISC, &rt2x00dev->flags) == !!promisc)
-
-	rt2x00dev->ops->lib->config_promisc(rt2x00dev, promisc);
+	    !test_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags))
+		return;
 
-	if (promisc)
-		__set_bit(INTERFACE_ENABLED_PROMISC, &rt2x00dev->flags);
-	else
-		__clear_bit(INTERFACE_ENABLED_PROMISC, &rt2x00dev->flags);
+	/*
+	 * Write configuration to device and clear the update flag.
+	 */
+	rt2x00dev->ops->lib->config_packet_filter(rt2x00dev, filter);
+	__clear_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
 }
 
 void rt2x00lib_config_type(struct rt2x00_dev *rt2x00dev, int type)

drivers/net/wireless/rt2x00lib.h +1 -1

index 4484381..1a28292 100644
@@ -50,7 +50,7 @@ void rt2x00lib_deinit_interface(struct rt2x00_dev *rt2x00dev);
  */
 void rt2x00lib_config_mac_addr(struct rt2x00_dev *rt2x00dev, u8 *mac);
 void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid);
-void rt2x00lib_config_promisc(struct rt2x00_dev *rt2x00dev, int promisc);
+void rt2x00lib_config_packet_filter(struct rt2x00_dev *rt2x00dev, int filter);
 void rt2x00lib_config_type(struct rt2x00_dev *rt2x00dev, int type);
 void rt2x00lib_config(struct rt2x00_dev *rt2x00dev, struct ieee80211_conf *conf);

drivers/net/wireless/rt2x00mac.c +25 ~~-15~~

index 0261fdc..c36ac4a 100644
@@ -148,7 +148,7 @@ int rt2x00mac_reset(struct ieee80211_hw *hw)
 	 */
 	rt2x00lib_config(rt2x00dev, &hw->conf);
 	rt2x00lib_config_type(rt2x00dev, intf->type);
-	rt2x00lib_config_promisc(rt2x00dev, intf->promisc);
+	rt2x00lib_config_packet_filter(rt2x00dev, intf->filter);
 	rt2x00lib_config_bssid(rt2x00dev, intf->bssid);
 
 	/*

@@ -192,7 +192,7 @@ int rt2x00mac_add_interface(struct ieee80211_hw *hw,
 		if (conf->type == IEEE80211_IF_TYPE_AP)
 			memcpy(&intf->bssid, conf->mac_addr, ETH_ALEN);
 		intf->mac = conf->mac_addr;
-		intf->promisc = 0;
+		intf->filter = 0;
 	}
 
 	return rt2x00lib_init_interface(rt2x00dev);

@@ -222,7 +222,7 @@ void rt2x00mac_remove_interface(struct ieee80211_hw *hw,
 		intf->type = INVALID_INTERFACE;
 		memset(&intf->bssid, 0x00, ETH_ALEN);
 		intf->mac = NULL;
-		intf->promisc = 0;
+		intf->filter = 0;
 	}
 
 	rt2x00lib_deinit_interface(rt2x00dev);

@@ -250,9 +250,9 @@ int rt2x00mac_config(struct ieee80211_hw *hw, struct ieee80211_conf *conf)
 	 * If promisc mode cannot be configured in irq context,
 	 * then it is now the time to configure it.
 	 */
-	if (test_bit(REQUIRE_SCHEDULE_PROMISC, &rt2x00dev->flags))
-		rt2x00lib_config_promisc(rt2x00dev,
-					 rt2x00dev->interface.promisc);
+	if (test_bit(PACKET_FILTER_SCHEDULED, &rt2x00dev->flags))
+		rt2x00lib_config_packet_filter(rt2x00dev,
+					       rt2x00dev->interface.filter);
 
 	/*
 	 * Reenable RX only if the radio should be on.

@@ -295,7 +295,7 @@ int rt2x00mac_config_interface(struct ieee80211_hw *hw, int if_id,
 	 * Enable configuration.
 	 */
 	rt2x00lib_config_type(rt2x00dev, conf->type);
-	rt2x00lib_config_promisc(rt2x00dev, intf->promisc);
+	rt2x00lib_config_packet_filter(rt2x00dev, intf->filter);
 	rt2x00lib_config_bssid(rt2x00dev, intf->bssid);
 
 	/*

@@ -318,25 +318,35 @@ void rt2x00mac_set_multicast_list(struct ieee80211_hw *hw,
 				  unsigned short flags, int mc_count)
 {
 	struct rt2x00_dev *rt2x00dev = hw->priv;
-	int promisc = !!(flags & IFF_PROMISC);
 
 	/*
-	 * Promisc mode is forced on for Monitor interfaces.
+	 * Check if the new state is different then the old state.
 	 */
-	if (is_monitor_present(&rt2x00dev->interface))
+	if (rt2x00dev->interface.filter == flags)
 		return;
 
+	rt2x00dev->interface.filter = flags;
+
 	/*
-	 * Check if the new state is different then the old state.
+	 * No filtering in monitor mode.
 	 */
-	if (test_bit(INTERFACE_ENABLED_PROMISC, &rt2x00dev->flags) == promisc)
+	if (is_monitor_present(&rt2x00dev->interface))
 		return;
 
-	rt2x00dev->interface.promisc = promisc;
+	/*
+	 * Raise the pending bit to indicate the
+	 * packet filter should be updated.
+	 */
+	__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
 
-	if (!test_bit(REQUIRE_SCHEDULE_PROMISC, &rt2x00dev->flags) ||
+	/*
+	 * Check if Packet filter actions are allowed in
+	 * atomic context. If not, raise the pending flag and
+	 * let it be.
+	 */
+	if (!test_bit(PACKET_FILTER_SCHEDULED, &rt2x00dev->flags) ||
 	    !in_atomic())
-		rt2x00lib_config_promisc(rt2x00dev, promisc);
+		rt2x00lib_config_packet_filter(rt2x00dev, flags);
 }
 EXPORT_SYMBOL_GPL(rt2x00mac_set_multicast_list);

drivers/net/wireless/rt61pci.c +8 -6

index 436b73c..d574297 100644
@@ -305,13 +305,18 @@ static void rt61pci_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid)
 	rt2x00pci_register_multiwrite(rt2x00dev, MAC_CSR4, &reg, sizeof(reg));
 }
 
-static void rt61pci_config_promisc(struct rt2x00_dev *rt2x00dev,
-				   const int promisc)
+static void rt61pci_config_packet_filter(struct rt2x00_dev *rt2x00dev,
+					 const int filter)
 {
+	int promisc = !!(filter & IFF_PROMISC);
+	int multicast = !!(filter & IFF_MULTICAST);
+	int broadcast = !!(filter & IFF_BROADCAST);
 	u32 reg;
 
 	rt2x00pci_register_read(rt2x00dev, TXRX_CSR0, &reg);
 	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_NOT_TO_ME, !promisc);
+	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_MULTICAST, !multicast);
+	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_BORADCAST, !broadcast);
 	rt2x00pci_register_write(rt2x00dev, TXRX_CSR0, reg);
 }

@@ -345,9 +350,6 @@ static void rt61pci_config_type(struct rt2x00_dev *rt2x00dev, const int type)
 		rt2x00_set_field32(&reg, TXRX_CSR0_DROP_ACK_CTS, 1);
 	}
 
-	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_MULTICAST, 0);
-	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_BORADCAST, 0);
-
 	rt2x00pci_register_write(rt2x00dev, TXRX_CSR0, reg);
 
 	/*

@@ -2376,7 +2378,7 @@ static const struct rt2x00lib_ops rt61pci_rt2x00_ops = {
 	.fill_rxdone		= rt61pci_fill_rxdone,
 	.config_mac_addr	= rt61pci_config_mac_addr,
 	.config_bssid		= rt61pci_config_bssid,
-	.config_promisc		= rt61pci_config_promisc,
+	.config_packet_filter	= rt61pci_config_packet_filter,
 	.config_type		= rt61pci_config_type,
 	.config			= rt61pci_config,
 };

drivers/net/wireless/rt73usb.c +10 -8

index e67f358..9875e39 100644
@@ -280,13 +280,18 @@ static void rt73usb_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid)
 	rt73usb_register_multiwrite(rt2x00dev, MAC_CSR4, &reg, sizeof(reg));
 }
 
-static void rt73usb_config_promisc(struct rt2x00_dev *rt2x00dev,
-				   const int promisc)
+static void rt73usb_config_packet_filter(struct rt2x00_dev *rt2x00dev,
+					 const int filter)
 {
+	int promisc = !!(filter & IFF_PROMISC);
+	int multicast = !!(filter & IFF_MULTICAST);
+	int broadcast = !!(filter & IFF_BROADCAST);
 	u32 reg;
 
 	rt73usb_register_read(rt2x00dev, TXRX_CSR0, &reg);
 	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_NOT_TO_ME, !promisc);
+	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_MULTICAST, !multicast);
+	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_BORADCAST, !broadcast);
 	rt73usb_register_write(rt2x00dev, TXRX_CSR0, reg);
 }

@@ -320,9 +325,6 @@ static void rt73usb_config_type(struct rt2x00_dev *rt2x00dev, const int type)
 		rt2x00_set_field32(&reg, TXRX_CSR0_DROP_ACK_CTS, 1);
 	}
 
-	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_MULTICAST, 0);
-	rt2x00_set_field32(&reg, TXRX_CSR0_DROP_BORADCAST, 0);
-
 	rt73usb_register_write(rt2x00dev, TXRX_CSR0, reg);
 
 	/*

@@ -1732,11 +1734,11 @@ static int rt73usb_probe_hw(struct rt2x00_dev *rt2x00dev)
 	rt73usb_probe_hw_mode(rt2x00dev);
 
 	/*
-	 * USB devices require scheduled promisc mode toggling
+	 * USB devices require scheduled packet filter toggling
 	 * This device requires firmware
 	 */
 	__set_bit(REQUIRE_FIRMWARE, &rt2x00dev->flags);
-	__set_bit(REQUIRE_SCHEDULE_PROMISC, &rt2x00dev->flags);
+	__set_bit(PACKET_FILTER_SCHEDULED, &rt2x00dev->flags);
 
 	/*
 	 * Set the rssi offset.

@@ -1850,7 +1852,7 @@ static const struct rt2x00lib_ops rt73usb_rt2x00_ops = {
 	.fill_rxdone		= rt73usb_fill_rxdone,
 	.config_mac_addr	= rt73usb_config_mac_addr,
 	.config_bssid		= rt73usb_config_bssid,
-	.config_promisc		= rt73usb_config_promisc,
+	.config_packet_filter	= rt73usb_config_packet_filter,
 	.config_type		= rt73usb_config_type,
 	.config			= rt73usb_config,
 };

Signature · 1 line

1.5.2.4

Johannes Berg

maintainer

2007-08-03 14:47 UTC #1 lore raw

drivers/net/wireless/rt2x00config.c void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 61

Quoted 5 lines · Ivo van Doorn

drivers/net/wireless/rt2x00config.c +5 -0

+	if (is_monitor_present(intf)) {
+		filter = IFF_PROMISC | IFF_MULTICAST | IFF_BROADCAST;
+		if (intf->filter != filter)
+			__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
+	}

Don't do that. There's no requirement that monitor mode interfaces always be promisc. Also, earlier in the code:

drivers/net/wireless/rt2500pci.c static void rt2500pci_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 291

Quoted 10 lines

drivers/net/wireless/rt2500pci.c +5 -0

+       int promisc = !!(filter & IFF_PROMISC);
+       int multicast = !!(filter & IFF_MULTICAST);
+       int broadcast = !!(filter & IFF_BROADCAST);
        u32 reg;
 
        rt2x00pci_register_read(rt2x00dev, RXCSR0, &reg);
        rt2x00_set_field32(&reg, RXCSR0_DROP_NOT_TO_ME, !promisc);
+       rt2x00_set_field32(&reg, RXCSR0_DROP_MCAST, !multicast);
+       rt2x00_set_field32(&reg, RXCSR0_DROP_BCAST, !broadcast);
        rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);

Depending on how the hardware behaves I think promisc should imply the other two, it seems likely that even with DROP_NOT_TO_ME turned off you won't be getting multicast traffic and promisc should see that too.

johannes

Ivo van Doorn

author

2007-08-03 15:05 UTC #2 lore raw

Quoted 10 lines · Johannes Berg

drivers/net/wireless/rt2x00config.c void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 61
Quoted 5 lines · Ivo van Doorn
drivers/net/wireless/rt2x00config.c +5 -0
+	if (is_monitor_present(intf)) {
+		filter = IFF_PROMISC | IFF_MULTICAST | IFF_BROADCAST;
+		if (intf->filter != filter)
+			__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
+	}
Don't do that. There's no requirement that monitor mode interfaces always be promisc. Also, earlier in the code:

Note that this "Enable promisc" on monitor mode here will only be send to the device. It does not mean that the non-monitor mode will have promisc mode enabled. In fact, when the monitor interface goes down, the normal setting will be used again.
If with monitor mode, IFF_PROMISC is not send to the device, monitor mode is useless since it will not catch anything except beacons (Especially true when you only have 1 interface which is in monitor mode)

Quoted 11 lines

drivers/net/wireless/rt2500pci.c static void rt2500pci_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 291

Quoted 10 lines

drivers/net/wireless/rt2500pci.c +5 -0

+       int promisc = !!(filter & IFF_PROMISC);
+       int multicast = !!(filter & IFF_MULTICAST);
+       int broadcast = !!(filter & IFF_BROADCAST);
        u32 reg;
 
        rt2x00pci_register_read(rt2x00dev, RXCSR0, &reg);
        rt2x00_set_field32(&reg, RXCSR0_DROP_NOT_TO_ME, !promisc);
+       rt2x00_set_field32(&reg, RXCSR0_DROP_MCAST, !multicast);
+       rt2x00_set_field32(&reg, RXCSR0_DROP_BCAST, !broadcast);
        rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);

Quoted 3 lines

Depending on how the hardware behaves I think promisc should imply the other two, it seems likely that even with DROP_NOT_TO_ME turned off you won't be getting multicast traffic and promisc should see that too.

Setting those 2 registers is having them at least configurable instead of having them always to 0. At least now the user has some control over them. ;)

Ivo

Johannes Berg

maintainer

2007-08-03 15:19 UTC #3 lore raw

Quoted 12 lines · Ivo van Doorn

Quoted 10 lines · Johannes Berg
drivers/net/wireless/rt2x00config.c void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 61
Quoted 5 lines · Ivo van Doorn
drivers/net/wireless/rt2x00config.c +5 -0
+	if (is_monitor_present(intf)) {
+		filter = IFF_PROMISC | IFF_MULTICAST | IFF_BROADCAST;
+		if (intf->filter != filter)
+			__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
+	}
Don't do that. There's no requirement that monitor mode interfaces always be promisc. Also, earlier in the code:

Quoted 7 lines

Note that this "Enable promisc" on monitor mode here will only be send to the device. It does not mean that the non-monitor mode will have promisc mode enabled. In fact, when the monitor interface goes down, the normal setting will be used again.
If with monitor mode, IFF_PROMISC is not send to the device, monitor mode is useless since it will not catch anything except beacons (Especially true when you only have 1 interface which is in monitor mode)

But don't you allow monitoring during operation? In that case this would make that monitor interface always promisc which isn't what you want unless you set the promisc bit on any of the interfaces.

johannes

Ivo van Doorn

author

2007-08-03 15:32 UTC #4 lore raw

Quoted 21 lines · Johannes Berg

Quoted 12 lines · Ivo van Doorn
Quoted 10 lines · Johannes Berg
drivers/net/wireless/rt2x00config.c void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 61
Quoted 5 lines · Ivo van Doorn
drivers/net/wireless/rt2x00config.c +5 -0
+	if (is_monitor_present(intf)) {
+		filter = IFF_PROMISC | IFF_MULTICAST | IFF_BROADCAST;
+		if (intf->filter != filter)
+			__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
+	}
Don't do that. There's no requirement that monitor mode interfaces always be promisc. Also, earlier in the code:
Quoted 7 lines
Note that this "Enable promisc" on monitor mode here will only be send to the device. It does not mean that the non-monitor mode will have promisc mode enabled. In fact, when the monitor interface goes down, the normal setting will be used again.
If with monitor mode, IFF_PROMISC is not send to the device, monitor mode is useless since it will not catch anything except beacons (Especially true when you only have 1 interface which is in monitor mode)

Quoted 3 lines

But don't you allow monitoring during operation? In that case this would make that monitor interface always promisc which isn't what you want unless you set the promisc bit on any of the interfaces.

But that would mean that when a non-monitor and monitor interface are enabled at the same time, the montitor interface is quite pointless since all frames can be caught on the regular interface as well.

A monitor interface is supposed to catch as many frames as possible, if a monitor interface does not catch frames not directly send to it, it won't catch anything except beacons.

Even when there is only 1 interface which is in monitor mode, the user shouldn't have to set the device into promisc mode right?

Ivo

Johannes Berg

maintainer

2007-08-03 15:40 UTC #5 lore raw

Quoted 3 lines · Ivo van Doorn

But that would mean that when a non-monitor and monitor interface are enabled at the same time, the montitor interface is quite pointless since all frames can be caught on the regular interface as well.

Not true, monitor mode interfaces gives you a lot more info (radiotap.)

Quoted 3 lines

A monitor interface is supposed to catch as many frames as possible, if a monitor interface does not catch frames not directly send to it, it won't catch anything except beacons.

Not if we want to use monitor interfaces for the userspace mlme.

Quoted 2 lines

Even when there is only 1 interface which is in monitor mode, the user shouldn't have to set the device into promisc mode right?

Actually, imho they should have to. And tcpdump always does anyway.

johannes

Ivo van Doorn

author

2007-08-03 15:59 UTC #6 lore raw

Quoted 12 lines · Johannes Berg

Quoted 3 lines · Ivo van Doorn
But that would mean that when a non-monitor and monitor interface are enabled at the same time, the montitor interface is quite pointless since all frames can be caught on the regular interface as well.
Not true, monitor mode interfaces gives you a lot more info (radiotap.)
Quoted 3 lines
A monitor interface is supposed to catch as many frames as possible, if a monitor interface does not catch frames not directly send to it, it won't catch anything except beacons.

Quoted 6 lines

Not if we want to use monitor interfaces for the userspace mlme.
Quoted 2 lines
Even when there is only 1 interface which is in monitor mode, the user shouldn't have to set the device into promisc mode right?
Actually, imho they should have to. And tcpdump always does anyway.

Well I hope so, otherwise I'll get quite a lot of bugreports. ;)

Next rt2x00 release will not treat monitor mode interfaces special. Just read the http://www.linuxwireless.org/en/developers/mac80211/semantics article So the next rt2x00 will comply with that.

Ivo

Johannes Berg

maintainer

2007-08-03 19:33 UTC #7 lore raw

Quoted 3 lines · Ivo van Doorn

Next rt2x00 release will not treat monitor mode interfaces special. Just read the http://www.linuxwireless.org/en/developers/mac80211/semantics article So the next rt2x00 will comply with that.

Wait until we finish discussing the CRC checks though :) I still think they should be done even in monitor mode and enabled by some extra flag; and only enabled when you have only monitor interfaces. Or mac80211 needs to be taught to drop packets with failed CRC on non-monitor interfaces.

johannes

Michael Buesch

2007-08-03 20:11 UTC #8 lore raw

In reply to Ivo van Doorn

Quoted 30 lines · Ivo van Doorn

Quoted 21 lines · Johannes Berg
Quoted 12 lines · Ivo van Doorn
Quoted 10 lines · Johannes Berg
drivers/net/wireless/rt2x00config.c void rt2x00lib_config_bssid(struct rt2x00_dev *rt2x00dev, u8 *bssid) · line 61
Quoted 5 lines · Ivo van Doorn
drivers/net/wireless/rt2x00config.c +5 -0
+	if (is_monitor_present(intf)) {
+		filter = IFF_PROMISC | IFF_MULTICAST | IFF_BROADCAST;
+		if (intf->filter != filter)
+			__set_bit(PACKET_FILTER_PENDING, &rt2x00dev->flags);
+	}
Don't do that. There's no requirement that monitor mode interfaces always be promisc. Also, earlier in the code:
Quoted 7 lines
Note that this "Enable promisc" on monitor mode here will only be send to the device. It does not mean that the non-monitor mode will have promisc mode enabled. In fact, when the monitor interface goes down, the normal setting will be used again.
If with monitor mode, IFF_PROMISC is not send to the device, monitor mode is useless since it will not catch anything except beacons (Especially true when you only have 1 interface which is in monitor mode)
Quoted 3 lines
But don't you allow monitoring during operation? In that case this would make that monitor interface always promisc which isn't what you want unless you set the promisc bit on any of the interfaces.
But that would mean that when a non-monitor and monitor interface are enabled at the same time, the montitor interface is quite pointless since all frames can be caught on the regular interface as well.

Quoted 6 lines

A monitor interface is supposed to catch as many frames as possible, if a monitor interface does not catch frames not directly send to it, it won't catch anything except beacons.
Even when there is only 1 interface which is in monitor mode, the user shouldn't have to set the device into promisc mode right?

You are wrong.
The promisc attribute is a _different_ setting from the monitor bits. If userspace wants promisc, it tells you (through set mcast_list stuff). And tcpdump/wireshark and stuff _do_ this.
If userspace wants a nonpromisc monitor, it should be allowed to have one. Don't put policy into the kernel and make bad assumptions like "monitor interface wants to be promisc".