All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.