Re: [PATCH wpan-next 1/6] ieee802154: Add support for user scanning requests

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Alexander,

> > > > > > +static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > > > > > +{
> > > > > > +       struct cfg802154_registered_device *rdev = info->user_ptr[0];
> > > > > > +       struct net_device *dev = info->user_ptr[1];
> > > > > > +       struct wpan_dev *wpan_dev = dev->ieee802154_ptr;
> > > > > > +       struct wpan_phy *wpan_phy = &rdev->wpan_phy;
> > > > > > +       struct cfg802154_scan_request *request;
> > > > > > +       u8 type;
> > > > > > +       int err;
> > > > > > +
> > > > > > +       /* Monitors are not allowed to perform scans */
> > > > > > +       if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > > > > > +               return -EPERM;  
> > > > >
> > > > > btw: why are monitors not allowed?  
> > > >
> > > > I guess I had the "active scan" use case in mind which of course does
> > > > not work with monitors. Maybe I can relax this a little bit indeed,
> > > > right now I don't remember why I strongly refused scans on monitors.  
> > >
> > > Isn't it that scans really work close to phy level? Means in this case
> > > we disable mostly everything of MAC filtering on the transceiver side.
> > > Then I don't see any reasons why even monitors can't do anything, they
> > > also can send something. But they really don't have any specific
> > > source address set, so long addresses are none for source addresses, I
> > > don't see any problem here. They also don't have AACK handling, but
> > > it's not required for scan anyway...  
> >
> > I think I remember why I did not want to enable scans on monitors: we
> > actually change the filtering level to "scan", which is very
> > different to what a monitor is supposed to receive, which means in scan
> > mode a monitor would no longer receive all what it is supposed to
> > receive. Nothing that cannot be workaround'ed by software, probably,
> > but I believe it is safer right now to avoid introducing potential
> > regressions. So I will just change the error code and still refuse
> > scans on monitor interfaces for now, until we figure out if it's
> > actually safe or not (and if we really want to allow it).
> >  
> 
> Okay, for scan yes we tell them to be in scan mode and then the
> transceiver can filter whatever it delivers to the next level which is
> necessary for filtering scan mac frames only. AACK handling is
> disabled for scan mode for all types != MONITORS.
> 
> For monitors we mostly allow everything and AACK is _always_ disabled.
> The transceiver filter is completely disabled for at least what looks
> like a 802.15.4 MAC header (even malformed). There are some frame
> length checks which are necessary for specific hardware because
> otherwise they would read out the frame buffer. For me it can still
> feed the mac802154 stack for scanning (with filtering level as what
> the monitor sets to, but currently our scan filter is equal to the
> monitor filter mode anyway (which probably can be changed in
> future?)). So in my opinion the monitor can do both -> feed the scan
> mac802154 deliver path and the packet layer. And I also think that on
> a normal interface type the packet layer should be feeded by those
> frames as well and do not hit the mac802154 layer scan path only.

Actually that would be an out-of-spec situation, here is a quote of
chapter "6.3.1.3 Active and passive channel scan"

	During an active or passive scan, the MAC sublayer shall
	discard all frames received over the PHY data service that are
	not Beacon frames.

I don't think this is possible to do anyway on devices with a single
hardware filter setting?

> However this can be done in future and I think, indeed there might be
> other problems to tackle to enable such functionality.

Indeed.

Thanks,
Miquèl