Re: [PATCH 3/4] mac80211: don't fragment packets when HW-fragmentation is on

[Luciano Coelho <luciano.coelho@nokia.com>]

On Thu, 2010-11-04 at 06:34 +0100, ext Arik Nemtsov wrote:
> On Thu, Nov 4, 2010 at 04:15, Johannes Berg <johannes@sipsolutions.net> wrote:
> >
> > On Wed, 2010-11-03 at 23:50 +0200, Arik Nemtsov wrote:
> > > If the driver supports hardware TX fragmentation, don't fragment
> > > packets in the stack.
> >
> > I'm not sure why you have three patches? Seems like it could all be a
> > single mac80211 patch.
> 
> Sure. I'll send a v2 with the mac80211 patches bundled into a single one.
> 
> >
> > > @@ -1181,8 +1183,10 @@ ieee80211_tx_prepare(struct ieee80211_sub_if_data *sdata,
> > >       /*
> > >        * Set this flag (used below to indicate "automatic fragmentation"),
> > >        * it will be cleared/left by radiotap as desired.
> > > +      * Only valid when fragmentation is done by the stack.
> > >        */
> > > -     tx->flags |= IEEE80211_TX_FRAGMENTED;
> > > +     if (!(local->hw.flags & IEEE80211_HW_TX_FRAGMENTATION))
> > > +             tx->flags |= IEEE80211_TX_FRAGMENTED;
> >
> > Do we really need the hw flag? Couldn't we go off the callback, like we
> > used to? I'm not really sure which one would perform better though, I
> > guess the flag might ...
> 
> I think the flag may be less confusing for new driver writers.
> Someone can implement the callback as a no-op and have a bug in his
> code as a result.

Well, if someone implements the fragmentation setting function as a
no-op, it is a bug in the driver and it should be fixed.  It's easy to
make that clear in the documentation.  "If the hardware doesn't support
fragmentation, set this function to NULL" or something.

That's how it's done already with other ops, such as hw_scan and so on.
I don't see a need to have another flag.

Performance-wise, if there's an impact it will be almost insignificant,
I believe.

-- 
Cheers,
Luca.