Re: [PATCH 1/3] [LLC]: skb allocation size for responses

[Arnaldo Carvalho de Melo <acme@redhat.com>]

Em Sat, Mar 29, 2008 at 03:06:55PM +0900, Joonwoo Park escreveu:
> On Fri, Mar 28, 2008 at 10:12:02AM -0300, Arnaldo Carvalho de Melo wrote:
> > 
> > > +static int llc_mac_header_len(unsigned short devtype)
> > > +{
> > > +	switch (devtype) {
> > > +	case ARPHRD_ETHER:
> > > +	case ARPHRD_LOOPBACK:
> > > +		return sizeof(struct ethhdr);
> > > +	}
> > > +
> > > +	return __llc_tr_mac_header_len(devtype);
> > > +}
> > 
> > Nah, just drop __llc_tr_mac_header_len altogether and have another case
> > entry for ARPHDR_IEEE802_TR.
> > 
> > >  /**
> > >   *	llc_alloc_frame - allocates sk_buff for frame
> > >   *	@dev: network device this skb will be sent over
> > > + *	@type: pdu type to allocate
> > > + *	@data_size: data size to allocate
> > >   *
> > >   *	Allocates an sk_buff for frame and initializes sk_buff fields.
> > >   *	Returns allocated skb or %NULL when out of memory.
> > >   */
> > > -struct sk_buff *llc_alloc_frame(struct sock *sk, struct net_device *dev)
> > > +struct sk_buff *llc_alloc_frame(struct sock *sk, struct net_device *dev,
> > > +				u8 type, u32 data_size)
> > >  {
> > > -	struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC);
> > > +	int hlen = type == LLC_PDU_TYPE_U ? 3 : 4;
> > > +	struct sk_buff *skb;
> > > +
> > > +	hlen += llc_mac_header_len(dev->type);
> > > +	skb = alloc_skb(hlen + data_size, GFP_ATOMIC);
> > >  
> > >  	if (skb) {
> > >  		skb_reset_mac_header(skb);
> > > -		skb_reserve(skb, 50);
> > > +		skb_reserve(skb, hlen);
> > 
> > Well done, now it is better documented (LLC_PDU_TYPE_U passed) and uses
> > less memory, no more magic numbers, good.
> > 
> > - Arnaldo
> 
> 
> I've respined the patch, thanks Arnaldo.
> 
> ---
> [LLC]: skb allocation size for responses
> 
> Allocate the skb for llc responses with the received packet size by
> using the size adjustable llc_frame_alloc.
> Don't allocate useless extra payload.
> Cleanup magic numbers.
> 
> So, this fixes oops.
> Reported by Jim Westfall:
> kernel: skb_over_panic: text:c0541fc7 len:1000 put:997 head:c166ac00 data:c166ac2f tail:0xc166b017 end:0xc166ac80 dev:eth0
> kernel: ------------[ cut here ]------------
> kernel: kernel BUG at net/core/skbuff.c:95!

Perfect, thanks a lot:

Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>

- Arnaldo