Hi, On Mon, Feb 13, 2023 at 12:35 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > 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... > > > > > > If this gets too complicated right now, then I am also fine with > > > returning an error here, we can enable it later but would it be better > > > to use ENOTSUPP or something like that in this case? EPERM sounds like > > > you can do that, but you don't have the permissions. > > > > > > > For me a scan should also be possible from iwpan phy $PHY scan (or > > whatever the scan command is, or just enable beacon)... to go over the > > dev is just a shortcut for "I mean whatever the phy is under this dev" > > ? > > Actually only coordinators (in a specific state) should be able to send > beacons, so I am kind of against allowing that shortcut, because there > are usually two dev interfaces on top of the phy's, a regular "NODE" > and a "COORD", so I don't think we should go that way. > > For scans however it makes sense, I've added the necessary changes in > wpan-tools. The TOP_LEVEL(scan) macro however does not support using > the same command name twice because it creates a macro, so this one > only supports a device name (the interface command has kind of the same > situation and uses a HIDDEN() macro which cannot be used here). > Yes, I was thinking about scanning only. > So in summary here is what is supported: > - dev <dev> beacon > - dev <dev> scan trigger|abort > - phy <phy> scan trigger|abort > - dev <dev> scan (the blocking one, which triggers, listens and returns) > > Do you agree? > Okay, yes. I trust you. - Alex