Subject: Re: ieee80211_mbuf_adjust
To: Christos Zoulas <christos@astron.com>
From: David Young <dyoung@pobox.com>
List: tech-net
Date: 08/15/2005 22:18:13
On Tue, Aug 16, 2005 at 02:23:21AM +0000, Christos Zoulas wrote:
> In article <20050816021259.101BF2DA27@cvs.netbsd.org>,
> David Young  <dyoung@netbsd.org> wrote:
> >
> >Module Name:	src
> >Committed By:	dyoung
> >Date:		Tue Aug 16 02:12:59 UTC 2005
> >
> >Modified Files:
> >	src/sys/net80211: ieee80211_output.c
> >
> >Log Message:
> >Fix previous patch for non-crypto operation: test for a NULL key
> >before testing the key flags.
> >
> >XXX Problems remain.  Nick Hudson points out my questionable
> >XXX M_COPY_PKTHDR usage.  Also, it seems to me that we may not be
> >XXX protected against writing a read-only mbuf during the crypto
> >XXX encapsulation stage, even if hardware does the actual crypto.
> 
> There is also the questionably usage of m_pullup and M_PREPEND in
> the code. Yes, the code makes sure that there is adequate space
> so that neither M_PREPEND or m_pullup will need to allocate a new
> mbuf, but this is not guaranteed (their implementation might change).
> I think that we either should change the api to pass an mbuf ** so
> that changes are propagated, or add KASSERTS to the code.

Let's KASSERT for now.  API changes need to be coordinated with FreeBSD.

I have attached a patch to ieee80211_mbuf_adjust that makes sure the
802.11 header + opt(crypto header) + LLC are writable, regardless of
crypto state.  If s/w crypto is enabled, it makes the entire chain
writable, as before.

There is still that matter of the questionable (most likely broken)
M_MOVE_PKTHDR use.  Seems to me that we should deprecate M_COPY_PKTHDR,
as FreeBSD has done, introducing M_MOVE_PKTHDR and M_DUP_PKTHDR.

Thoughts?

Dave  

-- 
David Young             OJC Technologies
dyoung@ojctech.com      Urbana, IL * (217) 278-3933