On Wed, 13 Jun 2018 00:34:28 +0200
Alexander Bluhm <[email protected]> wrote:
> 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?
IPv4 is used for that servers.
>> > + if (m->m_len + sizeof(struct tcphdr) >= MHLEN) {
>> is tweaked to
>> if (m->m_len + sizeof(struct tcphdr) > MHLEN) {
>
> Using > instead of >= is correct.
The condition was for MCLGET() not for keeping non-cluster.
> 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.
I also can't find the reason of previous calculation.
> 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);
Yes, it's possible and it seems better.
ok?
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 13 Jun 2018 04:18:25 -0000
@@ -191,6 +191,9 @@ tcp_template(struct tcpcb *tp)
struct mbuf *m;
struct tcphdr *th;
+ CTASSERT(sizeof(struct ip) + sizeof(struct tcphdr) <= MHLEN);
+ CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct tcphdr) <= MHLEN);
+
if ((m = tp->t_template) == 0) {
m = m_get(M_DONTWAIT, MT_HEADER);
if (m == NULL)
@@ -208,19 +211,6 @@ tcp_template(struct tcpcb *tp)
#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) {
- MCLGET(m, M_DONTWAIT);
- if ((m->m_flags & M_EXT) == 0) {
- m_free(m);
- return (0);
- }
- }
}
switch(tp->pf) {
