Date: Tue, 14 Feb 2012 20:50:02 -0800 From: Rui Paulo <rpaulo@freebsd.org> To: Monthadar Al Jaberi <monthadar@gmail.com> Cc: freebsd-wireless@freebsd.org, Bernhard Schmidt <bschmidt@freebsd.org> Subject: Re: Fragment and 11s inconsistency Message-ID: <3F2E258F-1F82-48D7-AAAD-546BCB03EDE2@freebsd.org> In-Reply-To: <CA%2BsBSoJGzabfyt8EXoT-g=GTJD0upX5jbRqfLWKQKzXu2FivaA@mail.gmail.com> References: <CA%2BsBSoJGzabfyt8EXoT-g=GTJD0upX5jbRqfLWKQKzXu2FivaA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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()?
Would be nice to test this before committing.
Thanks,
--
Rui Paulo
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3F2E258F-1F82-48D7-AAAD-546BCB03EDE2>