On Fri, 2010-10-08 at 11:25 -0700, Javier Cardona wrote:
> > But then you can't ever actually properly process a *matching*
> > PLINK_OPEN frame, afaict, because those definitely do have
> > "!sta && ftype == PLINK_OPEN"
>
> Not always: an sta can be created on reception of a beacon or probe
> response (mesh_neighbour_update()), so you could have ftype ==
> PLINK_OPEN and a non-null sta. Buy, yes, I agree that !sta && ftype ==
> PLINK_OPEN is a valid case.
Ah, indeed.
> > which is how the "if (!sta && ftype != PLINK_OPEN) return" came about,
> > afaict.
>
> Yes, and Christian's *second* patch (submitted as RFC on Sept 24) does
> not change that check. The matching PLINK_OPEN would be handled in
> the following branch:
>
> } else if (!sta) {
> /* ftype == PLINK_OPEN */
> u32 rates;
> (...)
> sta = mesh_plink_alloc(sdata, mgmt->sa, rates);
>
> > So I still don't think it's quite correct to fix it this way.
>
> Could it be that we are looking at different patches? Just to be
> sure, let me paste below the one we are referring to as sent by
> Christian:
Ahrg, yes, I always just went back to his first patch.
> ---
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index ea13a80..8b7efee 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -412,7 +412,7 @@ void mesh_rx_plink_frame(struct
> ieee80211_sub_if_data *sdata, struct ieee80211_m
> enum plink_event event;
> enum plink_frame_type ftype;
> size_t baselen;
> - bool deactivated;
> + bool deactivated, matches_local = true;
> u8 ie_len;
> u8 *baseaddr;
> __le16 plid, llid, reason;
> @@ -487,6 +487,7 @@ void mesh_rx_plink_frame(struct
> ieee80211_sub_if_data *sdata, struct ieee80211_m
> /* Now we will figure out the appropriate event... */
> event = PLINK_UNDEFINED;
> if (ftype != PLINK_CLOSE && (!mesh_matches_local(&elems, sdata))) {
> + matches_local = false;
> switch (ftype) {
> case PLINK_OPEN:
> event = OPN_RJCT;
> @@ -498,7 +499,15 @@ void mesh_rx_plink_frame(struct
> ieee80211_sub_if_data *sdata, struct ieee80211_m
> /* avoid warning */
> break;
> }
> - spin_lock_bh(&sta->lock);
> + }
> +
> + if (!sta && !matches_local) {
> + rcu_read_unlock();
> + reason = cpu_to_le16(MESH_CAPABILITY_POLICY_VIOLATION);
> + llid = 0;
> + mesh_plink_frame_tx(sdata, PLINK_CLOSE, mgmt->sa, llid,
> + plid, reason);
> + return;
> } else if (!sta) {
> /* ftype == PLINK_OPEN */
> u32 rates;
> @@ -522,7 +531,7 @@ void mesh_rx_plink_frame(struct
> ieee80211_sub_if_data *sdata, struct ieee80211_m
> }
> event = OPN_ACPT;
> spin_lock_bh(&sta->lock);
> - } else {
> + } else if (matches_local) {
> spin_lock_bh(&sta->lock);
> switch (ftype) {
> case PLINK_OPEN:
> @@ -564,6 +573,8 @@ void mesh_rx_plink_frame(struct
> ieee80211_sub_if_data *sdata, struct ieee80211_m
> rcu_read_unlock();
> return;
> }
> + } else {
> + spin_lock_bh(&sta->lock);
> }
Yeah, this looks good. Sorry!
johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
