From: Alexander Aring <aahringo@redhat.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Alexander Aring <alex.aring@gmail.com>,
Stefan Schmidt <stefan@datenfreihafen.org>,
linux-wpan@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>,
Guilhem Imberton <guilhem.imberton@qorvo.com>,
Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Eric Dumazet <edumazet@google.com>,
netdev@vger.kernel.org
Subject: Re: [PATCH wpan-next 1/3] ieee802154: Advertize coordinators discovery
Date: Sun, 6 Nov 2022 21:01:35 -0500 [thread overview]
Message-ID: <CAK-6q+iSzRyDDiNusXiRWvUsS5dSS5bSzAtNjSLTt6kgaxtbHg@mail.gmail.com> (raw)
In-Reply-To: <20221102151915.1007815-2-miquel.raynal@bootlin.com>
Hi,
On Wed, Nov 2, 2022 at 11:20 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Let's introduce the basics for advertizing discovered PANs and
> coordinators, which is:
> - A new "scan" netlink message group.
> - A couple of netlink command/attribute.
> - The main netlink helper to send a netlink message with all the
> necessary information to forward the main information to the user.
>
> Two netlink attributes are proactively added to support future UWB
> complex channels, but are not actually used yet.
>
> Co-developed-by: David Girault <david.girault@qorvo.com>
> Signed-off-by: David Girault <david.girault@qorvo.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> include/net/cfg802154.h | 20 +++++++
> include/net/nl802154.h | 44 ++++++++++++++
> net/ieee802154/nl802154.c | 121 ++++++++++++++++++++++++++++++++++++++
> net/ieee802154/nl802154.h | 6 ++
> 4 files changed, 191 insertions(+)
>
> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> index e1481f9cf049..8d67d9ed438d 100644
> --- a/include/net/cfg802154.h
> +++ b/include/net/cfg802154.h
> @@ -260,6 +260,26 @@ struct ieee802154_addr {
> };
> };
>
> +/**
> + * struct ieee802154_coord_desc - Coordinator descriptor
> + * @coord: PAN ID and coordinator address
> + * @page: page this coordinator is using
> + * @channel: channel this coordinator is using
> + * @superframe_spec: SuperFrame specification as received
> + * @link_quality: link quality indicator at which the beacon was received
> + * @gts_permit: the coordinator accepts GTS requests
> + * @node: list item
> + */
> +struct ieee802154_coord_desc {
> + struct ieee802154_addr *addr;
Why is this a pointer?
> + u8 page;
> + u8 channel;
> + u16 superframe_spec;
> + u8 link_quality;
> + bool gts_permit;
> + struct list_head node;
> +};
> +
> struct ieee802154_llsec_key_id {
> u8 mode;
> u8 id;
> diff --git a/include/net/nl802154.h b/include/net/nl802154.h
> index 145acb8f2509..cfe462288695 100644
> --- a/include/net/nl802154.h
> +++ b/include/net/nl802154.h
> @@ -58,6 +58,9 @@ enum nl802154_commands {
>
> NL802154_CMD_SET_WPAN_PHY_NETNS,
>
> + NL802154_CMD_NEW_COORDINATOR,
> + NL802154_CMD_KNOWN_COORDINATOR,
> +
NEW is something we never saw before and KNOWN we already saw before?
I am not getting that when I just want to maintain a list in the user
space and keep them updated, but I think we had this discussion
already or? Currently they do the same thing, just the command is
different. The user can use it to filter NEW and KNOWN? Still I am not
getting it why there is not just a start ... event, event, event ....
end. and let the user decide if it knows that it's new or old from its
perspective.
> /* add new commands above here */
>
> #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> @@ -133,6 +136,8 @@ enum nl802154_attrs {
> NL802154_ATTR_PID,
> NL802154_ATTR_NETNS_FD,
>
> + NL802154_ATTR_COORDINATOR,
> +
> /* add attributes here, update the policy in nl802154.c */
>
> #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> @@ -218,6 +223,45 @@ enum nl802154_wpan_phy_capability_attr {
> NL802154_CAP_ATTR_MAX = __NL802154_CAP_ATTR_AFTER_LAST - 1
> };
>
> +/**
> + * enum nl802154_coord - Netlink attributes for a coord
> + *
> + * @__NL802154_COORD_INVALID: invalid
> + * @NL802154_COORD_PANID: PANID of the coordinator (2 bytes)
> + * @NL802154_COORD_ADDR: coordinator address, (8 bytes or 2 bytes)
> + * @NL802154_COORD_CHANNEL: channel number, related to @NL802154_COORD_PAGE (u8)
> + * @NL802154_COORD_PAGE: channel page, related to @NL802154_COORD_CHANNEL (u8)
> + * @NL802154_COORD_PREAMBLE_CODE: Preamble code used when the beacon was received,
> + * this is PHY dependent and optional (u8)
> + * @NL802154_COORD_MEAN_PRF: Mean PRF used when the beacon was received,
> + * this is PHY dependent and optional (u8)
> + * @NL802154_COORD_SUPERFRAME_SPEC: superframe specification of the PAN (u16)
> + * @NL802154_COORD_LINK_QUALITY: signal quality of beacon in unspecified units,
> + * scaled to 0..255 (u8)
> + * @NL802154_COORD_GTS_PERMIT: set to true if GTS is permitted on this PAN
> + * @NL802154_COORD_PAYLOAD_DATA: binary data containing the raw data from the
> + * frame payload, (only if beacon or probe response had data)
> + * @NL802154_COORD_PAD: attribute used for padding for 64-bit alignment
> + * @NL802154_COORD_MAX: highest coordinator attribute
> + */
> +enum nl802154_coord {
> + __NL802154_COORD_INVALID,
> + NL802154_COORD_PANID,
> + NL802154_COORD_ADDR,
> + NL802154_COORD_CHANNEL,
> + NL802154_COORD_PAGE,
> + NL802154_COORD_PREAMBLE_CODE,
Interesting, if you do a scan and discover pans and others answers I
would think you would see only pans on the same preamble. How is this
working?
> + NL802154_COORD_MEAN_PRF,
> + NL802154_COORD_SUPERFRAME_SPEC,
> + NL802154_COORD_LINK_QUALITY,
not against it to have it, it's fine. I just think it is not very
useful. A way to dump all LQI values with some timestamp and having
something in user space to collect stats and do some heuristic may be
better?
> + NL802154_COORD_GTS_PERMIT,
> + NL802154_COORD_PAYLOAD_DATA,
> + NL802154_COORD_PAD,
> +
> + /* keep last */
> + NL802154_COORD_MAX,
> +};
> +
> /**
> * enum nl802154_cca_modes - cca modes
> *
> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> index e0b072aecf0f..f6fb7a228747 100644
> --- a/net/ieee802154/nl802154.c
> +++ b/net/ieee802154/nl802154.c
> @@ -26,10 +26,12 @@ static struct genl_family nl802154_fam;
> /* multicast groups */
> enum nl802154_multicast_groups {
> NL802154_MCGRP_CONFIG,
> + NL802154_MCGRP_SCAN,
> };
>
> static const struct genl_multicast_group nl802154_mcgrps[] = {
> [NL802154_MCGRP_CONFIG] = { .name = "config", },
> + [NL802154_MCGRP_SCAN] = { .name = "scan", },
> };
>
> /* returns ERR_PTR values */
> @@ -216,6 +218,9 @@ static const struct nla_policy nl802154_policy[NL802154_ATTR_MAX+1] = {
>
> [NL802154_ATTR_PID] = { .type = NLA_U32 },
> [NL802154_ATTR_NETNS_FD] = { .type = NLA_U32 },
> +
> + [NL802154_ATTR_COORDINATOR] = { .type = NLA_NESTED },
> +
> #ifdef CONFIG_IEEE802154_NL802154_EXPERIMENTAL
> [NL802154_ATTR_SEC_ENABLED] = { .type = NLA_U8, },
> [NL802154_ATTR_SEC_OUT_LEVEL] = { .type = NLA_U32, },
> @@ -1281,6 +1286,122 @@ static int nl802154_wpan_phy_netns(struct sk_buff *skb, struct genl_info *info)
> return err;
> }
>
> +static int nl802154_prep_new_coord_msg(struct sk_buff *msg,
> + struct cfg802154_registered_device *rdev,
> + struct wpan_dev *wpan_dev,
> + u32 portid, u32 seq, int flags, u8 cmd,
> + struct ieee802154_coord_desc *desc)
> +{
> + struct nlattr *nla;
> + void *hdr;
> +
> + hdr = nl802154hdr_put(msg, portid, seq, flags, cmd);
> + if (!hdr)
> + return -ENOBUFS;
> +
> + if (nla_put_u32(msg, NL802154_ATTR_WPAN_PHY, rdev->wpan_phy_idx))
> + goto nla_put_failure;
> +
> + if (wpan_dev->netdev &&
> + nla_put_u32(msg, NL802154_ATTR_IFINDEX, wpan_dev->netdev->ifindex))
> + goto nla_put_failure;
> +
> + if (nla_put_u64_64bit(msg, NL802154_ATTR_WPAN_DEV,
> + wpan_dev_id(wpan_dev), NL802154_ATTR_PAD))
> + goto nla_put_failure;
> +
> + nla = nla_nest_start_noflag(msg, NL802154_ATTR_COORDINATOR);
> + if (!nla)
> + goto nla_put_failure;
> +
> + if (nla_put(msg, NL802154_COORD_PANID, IEEE802154_PAN_ID_LEN,
> + &desc->addr->pan_id))
> + goto nla_put_failure;
> +
> + if (desc->addr->mode == IEEE802154_ADDR_SHORT) {
> + if (nla_put(msg, NL802154_COORD_ADDR,
> + IEEE802154_SHORT_ADDR_LEN,
> + &desc->addr->short_addr))
> + goto nla_put_failure;
> + } else {
> + if (nla_put(msg, NL802154_COORD_ADDR,
> + IEEE802154_EXTENDED_ADDR_LEN,
> + &desc->addr->extended_addr))
> + goto nla_put_failure;
> + }
> +
> + if (nla_put_u8(msg, NL802154_COORD_CHANNEL, desc->channel))
> + goto nla_put_failure;
> +
> + if (nla_put_u8(msg, NL802154_COORD_PAGE, desc->page))
> + goto nla_put_failure;
> +
> + if (nla_put_u16(msg, NL802154_COORD_SUPERFRAME_SPEC,
> + desc->superframe_spec))
> + goto nla_put_failure;
> +
> + if (nla_put_u8(msg, NL802154_COORD_LINK_QUALITY, desc->link_quality))
> + goto nla_put_failure;
> +
> + if (desc->gts_permit && nla_put_flag(msg, NL802154_COORD_GTS_PERMIT))
> + goto nla_put_failure;
> +
> + /* TODO: NL802154_COORD_PAYLOAD_DATA if any */
> +
> + nla_nest_end(msg, nla);
> +
> + genlmsg_end(msg, hdr);
> +
> + return 0;
> +
> + nla_put_failure:
> + genlmsg_cancel(msg, hdr);
> +
> + return -EMSGSIZE;
> +}
> +
> +static int nl802154_advertise_coordinator(struct wpan_phy *wpan_phy,
> + struct wpan_dev *wpan_dev, u8 cmd,
> + struct ieee802154_coord_desc *desc)
> +{
> + struct cfg802154_registered_device *rdev = wpan_phy_to_rdev(wpan_phy);
> + struct sk_buff *msg;
> + int ret;
> +
> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> + if (!msg)
> + return -ENOMEM;
> +
> + ret = nl802154_prep_new_coord_msg(msg, rdev, wpan_dev, 0, 0, 0, cmd, desc);
> + if (ret < 0) {
> + nlmsg_free(msg);
> + return ret;
> + }
> +
> + return genlmsg_multicast_netns(&nl802154_fam, wpan_phy_net(wpan_phy),
> + msg, 0, NL802154_MCGRP_SCAN, GFP_ATOMIC);
> +}
ah, okay that answers my previous question... regarding the trace event.
- Alex
next prev parent reply other threads:[~2022-11-07 2:02 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-02 15:19 [PATCH wpan-next 0/3] IEEE 802.15.4 PAN discovery handling Miquel Raynal
2022-11-02 15:19 ` [PATCH wpan-next 1/3] ieee802154: Advertize coordinators discovery Miquel Raynal
2022-11-07 2:01 ` Alexander Aring [this message]
2022-11-18 22:04 ` Miquel Raynal
2022-11-21 0:57 ` Alexander Aring
2022-11-21 1:01 ` Alexander Aring
2022-11-21 9:05 ` Miquel Raynal
2022-11-21 23:54 ` Alexander Aring
2022-11-23 17:07 ` Miquel Raynal
2022-11-24 1:49 ` Alexander Aring
2022-11-02 15:19 ` [PATCH wpan-next 2/3] ieee802154: Handle " Miquel Raynal
2022-11-07 2:03 ` Alexander Aring
2022-11-07 8:43 ` Miquel Raynal
2022-11-02 15:19 ` [PATCH wpan-next 3/3] ieee802154: Trace the registration of new PANs Miquel Raynal
2022-11-07 1:36 ` Alexander Aring
2022-11-07 8:49 ` 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=CAK-6q+iSzRyDDiNusXiRWvUsS5dSS5bSzAtNjSLTt6kgaxtbHg@mail.gmail.com \
--to=aahringo@redhat.com \
--cc=alex.aring@gmail.com \
--cc=davem@davemloft.net \
--cc=david.girault@qorvo.com \
--cc=edumazet@google.com \
--cc=frederic.blain@qorvo.com \
--cc=guilhem.imberton@qorvo.com \
--cc=kuba@kernel.org \
--cc=linux-wpan@vger.kernel.org \
--cc=miquel.raynal@bootlin.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).