Re: kern/159294: [em] em watchdog timeouts
Old Synopsis: em watchdog timeouts New Synopsis: [em] em watchdog timeouts Responsible-Changed-From-To: freebsd-bugs->freebsd-net Responsible-Changed-By: linimon Responsible-Changed-When: Fri Jul 29 20:42:18 UTC 2011 Responsible-Changed-Why: Over to maintainer(s). http://www.freebsd.org/cgi/query-pr.cgi?pr=159294 ___ 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: m_pkthdr.rcvif dangling pointer problem
On 7/14/11 8:44 AM, Gleb Smirnoff wrote:
Hi!
This problem is definitely known and is as old as network stack
is parallel. Those, who know the problem may skip to next paragraph.
Short description is the following: every mbuf that is allocated in
an interface receive procedure has a field where a pointer to the
corresponding struct ifnet is stored. Everything is okay, until you
are running a box, where configuration of interfaces is changing
rapidly, for example a PPP access concentrator. Utilizing any facility
that can delay packet processing for example netgraph(4) or dummynet(4)
would make probability of crash much bigger. Running INVARIANTS kernel
would crash a box with dummynet and disappearing interfaces in a few
minutes.
I see three approaches to this problem:
1) Every m_pkthdr.rcvif should if_ref() the interface. Releasing
references can be done in the mbuf allocator: mb_dtor_mbuf(),
mb_dtor_pack(). I'm afraid this approach would degrate performance,
since adding at least a couple of atomic ops on every mbuf for its
lifetime. That is +2 atomic ops per packet.
I did that in the 3.x timeframe in a branch.. it's probably in the cvs
tree still but was expensive.
better idea would be to use ifnet number and a generation number...
2) kib@ suggested to allocate ifnets from a UMA_ZONE_NOFREE zone.
I've made a compilable& working patch:
http://people.freebsd.org/~glebius/patches/ifnet.no_free
But on second though I find this a bad idea, this is just fooling
of INVARIANTS. Yes, we avoid thrashing of freed memory and rewriting
it by some other kernel allocation. But still out pointer point to
invalid ifnet. Even, if we make a check for IFF_DYING flag, we still
can not guarantee that an interface had been re-allocated for a new
instance. This would be not a panic condition, but subtle bugs in
firewalls.
3) As we now have a straight if_index table that can grow, what about
storing the if_index in the m_pkthdr? Lookup of interface by index
is fast enough if done lockless. Doing it lockless isn't perfect, but
better than current pointer dereferncing. Optionally it could be
done with locking and with putting a reference. To avoid situation
with with getting to a re-allocated interface with the same index,
we can use a unique cookie, that is incremented in if_alloc().
Smth like:
struct ifnet *
mbuf_rcvif(struct mbuf *m)
{
struct ifnet *ifp;
M_ASSERTPKTHDR(m);
if (m->m_pkthdr.rcvif_idx == 0)
return (NULL);
ifp = ifnet_byindex_locked(m->m_pkthdr.rcvif_idx);
if (ifp == NULL)
return (NULL);
if (ifp->if_unique != m->m_pkthdr.rcvif_unique)
return (NULL);
return (ifp);
}
struct ifnet *
mbuf_rcvif_ref(struct mbuf *m)
{
struct ifnet *ifp;
M_ASSERTPKTHDR(m);
if (m->m_pkthdr.rcvif_idx == 0)
return (NULL);
ifp = ifnet_byindex_ref(m->m_pkthdr.rcvif_idx);
if (ifp == NULL)
return (NULL);
if (ifp->if_unique != m->m_pkthdr.rcvif_unique) {
if_rele(ifp);
return (NULL);
}
return (ifp);
}
The size of struct mbuf isn't going to change on amd64:
@@ -111,7 +111,8 @@
* Record/packet header in first mbuf of chain; valid only if M_PKTHDR is set.
*/
struct pkthdr {
- struct ifnet*rcvif; /* rcv interface */
+ uint32_t rcvif_idx; /* rcv interface index */
+ uint32_t rcvif_unique; /* rcv interface unique id */
A proof-of-concept patch is available here:
http://people.freebsd.org/~glebius/patches/pkthdr.rcvif_idx
It doesn't cover entire kernel, LINT won't compile. It covers kern, netinet,
netinet6, several interface drivers and netgraph.
One of the problems is ongoing namespace pollution: not all code that include
mbuf.h includes if_var.h and vice versa. I've cut ifnet from m_devget(), but
my new function do declare it in parameter list :(. To deal with that I had
to declare functions in mbuf.h but implement them in if.c.
Other problem is net80211 code that abuses the rcvif pointer in mbuf packet
header for its own purposes casting it to private type. This can be fixed
utilizing mbuf_tags(9), I think. I haven't made this yet, that's why LINT
doesn't compile.
Comments are requested!
Any alternative ideas on solving the problem are welcome!
___
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: FIB separation
On 7/16/11 9:19 AM, Alexander V. Chernikov wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hiroki Sato wrote: Vlad Galu wrote in: du> Hello, du> du> A couple of years ago, Stef Walter proposed a patch[1] that enforced du> the scope of routing messages. The general consesus was that the best du> approach would be the OpenBSD way - transporting the FIB number in the du> message and letting the user applications filter out unwanted du> messages. du> du> Are there any plans to tackle this before 9.0? I am looking into this and investigating other possible extensions in rtsock messages such as addition of a fib member to rt_msghdr. I am not sure it can be done before 9.0, though... Actually there were an off-list discussion with bz@ and julian@ about interface fibs and rtsock changes several weeks ago. Initial messages: http://lists.freebsd.org/pipermail/freebsd-net/2011-June/029040.html I've got 3 different patches: 1) straight forwarded kern/134931 fix (no fib in rtsock, no breaking ABI, send to bz@) just got back from vacation in hungary so catching up...: Didn't he commit it? bz?? 2) adding fib in rtsock with rtsock versioning and other ABI keeping tricks 3) adding special RTA which can contain TLV pairs, with single defined TLV with routing socket As a result of discussion, first patch was sent to bz@. Since patches from kern/134931 are outdated attaching it here. ___ 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: LOR with nfsclient "sillyrename"
On Jul 22, 2011, at 5:11 PM, Rick Macklem wrote: > Please try the attached patch (which is also at): > http://people.freebsd.org/~rmacklem/oldsilly.patch > http://people.freebsd.org/~rmacklem/newsilly.patch > (for the old and new clients in -current, respectively) > > - I think oldsilly.patch should apply to the 7.n kernel > sources, although you might have to do the edit by hand? It applied with minimal futzing. > Please let me know how testing goes with it, rick Unfortunately we've never reproduced the original problem in the lab. Only in the field under heavy stress. I did build a kernel with the patch and run it under some of our tests, it seems to work correctly. We'll continue to test it, but I wanted to give you an update. Thanks a lot for your help. Jeremiah Lott Avere Systems ___ 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"
kern/159103
Additional data: First. Tio obtain bug you should place "lo0" as latest member of network interfaces set, i.e: network_interfaces="xl0 lo0" - bug will occured network_interfaces="lo0 xl0" - bug will not occured network_interfaces="xl0" - bug also will not occured Second. To eliminate bug you must down interface and configure it manually again What's a mess? I had have run with rc_debug=YES and the ONLY difference between "worked" and "non-worked" configuration is an order of network interfaces initialization! -- With Best Regards. Rashid N. Achilov (RNA1-RIPE), JID: cityc...@jabber.infos.ru OOO "ACK" telecommunications administrator, e-mail: achilov-rn [at] askd.ru PGP: 83 CD E2 A7 37 4A D5 81 D6 D6 52 BF C9 2F 85 AF 97 BE CB 0A ___ 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: bin/136661: [patch] ndp(8) ignores -f option
The following reply was made to PR bin/136661; it has been noted by GNATS. From: Sergey Kandaurov To: bug-follo...@freebsd.org, melif...@ipfw.ru Cc: Subject: Re: bin/136661: [patch] ndp(8) ignores -f option Date: Fri, 29 Jul 2011 14:47:28 +0400 The '-f' argument parsing was (accidentally?) removed with KAME sync in 2003. "make command line argument parsing POSIX compliant." http://svn.freebsd.org/changeset/base/122615 http://www.kame.net/dev/cvsweb2.cgi/kame/kame/kame/ndp/ndp.c#rev1.92 ___ 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"
bin/121359
The following reply was made to PR bin/121359; it has been noted by GNATS.
From: Maxim Konovalov
To: bug-follo...@freebsd.org
Cc:
Subject: bin/121359
Date: Fri, 29 Jul 2011 12:53:19 +0400 (MSD)
Try the following patch (ported from OpenBSD):
Index: systems.c
===
--- systems.c (revision 224489)
+++ systems.c (working copy)
@@ -64,9 +64,12 @@
fclose(fp);
}
-/* Move string from ``from'' to ``to'', interpreting ``~'' and $ */
+/*
+ * Move string from ``from'' to ``to'', interpreting ``~'' and $
+ * Returns NULL if string expansion failed due to lack of buffer space.
+ */
const char *
-InterpretArg(const char *from, char *to)
+InterpretArg(const char *from, char *to, size_t tosiz)
{
char *ptr, *startto, *endto;
struct passwd *pwd;
@@ -76,12 +79,14 @@
instring = 0;
startto = to;
- endto = to + LINE_LEN - 1;
+ endto = to + tosiz - 1;
while(issep(*from))
from++;
while (*from != '\0') {
+if (to >= endto)
+ return NULL;
switch (*from) {
case '"':
instring = !instring;
@@ -97,6 +102,8 @@
*to++ = '\\'; /* Pass the escapes on, maybe skipping \# */
break;
}
+if (to >= endto)
+ return NULL;
*to++ = *from++;
break;
case '$':
@@ -108,7 +115,7 @@
if (ptr) {
len = ptr - from - 2;
if (endto - to < (int)len )
- len = endto - to;
+ return NULL;
if (len) {
strncpy(to, from+2, len);
to[len] = '\0';
@@ -127,9 +134,13 @@
*ptr++ = *from;
*ptr = '\0';
}
+if (to >= endto)
+ return NULL;
if (*to == '\0')
*to++ = '$';
else if ((env = getenv(to)) != NULL) {
+ if (endto - to < (int)strlen(env))
+return NULL;
strncpy(to, env, endto - to);
*endto = '\0';
to += strlen(to);
@@ -142,19 +153,24 @@
if (len == 0)
pwd = getpwuid(ID0realuid());
else {
+ if (endto - to < (int)len)
+return NULL;
strncpy(to, from, len);
to[len] = '\0';
pwd = getpwnam(to);
}
+if (to >= endto)
+ return NULL;
if (pwd == NULL)
*to++ = '~';
else {
+ if (endto - to < (int)strlen(pwd->pw_dir))
+return NULL;
strncpy(to, pwd->pw_dir, endto - to);
*endto = '\0';
to += strlen(to);
from += len;
}
-endpwent();
break;
default:
@@ -179,12 +195,16 @@
#define CTRL_INCLUDE (1)
static int
-DecodeCtrlCommand(char *line, char *arg)
+DecodeCtrlCommand(char *line, char *arg, size_t argsiz)
{
const char *end;
if (!strncasecmp(line, "include", 7) && issep(line[7])) {
-end = InterpretArg(line+8, arg);
+end = InterpretArg(line+8, arg, argsiz);
+if (end == NULL) {
+ log_Printf(LogWARN, "Failed to expand command '%s': too long for the
destination buffer\n", line);
+ return CTRL_UNKNOWN;
+}
if (*end && *end != '#')
log_Printf(LogWARN, "usage: !include filename\n");
else
@@ -218,7 +238,6 @@
userok = 1;
break;
}
- endpwent();
return 0;
}
@@ -353,7 +372,7 @@
break;
case '!':
- switch (DecodeCtrlCommand(cp+1, arg)) {
+ switch (DecodeCtrlCommand(cp+1, arg, LINE_LEN)) {
case CTRL_INCLUDE:
log_Printf(LogCOMMAND, "%s: Including \"%s\"\n", filename, arg);
n = ReadSystem(bundle, name, arg, prompt, cx, how);
Index: systems.h
===
--- systems.h (revision 224489)
+++ systems.h (working copy)
@@ -40,4 +40,4 @@
extern void CloseSecret(FILE *);
extern int AllowUsers(struct cmdargs const *);
extern int AllowModes(struct cmdargs const *);
-extern const char *InterpretArg(const char *, char *);
+extern const char *InterpretArg(const char *, char *, size_t);
Index: command.c
===
--- command.c (revision 224489)
+++ command.c (working copy)
@@ -1139,7 +1134,11 @@
{
char buff2[LINE_LEN-offset];
- InterpretArg(buff, buff2);
+ if (InterpretArg(buff, buff2, sizeof buff2) == NULL) {
+log_Printf(LogWARN, "Failed to expand command '%s': too long for the
destination buffer\n", buff);
+return -1;
+ }
+
strncpy(buff, buff2, LINE_LEN - offset - 1);
buff[LINE_LEN - offset - 1] = '\0';
%%%
--
Maxim Konovalov
___
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubsc
Re: kern/109308: [pppd] [panic] Multiple panics kernel ppp suspected [regression]
Synopsis: [pppd] [panic] Multiple panics kernel ppp suspected [regression] State-Changed-From-To: open->closed State-Changed-By: maxim State-Changed-When: Fri Jul 29 08:07:19 UTC 2011 State-Changed-Why: pppd(8) and its kernel code was removed from the base long time ago. http://www.freebsd.org/cgi/query-pr.cgi?pr=109308 ___ 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"
