On Tue, Oct 25, 2016 at 00:22 +0200, Hrvoje Popovski wrote:
> On 24.10.2016. 23:36, Mike Belopuhov wrote:
> > On Mon, Oct 24, 2016 at 19:04 +0200, Hrvoje Popovski wrote:
> >> Hi all,
> >>
> >> OpenBSD box acts as transit router for /8 networks without pf and with
> >> this sysctls
> >>
> >> ddb.console=1
> >> kern.pool_debug=0
> >> net.inet.ip.forwarding=1
> >> net.inet.ip.ifq.maxlen=8192
> >>
> >> netstat
> >> 11/8 192.168.11.2 UGS 0 114466419 - 8
> >> ix0
> >> 12/8 192.168.12.2 UGS 0 0 - 8 ix1
> >> 13/8 192.168.13.2 UGS 0 0 - 8
> >> myx0
> >> 14/8 192.168.14.2 UGS 0 0 - 8
> >> myx1
> >> 15/8 192.168.15.2 UGS 0 0 - 8 em3
> >> 16/8 192.168.16.2 UGS 0 89907239 - 8 em2
> >> 17/8 192.168.17.2 UGS 0 65791508 - 8
> >> bge0
> >> 18/8 192.168.18.2 UGS 0 0 - 8
> >> bge1
> >>
> >> while testing dlg@ "mcl2k2 mbuf clusters" patch with todays -current i
> >> saw that performance with plain -current drops for about 300Kpps vs
> >> -current from 06.10.2016. by bisecting cvs tree it seems that this
> >> commit is guilty for this
> >>
> >> http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net/if_ethersubr.c?rev=1.240&content-type=text/x-cvsweb-markup
> >>
> >
> > I don't see how this change can affect performance in such a way
> > unless you're sending jumbo packets, but then the packet rates
> > are too high. Are you 100% sure it's this particular change?
> >
>
> No, no, i'm not 100% sure. I was doing this to try to find bottleneck:
>
> cvs -q checkout -D "2016-10-XX" -P src
>
> 2016-10-06 - 900kpps
> 2016-10-07 - 900kpps
> 2016-10-10 - 900kpps
> 2016-10-11 - 650kpps
> 2016-10-11 with if_ethersubr.c 1.239 - 900kpps
> ...
> 2016-10-14 - 650kpps
> 2016-10-14 with dlg@ patch - 900kpps
> 2016-10-14 with dlg@ patch and with if_ethersubr.c 1.239 - 880kpps
> ....
> 2016-10-24 - results are in mail ...
>
> and then i looked at networking diffs from 2016-10-10 and 2016-10-11 and
> it seems that if_ethersubr.c is guilty
>
> tests was done over ix only ...
>
> although as you can see with today's plain -current i'm getting 690kpps
> and with today's -current with if_ethersubr.c 1.239 i'm getting 910kpps
>
> so i thought that there must be something with if_ethersubr.c
>
I see. I will double check this tomorrow but your approach
looks solid.
> > What kind of traffic are you testing this with?
> > I assume small IP or UDP packets, correct?
> >
>
> yes, 64 byte UDP without flowcontrol..
>
> > Actually I'd like to know what causes this.
> >
> > So far I've noticed that the code generating ICMP error doesn't
> > reserve space for the link header but it's unlikely a culprit.
> > (The diff was only compile tested so far...)
> >
>
>
> with -current from few minutes ago and with this diff i'm getting panic
>
MH_ALIGN gets in the way... This should solve it, but needs to be
tested with large packets.
diff --git sys/netinet/ip_icmp.c sys/netinet/ip_icmp.c
index cdd60aa..5542f64 100644
--- sys/netinet/ip_icmp.c
+++ sys/netinet/ip_icmp.c
@@ -210,7 +210,8 @@ icmp_do_error(struct mbuf *n, int type, int code, u_int32_t
dest, int destmtu)
icmplen = MCLBYTES - ICMP_MINLEN - sizeof (struct ip);
m = m_gethdr(M_DONTWAIT, MT_HEADER);
- if (m && (sizeof (struct ip) + icmplen + ICMP_MINLEN > MHLEN)) {
+ if (m && (max_linkhdr + sizeof(struct ip) + icmplen +
+ ICMP_MINLEN > MHLEN)) {
MCLGET(m, M_DONTWAIT);
if ((m->m_flags & M_EXT) == 0) {
m_freem(m);
@@ -224,6 +225,8 @@ icmp_do_error(struct mbuf *n, int type, int code, u_int32_t
dest, int destmtu)
m->m_len = icmplen + ICMP_MINLEN;
if ((m->m_flags & M_EXT) == 0)
MH_ALIGN(m, m->m_len);
+ else
+ m->m_data += max_linkhdr;
icp = mtod(m, struct icmp *);
if ((u_int)type > ICMP_MAXTYPE)
panic("icmp_error");
