On Tue, Jun 12, 2018 at 01:46:48PM +0900, YASUOKA Masahiko wrote:
> > I found this because I wonder why my company, IIJ's HTTP proxy servers
> > started using a lot of mbuf clusters after a certain version.

Are you using IPv4 or IPv6?

> > +           if (m->m_len + sizeof(struct tcphdr) >= MHLEN) {
> is tweaked to
>               if (m->m_len + sizeof(struct tcphdr) > MHLEN) {

Using > instead of >= is correct.

I don't understand why MAX_TCPOPTLEN and max_linkhdr was added to
this calculation.  The mbuf is just a chunk of memory that has to
contain the data of the ip and tcp header.  So removing all this
seems reasonable.

The MCLGET() cannot be called anymore.  Can we remove it and add a
compile time assert instead?

        CTASSERT(sizeof(struct ip) + sizeof(struct tcphdr) <= MHLEN);
        CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct tcphdr) <= MHLEN);

bluhm

> Index: sys/netinet/tcp_subr.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/tcp_subr.c,v
> retrieving revision 1.171
> diff -u -p -r1.171 tcp_subr.c
> --- sys/netinet/tcp_subr.c    8 May 2018 15:10:33 -0000       1.171
> +++ sys/netinet/tcp_subr.c    12 Jun 2018 04:43:18 -0000
> @@ -207,20 +207,15 @@ tcp_template(struct tcpcb *tp)
>                       break;
>  #endif /* INET6 */
>               }
> -             m->m_len += sizeof (struct tcphdr);
>  
> -             /*
> -              * The link header, network header, TCP header, and TCP options
> -              * all must fit in this mbuf. For now, assume the worst case of
> -              * TCP options size. Eventually, compute this from tp flags.
> -              */
> -             if (m->m_len + MAX_TCPOPTLEN + max_linkhdr >= MHLEN) {
> +             if (m->m_len + sizeof(struct tcphdr) > MHLEN) {
>                       MCLGET(m, M_DONTWAIT);
>                       if ((m->m_flags & M_EXT) == 0) {
>                               m_free(m);
>                               return (0);
>                       }
>               }
> +             m->m_len += sizeof(struct tcphdr);
>       }
>  
>       switch(tp->pf) {

Reply via email to