Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type

[Alexander Aring <aahringo@redhat.com>]

Hi,

On Mon, Jun 6, 2022 at 11:43 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Fri, 3 Jun 2022 22:01:38 -0400:
>
> > Hi,
> >
> > On Fri, Jun 3, 2022 at 2:34 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > The current enum is wrong. A device can either be an RFD, an RFD-RX, an
> > > RFD-TX or an FFD. If it is an FFD, it can also be a coordinator. While
> > > defining a node type might make sense from a strict software point of
> > > view, opposing node and coordinator seems meaningless in the ieee
> > > 802.15.4 world. As this enumeration is not used anywhere, let's just
> > > drop it. We will in a second time add a new "node type" enumeration
> > > which apply only to nodes, and does differentiates the type of devices
> > > mentioned above.
> > >
> >
> > First you cannot say if this is not used anywhere else.
>
> Mmmh, that's tricky, I really don't see how that might be a
> problem because there is literally nowhere in the kernel that uses this
> type, besides ieee802154_setup_sdata() which would just BUG() if this
> type was to be used. So I assumed it was safe to be removed.
>

this header is somehow half uapi where we copy it into some other
software e.g. wpan-tools as you noticed.

> > 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. 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.

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.

3. coordinator (any $TYPE specific) userspace software

May the main argument. Some coordinator specific user space daemon
does specific type handling (e.g. hostapd) maybe because some library
is required. It is a pain to deal with changing roles during the
lifetime of an interface and synchronize user space software with it.
We should keep in mind that some of those handlings will maybe be
moved to user space instead of doing it in the kernel. I am fine with
the solution now, but keep in mind to offer such a possibility.

I think the above arguments are probably the same why wireless is
doing something similar and I would avoid running into issues or it's
really difficult to handle because you need to solve other Linux net
architecture handling at first.

> > You are mixing things here with "role in the network" and what the
> > transceiver capability (RFD, FFD) is, which are two different things.
>
> I don't think I am, however maybe our vision differ on what an
> interface should be.
>
> > You should use those defines and the user needs to create a new
> > interface type and probably have a different extended address to act
> > as a coordinator.
>
> Can't we just simply switch from coordinator to !coordinator (that's
> what I currently implemented)? Why would we need the user to create a
> new interface type *and* to provide a new address?
>
> Note that these are real questions that I am asking myself. I'm fine
> adapting my implementation, as long as I get the main idea.
>

See above.

- Alex