Re: [PATCH wpan-next 17/20] net: ieee802154: Handle limited devices with only datagram support

[Alexander Aring <aahringo@redhat.com>]

Hi,

On Fri, Aug 19, 2022 at 1:13 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alexander,
>
> aahringo@redhat.com wrote on Thu, 14 Jul 2022 23:16:33 -0400:
>
> > Hi,
> >
> > On Fri, Jul 1, 2022 at 10:37 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Some devices, like HardMAC ones can be a bit limited in the way they
> > > handle mac commands. In particular, they might just not support it at
> > > all and instead only be able to transmit and receive regular data
> > > packets. In this case, they cannot be used for any of the internal
> > > management commands that we have introduced so far and must be flagged
> > > accordingly.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  include/net/cfg802154.h   | 3 +++
> > >  net/ieee802154/nl802154.c | 6 ++++++
> > >  2 files changed, 9 insertions(+)
> > >
> > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> > > index d6ff60d900a9..20ac4df9dc7b 100644
> > > --- a/include/net/cfg802154.h
> > > +++ b/include/net/cfg802154.h
> > > @@ -178,12 +178,15 @@ wpan_phy_cca_cmp(const struct wpan_phy_cca *a, const struct wpan_phy_cca *b)
> > >   *     setting.
> > >   * @WPAN_PHY_FLAG_STATE_QUEUE_STOPPED: Indicates that the transmit queue was
> > >   *     temporarily stopped.
> > > + * @WPAN_PHY_FLAG_DATAGRAMS_ONLY: Indicates that transceiver is only able to
> > > + *     send/receive datagrams.
> > >   */
> > >  enum wpan_phy_flags {
> > >         WPAN_PHY_FLAG_TXPOWER           = BIT(1),
> > >         WPAN_PHY_FLAG_CCA_ED_LEVEL      = BIT(2),
> > >         WPAN_PHY_FLAG_CCA_MODE          = BIT(3),
> > >         WPAN_PHY_FLAG_STATE_QUEUE_STOPPED = BIT(4),
> > > +       WPAN_PHY_FLAG_DATAGRAMS_ONLY    = BIT(5),
> > >  };
> > >
> > >  struct wpan_phy {
> > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > index 00b03c33e826..b31a0bd36b08 100644
> > > --- a/net/ieee802154/nl802154.c
> > > +++ b/net/ieee802154/nl802154.c
> > > @@ -1404,6 +1404,9 @@ static int nl802154_trigger_scan(struct sk_buff *skb, struct genl_info *info)
> > >         if (wpan_dev->iftype == NL802154_IFTYPE_MONITOR)
> > >                 return -EPERM;
> > >
> > > +       if (wpan_phy->flags & WPAN_PHY_FLAG_DATAGRAMS_ONLY)
> > > +               return -EOPNOTSUPP;
> > > +
> >
> > for doing a scan it's also required to turn the transceiver into
> > promiscuous mode, right?
> >
> > There is currently a flag if a driver supports promiscuous mode or
> > not... I am not sure if all drivers have support for it. For me it
> > looks like a mandatory feature but I am not sure if every driver
> > supports it.
> > We have a similar situation with acknowledge retransmit handling...
> > some transceivers can't handle it and for normal dataframes we have a
> > default behaviour that we don't set it. However sometimes it's
> > required by the spec, then we can't do anything here.
> >
> > I think we should check on it but we should plan to drop this flag if
> > promiscuous mode is supported or not.
>
> Yes, you are right, I should check this flag is set, I will do it at
> the creation of the MONITOR interface, which anyway does not make sense
> if the device has no filtering support (promiscuous being a very
> standard one, but, as you said below, will later be replaced by some
> more advanced levels).
>
> > I also think that the
> > promiscuous driver_ops should be removed and moved as a parameter for
> > start() driver_ops to declare which "receive mode" should be
> > enabled... but we can do that in due course.
>
> Agreed.

We need to keep in mind that hwsim is a transceiver which only can run
in promiscuous mode and all receive paths need to be aware of this.
Yes, we can do that by saying on the ieee802154_rx() it always
receives frames in promiscuous mode and mac802154 does all the
filtering. I have the feeling there needs to be more done, because the
driver op to run into promiscuous mode and the mac802154 layer thinks
it's not BUT hwsim was it all the time. :-/

- Alex