From owner-freebsd-net Sun Oct 20 11:30:23 2002 Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DEB6637B401 for ; Sun, 20 Oct 2002 11:30:18 -0700 (PDT) Received: from InterJet.dellroad.org (adsl-63-194-81-26.dsl.snfc21.pacbell.net [63.194.81.26]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8D12F43E9C for ; Sun, 20 Oct 2002 11:30:17 -0700 (PDT) (envelope-from archie@dellroad.org) Received: from arch20m.dellroad.org (arch20m.dellroad.org [10.1.1.20]) by InterJet.dellroad.org (8.9.1a/8.9.1) with ESMTP id LAA30861; Sun, 20 Oct 2002 11:18:39 -0700 (PDT) Received: from arch20m.dellroad.org (localhost [127.0.0.1]) by arch20m.dellroad.org (8.12.6/8.12.6) with ESMTP id g9KIIdON048200; Sun, 20 Oct 2002 11:18:39 -0700 (PDT) (envelope-from archie@arch20m.dellroad.org) Received: (from archie@localhost) by arch20m.dellroad.org (8.12.6/8.12.6/Submit) id g9KIIdqF048199; Sun, 20 Oct 2002 11:18:39 -0700 (PDT) From: Archie Cobbs Message-Id: <200210201818.g9KIIdqF048199@arch20m.dellroad.org> Subject: Re: For those who had problems with MPD server and win clients In-Reply-To: <20021018132732.GA27692@otdel1.org> "from Nikolai Saoukh at Oct 18, 2002 05:27:32 pm" To: Nikolai Saoukh Date: Sun, 20 Oct 2002 11:18:39 -0700 (PDT) Cc: freebsd-net@FreeBSD.ORG X-Mailer: ELM [version 2.4ME+ PL88 (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org Nikolai Saoukh writes: > IMHO, there is a combination of two bugs. > > 1) MPD (3.9, at least) calculates and sets mtu (bund.c/BundUpdateParams()) > at wrong time -- when one of MIN() args is still zero. ioctl with bizzare mtu > value rejected, thus leaving the default (1500), which in turn is above > MRU requested from win client (1400 for multilink). See my comments below... > 2) MS clients has setting 'Negotiate multi-link for single link connections'. > Even when this option is negotiated, ms client does not understand partial MP > (RFC1990) fragments. When both (begin, end) flags set, then everything is > fine, as long as everything fits MRU. If this is true, then "set bundle enable round-robin" should work around the windows problem.. can you verify this? > --- bund.c.orig Tue Oct 8 13:40:15 2002 > +++ bund.c Fri Oct 18 17:17:23 2002 > @@ -548,22 +548,20 @@ > mtu = NG_IFACE_MTU_DEFAULT; /* Reset to default settings */ > break; > case 1: > - if (!Enabled(&bund->conf.options, BUND_CONF_MULTILINK)) { > - mtu = MIN(bund->links[the_link]->lcp.peer_mru, > - bund->links[the_link]->phys->type->mtu); > - break; > - } > - /* FALLTHROUGH */ > + mtu = bund->links[the_link]->lcp.peer_mru; > + break; > default: /* We fragment everything, use bundle MRRU */ > mtu = bund->mp.peer_mrru; > break; > } It doesn't seem possible to me that "bund->links[the_link]->phys->type->mtu" could ever be zero as you claim because this is a static variable initialized at compile time. However, this code is in fact broken. As sent in private email, I think the correct fix is this: --- bund.c 17 Oct 2002 18:48:47 -0000 1.8 +++ bund.c 20 Oct 2002 18:11:00 -0000 @@ -548,7 +548,7 @@ mtu = NG_IFACE_MTU_DEFAULT; /* Reset to default settings */ break; case 1: - if (!Enabled(&bund->conf.options, BUND_CONF_MULTILINK)) { + if (!bund->multilink) { /* If no multilink, use peer MRU */ mtu = MIN(bund->links[the_link]->lcp.peer_mru, bund->links[the_link]->phys->type->mtu); break; > - /* Subtract to make room for various frame-bloating protocols */ > - if (Enabled(&bund->conf.options, BUND_CONF_COMPRESSION)) > - mtu = CcpSubtractBloat(mtu); > - if (Enabled(&bund->conf.options, BUND_CONF_ENCRYPTION)) > - mtu = EcpSubtractBloat(mtu); > + if (bm->n_up > 0) { > + /* Subtract to make room for various frame-bloating protocols */ > + if (bund->ccp.xmit != NULL) > + mtu = CcpSubtractBloat(mtu); > + if (bund->ecp.xmit != NULL) > + mtu = EcpSubtractBloat(mtu); > + } Looks good, though I have a modified version that fixes another bug as well (we were subtracting from the MTU even if compression negotation failed). > --- iface.c.orig Tue Oct 8 14:28:09 2002 > +++ iface.c Sat Oct 12 11:54:36 2002 > @@ -346,6 +346,8 @@ > /* Sanity */ > assert(!iface->ip_up); > > + BundUpdateParams(); > + > /* Set addresses and bring interface up */ > snprintf(hisaddr, sizeof(hisaddr), "%s", inet_ntoa(iface->peer_addr)); > ExecCmd(LG_IFACE, "%s %s %s %s netmask 0xffffffff %slink0", Doesn't hurt :-) I've combined your and my changes into the combined patch below. Please apply & let me know if this fixes the problems you're seeing. Thanks, -Archie __________________________________________________________________________ Archie Cobbs * Packet Design * http://www.packetdesign.com Index: bund.c =================================================================== RCS file: /home/cvs/archie/mpd/src/bund.c,v retrieving revision 1.8 diff -u -r1.8 bund.c --- bund.c 17 Oct 2002 18:48:47 -0000 1.8 +++ bund.c 20 Oct 2002 18:17:28 -0000 @@ -548,7 +548,7 @@ mtu = NG_IFACE_MTU_DEFAULT; /* Reset to default settings */ break; case 1: - if (!Enabled(&bund->conf.options, BUND_CONF_MULTILINK)) { + if (!bund->multilink) { /* If no multilink, use peer MRU */ mtu = MIN(bund->links[the_link]->lcp.peer_mru, bund->links[the_link]->phys->type->mtu); break; @@ -560,10 +560,12 @@ } /* Subtract to make room for various frame-bloating protocols */ - if (Enabled(&bund->conf.options, BUND_CONF_COMPRESSION)) - mtu = CcpSubtractBloat(mtu); - if (Enabled(&bund->conf.options, BUND_CONF_ENCRYPTION)) - mtu = EcpSubtractBloat(mtu); + if (bm->n_up > 0) { + if (Enabled(&bund->conf.options, BUND_CONF_COMPRESSION)) + mtu = CcpSubtractBloat(mtu); + if (Enabled(&bund->conf.options, BUND_CONF_ENCRYPTION)) + mtu = EcpSubtractBloat(mtu); + } /* Update interface MTU */ if (mtu > BUND_MAX_MTU) Index: ccp.c =================================================================== RCS file: /home/cvs/archie/mpd/src/ccp.c,v retrieving revision 1.4 diff -u -r1.4 ccp.c --- ccp.c 17 Oct 2002 18:48:47 -0000 1.4 +++ ccp.c 20 Oct 2002 18:17:28 -0000 @@ -578,12 +578,14 @@ CcpState const ccp = &bund->ccp; /* Account for CCP's protocol number overhead */ - size -= CCP_OVERHEAD; + if (OPEN_STATE(ccp->fsm.state)) + size -= CCP_OVERHEAD; /* Account for transmit compression overhead */ - if (OPEN_STATE(ccp->fsm.state) && ccp->xmit && ccp->xmit->SubtractBloat) { + if (OPEN_STATE(ccp->fsm.state) && ccp->xmit && ccp->xmit->SubtractBloat) size = (*ccp->xmit->SubtractBloat)(size); - } + + /* Done */ return(size); } Index: ecp.c =================================================================== RCS file: /home/cvs/archie/mpd/src/ecp.c,v retrieving revision 1.4 diff -u -r1.4 ecp.c --- ecp.c 17 Oct 2002 18:48:48 -0000 1.4 +++ ecp.c 20 Oct 2002 18:17:28 -0000 @@ -583,12 +583,14 @@ EcpState const ecp = &bund->ecp; /* Account for ECP's protocol number overhead */ - size -= ECP_OVERHEAD; + if (OPEN_STATE(ecp->fsm.state)) + size -= ECP_OVERHEAD; /* Check transmit encryption */ - if (OPEN_STATE(ecp->fsm.state) && ecp->xmit && ecp->xmit->SubtractBloat) { + if (OPEN_STATE(ecp->fsm.state) && ecp->xmit && ecp->xmit->SubtractBloat) size = (*ecp->xmit->SubtractBloat)(size); - } + + /* Done */ return(size); } Index: iface.c =================================================================== RCS file: /home/cvs/archie/mpd/src/iface.c,v retrieving revision 1.4 diff -u -r1.4 iface.c --- iface.c 19 Oct 2002 18:14:26 -0000 1.4 +++ iface.c 20 Oct 2002 18:17:28 -0000 @@ -346,6 +346,9 @@ /* Sanity */ assert(!iface->ip_up); + /* For good measure */ + BundUpdateParams(); + /* Set addresses and bring interface up */ snprintf(hisaddr, sizeof(hisaddr), "%s", inet_ntoa(iface->peer_addr)); ExecCmd(LG_IFACE, "%s %s %s %s netmask 0xffffffff %slink0", To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message