Subject: Re: pppoe & mbuf chain
To: None <itojun@iijlab.net>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-net
Date: 06/22/2002 15:29:33
From: Jun-ichiro itojun Hagino <itojun@iijlab.net>
Subject: Re: pppoe & mbuf chain 
Date: Sat, 22 Jun 2002 14:40:42 +0900
> >> 	that have high possibility of failure (if m->m_pkthdr.len > MHLEN,
> >> 	m_pullup will fail), so i'd suggest rewriting pppoe code not to require
> >> 	"single mbuf" restriction.
> >ETHERTYPE_PPPOEDISC packets can be so large actually?
> 
> 	there are variable-length header present so i guess it safer not to
> 	assume maximum length.  in fact, if_pppoe.c do try to allocate cluster
> 	mbuf for outgoing PPPOEDISC packet.

i see.

> itojun@no test environment
> 
> Index: if_pppoe.c
> ===================================================================

i can test and debug, but...
please don't mix style changes and "real" changes in a diff. ;)

> +	m_copydata(m, 0, sizeof(eh), (caddr_t)&eh);
> +	m_adj(m, sizeof eh);

any point to copy and m_adj here while you're using m_pulldown?

>  static void
>  pppoe_disc_input(struct mbuf *m)
>  {
> -	u_int8_t *p;
> -	struct ether_header *eh;
>  
>  	/* avoid error messages if there is not a single pppoe instance */
>  	if (!LIST_EMPTY(&pppoe_softc_list)) {
> -		eh = mtod(m, struct ether_header*);
> -		m_adj(m, sizeof(struct ether_header));
> -		p = mtod(m, u_int8_t*);
>  		KASSERT(m->m_flags & M_PKTHDR);
> -		pppoe_dispatch_disc_pkt(p, m->m_len, m->m_pkthdr.rcvif, eh);
> +		pppoe_dispatch_disc_pkt(m, 0);
>  	}
> -	m_free(m);
>  }

you should free mbuf in the "no pppoe instance" case.

---
YAMAMOTO Takashi<yamt@mwd.biglobe.ne.jp>