diff mbox series

[wpan-next,v2,2/6] net: ieee802154: Ensure only FFDs can become PAN coordinators

Message ID 20220617193254.1275912-3-miquel.raynal@bootlin.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: ieee802154: PAN management | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 9 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Miquel Raynal June 17, 2022, 7:32 p.m. UTC
This is a limitation clearly listed in the specification. Now that we
have device types,let's ensure that only FFDs can become PAN
coordinators.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 net/ieee802154/nl802154.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Alexander Aring June 20, 2022, 12:24 a.m. UTC | #1
Hi,

On Fri, Jun 17, 2022 at 3:35 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> This is a limitation clearly listed in the specification. Now that we
> have device types,let's ensure that only FFDs can become PAN
> coordinators.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  net/ieee802154/nl802154.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> index 638bf544f102..0c6fc3385320 100644
> --- a/net/ieee802154/nl802154.c
> +++ b/net/ieee802154/nl802154.c
> @@ -924,6 +924,9 @@ static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
>                         return -EINVAL;
>         }
>
> +       if (type == NL802154_IFTYPE_COORD && !cfg802154_is_ffd(rdev))
> +               return -EINVAL;
> +

Look at my other mail regarding why the user needs to set this device
capability, change the errno to "EOPNOTSUPP"... it would be nice to
have an identically nl80211 handling like nl80211 to see which
interfaces are supported. Please look how wireless is doing that and
probably we should not take the standard about those "wording" too
seriously. What I mean is that according to FFD or RFD it's implied on
what interfaces you can create on.

- Alex
Alexander Aring June 20, 2022, 12:25 a.m. UTC | #2
On Sun, Jun 19, 2022 at 8:24 PM Alexander Aring <aahringo@redhat.com> wrote:
>
> Hi,
>
> On Fri, Jun 17, 2022 at 3:35 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > This is a limitation clearly listed in the specification. Now that we
> > have device types,let's ensure that only FFDs can become PAN
> > coordinators.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  net/ieee802154/nl802154.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > index 638bf544f102..0c6fc3385320 100644
> > --- a/net/ieee802154/nl802154.c
> > +++ b/net/ieee802154/nl802154.c
> > @@ -924,6 +924,9 @@ static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
> >                         return -EINVAL;
> >         }
> >
> > +       if (type == NL802154_IFTYPE_COORD && !cfg802154_is_ffd(rdev))
> > +               return -EINVAL;
> > +
>
> Look at my other mail regarding why the user needs to set this device
> capability, change the errno to "EOPNOTSUPP"... it would be nice to
> have an identically nl80211 handling like nl80211 to see which

s/like nl80211/in nl802154/

- Alex
Miquel Raynal June 20, 2022, 9:28 a.m. UTC | #3
Hi Alex,

aahringo@redhat.com wrote on Sun, 19 Jun 2022 20:24:48 -0400:

> Hi,
> 
> On Fri, Jun 17, 2022 at 3:35 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > This is a limitation clearly listed in the specification. Now that we
> > have device types,let's ensure that only FFDs can become PAN
> > coordinators.
> >
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  net/ieee802154/nl802154.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > index 638bf544f102..0c6fc3385320 100644
> > --- a/net/ieee802154/nl802154.c
> > +++ b/net/ieee802154/nl802154.c
> > @@ -924,6 +924,9 @@ static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
> >                         return -EINVAL;
> >         }
> >
> > +       if (type == NL802154_IFTYPE_COORD && !cfg802154_is_ffd(rdev))
> > +               return -EINVAL;
> > +  
> 
> Look at my other mail regarding why the user needs to set this device
> capability, change the errno to "EOPNOTSUPP"... it would be nice to
> have an identically nl80211 handling like nl80211 to see which
> interfaces are supported. Please look how wireless is doing that and
> probably we should not take the standard about those "wording" too
> seriously. What I mean is that according to FFD or RFD it's implied on
> what interfaces you can create on.

This is true, I don't need this _is_ffd() helper, checking on the type
of interface should be enough. I will drop the DEV(PHY)_TYPE enum
entirely.

Thanks,
Miquèl
Alexander Aring June 21, 2022, 2:03 a.m. UTC | #4
Hi,

On Mon, Jun 20, 2022 at 5:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Alex,
>
> aahringo@redhat.com wrote on Sun, 19 Jun 2022 20:24:48 -0400:
>
> > Hi,
> >
> > On Fri, Jun 17, 2022 at 3:35 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > This is a limitation clearly listed in the specification. Now that we
> > > have device types,let's ensure that only FFDs can become PAN
> > > coordinators.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  net/ieee802154/nl802154.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > index 638bf544f102..0c6fc3385320 100644
> > > --- a/net/ieee802154/nl802154.c
> > > +++ b/net/ieee802154/nl802154.c
> > > @@ -924,6 +924,9 @@ static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
> > >                         return -EINVAL;
> > >         }
> > >
> > > +       if (type == NL802154_IFTYPE_COORD && !cfg802154_is_ffd(rdev))
> > > +               return -EINVAL;
> > > +
> >
> > Look at my other mail regarding why the user needs to set this device
> > capability, change the errno to "EOPNOTSUPP"... it would be nice to
> > have an identically nl80211 handling like nl80211 to see which
> > interfaces are supported. Please look how wireless is doing that and
> > probably we should not take the standard about those "wording" too
> > seriously. What I mean is that according to FFD or RFD it's implied on
> > what interfaces you can create on.
>
> This is true, I don't need this _is_ffd() helper, checking on the type
> of interface should be enough. I will drop the DEV(PHY)_TYPE enum

as I said that the driver needs somehow to report which interface can
be created on the phy and such thing also exists in wireless inclusive
netlink attribute to check which iftypes are supported (by calling
iw)... you can map this information to if it's FFD or RFD. I am not
sure if such an option makes sense now because we mostly have FFD only
supported right now.

- Alex
Miquel Raynal June 21, 2022, 6:31 a.m. UTC | #5
Hi Alexander,

aahringo@redhat.com wrote on Mon, 20 Jun 2022 22:03:12 -0400:

> Hi,
> 
> On Mon, Jun 20, 2022 at 5:28 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Alex,
> >
> > aahringo@redhat.com wrote on Sun, 19 Jun 2022 20:24:48 -0400:
> >  
> > > Hi,
> > >
> > > On Fri, Jun 17, 2022 at 3:35 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > This is a limitation clearly listed in the specification. Now that we
> > > > have device types,let's ensure that only FFDs can become PAN
> > > > coordinators.
> > > >
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  net/ieee802154/nl802154.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> > > > index 638bf544f102..0c6fc3385320 100644
> > > > --- a/net/ieee802154/nl802154.c
> > > > +++ b/net/ieee802154/nl802154.c
> > > > @@ -924,6 +924,9 @@ static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
> > > >                         return -EINVAL;
> > > >         }
> > > >
> > > > +       if (type == NL802154_IFTYPE_COORD && !cfg802154_is_ffd(rdev))
> > > > +               return -EINVAL;
> > > > +  
> > >
> > > Look at my other mail regarding why the user needs to set this device
> > > capability, change the errno to "EOPNOTSUPP"... it would be nice to
> > > have an identically nl80211 handling like nl80211 to see which
> > > interfaces are supported. Please look how wireless is doing that and
> > > probably we should not take the standard about those "wording" too
> > > seriously. What I mean is that according to FFD or RFD it's implied on
> > > what interfaces you can create on.  
> >
> > This is true, I don't need this _is_ffd() helper, checking on the type
> > of interface should be enough. I will drop the DEV(PHY)_TYPE enum  
> 
> as I said that the driver needs somehow to report which interface can
> be created on the phy and such thing also exists in wireless inclusive
> netlink attribute to check which iftypes are supported (by calling
> iw)... you can map this information to if it's FFD or RFD.

Yes, this is how I ended up doing things, I'm checking we are on a
coordinator interface before allowing beacons, for instance.

> I am not
> sure if such an option makes sense now because we mostly have FFD only
> supported right now.

Yeah, I've dropped that part in v3 so that we can move forward, it will
not be too hard to had this information later if we ever have !FFD
devices which need to be supported anyway.

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 638bf544f102..0c6fc3385320 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -924,6 +924,9 @@  static int nl802154_new_interface(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
+	if (type == NL802154_IFTYPE_COORD && !cfg802154_is_ffd(rdev))
+		return -EINVAL;
+
 	if (info->attrs[NL802154_ATTR_EXTENDED_ADDR])
 		extended_addr = nla_get_le64(info->attrs[NL802154_ATTR_EXTENDED_ADDR]);