Date: Sun, 20 Oct 2002 11:18:39 -0700 (PDT) From: Archie Cobbs <archie@dellroad.org> To: Nikolai Saoukh <bsd#nms@otdel-1.org> Cc: freebsd-net@FreeBSD.ORG Subject: Re: For those who had problems with MPD server and win clients Message-ID: <200210201818.g9KIIdqF048199@arch20m.dellroad.org> In-Reply-To: <20021018132732.GA27692@otdel1.org> "from Nikolai Saoukh at Oct 18, 2002 05:27:32 pm"
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200210201818.g9KIIdqF048199>