[ofa-general] Re: [PATCH 2/10 REV5] [core] Add skb_blist & support for batching

[Krishna Kumar2]

Hi Evgeniy,

Evgeniy Polyakov <johnpol at 2ka.mipt.ru> wrote on 09/14/2007 06:16:38 PM:

> > +   if (dev->features & NETIF_F_BATCH_SKBS) {
> > +      /* Driver supports batching skb */
> > +      dev->skb_blist = kmalloc(sizeof *dev->skb_blist, GFP_KERNEL);
> > +      if (dev->skb_blist)
> > +         skb_queue_head_init(dev->skb_blist);
> > +   }
> > +
>
> A nitpick is that you should use sizeof(struct ...) and I think it
> requires flag clearing in cae of failed initialization?

I thought it is better to use *var name in case the name of the structure
changes. Also, the flag is not cleared since I could try to enable batching
later, and it could succeed at that time. When skb_blist is allocated, then
batching is enabled otherwise it is disabled (while features flag just
indicates that driver supports batching).

Thanks,

- KK