Message-ID: <20220609164240.4e7515d4@xps-13>
Date:   Thu, 9 Jun 2022 16:42:40 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Alexander Aring <aahringo@...hat.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>,
        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 1/6] net: ieee802154: Drop coordinator
 interface type

Hi Alex,

> > > > > Second I have
> > > > > a different opinion here that you cannot just "switch" the role from
> > > > > RFD, FFD, whatever.  
> > > >
> > > > I agree with this, and that's why I don't understand this enum.
> > > >
> > > > A device can either be a NODE (an active device) or a MONITOR (a
> > > > passive device) at a time. We can certainly switch from one to
> > > > another at run time.
> > > >
> > > > A NODE can be either an RFD or an FFD. That is a static property which
> > > > cannot change.
> > > >
> > > > However being a coordinator is just an additional property of a NODE
> > > > which is of type FFD, and this can change over time.
> > > >
> > > > So I don't get what having a coordinator interface would bring. What
> > > > was the idea behind its introduction then?
> > > >  
> > >
> > > There exists arguments which I have in my mind right now:
> > >
> > > 1. frame parsing/address filter (which is somehow missing in your patches)
> > >
> > > The parsing of frames is different from other types (just as monitor
> > > interfaces). You will notice that setting up the address filter will
> > > require a parameter if coordinator or not.  
> >
> > I think this is something that I completely missed until now, can you
> > point me to where/how this is expected to be done? I don't see anything
> > wpan specific filtering implementation. What is expected on this area
> > and is there code that I missed already?
> >  
> 
> https://elixir.bootlin.com/linux/v5.19-rc1/source/net/mac802154/rx.c#L284

Oh okay now I get what you mean. Indeed, I had to look into this
function to allow coordinators to receive packets with the IFACE_COORD
implementation, but so far the filtering is "the same" as for a node.
We can improve that later if needed.
 
> > > Changing the address
> > > filterung during runtime of an interface is somehow _not_ supported.
> > > The reason is that the datasheets tell you to first set up an address
> > > filter and then switch into receiving mode. Changing the address
> > > filter during receive mode (start()/stop()) is not a specified
> > > behaviour. Due to bus communication it also cannot be done atomically.
> > > This might be avoidable but is a pain to synchronize if you really
> > > depend on hw address filtering which we might do in future. It should
> > > end in a different receiving path e.g. node_rx/monitor_rx.  
> >
> > Got it.
> >  
> 
> I had some thoughts about this as well when going to promiscuous mode
> while in "receiving mode"... this is "normally" not supported...
> 
> > >
> > > 2. HardMAC transceivers
> > >
> > > The add_interface() callback will be directly forwarded to the driver
> > > and the driver will during the lifetime of this interface act as a
> > > coordinator and not a mixed mode which can be disabled and enabled
> > > anytime. I am not even sure if this can ever be handled in such a way
> > > from hardmac transceivers, it might depend on the transceiver
> > > interface but we should assume some strict "static" handling. Static
> > > handling means here that the transceiver is unable to switch from
> > > coordinator and vice versa after some initialization state.  
> >
> > Okay. I just completely missed the "interface add" command. So your
> > advice is to treat the "PAN coordinator" property as a static property
> > for a given interface, which seems reasonable.
> >
> > For now I will assume the same treatment when adding the interface is
> > required compared to a NODE, but if something comes to your mind,
> > please let me know.
> >
> > By the way, is there a mechanism limiting the number of interfaces on a
> > device? Should we prevent the introduction of a coordinator iface if
> > there is a node iface active?
> >  
> 
> such a mechanism already exists, look at the code when trying to ifup
> an interface in mac802154. You cannot simply have a monitor and node
> up at the same time. Hardware could have multiple address filters and
> run multiple mac stack instances on one phy, which is in my opinion
> not different than macvlan and in wireless running multiple access
> points on the same phy.

Oh nice, I didn't pay enough attention to figure out that this was
executed during ifup. So I already changed that code to refuse two node
*and* coordinators to be up at the same time, we should be on the safe
side.

Thanks,
Miquèl

Powered by blists - more mailing lists