Re: [PATCH wpan-next] mac802154: Allow the creation of coordinator interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, Oct 18, 2022 at 2:36 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
>
> As a first strep in introducing proper PAN management and association,
> we need to be able to create coordinator interfaces which might act as
> coordinator or PAN coordinator.
>
> Hence, let's add the minimum support to allow the creation of these
> interfaces. This support will be improved later, in particular regarding
> the filtering.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
>  net/mac802154/iface.c | 14 ++++++++------
>  net/mac802154/main.c  |  2 ++
>  net/mac802154/rx.c    | 11 +++++++----
>  3 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index d9b50884d34e..682249f3369b 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -262,13 +262,13 @@ ieee802154_check_concurrent_iface(struct ieee802154_sub_if_data *sdata,
>                 if (nsdata != sdata && ieee802154_sdata_running(nsdata)) {
>                         int ret;
>
> -                       /* TODO currently we don't support multiple node types
> -                        * we need to run skb_clone at rx path. Check if there
> -                        * exist really an use case if we need to support
> -                        * multiple node types at the same time.
> +                       /* TODO currently we don't support multiple node/coord
> +                        * types we need to run skb_clone at rx path. Check if
> +                        * there exist really an use case if we need to support
> +                        * multiple node/coord types at the same time.
>                          */
> -                       if (wpan_dev->iftype == NL802154_IFTYPE_NODE &&
> -                           nsdata->wpan_dev.iftype == NL802154_IFTYPE_NODE)
> +                       if (wpan_dev->iftype != NL802154_IFTYPE_MONITOR &&
> +                           nsdata->wpan_dev.iftype != NL802154_IFTYPE_MONITOR)
>                                 return -EBUSY;
>
>                         /* check all phy mac sublayer settings are the same.
> @@ -565,6 +565,7 @@ ieee802154_setup_sdata(struct ieee802154_sub_if_data *sdata,
>         wpan_dev->short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
>
>         switch (type) {
> +       case NL802154_IFTYPE_COORD:
>         case NL802154_IFTYPE_NODE:
>                 ieee802154_be64_to_le64(&wpan_dev->extended_addr,
>                                         sdata->dev->dev_addr);
> @@ -624,6 +625,7 @@ ieee802154_if_add(struct ieee802154_local *local, const char *name,
>         ieee802154_le64_to_be64(ndev->perm_addr,
>                                 &local->hw.phy->perm_extended_addr);
>         switch (type) {
> +       case NL802154_IFTYPE_COORD:
>         case NL802154_IFTYPE_NODE:
>                 ndev->type = ARPHRD_IEEE802154;
>                 if (ieee802154_is_valid_extended_unicast_addr(extended_addr)) {
> diff --git a/net/mac802154/main.c b/net/mac802154/main.c
> index 40fab08df24b..d03ecb747afc 100644
> --- a/net/mac802154/main.c
> +++ b/net/mac802154/main.c
> @@ -219,6 +219,8 @@ int ieee802154_register_hw(struct ieee802154_hw *hw)
>
>         if (hw->flags & IEEE802154_HW_PROMISCUOUS)
>                 local->phy->supported.iftypes |= BIT(NL802154_IFTYPE_MONITOR);
> +       else
> +               local->phy->supported.iftypes &= ~BIT(NL802154_IFTYPE_COORD);
>

So this means if somebody in the driver sets iftype COORD is supported
but then IEEE802154_HW_PROMISCUOUS is not supported it will not
support COORD?

Why is IEEE802154_HW_PROMISCUOUS required for COORD iftype? I thought
IEEE802154_HW_PROMISCUOUS is required to do a scan?

>         rc = wpan_phy_register(local->phy);
>         if (rc < 0)
> diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
> index 2ae23a2f4a09..aca348d7834b 100644
> --- a/net/mac802154/rx.c
> +++ b/net/mac802154/rx.c
> @@ -208,6 +208,7 @@ __ieee802154_rx_handle_packet(struct ieee802154_local *local,
>         int ret;
>         struct ieee802154_sub_if_data *sdata;
>         struct ieee802154_hdr hdr;
> +       struct sk_buff *skb2;
>
>         ret = ieee802154_parse_frame_start(skb, &hdr);
>         if (ret) {
> @@ -217,7 +218,7 @@ __ieee802154_rx_handle_packet(struct ieee802154_local *local,
>         }
>
>         list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> -               if (sdata->wpan_dev.iftype != NL802154_IFTYPE_NODE)
> +               if (sdata->wpan_dev.iftype == NL802154_IFTYPE_MONITOR)
>                         continue;

I guess this will work but I would like to see no logic about if it's
not MONITOR it's NODE or COORD, because introducing others requires
updating those again... however I think it's fine. I would like to see
a different receive path for coord_rx() and node_rx(), but yea
currently I guess they are mostly the same... in future I think they
are required as PACKTE_HOST, etc. will be changed regarding pan
coordinator or just coordinator (so far I understood).

>
>                 if (!ieee802154_sdata_running(sdata))
> @@ -230,9 +231,11 @@ __ieee802154_rx_handle_packet(struct ieee802154_local *local,
>                     sdata->required_filtering == IEEE802154_FILTERING_4_FRAME_FIELDS)
>                         continue;
>
> -               ieee802154_subif_frame(sdata, skb, &hdr);
> -               skb = NULL;
> -               break;
> +               skb2 = skb_clone(skb, GFP_ATOMIC);
> +               if (skb2) {
> +                       skb2->dev = sdata->dev;
> +                       ieee802154_subif_frame(sdata, skb2, &hdr);
> +               }
>         }
>
>         kfree_skb(skb);

If we do the clone above this kfree_skb() should be move to
ieee802154_rx() right after __ieee802154_rx_handle_packet(). This
patch also changes that we deliver one skb to multiple interfaces if
there are more than one and I was not aware that we currently do that.
:/

- Alex




[Index of Archives]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux