Fragment and 11s inconsistency

[Monthadar Al Jaberi]

sorry for the noise, I found out that I had an error when reading out
the QoS control field, it is corrected now and I updated the comment.

I tested the code without fragmentation and it should work fine now.

On Wed, Feb 15, 2012 at 10:26 AM, Monthadar Al Jaberi
<monthadar at gmail.com> wrote:
> On Wed, Feb 15, 2012 at 9:26 AM, Monthadar Al Jaberi
> <monthadar at gmail.com> wrote:
>> On Wed, Feb 15, 2012 at 5:50 AM, Rui Paulo <rpaulo at freebsd.org> wrote:
>>> On 2012/02/14, at 14:14, Monthadar Al Jaberi wrote:
>>>
>>>> Hi,
>>>>
>>>> I cant verify this yet, but isn't there something wrong in current FreeBSD?
>>>>
>>>> lets say an 11s data frame need to be fragmented in ieee80211_encap:
>>>> if (addqos)
>>>>               hdrsize = sizeof(struct ieee80211_qosframe);
>>>> else
>>>>               hdrsize = sizeof(struct ieee80211_frame);
>>>> ...
>>>> if (vap->iv_opmode == IEEE80211_M_MBSS) {
>>>> ...
>>>>               if (!IEEE80211_IS_MULTICAST(eh.ether_dhost))
>>>>                       hdrsize += IEEE80211_ADDR_LEN;  /* unicast are 4-addr */
>>>>               meshhdrsize = sizeof(struct ieee80211_meshcntl);
>>>> }
>>>> ...
>>>> if (__predict_true((m->m_flags & M_FF) == 0)) {
>>>>               /*
>>>>                * Normal frame.
>>>>                */
>>>>               m = ieee80211_mbuf_adjust(vap, hdrspace + meshhdrsize, key, m);
>>>> }
>>>> M_PREPEND(m, hdrspace + meshhdrsize, M_DONTWAIT);
>>>> if (txfrag && !ieee80211_fragment(vap, m, hdrsize,
>>>>           key != NULL ? key->wk_cipher->ic_header : 0, vap->iv_fragthreshold))
>>>>               goto bad;
>>>>
>>>> This means we send meshcontrol only in first segment, because we never
>>>> add meshhdrsize to hdrsize...
>>>
>>> Yes, this is a bug. Thanks for finding it.
>>>
>>>> but in mesh_input
>>>> switch (type) {
>>>>       case IEEE80211_FC0_TYPE_DATA:
>>>> ...
>>>> meshdrlen = sizeof(struct ieee80211_meshcntl) +
>>>>                   (mc->mc_flags & 3) * IEEE80211_ADDR_LEN;
>>>>               hdrspace += meshdrlen;
>>>> ...
>>>>               /*
>>>>                * Potentially forward packet.  See table s36 (p140)
>>>>                * for the rules.  XXX tap fwd'd packets not for us?
>>>>                */
>>>>               if (dir == IEEE80211_FC1_DIR_FROMDS ||
>>>>                   !mesh_isucastforme(vap, wh, mc)) {
>>>>                       mesh_forward(vap, m, mc);
>>>> ...
>>>> if (!IEEE80211_IS_MULTICAST(wh->i_addr1)) {
>>>>                       m = ieee80211_defrag(ni, m, hdrspace);
>>>>                       if (m == NULL) {
>>>>
>>>> This seems wrong to me, how can we potentially forward before defragin
>>>> the frame? this will fail cause sub-frag have no mesh control as code
>>>> shows above.
>>>>
>>>> shouldnt the defrag code be moved above?
>>>
>>> Yes, another bug.
>>>
>>>> I am attaching a patch that both moves the defrag, validates that mesh
>>>> control is present and makes 11s data frames QoS and set Mesh control
>>>> bit present to 1 as the amendment specifies.
>>>
>>> There are some typos in your patch ("This is in constrast to Draf 4.0.").
>>>
>>> Can you please rewrite this code:
>>> +               *(uint16_t *)qos = *(uint16_t *)
>>> +                   (((IEEE80211_IS_MULTICAST(wh->i_addr1) &&
>>> +                   dir == IEEE80211_FC1_DIR_FROMDS)) ?
>>> +                   ((struct ieee80211_qosframe *)wh)->i_qos :
>>> +                   ((struct ieee80211_qosframe_addr4 *)wh)->i_qos);
>>>
>>> In ieee80211_encap(), why didn't you add meshhdrsize to ieee80211_fragment()?
>>
>> Because the mesh control is not part of the header, it is in the frame
>> body and should only be present in the first fragment of a frame. So I
>> think this is correct.
>>
>
> Attaching patch again; this time Mesh Control field is set to not
> present for frags 1+.
>
>>>
>>> Would be nice to test this  before committing.
>>
>> Yes, Adrian and Bernhard are looking into the fragment code between
>> net80211 and ath. It seems the code is broken.
>>
>> I am reattaching the patch thanks for your comments.
>>
>>>
>>> Thanks,
>>> --
>>> Rui Paulo
>>>
>>
>>
>>
>> --
>> Monthadar Al Jaberi
>
> br
>
> --
> Monthadar Al Jaberi



-- 
Monthadar Al Jaberi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-mesh-data-frames-to-be-quality-of-service-QoS.patch
Type: text/x-patch
Size: 7641 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-wireless/attachments/20120215/58045567/0001-Make-mesh-data-frames-to-be-quality-of-service-QoS.bin