diff mbox

diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c

Message ID 4A815A3B.3080404@realsil.com.cn (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

David Woo Aug. 11, 2009, 11:47 a.m. UTC
Johannes Berg 写道:
> On Tue, 2009-08-11 at 18:22 +0800, David Woo wrote:
>> static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>>  				    struct ieee80211_mgmt *mgmt,
>> -				    u8 *preq_elem, u32 metric)
>> -{
>> +				    u8 *preq_elem, u32 metric) {
> 
> And other than adding this coding style mistake, what does this patch
> do?
> 
> johannes

It's my first time to submit the patch file, I just mean to submit the attched 
patch file.

[PATCH] mac80211: Fix preq frame process and peer link frame baselen.
  
This patch is just to fix rreq reply condition, and peer link confirm frame baselen.

Signed-off-by: David Woo <xinhua_wu@realsil.com.cn>
---

  
David

Comments

John W. Linville Aug. 11, 2009, 6:47 p.m. UTC | #1
David, thanks for the patch!  Unfortunately, I'm not really sure
what you are fixing.  Could you change your changelog to explain what
problem you are fixing?  Just telling me that you changed something
doesn't tell me what was wrong with it in the first place.

Also, please make sure you follow the suggestions here:

	http://linux.yyz.us/patch-format.html

I look forward to your next post...thanks!

John

On Tue, Aug 11, 2009 at 07:47:07PM +0800, david woo wrote:
> Johannes Berg 写道:
> > On Tue, 2009-08-11 at 18:22 +0800, David Woo wrote:
> >> static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
> >>  				    struct ieee80211_mgmt *mgmt,
> >> -				    u8 *preq_elem, u32 metric)
> >> -{
> >> +				    u8 *preq_elem, u32 metric) {
> > 
> > And other than adding this coding style mistake, what does this patch
> > do?
> > 
> > johannes
> 
> It's my first time to submit the patch file, I just mean to submit the attched 
> patch file.
> 
> [PATCH] mac80211: Fix preq frame process and peer link frame baselen.
>   
> This patch is just to fix rreq reply condition, and peer link confirm frame baselen.
> 
> Signed-off-by: David Woo <xinhua_wu@realsil.com.cn>
> ---
> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
> index e1a763e..c065854 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -397,7 +397,8 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
>  
>  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>  				    struct ieee80211_mgmt *mgmt,
> -				    u8 *preq_elem, u32 metric) {
> +				    u8 *preq_elem, u32 metric)
> +{
>  	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>  	struct mesh_path *mpath;
>  	u8 *dst_addr, *orig_addr;
> @@ -430,7 +431,7 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>  			if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
>  					DSN_LT(mpath->dsn, dst_dsn)) {
>  				mpath->dsn = dst_dsn;
> -				mpath->flags &= MESH_PATH_DSN_VALID;
> +				mpath->flags |= MESH_PATH_DSN_VALID;
>  			} else if ((!(dst_flags & MP_F_DO)) &&
>  					(mpath->flags & MESH_PATH_ACTIVE)) {
>  				reply = true;
> @@ -447,14 +448,15 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>  
>  	if (reply) {
>  		lifetime = PREQ_IE_LIFETIME(preq_elem);
> -		ttl = ifmsh->mshcfg.dot11MeshTTL;
> -		if (ttl != 0)
> +		ttl = PREQ_IE_TTL(preq_elem);
> +		if (ttl != 0) {
> +			ttl = ifmsh->mshcfg.dot11MeshTTL;
>  			mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
>  				cpu_to_le32(dst_dsn), 0, orig_addr,
>  				cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
>  				cpu_to_le32(lifetime), cpu_to_le32(metric),
>  				0, sdata);
> -		else
> +		} else
>  			ifmsh->mshstats.dropped_frames_ttl++;
>  	}
>  
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index cb14253..ffcbad7 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -409,7 +409,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
>  	baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
>  	if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
>  		baseaddr += 4;
> -		baselen -= 4;
> +		baselen += 4;
>  	}
>  	ieee802_11_parse_elems(baseaddr, len - baselen, &elems);
>  	if (!elems.peer_link) {
> 
>   
> David
> 

> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
> index e1a763e..c065854 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -397,7 +397,8 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
>  
>  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>  				    struct ieee80211_mgmt *mgmt,
> -				    u8 *preq_elem, u32 metric) {
> +				    u8 *preq_elem, u32 metric)
> +{
>  	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>  	struct mesh_path *mpath;
>  	u8 *dst_addr, *orig_addr;
> @@ -430,7 +431,7 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>  			if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
>  					DSN_LT(mpath->dsn, dst_dsn)) {
>  				mpath->dsn = dst_dsn;
> -				mpath->flags &= MESH_PATH_DSN_VALID;
> +				mpath->flags |= MESH_PATH_DSN_VALID;
>  			} else if ((!(dst_flags & MP_F_DO)) &&
>  					(mpath->flags & MESH_PATH_ACTIVE)) {
>  				reply = true;
> @@ -447,14 +448,15 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>  
>  	if (reply) {
>  		lifetime = PREQ_IE_LIFETIME(preq_elem);
> -		ttl = ifmsh->mshcfg.dot11MeshTTL;
> -		if (ttl != 0)
> +		ttl = PREQ_IE_TTL(preq_elem);
> +		if (ttl != 0) {
> +			ttl = ifmsh->mshcfg.dot11MeshTTL;
>  			mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
>  				cpu_to_le32(dst_dsn), 0, orig_addr,
>  				cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
>  				cpu_to_le32(lifetime), cpu_to_le32(metric),
>  				0, sdata);
> -		else
> +		} else
>  			ifmsh->mshstats.dropped_frames_ttl++;
>  	}
>  
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index cb14253..ffcbad7 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -409,7 +409,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
>  	baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
>  	if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
>  		baseaddr += 4;
> -		baselen -= 4;
> +		baselen += 4;
>  	}
>  	ieee802_11_parse_elems(baseaddr, len - baselen, &elems);
>  	if (!elems.peer_link) {
David Woo Aug. 12, 2009, 12:33 a.m. UTC | #2
_______________________________________
From: John W. Linville [linville@tuxdriver.com]
Sent: Wednesday, August 12, 2009 2:47 AM
To: 吴新华
Cc: Johannes Berg; javier@cozybit.com; linux-wireless@vger.kernel.org; andrey@cozybit.com; devel@lists.open80211s.org
Subject: Re: diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c

David, thanks for the patch!  Unfortunately, I'm not really sure
what you are fixing.  Could you change your changelog to explain what
problem you are fixing?  Just telling me that you changed something
doesn't tell me what was wrong with it in the first place.

Also, please make sure you follow the suggestions here:

        http://linux.yyz.us/patch-format.html

I look forward to your next post...thanks!

John

John,
thanks for your comments and suggestion.
With the mesh code reviewed, the following is just my opinion,
* When mesh RREQ frame received and its sequence number is greater than the previous      
   one or the previous installed one is invalid,  it seems that the mesh path flag could set to 
   be valid again after the sequence number updated.
* When try to ack the rreq with rrep, checking the original TTL value seems to be more
   proper.
* When peer link confirm frame received, the base address will step forward 4 bytes,  it
   makes more sense with the base length increased with 4 bytes.

David

On Tue, Aug 11, 2009 at 07:47:07PM +0800, david woo wrote:
> Johannes Berg 写道:
> > On Tue, 2009-08-11 at 18:22 +0800, David Woo wrote:
> >> static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
> >>                                struct ieee80211_mgmt *mgmt,
> >> -                              u8 *preq_elem, u32 metric)
> >> -{
> >> +                              u8 *preq_elem, u32 metric) {
> >
> > And other than adding this coding style mistake, what does this patch
> > do?
> >
> > johannes
>
> It's my first time to submit the patch file, I just mean to submit the attched
> patch file.
>
> [PATCH] mac80211: Fix preq frame process and peer link frame baselen.
>
> This patch is just to fix rreq reply condition, and peer link confirm frame baselen.
>
> Signed-off-by: David Woo <xinhua_wu@realsil.com.cn>
> ---
> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
> index e1a763e..c065854 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -397,7 +397,8 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
>
>  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>                                   struct ieee80211_mgmt *mgmt,
> -                                 u8 *preq_elem, u32 metric) {
> +                                 u8 *preq_elem, u32 metric)
> +{
>       struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>       struct mesh_path *mpath;
>       u8 *dst_addr, *orig_addr;
> @@ -430,7 +431,7 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>                       if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
>                                       DSN_LT(mpath->dsn, dst_dsn)) {
>                               mpath->dsn = dst_dsn;
> -                             mpath->flags &= MESH_PATH_DSN_VALID;
> +                             mpath->flags |= MESH_PATH_DSN_VALID;
>                       } else if ((!(dst_flags & MP_F_DO)) &&
>                                       (mpath->flags & MESH_PATH_ACTIVE)) {
>                               reply = true;
> @@ -447,14 +448,15 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>
>       if (reply) {
>               lifetime = PREQ_IE_LIFETIME(preq_elem);
> -             ttl = ifmsh->mshcfg.dot11MeshTTL;
> -             if (ttl != 0)
> +             ttl = PREQ_IE_TTL(preq_elem);
> +             if (ttl != 0) {
> +                     ttl = ifmsh->mshcfg.dot11MeshTTL;
>                       mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
>                               cpu_to_le32(dst_dsn), 0, orig_addr,
>                               cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
>                               cpu_to_le32(lifetime), cpu_to_le32(metric),
>                               0, sdata);
> -             else
> +             } else
>                       ifmsh->mshstats.dropped_frames_ttl++;
>       }
>
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index cb14253..ffcbad7 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -409,7 +409,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
>       baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
>       if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
>               baseaddr += 4;
> -             baselen -= 4;
> +             baselen += 4;
>       }
>       ieee802_11_parse_elems(baseaddr, len - baselen, &elems);
>       if (!elems.peer_link) {
>
>
> David
>

> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
> index e1a763e..c065854 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -397,7 +397,8 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
>
>  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>                                   struct ieee80211_mgmt *mgmt,
> -                                 u8 *preq_elem, u32 metric) {
> +                                 u8 *preq_elem, u32 metric)
> +{
>       struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>       struct mesh_path *mpath;
>       u8 *dst_addr, *orig_addr;
> @@ -430,7 +431,7 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>                       if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
>                                       DSN_LT(mpath->dsn, dst_dsn)) {
>                               mpath->dsn = dst_dsn;
> -                             mpath->flags &= MESH_PATH_DSN_VALID;
> +                             mpath->flags |= MESH_PATH_DSN_VALID;
>                       } else if ((!(dst_flags & MP_F_DO)) &&
>                                       (mpath->flags & MESH_PATH_ACTIVE)) {
>                               reply = true;
> @@ -447,14 +448,15 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>
>       if (reply) {
>               lifetime = PREQ_IE_LIFETIME(preq_elem);
> -             ttl = ifmsh->mshcfg.dot11MeshTTL;
> -             if (ttl != 0)
> +             ttl = PREQ_IE_TTL(preq_elem);
> +             if (ttl != 0) {
> +                     ttl = ifmsh->mshcfg.dot11MeshTTL;
>                       mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
>                               cpu_to_le32(dst_dsn), 0, orig_addr,
>                               cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
>                               cpu_to_le32(lifetime), cpu_to_le32(metric),
>                               0, sdata);
> -             else
> +             } else
>                       ifmsh->mshstats.dropped_frames_ttl++;
>       }
>
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index cb14253..ffcbad7 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -409,7 +409,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
>       baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
>       if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
>               baseaddr += 4;
> -             baselen -= 4;
> +             baselen += 4;
>       }
>       ieee802_11_parse_elems(baseaddr, len - baselen, &elems);
>       if (!elems.peer_link) {


--
John W. Linville                Someday the world will need a hero, and you
linville@tuxdriver.com                  might be all we have.  Be ready.


------Please consider the environment before printing this e-mail.
Javier Cardona Aug. 12, 2009, 1:42 a.m. UTC | #3
David,

Thanks for the patch.

2009/8/11 david woo <xinhua_wu@realsil.com.cn>:
> [PATCH] mac80211: Fix preq frame process and peer link frame baselen.
>
> This patch is just to fix rreq reply condition, and peer link confirm frame baselen.
>
> Signed-off-by: David Woo <xinhua_wu@realsil.com.cn>
> ---
> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
> index e1a763e..c065854 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -397,7 +397,8 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
>
>  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>                                    struct ieee80211_mgmt *mgmt,
> -                                   u8 *preq_elem, u32 metric) {
> +                                   u8 *preq_elem, u32 metric)
> +{

nak

>        struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>        struct mesh_path *mpath;
>        u8 *dst_addr, *orig_addr;
> @@ -430,7 +431,7 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>                        if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
>                                        DSN_LT(mpath->dsn, dst_dsn)) {
>                                mpath->dsn = dst_dsn;
> -                               mpath->flags &= MESH_PATH_DSN_VALID;
> +                               mpath->flags |= MESH_PATH_DSN_VALID;

This is a valid fix:  at this point we should set the "destination
sequence number" valid flag, not zero all flags.

>                        } else if ((!(dst_flags & MP_F_DO)) &&
>                                        (mpath->flags & MESH_PATH_ACTIVE)) {
>                                reply = true;
> @@ -447,14 +448,15 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>
>        if (reply) {
>                lifetime = PREQ_IE_LIFETIME(preq_elem);
> -               ttl = ifmsh->mshcfg.dot11MeshTTL;
> -               if (ttl != 0)
> +               ttl = PREQ_IE_TTL(preq_elem);
> +               if (ttl != 0) {
> +                       ttl = ifmsh->mshcfg.dot11MeshTTL;

Nak.  The reply to a preq should have the locally configured TTL and
not the TTL that was in the PREP.

>                        mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
>                                cpu_to_le32(dst_dsn), 0, orig_addr,
>                                cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
>                                cpu_to_le32(lifetime), cpu_to_le32(metric),
>                                0, sdata);
> -               else
> +               } else
>                        ifmsh->mshstats.dropped_frames_ttl++;
>        }
>
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index cb14253..ffcbad7 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -409,7 +409,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
>        baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
>        if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
>                baseaddr += 4;
> -               baselen -= 4;
> +               baselen += 4;

I'm confused with this.  Could you actually establish plinks after
changing this?

Can you resubmit the patch with only the MESH_PATH_DSN_VALID for now?

Thanks!

Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Woo Aug. 12, 2009, 6:39 a.m. UTC | #4
Javier,

Thanks for your comments.

Javier Cardona 写道:
> David,
> 
> Thanks for the patch.
> 
> 2009/8/11 david woo <xinhua_wu@realsil.com.cn>:
>> [PATCH] mac80211: Fix preq frame process and peer link frame baselen.
>>
>> This patch is just to fix rreq reply condition, and peer link confirm frame baselen.
>>
>> Signed-off-by: David Woo <xinhua_wu@realsil.com.cn>
>> ---
>> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
>> index e1a763e..c065854 100644
>> --- a/net/mac80211/mesh_hwmp.c
>> +++ b/net/mac80211/mesh_hwmp.c
>> @@ -397,7 +397,8 @@ static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
>>
>>  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>>                                    struct ieee80211_mgmt *mgmt,
>> -                                   u8 *preq_elem, u32 metric) {
>> +                                   u8 *preq_elem, u32 metric)
>> +{
> 
> nak
> 
>>        struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>>        struct mesh_path *mpath;
>>        u8 *dst_addr, *orig_addr;
>> @@ -430,7 +431,7 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>>                        if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
>>                                        DSN_LT(mpath->dsn, dst_dsn)) {
>>                                mpath->dsn = dst_dsn;
>> -                               mpath->flags &= MESH_PATH_DSN_VALID;
>> +                               mpath->flags |= MESH_PATH_DSN_VALID;
> 
> This is a valid fix:  at this point we should set the "destination
> sequence number" valid flag, not zero all flags.
> 
>>                        } else if ((!(dst_flags & MP_F_DO)) &&
>>                                        (mpath->flags & MESH_PATH_ACTIVE)) {
>>                                reply = true;
>> @@ -447,14 +448,15 @@ static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
>>
>>        if (reply) {
>>                lifetime = PREQ_IE_LIFETIME(preq_elem);
>> -               ttl = ifmsh->mshcfg.dot11MeshTTL;
>> -               if (ttl != 0)
>> +               ttl = PREQ_IE_TTL(preq_elem);
>> +               if (ttl != 0) {
>> +                       ttl = ifmsh->mshcfg.dot11MeshTTL;
> 
> Nak.  The reply to a preq should have the locally configured TTL and
> not the TTL that was in the PREP.
> 
  Yes, I agree with you, and this patch is just to check whether this PREQ is valid with 
  the ttl it contains instead of local TTL, and the local TTL has also been provided for 
  PREP further process.

>>                        mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
>>                                cpu_to_le32(dst_dsn), 0, orig_addr,
>>                                cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
>>                                cpu_to_le32(lifetime), cpu_to_le32(metric),
>>                                0, sdata);
>> -               else
>> +               } else
>>                        ifmsh->mshstats.dropped_frames_ttl++;
>>        }
>>
>> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
>> index cb14253..ffcbad7 100644
>> --- a/net/mac80211/mesh_plink.c
>> +++ b/net/mac80211/mesh_plink.c
>> @@ -409,7 +409,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
>>        baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
>>        if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
>>                baseaddr += 4;
>> -               baselen -= 4;
>> +               baselen += 4;
> 
> I'm confused with this.  Could you actually establish plinks after
> changing this?
  With the baseaddr steps with 4 bytes, while the baselen minus with 4 bytes, it's to avoid 
  following element parser in random with last 8 bytes.
  ===> ieee802_11_parse_elems(baseaddr, len - baselen, &elems);

 baseaddr               
  |<------------------->|

 baseaddr+4  
     |<--len - baselen->|<-4->|<-4->| (baselen -= 4)
     |<--len - baselen->|             (baselen += 4) 
           
> 
> Can you resubmit the patch with only the MESH_PATH_DSN_VALID for now?
> 
> Thanks!
> 
> Javier
> 
> 
> ------Please consider the environment before printing this e-mail. 
> 
> 

David
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Javier Cardona Aug. 12, 2009, 6:03 p.m. UTC | #5
> Yes, I agree with you, and this patch is just to check whether this PREQ is valid with
> the ttl it contains instead of local TTL, and the local TTL has also been provided for
> PREP further process.

I'm still unconvinced with that change.  The ttl check you refer to is done on
transmission: PREQs with ttl of zero are never transmitted.  See further down in
that same function:

	ttl = PREQ_IE_TTL(preq_elem);
	lifetime = PREQ_IE_LIFETIME(preq_elem);
	if (ttl <= 1) {
		/* supress frame */

Furthermore, with the change you propose, you open the possibility for PREPs to
be transmitted with ttl=0 (if ifmsh->mshcfg.dot11MeshTTL was set to zero).

Regarding your baselen explanation, I now understand it and agree to the
suggested change.  Your initial patch submission was reversed and I got confused
looking at two version of the same.

I've created a new series with the parts of your patch that I agree with.  They
follow this e-mail.

Thanks!

Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index e1a763e..c065854 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -397,7 +397,8 @@  static u32 hwmp_route_info_get(struct ieee80211_sub_if_data *sdata,
 
 static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
 				    struct ieee80211_mgmt *mgmt,
-				    u8 *preq_elem, u32 metric) {
+				    u8 *preq_elem, u32 metric)
+{
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	struct mesh_path *mpath;
 	u8 *dst_addr, *orig_addr;
@@ -430,7 +431,7 @@  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
 			if ((!(mpath->flags & MESH_PATH_DSN_VALID)) ||
 					DSN_LT(mpath->dsn, dst_dsn)) {
 				mpath->dsn = dst_dsn;
-				mpath->flags &= MESH_PATH_DSN_VALID;
+				mpath->flags |= MESH_PATH_DSN_VALID;
 			} else if ((!(dst_flags & MP_F_DO)) &&
 					(mpath->flags & MESH_PATH_ACTIVE)) {
 				reply = true;
@@ -447,14 +448,15 @@  static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
 
 	if (reply) {
 		lifetime = PREQ_IE_LIFETIME(preq_elem);
-		ttl = ifmsh->mshcfg.dot11MeshTTL;
-		if (ttl != 0)
+		ttl = PREQ_IE_TTL(preq_elem);
+		if (ttl != 0) {
+			ttl = ifmsh->mshcfg.dot11MeshTTL;
 			mesh_path_sel_frame_tx(MPATH_PREP, 0, dst_addr,
 				cpu_to_le32(dst_dsn), 0, orig_addr,
 				cpu_to_le32(orig_dsn), mgmt->sa, 0, ttl,
 				cpu_to_le32(lifetime), cpu_to_le32(metric),
 				0, sdata);
-		else
+		} else
 			ifmsh->mshstats.dropped_frames_ttl++;
 	}
 
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index cb14253..ffcbad7 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -409,7 +409,7 @@  void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
 	baselen = (u8 *) mgmt->u.action.u.plink_action.variable - (u8 *) mgmt;
 	if (mgmt->u.action.u.plink_action.action_code == PLINK_CONFIRM) {
 		baseaddr += 4;
-		baselen -= 4;
+		baselen += 4;
 	}
 	ieee802_11_parse_elems(baseaddr, len - baselen, &elems);
 	if (!elems.peer_link) {