Re: openbgpds not talking each other since 8.2-STABLE upgrade
On Jan 3, 2012, at 8:07 AM, Nikolay Denev wrote: > > On Jan 3, 2012, at 5:53 AM, Doug Barton wrote: > >> We have a pair of physical FreeBSD systems configured as routers >> designed to operate in an active/standby CARP configuration. Everything >> used to work fine, but since an upgrade to 8.2-STABLE on December 29th >> the two routers don't speak BGP to each other anymore. They both >> function fine individually, and failover works. It is only the openbgpd >> communication between them that's not flowing. Try compiling a kernel *without* TCP_MD5 and maybe without IPSEC support. I found out OpenBGPd has a problem. However, Quagga has been working for me flawlessly. Borja. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: how to debug non-working hole in nat
ignore. i sorted it. randy ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: how to debug non-working hole in nat
> add divert natd all from any to any via bridge0 This nat's all internal traffic on your lan. You probably don't want this. I'd place the nat on the tun0 interface. Which leads me to If you machine receives a syn from the tun0 interface, what firewall rule is in place to redirect the packet to the nat instance? I do not see any. ~Paul This message may contain confidential or privileged information. If you are not the intended recipient, please advise us immediately and delete this message. See http://www.datapipe.com/about-us-legal-email-disclaimer.htm for further information on confidentiality and the risks of non-secure electronic communication. If you cannot access these links, please notify us by reply message and we will send the contents to you. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Panic in the udp_input() under heavy load
On Fri, 30 Dec 2011, Maxim Sobolev wrote: On 12/30/2011 4:46 PM, Maxim Sobolev wrote: I see. Would you guys mind if I put that NULL pointer check into the code for the time being and turn it into some kind of big nasty warning in 8-stable branch only? I could also open a ticket, put all debug information collected to date in there. And encourage people to report to it once they see this warning on their system. Then it would provide more information about the exposure. It is definitely looks like locking issue somewhere, not just bad luck or flaky hardware, as we see it happening consistently on top 4 most UDP-loaded systems here, and it correlates well with the load. With my small NULL catch the machines have been running happily for a month now, so there is no visible side-effects. Please do file the PR so that all the information is in one place -- this is a network stack hacking week for me, so I should be able to take a closer look. Could you characterise the traffic load on these boxes a bit more? Also, is there regular monitoring using netstat/bsnmp/etc going on? I'd like to try and identify ways in which this workload differs from other common high-UDP workloads being used on 8.x that aren't seeing this problem... Robert ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On Tue, Jan 03, 2012 at 09:07:56AM +0200, Nikolay Denev wrote: > Since I've had similar problem with Quagga after updating to 8.2-STABLE I'd > suggest > you to try setting "net.inet.tcp.signature_verify_input=0" and see if that > would help. > > Here is another thread about the similar (if not the same) problem : > http://groups.google.com/group/mailing.freebsd.bugs/browse_thread/thread/ea347a919dbc165d/eeaa2965fc4f64c9?show_docid=eeaa2965fc4f64c9&pli=1 Thanks for the link Nikolay. Borja, I assume it's the PR submission form that gave you trouble - sorry for that. Based on your report it sounds to me like the bug is in OpenBGPd itself. If it works on OpenBSD with the TCP_MD5SIG option though I'd assume it's due to a difference in our (FreeBSD's) implementation of the option. Did you look at the OpenBSD/FreeBSD differences in your investigation? -Ed Another related query, where adding the TCP_SIGNATURE option reportedly fixed it: http://lists.freebsd.org/pipermail/freebsd-questions/2011-October/234503.html ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On Jan 3, 2012, at 4:29 PM, Ed Maste wrote: > On Tue, Jan 03, 2012 at 09:07:56AM +0200, Nikolay Denev wrote: > >> Since I've had similar problem with Quagga after updating to 8.2-STABLE I'd >> suggest >> you to try setting "net.inet.tcp.signature_verify_input=0" and see if that >> would help. >> >> Here is another thread about the similar (if not the same) problem : >> http://groups.google.com/group/mailing.freebsd.bugs/browse_thread/thread/ea347a919dbc165d/eeaa2965fc4f64c9?show_docid=eeaa2965fc4f64c9&pli=1 > > Thanks for the link Nikolay. > > Borja, I assume it's the PR submission form that gave you trouble - > sorry for that. Based on your report it sounds to me like the bug is Yes, I was filling a very detailed report and it was lost. Quite annoying... > in OpenBGPd itself. If it works on OpenBSD with the TCP_MD5SIG option > though I'd assume it's due to a difference in our (FreeBSD's) > implementation of the option. Did you look at the OpenBSD/FreeBSD > differences in your investigation? What I saw is that OpenBGPd *sets* the TCP_MD5SIG for the socket just to test if TCP_MD5 is available. Maybe in OpenBSD the behavior is different and unless you set the key TCP_MD5 won't be really enabled. In FreeBSD, the keys are set outside of your program using setkey(8). The second link you mention is simply a configuration problem. Unless you include TCP_SIGNATURE in the kernel config file, TCP_MD5 support will not be available. What I observed is this: if you have TCP_MD5 correctly available in the system, I mean, TCP_SIGNATURE and the rest of the IPSEC support correctly configured into the kernel, OpenBGPd is unable to work *without* TCP_MD5. Quagga works, I can use TCP_MD5 or not, my choice. Borja. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Use of spinlocks for TCP callouts
On Monday, January 02, 2012 11:35:31 pm Vijay Singh wrote: > I have see the following call sequence in profiles: > > > called/total parents > index %timeself descendents called+selfname index > called/total children > > 0.021.14 3822699/7559737 tcp_do_segment > [154]0.50.032.26 7559737callout_reset_on > 2.190.02 7573352/94883048 spinlock_exit > 0.010.04 7573352/11975031 callout_lock > > It is my perception that spinlocks are expensive. Since general TCP > locks are sleep mutexes, would there be any merit in building a TCP > callout facility using sleep mutexes? I can hack something up and > share here but wanted to check if people know of an existing facility > that might be used here? The spinlock in question is probably the callout_mtx itself. The problem is that you have to be able to acquire some sort of lock in the timer interrupt to check the timer state to see if a timer thread should be scheduled. That has to be a spin lock. -- John Baldwin ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Transitioning if_addr_lock to an rwlock
On Thursday, December 29, 2011 5:55:39 pm Gleb Smirnoff wrote:
> On Thu, Dec 29, 2011 at 03:27:26PM -0500, John Baldwin wrote:
> J> - if_addr_uses.patch This changes callers of the existing macros to use
> J> either read or write locks. This is the patch
> that
> J> could use the most review.
>
> Reviewing your patch I've found several problems not introduced by it,
> but already existing, and somewhat related to your patch:
>
> 1) Unneeded use of _SAFE version of TAILQ:
>
> igmp.c:3338
> in6.c:1339
> mld6.c:2993
I'll fix these.
> 2) Potential race when dropping a lock inside FOREACH loop:
>
> igmp.c:2058
> mld6.c:1419
> mld6.c:1704 (this one isn't even a SAFE, so would crash earlier)
These are not easy to fix. :( Dropping the lock is of course the
wrong thing to do. However, there are a few layering violations that
make this hard to fix properly. Actually, we might be able to use
an approach similar to that used in mld_ifdetach() and igmp_ifdetach()
to fix the cancel timers cases. Patch below
> 3) I've found that in6_ifawithifp() doesn't do what it is supposed
> to, as well as uses incorrect locking during this. As last resort
> it should run through global list of addresses, not run throgh the
> ifp one again. Patch attached.
This looks good to me. Maybe you can get bz@ to review it?
Index: netinet/igmp.c
===
--- netinet/igmp.c (revision 229389)
+++ netinet/igmp.c (working copy)
@@ -2004,7 +2003,7 @@
{
struct ifmultiaddr *ifma;
struct ifnet*ifp;
- struct in_multi *inm;
+ struct in_multi *inm, *tinm;
CTR3(KTR_IGMPV3, "%s: cancel v3 timers on ifp %p(%s)", __func__,
igi->igi_ifp, igi->igi_ifp->if_xname);
@@ -2050,14 +2049,8 @@
* transition to REPORTING to ensure the host leave
* message is sent upstream to the old querier --
* transition to NOT would lose the leave and race.
-*
-* SMPNG: Must drop and re-acquire IF_ADDR_LOCK
-* around inm_release_locked(), as it is not
-* a recursive mutex.
*/
- IF_ADDR_UNLOCK(ifp);
- inm_release_locked(inm);
- IF_ADDR_LOCK(ifp);
+ SLIST_INSERT_HEAD(&igi->igi_relinmhead, inm, inm_nrele);
/* FALLTHROUGH */
case IGMP_G_QUERY_PENDING_MEMBER:
case IGMP_SG_QUERY_PENDING_MEMBER:
@@ -2076,6 +2069,10 @@
_IF_DRAIN(&inm->inm_scq);
}
IF_ADDR_UNLOCK(ifp);
+ SLIST_FOREACH_SAFE(inm, &igi->igi_relinmhead, inm_nrele, tinm) {
+ SLIST_REMOVE_HEAD(&igi->igi_relinmhead, inm_nrele);
+ inm_release_locked(inm);
+ }
}
/*
Index: netinet6/mld6.c
===
--- netinet6/mld6.c (revision 229389)
+++ netinet6/mld6.c (working copy)
@@ -1656,7 +1656,7 @@
{
struct ifmultiaddr *ifma;
struct ifnet*ifp;
- struct in6_multi*inm;
+ struct in6_multi*inm, *tinm;
CTR3(KTR_MLD, "%s: cancel v2 timers on ifp %p(%s)", __func__,
mli->mli_ifp, mli->mli_ifp->if_xname);
@@ -1695,14 +1695,9 @@
* If we are leaving the group and switching
* version, we need to release the final
* reference held for issuing the INCLUDE {}.
-*
-* SMPNG: Must drop and re-acquire IF_ADDR_LOCK
-* around in6m_release_locked(), as it is not
-* a recursive mutex.
*/
- IF_ADDR_UNLOCK(ifp);
- in6m_release_locked(inm);
- IF_ADDR_LOCK(ifp);
+ SLIST_INSERT_HEAD(&mli->mli_relinmhead, inm,
+ in6m_nrele);
/* FALLTHROUGH */
case MLD_G_QUERY_PENDING_MEMBER:
case MLD_SG_QUERY_PENDING_MEMBER:
@@ -1720,6 +1715,10 @@
}
}
IF_ADDR_UNLOCK(ifp);
+ SLIST_FOREACH_SAFE(inm, &mli->mli_relinmhead, in6m_nrele, tinm) {
+ SLIST_REMOVE_HEAD(&mli->mli_relinmhead, in6m_nrele);
+ in6m_release_locked(inm);
+ }
}
/*
--
John Baldwin
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: how to debug non-working hole in nat
On Tue, 3 Jan 2012 17:52:53 +0900, Randy Bush wrote: > ignore. i sorted it. Too late, sucked in .. diff from prior config might be bone enough? cheers, Ian ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On 23. Dec 2011, at 20:08 , John Baldwin wrote:
> The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in
> in6_lifaddr_ioctl() does not grab a reference to an ifnet address structure
> that it uses after dropping the IF_ADDR_LOCK(). Based on other code that
> uses
> a similar pattern of finding an ifa while under the lock and then using it
> after dropping the lock, I believe it should be acquiring a reference on the
> ifa and then dropping that reference when it is done using the ifa. This
> (untested) patch should fix this I believe:
I almost assume it's been tested by now. From reading it looks right.
/bz
> Index: in6.c
> ===
> --- in6.c (revision 228777)
> +++ in6.c (working copy)
> @@ -1767,6 +1767,8 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> if (IN6_ARE_ADDR_EQUAL(&candidate, &match))
> break;
> }
> + if (ifa != NULL)
> + ifa_ref(ifa);
> IF_ADDR_UNLOCK(ifp);
> if (!ifa)
> return EADDRNOTAVAIL;
> @@ -1779,16 +1781,20 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> bcopy(&ia->ia_addr, &iflr->addr, ia->ia_addr.sin6_len);
> error = sa6_recoverscope(
> (struct sockaddr_in6 *)&iflr->addr);
> - if (error != 0)
> + if (error != 0) {
> + ifa_free(ifa);
> return (error);
> + }
>
> if ((ifp->if_flags & IFF_POINTOPOINT) != 0) {
> bcopy(&ia->ia_dstaddr, &iflr->dstaddr,
> ia->ia_dstaddr.sin6_len);
> error = sa6_recoverscope(
> (struct sockaddr_in6 *)&iflr->dstaddr);
> - if (error != 0)
> + if (error != 0) {
> + ifa_free(ifa);
> return (error);
> + }
> } else
> bzero(&iflr->dstaddr, sizeof(iflr->dstaddr));
>
> @@ -1796,6 +1802,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> in6_mask2len(&ia->ia_prefixmask.sin6_addr, NULL);
>
> iflr->flags = ia->ia6_flags;/* XXX */
> + ifa_free(ifa);
>
> return 0;
> } else {
> @@ -1819,6 +1826,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> ia->ia_prefixmask.sin6_len);
>
> ifra.ifra_flags = ia->ia6_flags;
> + ifa_free(ifa);
> return in6_control(so, SIOCDIFADDR_IN6, (caddr_t)&ifra,
> ifp, td);
> }
>
>
> --
> John Baldwin
--
Bjoern A. Zeeb You have to have visions!
It does not matter how good you are. It matters what good you do!
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On Jan 3, 2012, at 4:29 PM, Ed Maste wrote: > Thanks for the link Nikolay. > > Borja, I assume it's the PR submission form that gave you trouble - > sorry for that. Based on your report it sounds to me like the bug is > in OpenBGPd itself. If it works on OpenBSD with the TCP_MD5SIG option > though I'd assume it's due to a difference in our (FreeBSD's) > implementation of the option. Did you look at the OpenBSD/FreeBSD > differences in your investigation? Both bird and quagga work as expected on FreeBSD. You can leave TCP_MD5 enabled in the kernel. If you specify "password" options for a BGP peer, it will enable TCP_MD5. Of course in FreeBSD it's a bit clumsy and you have to use setkey(8) to set the keys. But it works. Borja. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On 3. Jan 2012, at 17:47 , Borja Marcos wrote: > > On Jan 3, 2012, at 4:29 PM, Ed Maste wrote: > >> Thanks for the link Nikolay. >> >> Borja, I assume it's the PR submission form that gave you trouble - >> sorry for that. Based on your report it sounds to me like the bug is >> in OpenBGPd itself. If it works on OpenBSD with the TCP_MD5SIG option >> though I'd assume it's due to a difference in our (FreeBSD's) >> implementation of the option. Did you look at the OpenBSD/FreeBSD >> differences in your investigation? > > Both bird and quagga work as expected on FreeBSD. You can leave TCP_MD5 > enabled in the kernel. If you specify "password" options for a BGP peer, it > will enable TCP_MD5. Of course in FreeBSD it's a bit clumsy and you have to > use setkey(8) to set the keys. But it works. The reason for setkey is just because the software (quagga, bird,...) didn't grow a proper key management integration on pfkey2. Would be easy. Might be needed soon anyway;-) Not having looked at the particular openbgpd patches in our ports tree I would almost expect there can only be a minor issue that it would stop to work for non-protected peers once MD5 support is present in the kernel and that should be easy to spot. Unfortunately Doug didn't say from where he updated to this December 8-STABLE to see if it could be the MFCs of the MD5 changes by Attilio could make OpenBGPd as in ports cranky? /bz -- Bjoern A. Zeeb You have to have visions! It does not matter how good you are. It matters what good you do! ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On Tuesday, January 03, 2012 12:35:30 pm Bjoern A. Zeeb wrote:
>
> On 23. Dec 2011, at 20:08 , John Baldwin wrote:
>
> > The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in
> > in6_lifaddr_ioctl() does not grab a reference to an ifnet address structure
> > that it uses after dropping the IF_ADDR_LOCK(). Based on other code that
> > uses
> > a similar pattern of finding an ifa while under the lock and then using it
> > after dropping the lock, I believe it should be acquiring a reference on
> > the
> > ifa and then dropping that reference when it is done using the ifa. This
> > (untested) patch should fix this I believe:
>
> I almost assume it's been tested by now. From reading it looks right.
Hmm, I don't have a good way to test it. :( I've booted a GENERIC kernel
with it, but I don't have IPv6 setup for my test machines.
> /bz
>
> > Index: in6.c
> > ===
> > --- in6.c (revision 228777)
> > +++ in6.c (working copy)
> > @@ -1767,6 +1767,8 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > if (IN6_ARE_ADDR_EQUAL(&candidate, &match))
> > break;
> > }
> > + if (ifa != NULL)
> > + ifa_ref(ifa);
> > IF_ADDR_UNLOCK(ifp);
> > if (!ifa)
> > return EADDRNOTAVAIL;
> > @@ -1779,16 +1781,20 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > bcopy(&ia->ia_addr, &iflr->addr, ia->ia_addr.sin6_len);
> > error = sa6_recoverscope(
> > (struct sockaddr_in6 *)&iflr->addr);
> > - if (error != 0)
> > + if (error != 0) {
> > + ifa_free(ifa);
> > return (error);
> > + }
> >
> > if ((ifp->if_flags & IFF_POINTOPOINT) != 0) {
> > bcopy(&ia->ia_dstaddr, &iflr->dstaddr,
> > ia->ia_dstaddr.sin6_len);
> > error = sa6_recoverscope(
> > (struct sockaddr_in6 *)&iflr->dstaddr);
> > - if (error != 0)
> > + if (error != 0) {
> > + ifa_free(ifa);
> > return (error);
> > + }
> > } else
> > bzero(&iflr->dstaddr, sizeof(iflr->dstaddr));
> >
> > @@ -1796,6 +1802,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > in6_mask2len(&ia->ia_prefixmask.sin6_addr, NULL);
> >
> > iflr->flags = ia->ia6_flags;/* XXX */
> > + ifa_free(ifa);
> >
> > return 0;
> > } else {
> > @@ -1819,6 +1826,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> > ia->ia_prefixmask.sin6_len);
> >
> > ifra.ifra_flags = ia->ia6_flags;
> > + ifa_free(ifa);
> > return in6_control(so, SIOCDIFADDR_IN6, (caddr_t)&ifra,
> > ifp, td);
> > }
> >
> >
> > --
> > John Baldwin
>
> --
> Bjoern A. Zeeb You have to have visions!
>It does not matter how good you are. It matters what good you do!
>
>
--
John Baldwin
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On 01/03/2012 10:03, Bjoern A. Zeeb wrote: > > On 3. Jan 2012, at 17:47 , Borja Marcos wrote: > >> >> On Jan 3, 2012, at 4:29 PM, Ed Maste wrote: >> >>> Thanks for the link Nikolay. >>> >>> Borja, I assume it's the PR submission form that gave you trouble - >>> sorry for that. Based on your report it sounds to me like the bug is >>> in OpenBGPd itself. If it works on OpenBSD with the TCP_MD5SIG option >>> though I'd assume it's due to a difference in our (FreeBSD's) >>> implementation of the option. Did you look at the OpenBSD/FreeBSD >>> differences in your investigation? >> >> Both bird and quagga work as expected on FreeBSD. You can leave TCP_MD5 >> enabled in the kernel. If you specify "password" options for a BGP peer, it >> will enable TCP_MD5. Of course in FreeBSD it's a bit clumsy and you have to >> use setkey(8) to set the keys. But it works. > > The reason for setkey is just because the software (quagga, bird,...) didn't > grow a proper key management integration on pfkey2. Would be easy. Might > be needed soon anyway;-) > > Not having looked at the particular openbgpd patches in our ports tree I > would almost expect there can only be a minor issue that it would stop to > work for non-protected peers once MD5 support is present in the kernel and > that should be easy to spot. > > Unfortunately Doug didn't say from where he updated to this December 8-STABLE > to see if it could be the MFCs of the MD5 changes by Attilio could make > OpenBGPd as in ports cranky? I mentioned December 29, sorry if that wasn't explicit enough, I didn't have the svn revision close to hand. Is r226260 the MFC that you're referring to? The log says, "Skip TCP_SIGNATURE calculation for INP_TIMEWAIT case." If so, that happened in October so we're well past that in our version of -stable. I'll be working on the various suggestions (thanks everyone for them, most helpful!) and report back on what works. Doug -- You can observe a lot just by watching. -- Yogi Berra Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
Doug Barton wrote in <4f027bc0.1080...@freebsd.org>: do> We have a pair of physical FreeBSD systems configured as routers do> designed to operate in an active/standby CARP configuration. Everything do> used to work fine, but since an upgrade to 8.2-STABLE on December 29th do> the two routers don't speak BGP to each other anymore. They both do> function fine individually, and failover works. It is only the openbgpd do> communication between them that's not flowing. Doug, does your kernel have TCP_SIGNATURE option? The patch[*] for net/openbgpd can be used as a workaround if it was due to TCP_MD5SIG option on the listening sockets. [*] http://people.allbsd.org/~hrs/FreeBSD/openbgpd.20120104-1.diff While this is an ugly hack and I will investigate more reasonable solution for that, I want to narrow down the cause first. Can anyone who are using a 8-STABLE kenrel with TCP_SIGNATURE let me know if this works or not? -- Hiroki pgpZ6ZqbthkKA.pgp Description: PGP signature
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On 3. Jan 2012, at 19:00 , Doug Barton wrote: > On 01/03/2012 10:03, Bjoern A. Zeeb wrote: >> >> On 3. Jan 2012, at 17:47 , Borja Marcos wrote: >> >>> >>> On Jan 3, 2012, at 4:29 PM, Ed Maste wrote: >>> Thanks for the link Nikolay. Borja, I assume it's the PR submission form that gave you trouble - sorry for that. Based on your report it sounds to me like the bug is in OpenBGPd itself. If it works on OpenBSD with the TCP_MD5SIG option though I'd assume it's due to a difference in our (FreeBSD's) implementation of the option. Did you look at the OpenBSD/FreeBSD differences in your investigation? >>> >>> Both bird and quagga work as expected on FreeBSD. You can leave TCP_MD5 >>> enabled in the kernel. If you specify "password" options for a BGP peer, it >>> will enable TCP_MD5. Of course in FreeBSD it's a bit clumsy and you have to >>> use setkey(8) to set the keys. But it works. >> >> The reason for setkey is just because the software (quagga, bird,...) didn't >> grow a proper key management integration on pfkey2. Would be easy. Might >> be needed soon anyway;-) >> >> Not having looked at the particular openbgpd patches in our ports tree I >> would almost expect there can only be a minor issue that it would stop to >> work for non-protected peers once MD5 support is present in the kernel and >> that should be easy to spot. >> >> Unfortunately Doug didn't say from where he updated to this December >> 8-STABLE to see if it could be the MFCs of the MD5 changes by Attilio could >> make OpenBGPd as in ports cranky? > > I mentioned December 29, sorry if that wasn't explicit enough, I didn't > have the svn revision close to hand. > > Is r226260 the MFC that you're referring to? The log says, "Skip > TCP_SIGNATURE calculation for INP_TIMEWAIT case." If so, that happened > in October so we're well past that in our version of -stable. > > I'll be working on the various suggestions (thanks everyone for them, > most helpful!) and report back on what works. I was wondering from *where* you were updating, not to which revision. I.e. was it an 8.2-RELEASE you were coming from or something earlier? -- Bjoern A. Zeeb You have to have visions! It does not matter how good you are. It matters what good you do! ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On 01/03/2012 11:16, Bjoern A. Zeeb wrote: > I was wondering from *where* you were updating, not to which revision. D'oh! Sorry ... the previous kernel was from stable/8 about 6 months ago. Well before Attilio's merge. Doug -- You can observe a lot just by watching. -- Yogi Berra Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
> Doug, does your kernel have TCP_SIGNATURE option? The patch[*] for > net/openbgpd can be used as a workaround if it was due to TCP_MD5SIG > option on the listening sockets. > > [*] http://people.allbsd.org/~hrs/FreeBSD/openbgpd.20120104-1.diff > > While this is an ugly hack and I will investigate more reasonable > solution for that, I want to narrow down the cause first. Can anyone > who are using a 8-STABLE kenrel with TCP_SIGNATURE let me know if > this works or not? 8-STABLE on several servers, csup'ed only a couple of days ago, with options TCP_SIGNATURE options IPSEC device crypto device cryptodev and Quagga bgpd talking to Juniper M/MX routers using MD5 key on the BGP sessions. No problems. Steinar Haug, Nethelp consulting, sth...@nethelp.no ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On 24 December 2011 00:08, John Baldwin wrote:
> The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in
> in6_lifaddr_ioctl() does not grab a reference to an ifnet address structure
> that it uses after dropping the IF_ADDR_LOCK(). Based on other code that uses
> a similar pattern of finding an ifa while under the lock and then using it
> after dropping the lock, I believe it should be acquiring a reference on the
> ifa and then dropping that reference when it is done using the ifa. This
> (untested) patch should fix this I believe:
[Some thoughts on this.]
FYI, a similar code exists in in_lifaddr_ioctl() under netinet/ which uses
an unreferenced ifa. Even when ifa reference is acquired, does this protect
ifa internals from concurrent changes? I would additionally lock ifa to
serialize multiple bcopy() operations. To do that, IFA_LOCK/UNLOCK() pair
exists to lock ifa with ifa_mtx. But there is only one place where such
locking is used explicitly. Initially IFA_LOCK/UNLOCK() were introduced in
2002 and used implicitly in IFAREF()/IFAFREE() to lock up ifaddr reference
counts. Two years later ifa_mtx started to be used explicitly in one place
to protect SIOCSIFNAME in net/if.c:ifhwioctl(). In 8.0 they are removed in
favor of refcount(9), and IFAREF/IFAFREE() moved to ifa_ref()/ifa_free(),
and now as said in r194602: "The ifa_mtx is now used for exactly one ioctl,
and possibly should be removed."
Now I'm losing the chain, sorry..
> Index: in6.c
> ===
> --- in6.c (revision 228777)
> +++ in6.c (working copy)
> @@ -1767,6 +1767,8 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> if (IN6_ARE_ADDR_EQUAL(&candidate, &match))
> break;
> }
> + if (ifa != NULL)
> + ifa_ref(ifa);
> IF_ADDR_UNLOCK(ifp);
> if (!ifa)
> return EADDRNOTAVAIL;
> @@ -1779,16 +1781,20 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> bcopy(&ia->ia_addr, &iflr->addr, ia->ia_addr.sin6_len);
> error = sa6_recoverscope(
> (struct sockaddr_in6 *)&iflr->addr);
> - if (error != 0)
> + if (error != 0) {
> + ifa_free(ifa);
> return (error);
> + }
>
> if ((ifp->if_flags & IFF_POINTOPOINT) != 0) {
> bcopy(&ia->ia_dstaddr, &iflr->dstaddr,
> ia->ia_dstaddr.sin6_len);
> error = sa6_recoverscope(
> (struct sockaddr_in6 *)&iflr->dstaddr);
> - if (error != 0)
> + if (error != 0) {
> + ifa_free(ifa);
> return (error);
> + }
> } else
> bzero(&iflr->dstaddr, sizeof(iflr->dstaddr));
>
> @@ -1796,6 +1802,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> in6_mask2len(&ia->ia_prefixmask.sin6_addr, NULL);
>
> iflr->flags = ia->ia6_flags; /* XXX */
> + ifa_free(ifa);
>
> return 0;
> } else {
> @@ -1819,6 +1826,7 @@ in6_lifaddr_ioctl(struct socket *so, u_long cmd, c
> ia->ia_prefixmask.sin6_len);
>
> ifra.ifra_flags = ia->ia6_flags;
> + ifa_free(ifa);
> return in6_control(so, SIOCDIFADDR_IN6, (caddr_t)&ifra,
> ifp, td);
> }
--
wbr,
pluknet
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Transitioning if_addr_lock to an rwlock
On 3. Jan 2012, at 16:23 , John Baldwin wrote:
> On Thursday, December 29, 2011 5:55:39 pm Gleb Smirnoff wrote:
>> On Thu, Dec 29, 2011 at 03:27:26PM -0500, John Baldwin wrote:
>> J> - if_addr_uses.patch This changes callers of the existing macros to
>> use
>> J> either read or write locks. This is the patch
>> that
>> J> could use the most review.
>>
>> Reviewing your patch I've found several problems not introduced by it,
>> but already existing, and somewhat related to your patch:
>>
>> 1) Unneeded use of _SAFE version of TAILQ:
>>
>> igmp.c:3338
>> in6.c:1339
>> mld6.c:2993
>
> I'll fix these.
>
>> 2) Potential race when dropping a lock inside FOREACH loop:
>>
>> igmp.c:2058
>> mld6.c:1419
>> mld6.c:1704 (this one isn't even a SAFE, so would crash earlier)
>
> These are not easy to fix. :( Dropping the lock is of course the
> wrong thing to do. However, there are a few layering violations that
> make this hard to fix properly. Actually, we might be able to use
> an approach similar to that used in mld_ifdetach() and igmp_ifdetach()
> to fix the cancel timers cases. Patch below
>
>> 3) I've found that in6_ifawithifp() doesn't do what it is supposed
>> to, as well as uses incorrect locking during this. As last resort
>> it should run through global list of addresses, not run throgh the
>> ifp one again. Patch attached.
>
> This looks good to me. Maybe you can get bz@ to review it?
Will look at that one next.
The two below seem fine. Would have loved the lock assertions that the
mld6 one has on igmp as well but that's a different (unrelated) story:-)
/bz
> Index: netinet/igmp.c
> ===
> --- netinet/igmp.c(revision 229389)
> +++ netinet/igmp.c(working copy)
> @@ -2004,7 +2003,7 @@
> {
> struct ifmultiaddr *ifma;
> struct ifnet*ifp;
> - struct in_multi *inm;
> + struct in_multi *inm, *tinm;
>
> CTR3(KTR_IGMPV3, "%s: cancel v3 timers on ifp %p(%s)", __func__,
> igi->igi_ifp, igi->igi_ifp->if_xname);
> @@ -2050,14 +2049,8 @@
>* transition to REPORTING to ensure the host leave
>* message is sent upstream to the old querier --
>* transition to NOT would lose the leave and race.
> - *
> - * SMPNG: Must drop and re-acquire IF_ADDR_LOCK
> - * around inm_release_locked(), as it is not
> - * a recursive mutex.
>*/
> - IF_ADDR_UNLOCK(ifp);
> - inm_release_locked(inm);
> - IF_ADDR_LOCK(ifp);
> + SLIST_INSERT_HEAD(&igi->igi_relinmhead, inm, inm_nrele);
> /* FALLTHROUGH */
> case IGMP_G_QUERY_PENDING_MEMBER:
> case IGMP_SG_QUERY_PENDING_MEMBER:
> @@ -2076,6 +2069,10 @@
> _IF_DRAIN(&inm->inm_scq);
> }
> IF_ADDR_UNLOCK(ifp);
> + SLIST_FOREACH_SAFE(inm, &igi->igi_relinmhead, inm_nrele, tinm) {
> + SLIST_REMOVE_HEAD(&igi->igi_relinmhead, inm_nrele);
> + inm_release_locked(inm);
> + }
> }
>
> /*
> Index: netinet6/mld6.c
> ===
> --- netinet6/mld6.c (revision 229389)
> +++ netinet6/mld6.c (working copy)
> @@ -1656,7 +1656,7 @@
> {
> struct ifmultiaddr *ifma;
> struct ifnet*ifp;
> - struct in6_multi*inm;
> + struct in6_multi*inm, *tinm;
>
> CTR3(KTR_MLD, "%s: cancel v2 timers on ifp %p(%s)", __func__,
> mli->mli_ifp, mli->mli_ifp->if_xname);
> @@ -1695,14 +1695,9 @@
>* If we are leaving the group and switching
>* version, we need to release the final
>* reference held for issuing the INCLUDE {}.
> - *
> - * SMPNG: Must drop and re-acquire IF_ADDR_LOCK
> - * around in6m_release_locked(), as it is not
> - * a recursive mutex.
>*/
> - IF_ADDR_UNLOCK(ifp);
> - in6m_release_locked(inm);
> - IF_ADDR_LOCK(ifp);
> + SLIST_INSERT_HEAD(&mli->mli_relinmhead, inm,
> + in6m_nrele);
> /* FALLTHROUGH */
> case MLD_G_QUERY_PENDING_MEMBER:
> case MLD_SG_QUERY_PENDING_MEMBER:
> @@ -1720,6 +1715,10 @@
> }
> }
> IF_ADDR_UNLOCK(ifp);
> + SLIST_FOREACH_SAFE(inm, &mli->mli_relinmhead, in6m_nrele, tinm) {
> + SLIST_REMOVE_HEAD(&mli->mli_relinmhead, in6m_nrele);
> + in6m_release_locked(inm);
> + }
> }
>
> /*
>
> --
> John Baldwin
>
Re: [PATCH] Use of unreferenced ifa in in6
On Tuesday, January 03, 2012 2:36:25 pm Sergey Kandaurov wrote:
> On 24 December 2011 00:08, John Baldwin wrote:
> > The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in
> > in6_lifaddr_ioctl() does not grab a reference to an ifnet address structure
> > that it uses after dropping the IF_ADDR_LOCK(). Based on other code that
> > uses
> > a similar pattern of finding an ifa while under the lock and then using it
> > after dropping the lock, I believe it should be acquiring a reference on the
> > ifa and then dropping that reference when it is done using the ifa. This
> > (untested) patch should fix this I believe:
>
> [Some thoughts on this.]
>
> FYI, a similar code exists in in_lifaddr_ioctl() under netinet/ which uses
> an unreferenced ifa. Even when ifa reference is acquired, does this protect
> ifa internals from concurrent changes? I would additionally lock ifa to
> serialize multiple bcopy() operations. To do that, IFA_LOCK/UNLOCK() pair
> exists to lock ifa with ifa_mtx. But there is only one place where such
> locking is used explicitly. Initially IFA_LOCK/UNLOCK() were introduced in
> 2002 and used implicitly in IFAREF()/IFAFREE() to lock up ifaddr reference
> counts. Two years later ifa_mtx started to be used explicitly in one place
> to protect SIOCSIFNAME in net/if.c:ifhwioctl(). In 8.0 they are removed in
> favor of refcount(9), and IFAREF/IFAFREE() moved to ifa_ref()/ifa_free(),
> and now as said in r194602: "The ifa_mtx is now used for exactly one ioctl,
> and possibly should be removed."
>
> Now I'm losing the chain, sorry..
Hmm, I'm not sure if ifa objects become immutable or not once they are
referenced in the list. Other places in the code seem to use the ifa
without locking it though, merely obtaining a reference.
The in.c code doesn't even grab the IF_ADDR_LOCK(). :( The below patch
should fix that and add the same fix as done to the in6.c code.
Index: in.c
===
--- in.c(revision 229406)
+++ in.c(working copy)
@@ -784,6 +784,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
}
}
+ IF_ADDR_LOCK(ifp);
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
if (ifa->ifa_addr->sa_family != AF_INET6)
continue;
@@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
if (candidate.s_addr == match.s_addr)
break;
}
+ if (ifa != NULL)
+ ifa_ref(ifa);
+ IF_ADDR_UNLOCK(ifp);
if (ifa == NULL)
return (EADDRNOTAVAIL);
ia = (struct in_ifaddr *)ifa;
@@ -812,6 +816,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
in_mask2len(&ia->ia_sockmask.sin_addr);
iflr->flags = 0;/*XXX*/
+ ifa_free(ifa);
return (0);
} else {
@@ -830,6 +835,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
}
bcopy(&ia->ia_sockmask, &ifra.ifra_dstaddr,
ia->ia_sockmask.sin_len);
+ ifa_free(ifa);
return (in_control(so, SIOCDIFADDR, (caddr_t)&ifra,
ifp, td));
--
John Baldwin
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On 4 January 2012 00:17, John Baldwin wrote:
> On Tuesday, January 03, 2012 2:36:25 pm Sergey Kandaurov wrote:
>> On 24 December 2011 00:08, John Baldwin wrote:
>> > The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in
>> > in6_lifaddr_ioctl() does not grab a reference to an ifnet address structure
>> > that it uses after dropping the IF_ADDR_LOCK(). Based on other code that
>> > uses
>> > a similar pattern of finding an ifa while under the lock and then using it
>> > after dropping the lock, I believe it should be acquiring a reference on
>> > the
>> > ifa and then dropping that reference when it is done using the ifa. This
>> > (untested) patch should fix this I believe:
>>
>> [Some thoughts on this.]
>>
>> FYI, a similar code exists in in_lifaddr_ioctl() under netinet/ which uses
>> an unreferenced ifa. Even when ifa reference is acquired, does this protect
>> ifa internals from concurrent changes? I would additionally lock ifa to
>> serialize multiple bcopy() operations. To do that, IFA_LOCK/UNLOCK() pair
>> exists to lock ifa with ifa_mtx. But there is only one place where such
>> locking is used explicitly. Initially IFA_LOCK/UNLOCK() were introduced in
>> 2002 and used implicitly in IFAREF()/IFAFREE() to lock up ifaddr reference
>> counts. Two years later ifa_mtx started to be used explicitly in one place
>> to protect SIOCSIFNAME in net/if.c:ifhwioctl(). In 8.0 they are removed in
>> favor of refcount(9), and IFAREF/IFAFREE() moved to ifa_ref()/ifa_free(),
>> and now as said in r194602: "The ifa_mtx is now used for exactly one ioctl,
>> and possibly should be removed."
>>
>> Now I'm losing the chain, sorry..
>
> Hmm, I'm not sure if ifa objects become immutable or not once they are
> referenced in the list. Other places in the code seem to use the ifa
> without locking it though, merely obtaining a reference.
Yes, this is a main concern.
> The in.c code doesn't even grab the IF_ADDR_LOCK(). :( The below patch
> should fix that and add the same fix as done to the in6.c code.
>
> Index: in.c
> ===
> --- in.c (revision 229406)
> +++ in.c (working copy)
> @@ -784,6 +784,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> }
> }
>
> + IF_ADDR_LOCK(ifp);
> TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
> if (ifa->ifa_addr->sa_family != AF_INET6)
> continue;
> @@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> if (candidate.s_addr == match.s_addr)
> break;
> }
> + if (ifa != NULL)
> + ifa_ref(ifa);
> + IF_ADDR_UNLOCK(ifp);
> if (ifa == NULL)
> return (EADDRNOTAVAIL);
> ia = (struct in_ifaddr *)ifa;
> @@ -812,6 +816,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> in_mask2len(&ia->ia_sockmask.sin_addr);
>
> iflr->flags = 0; /*XXX*/
> + ifa_free(ifa);
>
> return (0);
> } else {
> @@ -830,6 +835,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> }
> bcopy(&ia->ia_sockmask, &ifra.ifra_dstaddr,
> ia->ia_sockmask.sin_len);
> + ifa_free(ifa);
>
> return (in_control(so, SIOCDIFADDR, (caddr_t)&ifra,
> ifp, td));
>
>
With this patch in_lifaddr_ioctl() now looks more syntactically similar
to in6_lifaddr_ioctl(). They could look even more similar by eliminating
a lot of whitespace changes present here or there.
--
wbr,
pluknet
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On 01/03/2012 11:06, Hiroki Sato wrote: > Doug Barton wrote > in <4f027bc0.1080...@freebsd.org>: > > do> We have a pair of physical FreeBSD systems configured as routers > do> designed to operate in an active/standby CARP configuration. Everything > do> used to work fine, but since an upgrade to 8.2-STABLE on December 29th > do> the two routers don't speak BGP to each other anymore. They both > do> function fine individually, and failover works. It is only the openbgpd > do> communication between them that's not flowing. > > Doug, does your kernel have TCP_SIGNATURE option? Yes. > The patch[*] for > net/openbgpd can be used as a workaround if it was due to TCP_MD5SIG > option on the listening sockets. > > [*] http://people.allbsd.org/~hrs/FreeBSD/openbgpd.20120104-1.diff > > While this is an ugly hack and I will investigate more reasonable > solution for that, I want to narrow down the cause first. Can anyone > who are using a 8-STABLE kenrel with TCP_SIGNATURE let me know if > this works or not? This patch works even if net.inet.tcp.signature_verify_input=1. If I turn that sysctl off on both sides they can talk to each other even without the patch. So that would definitely seem to indicate that the tcp_signature stuff is the source of the problem. What unfortunately did not work is configuring signatures on both sides. With the sysctl enabled, IPSEC set up on both hosts, and the tcp md5sig option in both bgpd.conf files, we got the same result as before, no communication between them. When -HUP'ing and/or restarting openbgpd with the tcp md5sig option enabled we get "pfkey setup failed." So, "working iBGP + no signatures" is a good next step. "iBGP + signatures" would be an even better one. :) We're happy to test more patches, etc.; and thanks again to everyone who has responded so far. Doug -- You can observe a lot just by watching. -- Yogi Berra Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
Doug Barton wrote in <4f036a7f.9030...@freebsd.org>: do> This patch works even if net.inet.tcp.signature_verify_input=1. If I do> turn that sysctl off on both sides they can talk to each other even do> without the patch. So that would definitely seem to indicate that the do> tcp_signature stuff is the source of the problem. do> do> What unfortunately did not work is configuring signatures on both sides. do> With the sysctl enabled, IPSEC set up on both hosts, and the tcp md5sig do> option in both bgpd.conf files, we got the same result as before, no do> communication between them. When -HUP'ing and/or restarting openbgpd do> with the tcp md5sig option enabled we get "pfkey setup failed." do> do> So, "working iBGP + no signatures" is a good next step. "iBGP + do> signatures" would be an even better one. :) We're happy to test more do> patches, etc.; and thanks again to everyone who has responded so far. Okay, thank you for your report. I will take some time to fix TCP_MD5SIG support in openbgpd and inform you when another patch is ready. -- Hiroki pgpPR2vhn4hcv.pgp Description: PGP signature
Re: Transitioning if_addr_lock to an rwlock
On 29. Dec 2011, at 22:55 , Gleb Smirnoff wrote: > 3) I've found that in6_ifawithifp() doesn't do what it is supposed > to, as well as uses incorrect locking during this. As last resort > it should run through global list of addresses, not run throgh the > ifp one again. Patch attached. the first half of the patch is simple style which either goes on its own or not at all please. I think the second half of the patch is wrong. Mislead by either the KAME rev 1.1 comment or the locking. The locking sneaked in at http://svnweb.freebsd.org/base?view=revision&revision=194971 and I am not sure why reading it now. I guess an oversight by both Robert and I. The difference between the first and 2nd loop is the scope check and it's an "withifp" function, meaning we want an address of that interface and if you look at the highly expensive part in ip6_output() where we call it we expect the address to be on the given interface or to not be returned. So I'd say fix the locking and be done (and don't do the besta -> ia) change. If we want to optimize all this we might ponder to cache the first non-scope matching address and the 1st non-scope matching deprecated address and return that saving the 2nd loop. However not sure if the 2nd loop or the ifa_ref()s would be more expensive... Also wonder that for Apple... http://fxr.watson.org/fxr/source/bsd/netinet6/in6.c?v=xnu-1699.24.8#L3042 /bz -- Bjoern A. Zeeb You have to have visions! It does not matter how good you are. It matters what good you do! ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: how to debug non-working hole in nat
>> ignore. i sorted it. > Too late, sucked in .. diff from prior config might be bone enough? i had forgotten to remove the nat enable from /etc/ppp/ppp.conf when i moved to natd. randy ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On Tuesday, January 03, 2012 3:44:50 pm Sergey Kandaurov wrote:
> On 4 January 2012 00:17, John Baldwin wrote:
> > On Tuesday, January 03, 2012 2:36:25 pm Sergey Kandaurov wrote:
> >> On 24 December 2011 00:08, John Baldwin wrote:
> >> > The code to handle the SIOCGLIFADDR and SIOCDLIFADDR ioctls in
> >> > in6_lifaddr_ioctl() does not grab a reference to an ifnet address
structure
> >> > that it uses after dropping the IF_ADDR_LOCK(). Based on other code
that uses
> >> > a similar pattern of finding an ifa while under the lock and then using
it
> >> > after dropping the lock, I believe it should be acquiring a reference
on the
> >> > ifa and then dropping that reference when it is done using the ifa.
This
> >> > (untested) patch should fix this I believe:
> >>
> >> [Some thoughts on this.]
> >>
> >> FYI, a similar code exists in in_lifaddr_ioctl() under netinet/ which
uses
> >> an unreferenced ifa. Even when ifa reference is acquired, does this
protect
> >> ifa internals from concurrent changes? I would additionally lock ifa to
> >> serialize multiple bcopy() operations. To do that, IFA_LOCK/UNLOCK() pair
> >> exists to lock ifa with ifa_mtx. But there is only one place where such
> >> locking is used explicitly. Initially IFA_LOCK/UNLOCK() were introduced
in
> >> 2002 and used implicitly in IFAREF()/IFAFREE() to lock up ifaddr
reference
> >> counts. Two years later ifa_mtx started to be used explicitly in one
place
> >> to protect SIOCSIFNAME in net/if.c:ifhwioctl(). In 8.0 they are removed
in
> >> favor of refcount(9), and IFAREF/IFAFREE() moved to ifa_ref()/ifa_free(),
> >> and now as said in r194602: "The ifa_mtx is now used for exactly one
ioctl,
> >> and possibly should be removed."
> >>
> >> Now I'm losing the chain, sorry..
> >
> > Hmm, I'm not sure if ifa objects become immutable or not once they are
> > referenced in the list. Other places in the code seem to use the ifa
> > without locking it though, merely obtaining a reference.
>
> Yes, this is a main concern.
>
> > The in.c code doesn't even grab the IF_ADDR_LOCK(). :( The below patch
> > should fix that and add the same fix as done to the in6.c code.
> >
> > Index: in.c
> > ===
> > --- in.c(revision 229406)
> > +++ in.c(working copy)
> > @@ -784,6 +784,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> >}
> >}
> >
> > + IF_ADDR_LOCK(ifp);
> >TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
> >if (ifa->ifa_addr->sa_family != AF_INET6)
> >continue;
> > @@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> >if (candidate.s_addr == match.s_addr)
> >break;
> >}
> > + if (ifa != NULL)
> > + ifa_ref(ifa);
> > + IF_ADDR_UNLOCK(ifp);
> >if (ifa == NULL)
> >return (EADDRNOTAVAIL);
> >ia = (struct in_ifaddr *)ifa;
> > @@ -812,6 +816,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> >in_mask2len(&ia->ia_sockmask.sin_addr);
> >
> >iflr->flags = 0;/*XXX*/
> > + ifa_free(ifa);
> >
> >return (0);
> >} else {
> > @@ -830,6 +835,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> >}
> >bcopy(&ia->ia_sockmask, &ifra.ifra_dstaddr,
> >ia->ia_sockmask.sin_len);
> > + ifa_free(ifa);
> >
> >return (in_control(so, SIOCDIFADDR, (caddr_t)&ifra,
> >ifp, td));
> >
> >
>
> With this patch in_lifaddr_ioctl() now looks more syntactically similar
> to in6_lifaddr_ioctl(). They could look even more similar by eliminating
> a lot of whitespace changes present here or there.
Hmmm. Actually, it seems to be a bit more broken. Note that it is expecting
to get a sockaddr_in, but it is checking for AF_INET6, not AF_INET in its
loop. That bug seems to go back to the original import from KAME. I'm not
sure if the two can be merged since they work on different underyling data
structures though.
--
John Baldwin
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Transitioning if_addr_lock to an rwlock
On Thursday, December 29, 2011 5:55:39 pm Gleb Smirnoff wrote:
> Reviewing your patch I've found several problems not introduced by it,
> but already existing, and somewhat related to your patch:
>
> 2) Potential race when dropping a lock inside FOREACH loop:
>
> igmp.c:2058
> mld6.c:1419
> mld6.c:1704 (this one isn't even a SAFE, so would crash earlier)
Ok, I think I have a patch to fix the middle one. I've (ab)used the list used
to defer calls to in6m_release_locked() and used it to defer calls to
mld_v1_transmit_report() from under the loop. Note that the v2 timers in
the same loop already use this list to defer calls to in6m_release_locked()
so it should be safe to use the temporary list for v1 as well.
Index: mld6.c
===
--- mld6.c (revision 229420)
+++ mld6.c (working copy)
@@ -121,7 +121,8 @@ static int mld_v1_input_query(struct ifnet *, cons
/*const*/ struct mld_hdr *);
static int mld_v1_input_report(struct ifnet *, const struct ip6_hdr *,
/*const*/ struct mld_hdr *);
-static voidmld_v1_process_group_timer(struct in6_multi *, const int);
+static voidmld_v1_process_group_timer(struct mld_ifinfo *,
+ struct in6_multi *);
static voidmld_v1_process_querier_timers(struct mld_ifinfo *);
static int mld_v1_transmit_report(struct in6_multi *, const int);
static voidmld_v1_update_group(struct in6_multi *, const int);
@@ -1336,8 +1337,8 @@ mld_fasttimo_vnet(void)
struct ifqueue qrq; /* Query response packets */
struct ifnet*ifp;
struct mld_ifinfo *mli;
- struct ifmultiaddr *ifma, *tifma;
- struct in6_multi*inm;
+ struct ifmultiaddr *ifma;
+ struct in6_multi*inm, *tinm;
int uri_fasthz;
uri_fasthz = 0;
@@ -1401,24 +1402,14 @@ mld_fasttimo_vnet(void)
}
IF_ADDR_LOCK(ifp);
- TAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link,
- tifma) {
+ TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
if (ifma->ifma_addr->sa_family != AF_INET6 ||
ifma->ifma_protospec == NULL)
continue;
inm = (struct in6_multi *)ifma->ifma_protospec;
switch (mli->mli_version) {
case MLD_VERSION_1:
- /*
-* XXX Drop IF_ADDR lock temporarily to
-* avoid recursion caused by a potential
-* call by in6ifa_ifpforlinklocal().
-* rwlock candidate?
-*/
- IF_ADDR_UNLOCK(ifp);
- mld_v1_process_group_timer(inm,
- mli->mli_version);
- IF_ADDR_LOCK(ifp);
+ mld_v1_process_group_timer(mli, inm);
break;
case MLD_VERSION_2:
mld_v2_process_group_timers(mli, &qrq,
@@ -1428,9 +1419,25 @@ mld_fasttimo_vnet(void)
}
IF_ADDR_UNLOCK(ifp);
- if (mli->mli_version == MLD_VERSION_2) {
- struct in6_multi*tinm;
-
+ switch (mli->mli_version) {
+ case MLD_VERSION_1:
+ /*
+* Transmit reports for this lifecycle. This
+* is done while not holding IF_ADDR_LOCK
+* since this can call
+* in6ifa_ifpforlinklocal() which locks
+* IF_ADDR_LOCK internally as well as
+* ip6_output() to transmit a packet.
+*/
+ SLIST_FOREACH_SAFE(inm, &mli->mli_relinmhead,
+ in6m_nrele, tinm) {
+ SLIST_REMOVE_HEAD(&mli->mli_relinmhead,
+ in6m_nrele);
+ (void)mld_v1_transmit_report(inm,
+ MLD_LISTENER_REPORT);
+ }
+ break;
+ case MLD_VERSION_2:
mld_dispatch_queue(&qrq, 0);
mld_dispatch_queue(&scq, 0);
@@ -1444,6 +1451,7 @@ mld_fasttimo_vnet(void)
in6m_nrele);
in6m_release_locked(inm);
}
+ break;
}
}
@@ -1457,7 +1465,7 @@ out_locked:
* Will update the global pending timer flags.
*/
static void
-mld_v1_process_group_timer(str
Re: [PATCH] Use of unreferenced ifa in in6
John Baldwin wrote
in <201201031608.59688@freebsd.org>:
jh> > With this patch in_lifaddr_ioctl() now looks more syntactically similar
jh> > to in6_lifaddr_ioctl(). They could look even more similar by eliminating
jh> > a lot of whitespace changes present here or there.
jh>
jh> Hmmm. Actually, it seems to be a bit more broken. Note that it is
expecting
jh> to get a sockaddr_in, but it is checking for AF_INET6, not AF_INET in its
jh> loop. That bug seems to go back to the original import from KAME. I'm not
jh> sure if the two can be merged since they work on different underyling data
jh> structures though.
Hmm, a fix for that bug was not merged for some reason. Something
like the attached patch should be applied.
-- Hiroki
Index: in.c
===
--- in.c (revision 229434)
+++ in.c (working copy)
@@ -735,7 +735,7 @@
if (iflr->flags & IFLR_PREFIX)
return (EINVAL);
- /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR_IN6). */
+ /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR_IN). */
bzero(&ifra, sizeof(ifra));
bcopy(iflr->iflr_name, ifra.ifra_name,
sizeof(ifra.ifra_name));
@@ -785,7 +785,7 @@
}
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
- if (ifa->ifa_addr->sa_family != AF_INET6)
+ if (ifa->ifa_addr->sa_family != AF_INET)
continue;
if (match.s_addr == 0)
break;
@@ -817,7 +817,7 @@
} else {
struct in_aliasreq ifra;
- /* fill in_aliasreq and do ioctl(SIOCDIFADDR_IN6) */
+ /* fill in_aliasreq and do ioctl(SIOCDIFADDR_IN) */
bzero(&ifra, sizeof(ifra));
bcopy(iflr->iflr_name, ifra.ifra_name,
sizeof(ifra.ifra_name));
pgpDCHKwHwYmL.pgp
Description: PGP signature
Re: Transitioning if_addr_lock to an rwlock
On 29. Dec 2011, at 20:27 , John Baldwin wrote: > I've gone ahead with this approach. I have three separate patches that should > implement Phase 1. All of them can be found at > http://www.FreeBSD.org/~jhb/patches/ > > - if_addr_dev.patch This fixes a few new device drivers that were using > the locking macros directly rather than the wrapper > functions Robert added. I've already sent this > directly to the relevant driver maintainers for their > review. > - if_addr_macros.patch This adds new locking macros to support read locks vs > write locks. However, they all still map to mutex > operations. The first two look good. I wondered why you didn't need the r-wraper-functions but obviously they had been named like that already:) I'll look at the one below in more detail and get back to you. > - if_addr_uses.patch This changes callers of the existing macros to use > either read or write locks. This is the patch that > could use the most review. -- Bjoern A. Zeeb You have to have visions! It does not matter how good you are. It matters what good you do! ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On Tuesday, January 03, 2012 5:14:22 pm Hiroki Sato wrote:
> John Baldwin wrote
> in <201201031608.59688@freebsd.org>:
>
> jh> > With this patch in_lifaddr_ioctl() now looks more syntactically similar
> jh> > to in6_lifaddr_ioctl(). They could look even more similar by eliminating
> jh> > a lot of whitespace changes present here or there.
> jh>
> jh> Hmmm. Actually, it seems to be a bit more broken. Note that it is
> expecting
> jh> to get a sockaddr_in, but it is checking for AF_INET6, not AF_INET in its
> jh> loop. That bug seems to go back to the original import from KAME. I'm
> not
> jh> sure if the two can be merged since they work on different underyling data
> jh> structures though.
>
> Hmm, a fix for that bug was not merged for some reason. Something
> like the attached patch should be applied.
Ah, great, I've merged that into the patch, thanks!
Index: in.c
===
--- in.c(revision 229406)
+++ in.c(working copy)
@@ -735,7 +735,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
if (iflr->flags & IFLR_PREFIX)
return (EINVAL);
- /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR_IN6). */
+ /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR). */
bzero(&ifra, sizeof(ifra));
bcopy(iflr->iflr_name, ifra.ifra_name,
sizeof(ifra.ifra_name));
@@ -784,8 +784,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
}
}
+ IF_ADDR_LOCK(ifp);
TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
- if (ifa->ifa_addr->sa_family != AF_INET6)
+ if (ifa->ifa_addr->sa_family != AF_INET)
continue;
if (match.s_addr == 0)
break;
@@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
if (candidate.s_addr == match.s_addr)
break;
}
+ if (ifa != NULL)
+ ifa_ref(ifa);
+ IF_ADDR_UNLOCK(ifp);
if (ifa == NULL)
return (EADDRNOTAVAIL);
ia = (struct in_ifaddr *)ifa;
@@ -812,12 +816,13 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
in_mask2len(&ia->ia_sockmask.sin_addr);
iflr->flags = 0;/*XXX*/
+ ifa_free(ifa);
return (0);
} else {
struct in_aliasreq ifra;
- /* fill in_aliasreq and do ioctl(SIOCDIFADDR_IN6) */
+ /* fill in_aliasreq and do ioctl(SIOCDIFADDR) */
bzero(&ifra, sizeof(ifra));
bcopy(iflr->iflr_name, ifra.ifra_name,
sizeof(ifra.ifra_name));
@@ -830,6 +835,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
}
bcopy(&ia->ia_sockmask, &ifra.ifra_dstaddr,
ia->ia_sockmask.sin_len);
+ ifa_free(ifa);
return (in_control(so, SIOCDIFADDR, (caddr_t)&ifra,
ifp, td));
--
John Baldwin
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: [PATCH] Use of unreferenced ifa in in6
On 3. Jan 2012, at 22:22 , John Baldwin wrote:
> On Tuesday, January 03, 2012 5:14:22 pm Hiroki Sato wrote:
>> John Baldwin wrote
>> in <201201031608.59688@freebsd.org>:
>>
>> jh> > With this patch in_lifaddr_ioctl() now looks more syntactically similar
>> jh> > to in6_lifaddr_ioctl(). They could look even more similar by
>> eliminating
>> jh> > a lot of whitespace changes present here or there.
>> jh>
>> jh> Hmmm. Actually, it seems to be a bit more broken. Note that it is
>> expecting
>> jh> to get a sockaddr_in, but it is checking for AF_INET6, not AF_INET in its
>> jh> loop. That bug seems to go back to the original import from KAME. I'm
>> not
>> jh> sure if the two can be merged since they work on different underyling
>> data
>> jh> structures though.
>>
>> Hmm, a fix for that bug was not merged for some reason. Something
>> like the attached patch should be applied.
>
> Ah, great, I've merged that into the patch, thanks!
>
> Index: in.c
> ===
> --- in.c (revision 229406)
> +++ in.c (working copy)
> @@ -735,7 +735,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> if (iflr->flags & IFLR_PREFIX)
> return (EINVAL);
>
> - /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR_IN6). */
> + /* copy args to in_aliasreq, perform ioctl(SIOCAIFADDR). */
> bzero(&ifra, sizeof(ifra));
> bcopy(iflr->iflr_name, ifra.ifra_name,
> sizeof(ifra.ifra_name));
> @@ -784,8 +784,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> }
> }
>
> + IF_ADDR_LOCK(ifp);
> TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
> - if (ifa->ifa_addr->sa_family != AF_INET6)
> + if (ifa->ifa_addr->sa_family != AF_INET)
> continue;
> if (match.s_addr == 0)
> break;
When doing "noise changes" like fixing comment in the same go (I'd prefer
that to be independent of the locking/refcount changes if possible, you
could also wrap the long line:
792 candidate.s_addr = ((struct sockaddr_in
*)&ifa->ifa_addr)->sin_addr.s_addr;
Otherwise looks good.
> @@ -794,6 +795,9 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> if (candidate.s_addr == match.s_addr)
> break;
> }
> + if (ifa != NULL)
> + ifa_ref(ifa);
> + IF_ADDR_UNLOCK(ifp);
> if (ifa == NULL)
> return (EADDRNOTAVAIL);
> ia = (struct in_ifaddr *)ifa;
> @@ -812,12 +816,13 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> in_mask2len(&ia->ia_sockmask.sin_addr);
>
> iflr->flags = 0;/*XXX*/
> + ifa_free(ifa);
>
> return (0);
> } else {
> struct in_aliasreq ifra;
>
> - /* fill in_aliasreq and do ioctl(SIOCDIFADDR_IN6) */
> + /* fill in_aliasreq and do ioctl(SIOCDIFADDR) */
> bzero(&ifra, sizeof(ifra));
> bcopy(iflr->iflr_name, ifra.ifra_name,
> sizeof(ifra.ifra_name));
> @@ -830,6 +835,7 @@ in_lifaddr_ioctl(struct socket *so, u_long cmd, ca
> }
> bcopy(&ia->ia_sockmask, &ifra.ifra_dstaddr,
> ia->ia_sockmask.sin_len);
> + ifa_free(ifa);
>
> return (in_control(so, SIOCDIFADDR, (caddr_t)&ifra,
> ifp, td));
>
> --
> John Baldwin
> ___
> freebsd-net@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-net
> To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
--
Bjoern A. Zeeb You have to have visions!
It does not matter how good you are. It matters what good you do!
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: Transitioning if_addr_lock to an rwlock
On 3. Jan 2012, at 21:45 , John Baldwin wrote:
> On Thursday, December 29, 2011 5:55:39 pm Gleb Smirnoff wrote:
>> Reviewing your patch I've found several problems not introduced by it,
>> but already existing, and somewhat related to your patch:
>>
>> 2) Potential race when dropping a lock inside FOREACH loop:
>>
>> igmp.c:2058
>> mld6.c:1419
>> mld6.c:1704 (this one isn't even a SAFE, so would crash earlier)
>
> Ok, I think I have a patch to fix the middle one. I've (ab)used the list used
> to defer calls to in6m_release_locked() and used it to defer calls to
> mld_v1_transmit_report() from under the loop. Note that the v2 timers in
> the same loop already use this list to defer calls to in6m_release_locked()
> so it should be safe to use the temporary list for v1 as well.
It seems to look fine indeed.
> Index: mld6.c
> ===
> --- mld6.c(revision 229420)
> +++ mld6.c(working copy)
> @@ -121,7 +121,8 @@ static intmld_v1_input_query(struct ifnet *, cons
> /*const*/ struct mld_hdr *);
> static intmld_v1_input_report(struct ifnet *, const struct ip6_hdr *,
> /*const*/ struct mld_hdr *);
> -static void mld_v1_process_group_timer(struct in6_multi *, const int);
> +static void mld_v1_process_group_timer(struct mld_ifinfo *,
> + struct in6_multi *);
> static void mld_v1_process_querier_timers(struct mld_ifinfo *);
> static intmld_v1_transmit_report(struct in6_multi *, const int);
> static void mld_v1_update_group(struct in6_multi *, const int);
> @@ -1336,8 +1337,8 @@ mld_fasttimo_vnet(void)
> struct ifqueue qrq; /* Query response packets */
> struct ifnet*ifp;
> struct mld_ifinfo *mli;
> - struct ifmultiaddr *ifma, *tifma;
> - struct in6_multi*inm;
> + struct ifmultiaddr *ifma;
> + struct in6_multi*inm, *tinm;
> int uri_fasthz;
>
> uri_fasthz = 0;
> @@ -1401,24 +1402,14 @@ mld_fasttimo_vnet(void)
> }
>
> IF_ADDR_LOCK(ifp);
> - TAILQ_FOREACH_SAFE(ifma, &ifp->if_multiaddrs, ifma_link,
> - tifma) {
> + TAILQ_FOREACH(ifma, &ifp->if_multiaddrs, ifma_link) {
> if (ifma->ifma_addr->sa_family != AF_INET6 ||
> ifma->ifma_protospec == NULL)
> continue;
> inm = (struct in6_multi *)ifma->ifma_protospec;
> switch (mli->mli_version) {
> case MLD_VERSION_1:
> - /*
> - * XXX Drop IF_ADDR lock temporarily to
> - * avoid recursion caused by a potential
> - * call by in6ifa_ifpforlinklocal().
> - * rwlock candidate?
> - */
> - IF_ADDR_UNLOCK(ifp);
> - mld_v1_process_group_timer(inm,
> - mli->mli_version);
> - IF_ADDR_LOCK(ifp);
> + mld_v1_process_group_timer(mli, inm);
> break;
> case MLD_VERSION_2:
> mld_v2_process_group_timers(mli, &qrq,
> @@ -1428,9 +1419,25 @@ mld_fasttimo_vnet(void)
> }
> IF_ADDR_UNLOCK(ifp);
>
> - if (mli->mli_version == MLD_VERSION_2) {
> - struct in6_multi*tinm;
> -
> + switch (mli->mli_version) {
> + case MLD_VERSION_1:
> + /*
> + * Transmit reports for this lifecycle. This
> + * is done while not holding IF_ADDR_LOCK
> + * since this can call
> + * in6ifa_ifpforlinklocal() which locks
> + * IF_ADDR_LOCK internally as well as
> + * ip6_output() to transmit a packet.
> + */
> + SLIST_FOREACH_SAFE(inm, &mli->mli_relinmhead,
> + in6m_nrele, tinm) {
> + SLIST_REMOVE_HEAD(&mli->mli_relinmhead,
> + in6m_nrele);
> + (void)mld_v1_transmit_report(inm,
> + MLD_LISTENER_REPORT);
> + }
> + break;
> + case MLD_VERSION_2:
> mld_dispatch_queue(&qrq, 0);
> mld_dispatch_queue(&scq, 0);
>
> @@ -1444,6 +1451,7 @@ mld_fasttimo_vnet(void)
> in6m_nrele);
> in6m_release_locked(inm);
> }
> + break;
> }
> }
>
>
NDP Problem
Hi lists, I'm observing something strange. ipv6_enable="YES" ipv6_gateway_enable="YES" ipv6_network_interfaces="vlan3901" ipv6_ifconfig_vlan3901="2001:7f8:42::a503:9310:1/64" vlan3901: flags=8943 metric 0 mtu 1500 options=3 ether 00:15:17:38:13:7f inet6 fe80::92e2:baff:fe02:20bc%vlan3901 prefixlen 64 scopeid 0xf inet 195.182.218.230 netmask 0xfe00 broadcast 195.182.219.255 inet6 2001:7f8:42::a503:9310:1 prefixlen 64 nd6 options=3 media: Ethernet autoselect (1000baseT ) status: active vlan: 3901 parent interface: igb1 NDP doesn't work. After adding a static entry with ndp -s, communication is possible. net.inet6.ip6.fw.enable: 0 Any ideas? :) Pawel. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: NDP Problem
Pawel Tyll wrote
in <1609249417.20120104033...@nitronet.pl>:
pt> Hi lists,
pt>
pt> I'm observing something strange.
pt>
pt> ipv6_enable="YES"
pt> ipv6_gateway_enable="YES"
pt> ipv6_network_interfaces="vlan3901"
pt> ipv6_ifconfig_vlan3901="2001:7f8:42::a503:9310:1/64"
pt>
pt> vlan3901: flags=8943 metric
0 mtu 1500
pt> options=3
pt> ether 00:15:17:38:13:7f
pt> inet6 fe80::92e2:baff:fe02:20bc%vlan3901 prefixlen 64 scopeid 0xf
pt> inet 195.182.218.230 netmask 0xfe00 broadcast 195.182.219.255
pt> inet6 2001:7f8:42::a503:9310:1 prefixlen 64
pt> nd6 options=3
pt> media: Ethernet autoselect (1000baseT )
pt> status: active
pt> vlan: 3901 parent interface: igb1
pt>
pt> NDP doesn't work.
pt>
pt> After adding a static entry with ndp -s, communication is possible.
pt> net.inet6.ip6.fw.enable: 0
pt>
pt> Any ideas? :)
Does the attached patch (for 8.x kernel) fix your problem?
-- Hiroki
Index: sys/netinet6/in6_ifattach.c
===
RCS file: /home/ncvs/src/sys/netinet6/in6_ifattach.c,v
retrieving revision 1.74.2.3.4.1
diff -u -r1.74.2.3.4.1 in6_ifattach.c
--- sys/netinet6/in6_ifattach.c 21 Dec 2010 17:09:25 - 1.74.2.3.4.1
+++ sys/netinet6/in6_ifattach.c 4 Jan 2012 03:17:29 -
@@ -267,6 +267,7 @@
/* get EUI64 */
switch (ifp->if_type) {
case IFT_ETHER:
+ case IFT_L2VLAN:
case IFT_FDDI:
case IFT_ISO88025:
case IFT_ATM:
pgpUfgkspStby.pgp
Description: PGP signature
Re: NDP Problem
Hi Hiroki, > Does the attached patch (for 8.x kernel) fix your problem? Unfortunately, it doesn't. :( ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: NDP Problem
Pawel Tyll wrote in <1161157726.20120104052...@nitronet.pl>: pt> Hi Hiroki, pt> pt> > Does the attached patch (for 8.x kernel) fix your problem? pt> Unfortunately, it doesn't. :( Okay, so could you explain in more detail what symptoms made you think NDP didn't work properly? The results of "ifconfig -a", "ndp -na", "netstat -nrf inet6", and "ifmcstat" would be helpful for further diagnosis. -- Hiroki pgptmandMzCNT.pgp Description: PGP signature
pf not seeing inbound packets on netgraph interface
Hi freebsd-net, I originally sent this to -questions@, but was redirected here by that list. My original question is below: I am running into a roadblock getting PF to filter traffic on a Netgraph interface representing an L2TP/IPSec connection. I have done some narrowing down of the problem, but was hoping to get some advice on figuring out where to go digging next, or things to try. For context, here is what I have setup so far: I am running FreeBSD 8.2 with IPSec support compiled into the kernel. Here's the details from uname: # uname -a FreeBSD 8.2-RELEASE-p4 FreeBSD 8.2-RELEASE-p4 #2: Sun Nov 27 04:12:16 PST 2011 i386 I am following what seems like a typical setup of racoon(8) and setkey(8), and am having mpd5 handle construction of the L2TP nodes in netgraph. I can provide the details on the configuration of each of these, if desired. When I startup racoon in the foreground and ask mpd to construct an L2TP link, I can see both the IPSec tunnel and the L2TP link establish without a problem. I am able to ping the remote end, and, if I set up a routing rule, can contact and ssh to hosts at the other end of the tunnel. Here's how netgraph sees the world when thing are established: # ngctl list There are 13 total nodes: Name:Type: ksocket ID: 01b3 Num hooks: 1 Name:Type: l2tpID: 01b1 Num hooks: 3 Name:Type: socket ID: 01b0 Num hooks: 1 Name: ng0 Type: iface ID: 01b6 Num hooks: 1 Name: ngctl26124 Type: socket ID: 01bd Num hooks: 0 Name: ngctl19375 Type: socket ID: 00ba Num hooks: 0 Name: mpd25875-stats Type: socket ID: 01b8 Num hooks: 0 Name: mpd25875-WPLink-lt Type: tee ID: 01af Num hooks: 2 Name: mpd25875-csoType: socket ID: 01ad Num hooks: 0 Name: mpd25875-esoType: socket ID: 01ae Num hooks: 0 Name: mpd25875-lsoType: socket ID: 01ac Num hooks: 1 Name: mpd25875-WPBundle-1 Type: ppp ID: 01b7 Num hooks: 3 Name: ng0-tee Type: tee ID: 01b9 Num hooks: 2 # The problem I have is that PF only sees traffic on the outbound side of the netgraph interface. But, the rest of the network stack appears to see data going both ways: # ifconfig ng0 ng0: flags=88d1 metric 0 mtu 1322 inet 10.10.7.40 --> 10.10.0.2 netmask 0x # pfctl -vvs Interfaces -i ng0 ng0 Cleared: Sun Dec 25 21:14:44 2011 References: [ States: 0 Rules: 9 ] In4/Pass:[ Packets: 0 Bytes: 0 ] In4/Block: [ Packets: 0 Bytes: 0 ] Out4/Pass: [ Packets: Bytes: 446225 ] Out4/Block: [ Packets: 622Bytes: 56336 ] In6/Pass:[ Packets: 0 Bytes: 0 ] In6/Block: [ Packets: 0 Bytes: 0 ] Out6/Pass: [ Packets: 0 Bytes: 0 ] Out6/Block: [ Packets: 0 Bytes: 0 ] # netstat -I ng0 -bn NameMtu Network Address Ipkts Ierrs Idrop Ibytes Opkts Oerrs Obytes Coll ng01322 56 0 0 5069 98 0 6032 0 ng01322 10.10.7.40/32 10.10.7.40 56 - - 5069 54 - 3472 - I have stood up this interface several times, hence the differing numbers between the PF and netstat. The cause for concern is the lack of packets going through PF when inbound on ng0 -- no problem both seeing them and applying rules going outbound. There isn't a peep about the inbound traffic within pflog0, either. I can see traffic going both ways in tcpdump, and nothing looks peculiar about the packets. # tcpdump -c 10 -i ng0 tcpdump: verbose output suppressed, use -v or -vv for full protocol decode listening on ng0, link-type NULL (BSD loopback), capture size 96 bytes 22:06:37.201732 IP 10.10.7.40.43113 > 10.10.4.3.ssh: Flags [S], seq 3442571726, win 65535, options [mss 1282,nop,wscale 3,sackOK,TS val 694436002 ecr 0], length 0 22:06:37.263336 IP 10.10.4.3.ssh > 10.10.7.40.43113: Flags [S.], seq 1974232057, ack 3442571727, win 14480, options [mss 1282,sackOK,TS val 370681934 ecr 694436002,nop,wscale 7], length 0 22:06:37.263577 IP 10.10.7.40.43113 > 10.10.4.3.ssh: Flags [.], ack 1, win 8255, options [nop,nop,TS val 694436064 ecr 370681934], length 0 22:06:37.282835 IP 10.10.4.3.ssh > 10.10.7.40.43113: Flags [P.], ack 1, win 114, options [nop,nop,TS val 370681940 ecr 694436064], length 21 22:06:37.283931 IP 10.10.7.40.43113 > 10.10.4.3.ssh: Flags [P.], ack 22, win 8255, options [nop,nop,TS val 694436084 ecr 370681940], length 40 22:06:37.300729 IP 10.10.4.3.ssh > 10.10.7.40.43113: Flags [.], ack 41, win 114, options [nop,nop,TS val 370681945 e
Any recommendations for a 10G NIC from Broadcom
Hi. I would like to try out a 10G NIC from Broadcom. The BCM5716 seems promising. I am looking for features such as multi-queue, MSI-X, TSO etc. Any recommendations would be greatly appreciated. -vijay PS: I'd be using FreeBSD 8.2 initially, and FreeBSD 9.x in a few months. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On Jan 3, 2012, at 10:52 PM, Doug Barton wrote: > On 01/03/2012 11:06, Hiroki Sato wrote: >> Doug Barton wrote >> in <4f027bc0.1080...@freebsd.org>: >> >> do> We have a pair of physical FreeBSD systems configured as routers >> do> designed to operate in an active/standby CARP configuration. Everything >> do> used to work fine, but since an upgrade to 8.2-STABLE on December 29th >> do> the two routers don't speak BGP to each other anymore. They both >> do> function fine individually, and failover works. It is only the openbgpd >> do> communication between them that's not flowing. >> >> Doug, does your kernel have TCP_SIGNATURE option? > > Yes. > >> The patch[*] for >> net/openbgpd can be used as a workaround if it was due to TCP_MD5SIG >> option on the listening sockets. >> >> [*] http://people.allbsd.org/~hrs/FreeBSD/openbgpd.20120104-1.diff >> >> While this is an ugly hack and I will investigate more reasonable >> solution for that, I want to narrow down the cause first. Can anyone >> who are using a 8-STABLE kenrel with TCP_SIGNATURE let me know if >> this works or not? > > This patch works even if net.inet.tcp.signature_verify_input=1. If I > turn that sysctl off on both sides they can talk to each other even > without the patch. So that would definitely seem to indicate that the > tcp_signature stuff is the source of the problem. > > What unfortunately did not work is configuring signatures on both sides. > With the sysctl enabled, IPSEC set up on both hosts, and the tcp md5sig > option in both bgpd.conf files, we got the same result as before, no > communication between them. When -HUP'ing and/or restarting openbgpd > with the tcp md5sig option enabled we get "pfkey setup failed." > > So, "working iBGP + no signatures" is a good next step. "iBGP + > signatures" would be an even better one. :) We're happy to test more > patches, etc.; and thanks again to everyone who has responded so far. > > > Doug > > -- > > You can observe a lot just by watching. -- Yogi Berra > > Breadth of IT experience, and depth of knowledge in the DNS. > Yours for the right price. :) http://SupersetSolutions.com/ > You are setting the keys with setkey for both directions of a single session, right? i.e.: add X.X.X.X Y.Y.Y.Y tcp 0x1000 -A tcp-md5 "SomePass"; add Y.Y.Y.Y X.X.X.X tcp 0x1000 -A tcp-md5 "SomePass"; As before it was only needed to set the "outgoing" direction key, which should not work anymore unless net.inet.tcp.signature_verify_input is zero. ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On 01/03/2012 21:23, Nikolay Denev wrote: > You are setting the keys with setkey for both directions of a single session, > right? Yes. But thanks for asking. :) Doug -- You can observe a lot just by watching. -- Yogi Berra Breadth of IT experience, and depth of knowledge in the DNS. Yours for the right price. :) http://SupersetSolutions.com/ ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
Re: openbgpds not talking each other since 8.2-STABLE upgrade
On Jan 3, 2012, at 9:36 PM, sth...@nethelp.no wrote: >> Doug, does your kernel have TCP_SIGNATURE option? The patch[*] for >> net/openbgpd can be used as a workaround if it was due to TCP_MD5SIG >> option on the listening sockets. >> >> [*] http://people.allbsd.org/~hrs/FreeBSD/openbgpd.20120104-1.diff >> >> While this is an ugly hack and I will investigate more reasonable >> solution for that, I want to narrow down the cause first. Can anyone >> who are using a 8-STABLE kenrel with TCP_SIGNATURE let me know if >> this works or not? > > 8-STABLE on several servers, csup'ed only a couple of days ago, with > > options TCP_SIGNATURE > options IPSEC > device crypto > device cryptodev > > and Quagga bgpd talking to Juniper M/MX routers using MD5 key on the > BGP sessions. No problems. > > Steinar Haug, Nethelp consulting, sth...@nethelp.no This was always working for me. My problem was that I had two routers having BGP sessions to an ISP with md5 and a session between themselves without md5. After I upgraded to 8-STABLE some time ago, the md5 sessions still worked, but the ones without did not. tcpdump showed packets with md5 digest fields all zeroes. If one of the machines does not have md5 signature support it will probably work, since when one of the routers tries to speak tcpmd5 even with incorrect digest field, the other one tries to respond also with tcpmd5. Also there are some things in the tcp(4) manual page that should be fixed to reflect the new behaviour (the part mentioning that incoming digests are not verified): TCP_MD5SIG This option enables the use of MD5 digests (also known as TCP-MD5) on writes to the specified socket. In the current release, only outgoing traffic is digested; digests on incoming traffic are not verified. The current default behavior for the system is to respond to a system advertis- ing this option with TCP-MD5; this may change. Also in the case of my failing BGP sessions I expected to see errors as per the man page : If an SADB entry cannot be found for the destination, the outgoing traffic will have an invalid digest option prepended, and the following error message will be visible on the system console: tcp_signature_compute: SADB lookup failed for %d.%d.%d.%d. But this was not happening. Regards, Nikolay ___ freebsd-net@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-net To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
