Date:   Thu, 25 Aug 2022 21:05:09 -0400
From:   Alexander Aring <aahringo@...hat.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Alexander Aring <alex.aring@...il.com>,
        Stefan Schmidt <stefan@...enfreihafen.org>,
        linux-wpan - ML <linux-wpan@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Network Development <netdev@...r.kernel.org>,
        David Girault <david.girault@...vo.com>,
        Romuald Despres <romuald.despres@...vo.com>,
        Frederic Blain <frederic.blain@...vo.com>,
        Nicolas Schodet <nico@...fr.eu.org>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH wpan-next 01/20] net: mac802154: Allow the creation of
 coordinator interfaces

Hi,

On Thu, Aug 25, 2022 at 8:58 AM Miquel Raynal <miquel.raynal@...tlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@...hat.com wrote on Wed, 24 Aug 2022 17:53:45 -0400:
>
> > Hi,
> >
> > On Wed, Aug 24, 2022 at 9:27 AM Miquel Raynal <miquel.raynal@...tlin.com> wrote:
> > >
> > > Hi Alexander,
> > >
> > > aahringo@...hat.com wrote on Wed, 24 Aug 2022 08:43:20 -0400:
> > >
> > > > Hi,
> > > >
> > > > On Wed, Aug 24, 2022 at 6:21 AM Miquel Raynal <miquel.raynal@...tlin.com> wrote:
> > > > ...
> > > > >
> > > > > Actually right now the second level is not enforced, and all the
> > > > > filtering levels are a bit fuzzy and spread everywhere in rx.c.
> > > > >
> > > > > I'm gonna see if I can at least clarify all of that and only make
> > > > > coord-dependent the right section because right now a
> > > > > ieee802154_coord_rx() path in ieee802154_rx_handle_packet() does not
> > > > > really make sense given that the level 3 filtering rules are mostly
> > > > > enforced in ieee802154_subif_frame().
> > > >
> > > > One thing I mentioned before is that we probably like to have a
> > > > parameter for rx path to give mac802154 a hint on which filtering
> > > > level it was received. We don't have that, I currently see that this
> > > > is a parameter for hwsim receiving it on promiscuous level only and
> > > > all others do third level filtering.
> > > > We need that now, because the promiscuous mode was only used for
> > > > sniffing which goes directly into the rx path for monitors. With scan
> > > > we mix things up here and in my opinion require such a parameter and
> > > > do filtering if necessary.
> > >
> > > I am currently trying to implement a slightly different approach. The
> > > core does not know hwsim is always in promiscuous mode, but it does
> > > know that it does not check FCS. So the core checks it. This is
> > > level 1 achieved. Then in level 2 we want to know if the core asked
> > > the transceiver to enter promiscuous mode, which, if it did, should
> > > not imply more filtering. If the device is working in promiscuous
> > > mode but this was not asked explicitly by the core, we don't really
> > > care, software filtering will apply anyway.
> > >
> >
> > I doubt that I will be happy with this solution, this all sounds like
> > "for the specific current behaviour that we support 2 filtering levels
> > it will work", just do a parameter on which 802.15.4 filtering level
> > it was received and the rx path will check what kind of filter is
> > required and which not.
> > As driver ops start() callback you should say which filtering level
> > the receive mode should start with.
> >
> > > I am reworking the rx path to clarify what is being done and when,
> > > because I found this part very obscure right now. In the end I don't
> > > think we need additional rx info from the drivers. Hopefully my
> > > proposal will clarify why this is (IMHO) not needed.
> > >
> >
> > Never looked much in 802.15.4 receive path as it just worked but I
> > said that there might be things to clean up when filtering things on
> > hardware and when on software and I have the feeling we are doing
> > things twice. Sometimes it is also necessary to set some skb fields
> > e.g. PACKET_HOST, etc. and I think this is what the most important
> > part of it is there. However, there are probably some tune ups if we
> > know we are in third leveling filtering...
>
> Ok, I've done the following.
>
> - Adding a PHY parameter which reflects the actual filtering level of
>   the transceiver, the default level is 4 (standard situation, you're

3?

>   receiving data) but of course if the PHY does not support this state
>   (like hwsim) it should overwrite this value by setting the actual
>   filtering level (none, in the hwsim case) so that the core knows what
>   it receives.
>

ok.

> - I've replaced the specific "do not check the FCS" flag only used by
>   hwsim by this filtering level, which gives all the information we
>   need.
>

ok.

> - I've added a real promiscuous filtering mode which truly does not
>   care about the content of the frame but only checks the FCS if not
>   already done by the xceiver.
>

not sure what a "real promiscuous filtering here is" people have
different understanding about it, but 802.15.4 has a definition for
it. You should consider that having monitors, frames with bad fcs
should not be filtered out by hardware. There it comes back what I
said before, the filtering level should be a parameter for start()
driver ops.

> - I've also implemented in software filtering level 4 for most regular

3?

>   data packets. Without changing the default PHY level mentioned in the
>   first item above, this additional filtering will be skipped which
>   ensures we keep the same behavior of most driver. In the case of hwsim
>   however, these filters will become active if the MAC is not in
>   promiscuous mode or in scan mode, which is actually what people
>   should be expecting.
>

To give feedback to that I need to see code. And please don't send the
whole feature stuff again, just this specific part of it. Thanks.

> Hopefully all this fits what you had in mind.
>
> I have one item left on my current todo list: improving a bit the
> userspace tool with a "monitor" command.
>
> Otherwise the remaining things to do are to discuss the locking design
> which might need to be changed to avoid lockdep issues and keep the
> rtnl locked eg. during a channel change. I still don't know how to do
> that, so it's likely that the right next version will not include any
> change in this area unless something pops up.

I try to look at that on the weekend.

- Alex

Powered by blists - more mailing lists