Hi, On Wed, Sep 21, 2022 at 11:46 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > Hi Alexander, > > aahringo@xxxxxxxxxx wrote on Thu, 8 Sep 2022 20:41:14 -0400: > > > Hi, > > > > On Thu, Sep 8, 2022 at 3:37 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > > > > Hi Alexander, > > > > > > aahringo@xxxxxxxxxx wrote on Wed, 7 Sep 2022 21:40:13 -0400: > > > > > > > Hi, > > > > > > > > On Mon, Sep 5, 2022 at 4:34 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote: > > > > > > > > > > Hello, > > > > > > > > > > A third version of this series, dropping the scan patches for now > > > > > because before we need to settle on the filtering topic and the > > > > > coordinator interface topic. Here is just the filtering part, I've > > > > > integrated Alexander's patches, as well as the atusb fix. Once this is > > > > > merge there are a few coordinator-related patches, and finally the > > > > > scan. > > > > > > > > I think we have a communication problem here and we should talk about > > > > what the problems are and agree on a way to solve them. > > > > > > > > The problems are: > > > > > > > > 1. We never supported switching from an operating phy (interfaces are > > > > up) into another filtering mode. > > > > > > In the trigger scan path there is a: > > > > > > mlme_op_pre() // stop Tx > > > drv_stop() // stop Rx > > > synchronize_net() > > > drv_start(params) // restart Rx with another hw filtering level > > > > > > > Okay, that's looking good. > > > > > > 2. Scan requires to be in "promiscuous mode" (according to the > > > > 802.15.4 spec promiscuous mode). We don't support promiscuous mode > > > > (according to the 802.15.4 spec promiscuous mode). We "can" however > > > > use the currently supported mode which does not filter anything > > > > (IEEE802154_FILTERING_NONE) when we do additional filtering in > > > > mac802154. _But_ this is only required when the phy is scanning, it > > > > will also deliver anything to the upper layers. > > > > > > > > This patch-series tries to do the second thing, okay that's fine. But > > > > I thought this should only be done while the phy is in "scanning > > > > mode"? > > > > > > I don't understand what's wrong then. We ask for the "scan mode" > > > filtering level when starting the scan [1] and we ask for the normal > > > filtering level when it's done/aborted [2] [3]. > > > > > > > There is no problem with that. There is for me a problem with the > > receive path when certain filtering levels are active. > > > > > [1] https://github.com/miquelraynal/linux/blob/wpan-next/scan/net/mac802154/scan.c#L326 > > > [2] https://github.com/miquelraynal/linux/blob/wpan-next/scan/net/mac802154/scan.c#L55 > > > > > > > The other receive path while not in promiscuous mode > > > > (phy->filtering == IEEE802154_FILTERING_4_FRAME_FIELDS) should never > > > > require any additional filtering. I somehow miss this point here. > > > > > > Maybe the drv_start() function should receive an sdata pointer. This way > > > instead of changing the PHY filtering level to what has just be asked > > > blindly, the code should look at the filtering level of all the > > > interfaces up on the PHY and apply the lowest filtering level by > > > hardware, knowing that on a per interface basis, the software will > > > compensate. > > > > > > It should work just fine because local->phy->filtering shows the actual > > > filtering level of the PHY while sdata->requested_filtering shows the > > > level of filtering that was expected on each interface. If you don't > > > like the idea of having a mutable sdata->requested_filtering entry, I > > > can have an sdata->base_filtering which should only be set by > > > ieee802154_setup_sdata() and an sdata->expected_filtering which would > > > reflect what the mac expects on this interface at the present moment. > > > > > > > From my view is that if we disable address filters (all filtering > > modes except IEEE802154_FILTERING_4_FRAME_FIELDS) we never can call > > netif_receive_skb(). This patch series tries to "compensate" the > > missing filtering on phy which is fine only to handle things related > > for the scan operation but nothing else. > > > > The reason why we can't call netif_receive_skb() is because we don't > > have ackknowledge handling, whereas for scanning we ignore ack frames > > and that's why we don't need it. > > I've digested all of that right before the conference so I think I now > understand all your fears regarding the possible absence of ACK in > certain situations. I even share them now. > > > > > For 1), the driver should change the filtering mode" when we start to > > > > "listen", this is done by the start() driver callback. They should get > > > > all receive parameters and set up receiving to whatever mac802154, > > > > currently there is a bit of chaos there. To move it into drv_start() > > > > is just a workaround to begin this step that we move it at some point > > > > to the driver. I mention 1) here because that should be part of the > > > > picture how everything works together when the phy is switched to a > > > > different filter level while it's operating (I mean there are running > > > > interfaces on it which requires IEEE802154_FILTERING_4_FRAME_FIELDS) > > > > which then activates the different receive path for the use case of > > > > scanning (something like (phy->state & WPANPHY_SCANING) == true)? > > > > > > Scanning is a dedicated filtering level per-se because it must discard > > > !beacon frames, that's why I request this level of filtering (which > > > maybe I should do on a per-interface basis instead of using the *local > > > poiner). > > > > > > > We only can do a per filter level per interface if the hardware has > > support for such a thing. Currently there is one address filter and if > > it's disabled we lose ackknowledge handling (as general rule), we > > can't compensate by doing any additional filtering by software in this > > mode. > > Yes. > > > > > > > I am sorry, but I somehow miss the picture of how those things work > > > > together. It is not clear for me and I miss those parts to get a whole > > > > picture of this. For me it's not clear that those patches are going in > > > > this direction. > > > > > > Please tell me if it's more clear and if you agree with this vision. I > > > don't have time to draft something this week. > > > > > > > That's fine. We should agree on things like compensate lower filter > > levels by doing additional softmac filtering to reach > > IEEE802154_FILTERING_4_FRAME_FIELDS filtering from others because we > > will lose AACK handling. It is only fine to do that in mac802154 > > receive path but don't deliver it to the upper layer. > > So actually, if I try to summarize the situation. > > I've tried to make several different subif working on a single PHY. > Unfortunately, today there is only one address filter per PHY, so correct. But you are assuming hardware which is currently around. As I said atusb is one candidate to make a more clever hardware because it has a co-processor which eventually could handle ack handling if done right. > disabling the address filter on one interface would also disable it on > the other, leading to the ACKs not being handled anymore, which we > cannot afford. > For hardware which has such limitation of one address filter yes. > So overall I guess we should settle on how we want to handle the > situation. I propose, to move forward, that we continue to assume that > it is *not* possible to have several different interfaces running at the > same time on a single PHY. This involves dropping all the "software > address filtering" which I proposed, but I guess that's fine. I agree here, but don't remove code which could add such handling and allow multiple monitors at the same time. - Alex
