2013-02-18 11:47:22

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

Unicast frame with unknown forwarding information always trigger
the path discovery assuming destination is always located inside the
MBSS. This patch allows the forwarding to look for mesh gate if path
discovery inside the MBSS has failed.

Reported-by: Cedric Voncken <[email protected]>
Signed-off-by: Chun-Yeow Yeoh <[email protected]>
---
net/mac80211/tx.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5b9602b..dce3af3 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1844,9 +1844,18 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
}

if (!is_multicast_ether_addr(skb->data)) {
- mpath = mesh_path_lookup(sdata, skb->data);
- if (!mpath)
- mppath = mpp_path_lookup(sdata, skb->data);
+ struct sta_info *next_hop;
+
+ mpath = mesh_path_lookup(skb->data, sdata);
+ if (mpath)
+ next_hop = rcu_dereference(mpath->next_hop);
+
+ if (!mpath || (mpath && (!next_hop ||
+ !(mpath->flags & MESH_PATH_ACTIVE))))
+ mppath = mpp_path_lookup(skb->data, sdata);
+
+ if (mppath && mpath)
+ mesh_path_del(mpath->dst, mpath->sdata);
}

/*
--
1.7.0.4



2013-02-18 18:52:14

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

On Mon, Feb 18, 2013 at 3:16 AM, Chun-Yeow Yeoh <[email protected]> wrote:
> Unicast frame with unknown forwarding information always trigger
> the path discovery assuming destination is always located inside the
> MBSS. This patch allows the forwarding to look for mesh gate if path
> discovery inside the MBSS has failed.
>
> Reported-by: Cedric Voncken <[email protected]>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> net/mac80211/tx.c | 15 ++++++++++++---
> 1 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 5b9602b..dce3af3 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1844,9 +1844,18 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> }
>
> if (!is_multicast_ether_addr(skb->data)) {
> - mpath = mesh_path_lookup(sdata, skb->data);
> - if (!mpath)
> - mppath = mpp_path_lookup(sdata, skb->data);
> + struct sta_info *next_hop;
> +
> + mpath = mesh_path_lookup(skb->data, sdata);

The API is now mesh_path_lookup(sdata, skb->data) :)

> + if (mpath)
> + next_hop = rcu_dereference(mpath->next_hop);
> +
> + if (!mpath || (mpath && (!next_hop ||
> + !(mpath->flags & MESH_PATH_ACTIVE))))

The mpath could still be resolving?

> + mppath = mpp_path_lookup(skb->data, sdata);
> +
> + if (mppath && mpath)
> + mesh_path_del(mpath->dst, mpath->sdata);

In general the mpath / mppath path selection code is very similar to
the point of duplication. I think these are ripe for a refactoring,
where a single mpath could represent an intra-MBSS or mesh gate
proxied destination. This is obviously out of scope for your patch,
but hopefully some day we can make all this mpath code readable :)

--
Thomas

2013-02-18 19:02:00

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

On Mon, 2013-02-18 at 19:16 +0800, Chun-Yeow Yeoh wrote:
> Unicast frame with unknown forwarding information always trigger
> the path discovery assuming destination is always located inside the
> MBSS. This patch allows the forwarding to look for mesh gate if path
> discovery inside the MBSS has failed.
>
> Reported-by: Cedric Voncken <[email protected]>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> net/mac80211/tx.c | 15 ++++++++++++---
> 1 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 5b9602b..dce3af3 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1844,9 +1844,18 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> }
>
> if (!is_multicast_ether_addr(skb->data)) {
> - mpath = mesh_path_lookup(sdata, skb->data);
> - if (!mpath)
> - mppath = mpp_path_lookup(sdata, skb->data);
> + struct sta_info *next_hop;
> +
> + mpath = mesh_path_lookup(skb->data, sdata);

as Thomas said, please rebase on mac80211-next.

> + if (mpath)
> + next_hop = rcu_dereference(mpath->next_hop);
> +
> + if (!mpath || (mpath && (!next_hop ||
> + !(mpath->flags & MESH_PATH_ACTIVE))))

indentation here is also wrong.

johannes



2013-02-18 19:03:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

On Mon, 2013-02-18 at 19:16 +0800, Chun-Yeow Yeoh wrote:
> Unicast frame with unknown forwarding information always trigger
> the path discovery assuming destination is always located inside the
> MBSS. This patch allows the forwarding to look for mesh gate if path
> discovery inside the MBSS has failed.
>
> Reported-by: Cedric Voncken <[email protected]>
> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
> ---
> net/mac80211/tx.c | 15 ++++++++++++---
> 1 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 5b9602b..dce3af3 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1844,9 +1844,18 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
> }
>
> if (!is_multicast_ether_addr(skb->data)) {
> - mpath = mesh_path_lookup(sdata, skb->data);
> - if (!mpath)
> - mppath = mpp_path_lookup(sdata, skb->data);
> + struct sta_info *next_hop;
> +
> + mpath = mesh_path_lookup(skb->data, sdata);
> + if (mpath)
> + next_hop = rcu_dereference(mpath->next_hop);
> +
> + if (!mpath || (mpath && (!next_hop ||
> + !(mpath->flags & MESH_PATH_ACTIVE))))
> + mppath = mpp_path_lookup(skb->data, sdata);

Heck, even the logic is wrong. Ok actually just weird -- if (!mpath || !
next_hop || ...) would be totally sufficient.

> + if (mppath && mpath)
> + mesh_path_del(mpath->dst, mpath->sdata);

You really delete paths because packets are transmitted along them?

johannes


2013-02-19 01:17:12

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

>> + if (!mpath || (mpath && (!next_hop ||
>> + !(mpath->flags & MESH_PATH_ACTIVE))))
>> + mppath = mpp_path_lookup(skb->data, sdata);
>
> Heck, even the logic is wrong. Ok actually just weird -- if (!mpath || !
> next_hop || ...) would be totally sufficient.

We go to lookup for mppath if mpath is unavailable, or if mpath is
available but the next_hop is NULL or the path is inactive.

---
Chun-Yeow

2013-02-18 19:11:48

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

On Mon, Feb 18, 2013 at 11:03 AM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2013-02-18 at 19:16 +0800, Chun-Yeow Yeoh wrote:
>> Unicast frame with unknown forwarding information always trigger
>> the path discovery assuming destination is always located inside the
>> MBSS. This patch allows the forwarding to look for mesh gate if path
>> discovery inside the MBSS has failed.
>>
>> Reported-by: Cedric Voncken <[email protected]>
>> Signed-off-by: Chun-Yeow Yeoh <[email protected]>
>> ---
>> net/mac80211/tx.c | 15 ++++++++++++---
>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
>> index 5b9602b..dce3af3 100644
>> --- a/net/mac80211/tx.c
>> +++ b/net/mac80211/tx.c
>> @@ -1844,9 +1844,18 @@ netdev_tx_t ieee80211_subif_start_xmit(struct sk_buff *skb,
>> }
>>
>> if (!is_multicast_ether_addr(skb->data)) {
>> - mpath = mesh_path_lookup(sdata, skb->data);
>> - if (!mpath)
>> - mppath = mpp_path_lookup(sdata, skb->data);
>> + struct sta_info *next_hop;
>> +
>> + mpath = mesh_path_lookup(skb->data, sdata);
>> + if (mpath)
>> + next_hop = rcu_dereference(mpath->next_hop);
>> +
>> + if (!mpath || (mpath && (!next_hop ||
>> + !(mpath->flags & MESH_PATH_ACTIVE))))
>> + mppath = mpp_path_lookup(skb->data, sdata);
>
> Heck, even the logic is wrong. Ok actually just weird -- if (!mpath || !
> next_hop || ...) would be totally sufficient.
>
>> + if (mppath && mpath)
>> + mesh_path_del(mpath->dst, mpath->sdata);
>
> You really delete paths because packets are transmitted along them?

I think he's deleting the mpath if an mppath was found (mpath is not
valid), but this cleanup will happen in the housekeeping work anyway,
so it's not needed here.

--
Thomas

2013-02-19 00:50:07

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the problem of unicast forwarding from DS to DS in Mesh

> I think he's deleting the mpath if an mppath was found (mpath is not
> valid), but this cleanup will happen in the housekeeping work anyway,
> so it's not needed here.

Unfortunately, the housekeeping takes about 10 minutes to delete the
mpath. In that case, if the mesh STA moves in and out from DS to MBSS
or vice versa, the connectivity is loss for that duration.

---
Chun-Yeow