Re: [PATCH 18/24] rt2x00: Add Multicast/Broadcast filtering

[Michael Buesch <mb@bu3sch.de>]

On Friday 03 August 2007, Ivo van Doorn wrote:
> On Friday 03 August 2007, Johannes Berg wrote:
> > On Fri, 2007-08-03 at 17:09 +0200, Ivo van Doorn wrote:
> > > On Friday 03 August 2007, Johannes Berg wrote:
> > > > On Tue, 2007-07-31 at 20:37 +0200, Ivo van Doorn wrote:
> > > > 
> > > > > +	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)
> > 
> > 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?

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".