netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Alexander Aring <aahringo@redhat.com>
Cc: Alexander Aring <alex.aring@gmail.com>,
	Stefan Schmidt <stefan@datenfreihafen.org>,
	linux-wpan - ML <linux-wpan@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Network Development <netdev@vger.kernel.org>,
	David Girault <david.girault@qorvo.com>,
	Romuald Despres <romuald.despres@qorvo.com>,
	Frederic Blain <frederic.blain@qorvo.com>,
	Nicolas Schodet <nico@ni.fr.eu.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type
Date: Thu, 9 Jun 2022 16:42:40 +0200	[thread overview]
Message-ID: <20220609164240.4e7515d4@xps-13> (raw)
In-Reply-To: <CAK-6q+gf2_aVt4m7z77aLH+Rkc_sRTEjoykk5Dn+04wbu5n7Xg@mail.gmail.com>

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

  reply	other threads:[~2022-06-09 14:42 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-03 18:21 [PATCH wpan-next 0/6] net: ieee802154: PAN management Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 1/6] net: ieee802154: Drop coordinator interface type Miquel Raynal
2022-06-04  2:01   ` Alexander Aring
2022-06-06 15:43     ` Miquel Raynal
2022-06-07  3:04       ` Alexander Aring
2022-06-07 16:16         ` Miquel Raynal
2022-06-08 13:47           ` Miquel Raynal
2022-06-08 14:37             ` Miquel Raynal
2022-06-09  2:06               ` Alexander Aring
2022-06-09  2:23                 ` Alexander Aring
2022-06-09 15:43                   ` Miquel Raynal
2022-06-11 12:05                     ` Alexander Aring
2022-06-15  9:15                       ` Miquel Raynal
2022-06-09  1:56             ` Alexander Aring
2022-06-09 15:52               ` Miquel Raynal
     [not found]                 ` <CAK-6q+jchHcge2_hMznO6fwx=xoUEpmoZTFYLAUwqM2Ue4Lx-A@mail.gmail.com>
2022-06-17 15:12                   ` Miquel Raynal
2022-06-20  0:13                     ` Alexander Aring
2022-06-20  9:19                       ` Miquel Raynal
2022-06-21  1:54                         ` Alexander Aring
2022-06-21  6:27                           ` Miquel Raynal
2022-06-26  1:36                             ` Alexander Aring
2022-06-27  8:17                               ` Miquel Raynal
2022-06-09  1:42           ` Alexander Aring
2022-06-09 14:42             ` Miquel Raynal [this message]
2022-06-03 18:21 ` [PATCH wpan-next 2/6] net: ieee802154: Add support for internal PAN management Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 3/6] net: ieee802154: Create a node type Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 4/6] net: ieee802154: Add the PAN coordinator information Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 5/6] net: ieee802154: Full PAN management Miquel Raynal
2022-06-03 18:21 ` [PATCH wpan-next 6/6] net: ieee802154: Trace the registration of new PANs Miquel Raynal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220609164240.4e7515d4@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=aahringo@redhat.com \
    --cc=alex.aring@gmail.com \
    --cc=davem@davemloft.net \
    --cc=david.girault@qorvo.com \
    --cc=frederic.blain@qorvo.com \
    --cc=kuba@kernel.org \
    --cc=linux-wpan@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nico@ni.fr.eu.org \
    --cc=pabeni@redhat.com \
    --cc=romuald.despres@qorvo.com \
    --cc=stefan@datenfreihafen.org \
    --cc=thomas.petazzoni@bootlin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).