diff mbox series

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

Message ID 20220701143052.1267509-18-miquel.raynal@bootlin.com (mailing list archive)
State Superseded
Headers show
Series net: ieee802154: Support scanning/beaconing | expand

Commit Message

Miquel Raynal July 1, 2022, 2:30 p.m. UTC
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(+)

Comments

Alexander Aring July 15, 2022, 3:16 a.m. UTC | #1
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. 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.

- Alex
Miquel Raynal Aug. 19, 2022, 5:13 p.m. UTC | #2
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.

Thanks,
Miquèl
Alexander Aring Aug. 23, 2022, 12:43 p.m. UTC | #3
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
diff mbox series

Patch

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;
+
 	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
@@ -1585,6 +1588,9 @@  nl802154_send_beacons(struct sk_buff *skb, struct genl_info *info)
 		return -EPERM;
 	}
 
+	if (wpan_phy->flags & WPAN_PHY_FLAG_DATAGRAMS_ONLY)
+		return -EOPNOTSUPP;
+
 	request = kzalloc(sizeof(*request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;