[ofa-general] Re: [PATCH 03/10] dev.c changes.

[Krishna Kumar2]

Hi Patrick,

Patrick McHardy <kaber at trash.net> wrote on 07/20/2007 04:50:37 PM:

> > I have a TODO comment in net-sysfs.c which is to catch this case.
> >
>
> I noticed that. Still wondering why it is important at all though.

I saw another mail of yours on the marc list on this same topic (which
still hasn't come to me in the mail), so I will answer both :

> Is there any downside in using batching with smaller queue sizes?

I think there is, but as yet I don't have any data (and 16 is probably
higher
than reqd) to show it. If the queue size is very small (like 4), the extra
processing to maintain this list may take more cycles than the performance
gains for sending out few skbs, esp since most xmits will send out 1 skb
and
skb batching takes places less often (when tx lock fails or queue gets
full).

OTOH, there might be a gain to even send out 2 skbs, the problem is in
doing
the extra processing before xmit and not at the time of xmit.

Does this sound OK ? If so, I will add the code to implement the TODO for
tx_queue_len checking too.

> > Without going into GSO, it is wasting some 32 bytes on i386 since most
> > drivers don't export this API.
>
> 32 bytes? I count 16, - 4 for the pointer, so its 12 bytes of waste.
> If you'd use it for gso_skb it would come down to 8 bytes. struct
> net_device is a pig already, and there are better ways to reduce this
> than starting to allocating single members with a few bytes IMO.

Sorry, I wanted to say 12 bytes on 32 bit system but mixed it up and
said 32 bytes. So I guess static allocation is better then, and it will
also help in performance as memory access is not required (offsetof
should work).

> Yes, packets can be holding references to various stuff and
> these should be released on device down. As I said above I
> don't really like the allocation, but even if you want to
> keep it, just do the purging and dev_deactivate and keep the
> freeing in unregister_netdev (actually I guess it should be
> free_netdev to handle register_netdevice errors).

Right, that makes it clean to do (and avoid stale packets on down).
I will make both these changes now.

Thanks for these suggestions,

- KK