| Message ID | 20230601154817.754519-5-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ieee802154: Associations between devices | expand |
On Thu, Jun 01, 2023 at 05:48:10PM +0200, Miquel Raynal wrote: > Joining a PAN officially goes by associating with a coordinator. This > coordinator may have been discovered thanks to the beacons it sent in > the past. Add support to the MAC layer for these associations, which > require: > - Sending an association request > - Receiving an association response > > The association response contains the association status, eventually a > reason if the association was unsuccessful, and finally a short address > that we should use for intra-PAN communication from now on, if we > required one (which is the default, and not yet configurable). > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> ... > diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c > index cd69bdbfd59f..8bf01bb7e858 100644 > --- a/net/ieee802154/core.c > +++ b/net/ieee802154/core.c > @@ -198,6 +198,18 @@ void wpan_phy_free(struct wpan_phy *phy) > } > EXPORT_SYMBOL(wpan_phy_free); > > +static void cfg802154_free_peer_structures(struct wpan_dev *wpan_dev) > +{ > + mutex_lock(&wpan_dev->association_lock); > + > + if (wpan_dev->parent) > + kfree(wpan_dev->parent); Hi Miquel, a minor nit from my side: There no need to check for NULL before calling kfree, because kfree will do nothing with a NULL argument. > + > + wpan_dev->association_generation++; > + > + mutex_unlock(&wpan_dev->association_lock); > +} > + > int cfg802154_switch_netns(struct cfg802154_registered_device *rdev, > struct net *net) > { ...
Hi, On Thu, Jun 1, 2023 at 11:50 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > Joining a PAN officially goes by associating with a coordinator. This > coordinator may have been discovered thanks to the beacons it sent in > the past. Add support to the MAC layer for these associations, which > require: > - Sending an association request > - Receiving an association response > > The association response contains the association status, eventually a > reason if the association was unsuccessful, and finally a short address > that we should use for intra-PAN communication from now on, if we > required one (which is the default, and not yet configurable). > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > include/linux/ieee802154.h | 1 + > include/net/cfg802154.h | 1 + > include/net/ieee802154_netdev.h | 5 ++ > net/ieee802154/core.c | 14 ++++ > net/mac802154/cfg.c | 72 ++++++++++++++++++ > net/mac802154/ieee802154_i.h | 19 +++++ > net/mac802154/main.c | 2 + > net/mac802154/rx.c | 9 +++ > net/mac802154/scan.c | 127 ++++++++++++++++++++++++++++++++ > 9 files changed, 250 insertions(+) > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h > index 140f61ec0f5f..c72bd76cac1b 100644 > --- a/include/linux/ieee802154.h > +++ b/include/linux/ieee802154.h > @@ -37,6 +37,7 @@ > IEEE802154_FCS_LEN) > > #define IEEE802154_PAN_ID_BROADCAST 0xffff > +#define IEEE802154_ADDR_LONG_BROADCAST 0xffffffffffffffffULL > #define IEEE802154_ADDR_SHORT_BROADCAST 0xffff > #define IEEE802154_ADDR_SHORT_UNSPEC 0xfffe > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > index 3b9d65455b9a..dd0964d351cd 100644 > --- a/include/net/cfg802154.h > +++ b/include/net/cfg802154.h > @@ -502,6 +502,7 @@ struct wpan_dev { > struct mutex association_lock; > struct ieee802154_pan_device *parent; > struct list_head children; > + unsigned int association_generation; > }; > > #define to_phy(_dev) container_of(_dev, struct wpan_phy, dev) > diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h > index ca8c827d0d7f..e26ffd079556 100644 > --- a/include/net/ieee802154_netdev.h > +++ b/include/net/ieee802154_netdev.h > @@ -149,6 +149,11 @@ struct ieee802154_assoc_req_pl { > #endif > } __packed; > > +struct ieee802154_assoc_resp_pl { > + __le16 short_addr; > + u8 status; > +} __packed; > + > enum ieee802154_frame_version { > IEEE802154_2003_STD, > IEEE802154_2006_STD, > diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c > index cd69bdbfd59f..8bf01bb7e858 100644 > --- a/net/ieee802154/core.c > +++ b/net/ieee802154/core.c > @@ -198,6 +198,18 @@ void wpan_phy_free(struct wpan_phy *phy) > } > EXPORT_SYMBOL(wpan_phy_free); > > +static void cfg802154_free_peer_structures(struct wpan_dev *wpan_dev) > +{ > + mutex_lock(&wpan_dev->association_lock); > + > + if (wpan_dev->parent) > + kfree(wpan_dev->parent); > + > + wpan_dev->association_generation++; > + > + mutex_unlock(&wpan_dev->association_lock); > +} > + > int cfg802154_switch_netns(struct cfg802154_registered_device *rdev, > struct net *net) > { > @@ -293,6 +305,8 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb, > rdev->opencount++; > break; > case NETDEV_UNREGISTER: > + cfg802154_free_peer_structures(wpan_dev); > + I think the comment below is not relevant here, but I have also no idea if this is still the case. > /* It is possible to get NETDEV_UNREGISTER > * multiple times. To detect that, check > * that the interface is still on the list > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c > index 5c3cb019f751..89112d2bcee7 100644 > --- a/net/mac802154/cfg.c > +++ b/net/mac802154/cfg.c > @@ -315,6 +315,77 @@ static int mac802154_stop_beacons(struct wpan_phy *wpan_phy, > return mac802154_stop_beacons_locked(local, sdata); > } > > +static int mac802154_associate(struct wpan_phy *wpan_phy, > + struct wpan_dev *wpan_dev, > + struct ieee802154_addr *coord) > +{ > + struct ieee802154_local *local = wpan_phy_priv(wpan_phy); > + u64 ceaddr = swab64((__force u64)coord->extended_addr); > + struct ieee802154_sub_if_data *sdata; > + struct ieee802154_pan_device *parent; > + __le16 short_addr; > + int ret; > + > + ASSERT_RTNL(); > + > + sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev); > + > + if (wpan_dev->parent) { > + dev_err(&sdata->dev->dev, > + "Device %8phC is already associated\n", &ceaddr); > + return -EPERM; > + } > + > + parent = kzalloc(sizeof(*parent), GFP_KERNEL); > + if (!parent) > + return -ENOMEM; > + > + parent->pan_id = coord->pan_id; > + parent->mode = coord->mode; > + if (parent->mode == IEEE802154_SHORT_ADDRESSING) { > + parent->short_addr = coord->short_addr; > + parent->extended_addr = cpu_to_le64(IEEE802154_ADDR_LONG_BROADCAST); There is no IEEE802154_ADDR_LONG_BROADCAST (extended address) address. The broadcast address is always a short address 0xffff. (Talkin about destination addresses). Just to clarify we can have here two different types/length of mac addresses being used, whereas the extended address is always present. We have the monitor interface set to an invalid extended address 0x0...0 (talking about source address) which is a reserved EUI64 (what long/extended address is) address, 0xffff...ffff is also being reserved. Monitors get their address from the socket interface. If there is a parent, an extended address is always present and known. A short address can be set, but is not required as a node to have. Sure if a node has a short address, you want to use a short address because it saves payload. Also remember when an address is unique in the network, an extended address (LONG) is always being unique, a short address is unique in combination of pan id + short address. If you save some neighbors you want to store the extended and if present panid/shortaddress. Or I do not understand something here? btw: as you probably noticed, the netdev interface dev_addr is an extended address (because it's always present). Now there comes the ugly part, netdevs cannot deal with other dev_addrs with different length, that's why it's stored in the wpan specific dev structure and things don't get easy and solutions need to be found how to make it working... get prepared to get crazy... - Alex
Hi, On Sat, Jun 3, 2023 at 6:09 AM Alexander Aring <aahringo@redhat.com> wrote: > > Hi, > > On Thu, Jun 1, 2023 at 11:50 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > > Joining a PAN officially goes by associating with a coordinator. This > > coordinator may have been discovered thanks to the beacons it sent in > > the past. Add support to the MAC layer for these associations, which > > require: > > - Sending an association request > > - Receiving an association response > > > > The association response contains the association status, eventually a > > reason if the association was unsuccessful, and finally a short address > > that we should use for intra-PAN communication from now on, if we > > required one (which is the default, and not yet configurable). > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > include/linux/ieee802154.h | 1 + > > include/net/cfg802154.h | 1 + > > include/net/ieee802154_netdev.h | 5 ++ > > net/ieee802154/core.c | 14 ++++ > > net/mac802154/cfg.c | 72 ++++++++++++++++++ > > net/mac802154/ieee802154_i.h | 19 +++++ > > net/mac802154/main.c | 2 + > > net/mac802154/rx.c | 9 +++ > > net/mac802154/scan.c | 127 ++++++++++++++++++++++++++++++++ > > 9 files changed, 250 insertions(+) > > > > diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h > > index 140f61ec0f5f..c72bd76cac1b 100644 > > --- a/include/linux/ieee802154.h > > +++ b/include/linux/ieee802154.h > > @@ -37,6 +37,7 @@ > > IEEE802154_FCS_LEN) > > > > #define IEEE802154_PAN_ID_BROADCAST 0xffff > > +#define IEEE802154_ADDR_LONG_BROADCAST 0xffffffffffffffffULL > > #define IEEE802154_ADDR_SHORT_BROADCAST 0xffff > > #define IEEE802154_ADDR_SHORT_UNSPEC 0xfffe > > > > diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h > > index 3b9d65455b9a..dd0964d351cd 100644 > > --- a/include/net/cfg802154.h > > +++ b/include/net/cfg802154.h > > @@ -502,6 +502,7 @@ struct wpan_dev { > > struct mutex association_lock; > > struct ieee802154_pan_device *parent; > > struct list_head children; > > + unsigned int association_generation; > > }; > > > > #define to_phy(_dev) container_of(_dev, struct wpan_phy, dev) > > diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h > > index ca8c827d0d7f..e26ffd079556 100644 > > --- a/include/net/ieee802154_netdev.h > > +++ b/include/net/ieee802154_netdev.h > > @@ -149,6 +149,11 @@ struct ieee802154_assoc_req_pl { > > #endif > > } __packed; > > > > +struct ieee802154_assoc_resp_pl { > > + __le16 short_addr; > > + u8 status; > > +} __packed; > > + > > enum ieee802154_frame_version { > > IEEE802154_2003_STD, > > IEEE802154_2006_STD, > > diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c > > index cd69bdbfd59f..8bf01bb7e858 100644 > > --- a/net/ieee802154/core.c > > +++ b/net/ieee802154/core.c > > @@ -198,6 +198,18 @@ void wpan_phy_free(struct wpan_phy *phy) > > } > > EXPORT_SYMBOL(wpan_phy_free); > > > > +static void cfg802154_free_peer_structures(struct wpan_dev *wpan_dev) > > +{ > > + mutex_lock(&wpan_dev->association_lock); > > + > > + if (wpan_dev->parent) > > + kfree(wpan_dev->parent); > > + > > + wpan_dev->association_generation++; > > + > > + mutex_unlock(&wpan_dev->association_lock); > > +} > > + > > int cfg802154_switch_netns(struct cfg802154_registered_device *rdev, > > struct net *net) > > { > > @@ -293,6 +305,8 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb, > > rdev->opencount++; > > break; > > case NETDEV_UNREGISTER: > > + cfg802154_free_peer_structures(wpan_dev); > > + > > I think the comment below is not relevant here, but I have also no > idea if this is still the case. > > > /* It is possible to get NETDEV_UNREGISTER > > * multiple times. To detect that, check > > * that the interface is still on the list > > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c > > index 5c3cb019f751..89112d2bcee7 100644 > > --- a/net/mac802154/cfg.c > > +++ b/net/mac802154/cfg.c > > @@ -315,6 +315,77 @@ static int mac802154_stop_beacons(struct wpan_phy *wpan_phy, > > return mac802154_stop_beacons_locked(local, sdata); > > } > > > > +static int mac802154_associate(struct wpan_phy *wpan_phy, > > + struct wpan_dev *wpan_dev, > > + struct ieee802154_addr *coord) > > +{ > > + struct ieee802154_local *local = wpan_phy_priv(wpan_phy); > > + u64 ceaddr = swab64((__force u64)coord->extended_addr); > > + struct ieee802154_sub_if_data *sdata; > > + struct ieee802154_pan_device *parent; > > + __le16 short_addr; > > + int ret; > > + > > + ASSERT_RTNL(); > > + > > + sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev); > > + > > + if (wpan_dev->parent) { > > + dev_err(&sdata->dev->dev, > > + "Device %8phC is already associated\n", &ceaddr); > > + return -EPERM; > > + } > > + > > + parent = kzalloc(sizeof(*parent), GFP_KERNEL); > > + if (!parent) > > + return -ENOMEM; > > + > > + parent->pan_id = coord->pan_id; > > + parent->mode = coord->mode; > > + if (parent->mode == IEEE802154_SHORT_ADDRESSING) { > > + parent->short_addr = coord->short_addr; > > + parent->extended_addr = cpu_to_le64(IEEE802154_ADDR_LONG_BROADCAST); > > There is no IEEE802154_ADDR_LONG_BROADCAST (extended address) address. > The broadcast address is always a short address 0xffff. (Talkin about > destination addresses). > > Just to clarify we can have here two different types/length of mac > addresses being used, whereas the extended address is always present. > We have the monitor interface set to an invalid extended address > 0x0...0 (talking about source address) which is a reserved EUI64 (what > long/extended address is) address, 0xffff...ffff is also being > reserved. Monitors get their address from the socket interface. > > If there is a parent, an extended address is always present and known. I want to weaken this, we can also have only the short address of the neighbor. But it depends on assoc/deassoc, I would think the extended address should be known. If you look on air and make per neighbor stats... you can see a neighbor with either a short or extended address being used. Map them to one neighbor if using a short address is only possible if you know the mapping... (but this is so far I see not the case here). We need some kind of policy here, but with assoc/deassoc we should always know this mapping. - Alex
Hi Simon, simon.horman@corigine.com wrote on Fri, 2 Jun 2023 17:54:40 +0200: > On Thu, Jun 01, 2023 at 05:48:10PM +0200, Miquel Raynal wrote: > > Joining a PAN officially goes by associating with a coordinator. This > > coordinator may have been discovered thanks to the beacons it sent in > > the past. Add support to the MAC layer for these associations, which > > require: > > - Sending an association request > > - Receiving an association response > > > > The association response contains the association status, eventually a > > reason if the association was unsuccessful, and finally a short address > > that we should use for intra-PAN communication from now on, if we > > required one (which is the default, and not yet configurable). > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > ... > > > diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c > > index cd69bdbfd59f..8bf01bb7e858 100644 > > --- a/net/ieee802154/core.c > > +++ b/net/ieee802154/core.c > > @@ -198,6 +198,18 @@ void wpan_phy_free(struct wpan_phy *phy) > > } > > EXPORT_SYMBOL(wpan_phy_free); > > > > +static void cfg802154_free_peer_structures(struct wpan_dev *wpan_dev) > > +{ > > + mutex_lock(&wpan_dev->association_lock); > > + > > + if (wpan_dev->parent) > > + kfree(wpan_dev->parent); > > Hi Miquel, > > a minor nit from my side: There no need to check for NULL before calling > kfree, because kfree will do nothing with a NULL argument. Sorry for the delay, yes of course, I will drop the extra check. Thanks, Miquèl
Hi Alexander, > > @@ -293,6 +305,8 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb, > > rdev->opencount++; > > break; > > case NETDEV_UNREGISTER: > > + cfg802154_free_peer_structures(wpan_dev); > > + > > I think the comment below is not relevant here, but I have also no > idea if this is still the case. Yeah, I did not bother with it as it was off topic. I believe the call above would anyway not be affected. > > /* It is possible to get NETDEV_UNREGISTER > > * multiple times. To detect that, check > > * that the interface is still on the list > > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c > > index 5c3cb019f751..89112d2bcee7 100644 > > --- a/net/mac802154/cfg.c > > +++ b/net/mac802154/cfg.c > > @@ -315,6 +315,77 @@ static int mac802154_stop_beacons(struct wpan_phy *wpan_phy, > > return mac802154_stop_beacons_locked(local, sdata); > > } > > > > +static int mac802154_associate(struct wpan_phy *wpan_phy, > > + struct wpan_dev *wpan_dev, > > + struct ieee802154_addr *coord) > > +{ > > + struct ieee802154_local *local = wpan_phy_priv(wpan_phy); > > + u64 ceaddr = swab64((__force u64)coord->extended_addr); > > + struct ieee802154_sub_if_data *sdata; > > + struct ieee802154_pan_device *parent; > > + __le16 short_addr; > > + int ret; > > + > > + ASSERT_RTNL(); > > + > > + sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev); > > + > > + if (wpan_dev->parent) { > > + dev_err(&sdata->dev->dev, > > + "Device %8phC is already associated\n", &ceaddr); > > + return -EPERM; > > + } > > + > > + parent = kzalloc(sizeof(*parent), GFP_KERNEL); > > + if (!parent) > > + return -ENOMEM; > > + > > + parent->pan_id = coord->pan_id; > > + parent->mode = coord->mode; > > + if (parent->mode == IEEE802154_SHORT_ADDRESSING) { > > + parent->short_addr = coord->short_addr; > > + parent->extended_addr = cpu_to_le64(IEEE802154_ADDR_LONG_BROADCAST); > > There is no IEEE802154_ADDR_LONG_BROADCAST (extended address) address. > The broadcast address is always a short address 0xffff. (Talkin about > destination addresses). You're totally right, this is misleading. I will just drop this definition and its use, see below. > Just to clarify we can have here two different types/length of mac > addresses being used, whereas the extended address is always present. > We have the monitor interface set to an invalid extended address > 0x0...0 (talking about source address) which is a reserved EUI64 (what > long/extended address is) address, 0xffff...ffff is also being > reserved. Monitors get their address from the socket interface. > > If there is a parent, an extended address is always present and known. > A short address can be set, but is not required as a node to have. > Sure if a node has a short address, you want to use a short address > because it saves payload. > Also remember when an address is unique in the network, an extended > address (LONG) is always being unique, a short address is unique in > combination of pan id + short address. Absolutely. > If you save some neighbors you want to store the extended and if > present panid/shortaddress. The code above was misleading, I will clarify it to only accept NL association commands with an extended address. In fact the coord structure is created in the nl802154 layer and it already only accepts extended addresses (we don't expect the short address through the nl command) so to answer your request: yes, the extended address will always be there because we must know it. Regarding the possibility to provide a short address, as we no longer keep details about the surrounding devices inside the kernel (we only keep the associated devices, either the parent or the children) keeping this information would be useless. I don't see a need for it. The NL interface can evolve later without breaking the compatibility if we ever want to provide a short address as well and make something with it (actually even today one could send the short address, it would simply be ingored). One thing however regarding the fact that we might want to store both a short *and* an extended address in some cases: I created a "PAN device" structure which allows this (because otherwise we had a union) as we need to do it when we are parent as we are in charge of allocating the short addresses: +/** + * struct ieee802154_pan_device - PAN device information + * @pan_id: the PAN ID of this device + * @mode: the preferred mode to reach the device + * @short_addr: the short address of this device + * @extended_addr: the extended address of this device + * @node: the list node + */ +struct ieee802154_pan_device { + __le16 pan_id; + u8 mode; + __le16 short_addr; + __le64 extended_addr; + struct list_head node; +}; > Or I do not understand something here? No no, I think we are fully aligned :-) > btw: as you probably noticed, the netdev interface dev_addr is an > extended address (because it's always present). Now there comes the > ugly part, netdevs cannot deal with other dev_addrs with different > length, that's why it's stored in the wpan specific dev structure and > things don't get easy and solutions need to be found how to make it > working... get prepared to get crazy... Yeah but I believe once we have proper PAN IDs/short address vs extended address mappings we could have the translation internally at least. I haven't investigated this for now. Thanks, Miquèl
Hi Alexander, > > @@ -293,6 +305,8 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb, > > rdev->opencount++; > > break; > > case NETDEV_UNREGISTER: > > + cfg802154_free_peer_structures(wpan_dev); > > + > > I think the comment below is not relevant here, but I have also no > idea if this is still the case. Yeah, I did not bother with it as it was off topic. I believe the call above would anyway not be affected. > > /* It is possible to get NETDEV_UNREGISTER > > * multiple times. To detect that, check > > * that the interface is still on the list > > diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c > > index 5c3cb019f751..89112d2bcee7 100644 > > --- a/net/mac802154/cfg.c > > +++ b/net/mac802154/cfg.c > > @@ -315,6 +315,77 @@ static int mac802154_stop_beacons(struct wpan_phy *wpan_phy, > > return mac802154_stop_beacons_locked(local, sdata); > > } > > > > +static int mac802154_associate(struct wpan_phy *wpan_phy, > > + struct wpan_dev *wpan_dev, > > + struct ieee802154_addr *coord) > > +{ > > + struct ieee802154_local *local = wpan_phy_priv(wpan_phy); > > + u64 ceaddr = swab64((__force u64)coord->extended_addr); > > + struct ieee802154_sub_if_data *sdata; > > + struct ieee802154_pan_device *parent; > > + __le16 short_addr; > > + int ret; > > + > > + ASSERT_RTNL(); > > + > > + sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev); > > + > > + if (wpan_dev->parent) { > > + dev_err(&sdata->dev->dev, > > + "Device %8phC is already associated\n", &ceaddr); > > + return -EPERM; > > + } > > + > > + parent = kzalloc(sizeof(*parent), GFP_KERNEL); > > + if (!parent) > > + return -ENOMEM; > > + > > + parent->pan_id = coord->pan_id; > > + parent->mode = coord->mode; > > + if (parent->mode == IEEE802154_SHORT_ADDRESSING) { > > + parent->short_addr = coord->short_addr; > > + parent->extended_addr = cpu_to_le64(IEEE802154_ADDR_LONG_BROADCAST); > > There is no IEEE802154_ADDR_LONG_BROADCAST (extended address) address. > The broadcast address is always a short address 0xffff. (Talkin about > destination addresses). You're totally right, this is misleading. I will just drop this definition and its use, see below. > Just to clarify we can have here two different types/length of mac > addresses being used, whereas the extended address is always present. > We have the monitor interface set to an invalid extended address > 0x0...0 (talking about source address) which is a reserved EUI64 (what > long/extended address is) address, 0xffff...ffff is also being > reserved. Monitors get their address from the socket interface. > > If there is a parent, an extended address is always present and known. > A short address can be set, but is not required as a node to have. > Sure if a node has a short address, you want to use a short address > because it saves payload. > Also remember when an address is unique in the network, an extended > address (LONG) is always being unique, a short address is unique in > combination of pan id + short address. Absolutely. > If you save some neighbors you want to store the extended and if > present panid/shortaddress. The code above was misleading, I will clarify it to only accept NL association commands with an extended address. In fact the coord structure is created in the nl802154 layer and it already only accepts extended addresses (we don't expect the short address through the nl command) so to answer your request: yes, the extended address will always be there because we must know it. Regarding the possibility to provide a short address, as we no longer keep details about the surrounding devices inside the kernel (we only keep the associated devices, either the parent or the children) keeping this information would be useless. I don't see a need for it. The NL interface can evolve later without breaking the compatibility if we ever want to provide a short address as well and make something with it (actually even today one could send the short address, it would simply be ingored). One thing however regarding the fact that we might want to store both a short *and* an extended address in some cases: I created a "PAN device" structure which allows this (because otherwise we had a union) as we need to do it when we are parent as we are in charge of allocating the short addresses: +/** + * struct ieee802154_pan_device - PAN device information + * @pan_id: the PAN ID of this device + * @mode: the preferred mode to reach the device + * @short_addr: the short address of this device + * @extended_addr: the extended address of this device + * @node: the list node + */ +struct ieee802154_pan_device { + __le16 pan_id; + u8 mode; + __le16 short_addr; + __le64 extended_addr; + struct list_head node; +}; > Or I do not understand something here? No no, I think we are fully aligned :-) > btw: as you probably noticed, the netdev interface dev_addr is an > extended address (because it's always present). Now there comes the > ugly part, netdevs cannot deal with other dev_addrs with different > length, that's why it's stored in the wpan specific dev structure and > things don't get easy and solutions need to be found how to make it > working... get prepared to get crazy... Yeah but I believe once we have proper PAN IDs/short address vs extended address mappings we could have the translation internally at least. I haven't investigated this for now. Thanks, Miquèl
diff --git a/include/linux/ieee802154.h b/include/linux/ieee802154.h index 140f61ec0f5f..c72bd76cac1b 100644 --- a/include/linux/ieee802154.h +++ b/include/linux/ieee802154.h @@ -37,6 +37,7 @@ IEEE802154_FCS_LEN) #define IEEE802154_PAN_ID_BROADCAST 0xffff +#define IEEE802154_ADDR_LONG_BROADCAST 0xffffffffffffffffULL #define IEEE802154_ADDR_SHORT_BROADCAST 0xffff #define IEEE802154_ADDR_SHORT_UNSPEC 0xfffe diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h index 3b9d65455b9a..dd0964d351cd 100644 --- a/include/net/cfg802154.h +++ b/include/net/cfg802154.h @@ -502,6 +502,7 @@ struct wpan_dev { struct mutex association_lock; struct ieee802154_pan_device *parent; struct list_head children; + unsigned int association_generation; }; #define to_phy(_dev) container_of(_dev, struct wpan_phy, dev) diff --git a/include/net/ieee802154_netdev.h b/include/net/ieee802154_netdev.h index ca8c827d0d7f..e26ffd079556 100644 --- a/include/net/ieee802154_netdev.h +++ b/include/net/ieee802154_netdev.h @@ -149,6 +149,11 @@ struct ieee802154_assoc_req_pl { #endif } __packed; +struct ieee802154_assoc_resp_pl { + __le16 short_addr; + u8 status; +} __packed; + enum ieee802154_frame_version { IEEE802154_2003_STD, IEEE802154_2006_STD, diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c index cd69bdbfd59f..8bf01bb7e858 100644 --- a/net/ieee802154/core.c +++ b/net/ieee802154/core.c @@ -198,6 +198,18 @@ void wpan_phy_free(struct wpan_phy *phy) } EXPORT_SYMBOL(wpan_phy_free); +static void cfg802154_free_peer_structures(struct wpan_dev *wpan_dev) +{ + mutex_lock(&wpan_dev->association_lock); + + if (wpan_dev->parent) + kfree(wpan_dev->parent); + + wpan_dev->association_generation++; + + mutex_unlock(&wpan_dev->association_lock); +} + int cfg802154_switch_netns(struct cfg802154_registered_device *rdev, struct net *net) { @@ -293,6 +305,8 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb, rdev->opencount++; break; case NETDEV_UNREGISTER: + cfg802154_free_peer_structures(wpan_dev); + /* It is possible to get NETDEV_UNREGISTER * multiple times. To detect that, check * that the interface is still on the list diff --git a/net/mac802154/cfg.c b/net/mac802154/cfg.c index 5c3cb019f751..89112d2bcee7 100644 --- a/net/mac802154/cfg.c +++ b/net/mac802154/cfg.c @@ -315,6 +315,77 @@ static int mac802154_stop_beacons(struct wpan_phy *wpan_phy, return mac802154_stop_beacons_locked(local, sdata); } +static int mac802154_associate(struct wpan_phy *wpan_phy, + struct wpan_dev *wpan_dev, + struct ieee802154_addr *coord) +{ + struct ieee802154_local *local = wpan_phy_priv(wpan_phy); + u64 ceaddr = swab64((__force u64)coord->extended_addr); + struct ieee802154_sub_if_data *sdata; + struct ieee802154_pan_device *parent; + __le16 short_addr; + int ret; + + ASSERT_RTNL(); + + sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev); + + if (wpan_dev->parent) { + dev_err(&sdata->dev->dev, + "Device %8phC is already associated\n", &ceaddr); + return -EPERM; + } + + parent = kzalloc(sizeof(*parent), GFP_KERNEL); + if (!parent) + return -ENOMEM; + + parent->pan_id = coord->pan_id; + parent->mode = coord->mode; + if (parent->mode == IEEE802154_SHORT_ADDRESSING) { + parent->short_addr = coord->short_addr; + parent->extended_addr = cpu_to_le64(IEEE802154_ADDR_LONG_BROADCAST); + } else { + parent->extended_addr = coord->extended_addr; + parent->short_addr = cpu_to_le16(IEEE802154_ADDR_SHORT_BROADCAST); + } + + /* Set the PAN ID hardware address filter beforehand to avoid dropping + * the association response with a destination PAN ID field set to the + * "new" PAN ID. + */ + if (local->hw.flags & IEEE802154_HW_AFILT) { + ret = drv_set_pan_id(local, coord->pan_id); + if (ret < 0) + goto free_parent; + } + + ret = mac802154_perform_association(sdata, parent, &short_addr); + if (ret) + goto reset_panid; + + if (local->hw.flags & IEEE802154_HW_AFILT) { + ret = drv_set_short_addr(local, short_addr); + if (ret < 0) + goto reset_panid; + } + + wpan_dev->pan_id = coord->pan_id; + wpan_dev->short_addr = short_addr; + wpan_dev->parent = parent; + wpan_dev->association_generation++; + + return 0; + +reset_panid: + if (local->hw.flags & IEEE802154_HW_AFILT) + drv_set_pan_id(local, IEEE802154_PAN_ID_BROADCAST); + +free_parent: + kfree(parent); + return ret; +} + #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL static void ieee802154_get_llsec_table(struct wpan_phy *wpan_phy, @@ -526,6 +597,7 @@ const struct cfg802154_ops mac802154_config_ops = { .abort_scan = mac802154_abort_scan, .send_beacons = mac802154_send_beacons, .stop_beacons = mac802154_stop_beacons, + .associate = mac802154_associate, #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL .get_llsec_table = ieee802154_get_llsec_table, .lock_llsec_table = ieee802154_lock_llsec_table, diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h index c347ec9ff8c9..fff67676b400 100644 --- a/net/mac802154/ieee802154_i.h +++ b/net/mac802154/ieee802154_i.h @@ -24,6 +24,7 @@ enum ieee802154_ongoing { IEEE802154_IS_SCANNING = BIT(0), IEEE802154_IS_BEACONING = BIT(1), + IEEE802154_IS_ASSOCIATING = BIT(2), }; /* mac802154 device private data */ @@ -74,6 +75,13 @@ struct ieee802154_local { struct list_head rx_mac_cmd_list; struct work_struct rx_mac_cmd_work; + /* Association */ + struct ieee802154_pan_device *assoc_dev; + struct completion assoc_done; + __le16 assoc_addr; + u8 assoc_status; + struct work_struct assoc_work; + bool started; bool suspended; unsigned long ongoing; @@ -296,6 +304,17 @@ static inline bool mac802154_is_beaconing(struct ieee802154_local *local) void mac802154_rx_mac_cmd_worker(struct work_struct *work); +int mac802154_perform_association(struct ieee802154_sub_if_data *sdata, + struct ieee802154_pan_device *coord, + __le16 *short_addr); +int mac802154_process_association_resp(struct ieee802154_sub_if_data *sdata, + struct sk_buff *skb); + +static inline bool mac802154_is_associating(struct ieee802154_local *local) +{ + return test_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing); +} + /* interface handling */ int ieee802154_iface_init(void); void ieee802154_iface_exit(void); diff --git a/net/mac802154/main.c b/net/mac802154/main.c index 357ece67432b..9ab7396668d2 100644 --- a/net/mac802154/main.c +++ b/net/mac802154/main.c @@ -103,6 +103,8 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) INIT_DELAYED_WORK(&local->beacon_work, mac802154_beacon_worker); INIT_WORK(&local->rx_mac_cmd_work, mac802154_rx_mac_cmd_worker); + init_completion(&local->assoc_done); + /* init supported flags with 802.15.4 default ranges */ phy->supported.max_minbe = 8; phy->supported.min_maxbe = 3; diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c index e2434b4fe514..d0e08613a36b 100644 --- a/net/mac802154/rx.c +++ b/net/mac802154/rx.c @@ -93,6 +93,15 @@ void mac802154_rx_mac_cmd_worker(struct work_struct *work) queue_delayed_work(local->mac_wq, &local->beacon_work, 0); break; + + case IEEE802154_CMD_ASSOCIATION_RESP: + dev_dbg(&mac_pkt->sdata->dev->dev, "processing ASSOC RESP\n"); + if (!mac802154_is_associating(local)) + break; + + mac802154_process_association_resp(mac_pkt->sdata, mac_pkt->skb); + break; + default: break; } diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c index d9658f2c4ae6..5dd50e1ce329 100644 --- a/net/mac802154/scan.c +++ b/net/mac802154/scan.c @@ -510,3 +510,130 @@ int mac802154_send_beacons_locked(struct ieee802154_sub_if_data *sdata, return 0; } + +int mac802154_perform_association(struct ieee802154_sub_if_data *sdata, + struct ieee802154_pan_device *coord, + __le16 *short_addr) +{ + u64 ceaddr = swab64((__force u64)coord->extended_addr); + struct ieee802154_association_req_frame frame = {}; + struct ieee802154_local *local = sdata->local; + struct wpan_dev *wpan_dev = &sdata->wpan_dev; + struct sk_buff *skb; + int ret; + + frame.mhr.fc.type = IEEE802154_FC_TYPE_MAC_CMD; + frame.mhr.fc.security_enabled = 0; + frame.mhr.fc.frame_pending = 0; + frame.mhr.fc.ack_request = 1; /* We always expect an ack here */ + frame.mhr.fc.intra_pan = 0; + frame.mhr.fc.dest_addr_mode = (coord->mode == IEEE802154_ADDR_LONG) ? + IEEE802154_EXTENDED_ADDRESSING : IEEE802154_SHORT_ADDRESSING; + frame.mhr.fc.version = IEEE802154_2003_STD; + frame.mhr.fc.source_addr_mode = IEEE802154_EXTENDED_ADDRESSING; + frame.mhr.source.mode = IEEE802154_ADDR_LONG; + frame.mhr.source.pan_id = cpu_to_le16(IEEE802154_PANID_BROADCAST); + frame.mhr.source.extended_addr = wpan_dev->extended_addr; + frame.mhr.dest.mode = coord->mode; + frame.mhr.dest.pan_id = coord->pan_id; + if (coord->mode == IEEE802154_ADDR_LONG) + frame.mhr.dest.extended_addr = coord->extended_addr; + else + frame.mhr.dest.short_addr = coord->short_addr; + frame.mhr.seq = atomic_inc_return(&wpan_dev->dsn) & 0xFF; + frame.mac_pl.cmd_id = IEEE802154_CMD_ASSOCIATION_REQ; + frame.assoc_req_pl.device_type = 1; + frame.assoc_req_pl.power_source = 1; + frame.assoc_req_pl.rx_on_when_idle = 1; + frame.assoc_req_pl.alloc_addr = 1; + + skb = alloc_skb(IEEE802154_MAC_CMD_SKB_SZ + sizeof(frame.assoc_req_pl), + GFP_KERNEL); + if (!skb) + return -ENOBUFS; + + skb->dev = sdata->dev; + + ret = ieee802154_mac_cmd_push(skb, &frame, &frame.assoc_req_pl, + sizeof(frame.assoc_req_pl)); + if (ret) { + kfree_skb(skb); + return ret; + } + + local->assoc_dev = coord; + reinit_completion(&local->assoc_done); + set_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing); + + ret = ieee802154_mlme_tx_one_locked(local, sdata, skb); + if (ret) { + if (ret > 0) + ret = (ret == IEEE802154_NO_ACK) ? -EREMOTEIO : -EIO; + dev_warn(&sdata->dev->dev, + "No ASSOC REQ ACK received from %8phC\n", &ceaddr); + goto clear_assoc; + } + + ret = wait_for_completion_killable_timeout(&local->assoc_done, 10 * HZ); + if (ret <= 0) { + dev_warn(&sdata->dev->dev, + "No ASSOC RESP received from %8phC\n", &ceaddr); + ret = -ETIMEDOUT; + goto clear_assoc; + } + + if (local->assoc_status != IEEE802154_ASSOCIATION_SUCCESSFUL) { + if (local->assoc_status == IEEE802154_PAN_AT_CAPACITY) + ret = -ERANGE; + else + ret = -EPERM; + + dev_warn(&sdata->dev->dev, + "Negative ASSOC RESP received from %8phC: %s\n", &ceaddr, + local->assoc_status == IEEE802154_PAN_AT_CAPACITY ? + "PAN at capacity" : "access denied"); + } + + ret = 0; + *short_addr = local->assoc_addr; + +clear_assoc: + clear_bit(IEEE802154_IS_ASSOCIATING, &local->ongoing); + local->assoc_dev = NULL; + + return ret; +} + +int mac802154_process_association_resp(struct ieee802154_sub_if_data *sdata, + struct sk_buff *skb) +{ + struct ieee802154_addr *src = &mac_cb(skb)->source; + struct ieee802154_addr *dest = &mac_cb(skb)->dest; + u64 deaddr = swab64((__force u64)dest->extended_addr); + struct ieee802154_local *local = sdata->local; + struct wpan_dev *wpan_dev = &sdata->wpan_dev; + struct ieee802154_assoc_resp_pl resp_pl = {}; + + if (skb->len != sizeof(resp_pl)) + return -EINVAL; + + if (unlikely(src->mode != IEEE802154_EXTENDED_ADDRESSING || + dest->mode != IEEE802154_EXTENDED_ADDRESSING)) + return -EINVAL; + + if (unlikely(dest->extended_addr != wpan_dev->extended_addr || + src->extended_addr != local->assoc_dev->extended_addr)) + return -ENODEV; + + memcpy(&resp_pl, skb->data, sizeof(resp_pl)); + local->assoc_addr = resp_pl.short_addr; + local->assoc_status = resp_pl.status; + + dev_dbg(&skb->dev->dev, + "ASSOC RESP 0x%x received from %8phC, getting short address %04x\n", + local->assoc_status, &deaddr, local->assoc_addr); + + complete(&local->assoc_done); + + return 0; +}
Joining a PAN officially goes by associating with a coordinator. This coordinator may have been discovered thanks to the beacons it sent in the past. Add support to the MAC layer for these associations, which require: - Sending an association request - Receiving an association response The association response contains the association status, eventually a reason if the association was unsuccessful, and finally a short address that we should use for intra-PAN communication from now on, if we required one (which is the default, and not yet configurable). Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- include/linux/ieee802154.h | 1 + include/net/cfg802154.h | 1 + include/net/ieee802154_netdev.h | 5 ++ net/ieee802154/core.c | 14 ++++ net/mac802154/cfg.c | 72 ++++++++++++++++++ net/mac802154/ieee802154_i.h | 19 +++++ net/mac802154/main.c | 2 + net/mac802154/rx.c | 9 +++ net/mac802154/scan.c | 127 ++++++++++++++++++++++++++++++++ 9 files changed, 250 insertions(+)