Re: fix seekdir(3)
Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100:
I will send a minimal one-line patch to just fix the bug and do nothing
else. We should get that one in quickly.
Done.
[...]
Then i will send two cleanup patches to remove useless stuff
and put the code into the right place, not changing any functionality.
Here it is (one patch only).
- put the seekdir() implementation where it belongs
and remove a related lie from a comment
- remove unused headers and an unused prototype
- remove two trivial functions used only once, in the same file
- avoid code duplication
No functional change.
OK?
Ingo
Index: rewinddir.c
===
RCS file: /cvs/src/lib/libc/gen/rewinddir.c,v
retrieving revision 1.10
diff -u -p -r1.10 rewinddir.c
--- rewinddir.c 13 Aug 2013 05:52:12 - 1.10
+++ rewinddir.c 5 Nov 2013 09:54:32 -
@@ -1,5 +1,5 @@
/* $OpenBSD: rewinddir.c,v 1.10 2013/08/13 05:52:12 guenther Exp $ */
-/*-
+/*
* Copyright (c) 1990, 1993
* The Regents of the University of California. All rights reserved.
*
@@ -28,16 +28,12 @@
* SUCH DAMAGE.
*/
-#include sys/types.h
#include dirent.h
-
#include thread_private.h
#include telldir.h
void
rewinddir(DIR *dirp)
{
- _MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, 0);
- _MUTEX_UNLOCK(dirp-dd_lock);
+ seekdir(dirp, 0);
}
Index: seekdir.c
===
RCS file: /cvs/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.9
diff -u -p -r1.9 seekdir.c
--- seekdir.c 5 Jun 2007 18:11:48 - 1.9
+++ seekdir.c 5 Nov 2013 09:54:32 -
@@ -28,19 +28,21 @@
* SUCH DAMAGE.
*/
-#include sys/param.h
#include dirent.h
+#include unistd.h
+
#include thread_private.h
#include telldir.h
/*
* Seek to an entry in a directory.
- * __seekdir is in telldir.c so that it can share opaque data structures.
+ * Only values returned by telldir should be passed to seekdir.
*/
void
seekdir(DIR *dirp, long loc)
{
_MUTEX_LOCK(dirp-dd_lock);
- __seekdir(dirp, loc);
+ dirp-dd_loc = 0;
+ dirp-dd_curpos = lseek(dirp-dd_fd, loc, SEEK_SET);
_MUTEX_UNLOCK(dirp-dd_lock);
}
Index: telldir.c
===
RCS file: /cvs/src/lib/libc/gen/telldir.c,v
retrieving revision 1.16
diff -u -p -r1.16 telldir.c
--- telldir.c 5 Nov 2013 09:36:05 - 1.16
+++ telldir.c 5 Nov 2013 09:54:33 -
@@ -28,45 +28,21 @@
* SUCH DAMAGE.
*/
-#include sys/param.h
-#include sys/queue.h
#include dirent.h
-#include stdlib.h
-#include unistd.h
-
#include thread_private.h
#include telldir.h
-int _readdir_unlocked(DIR *, struct dirent **, int);
-
/*
* return a pointer into a directory
*/
long
-_telldir_unlocked(DIR *dirp)
-{
- return (dirp-dd_curpos);
-}
-
-long
telldir(DIR *dirp)
{
long i;
_MUTEX_LOCK(dirp-dd_lock);
- i = _telldir_unlocked(dirp);
+ i = dirp-dd_curpos;
_MUTEX_UNLOCK(dirp-dd_lock);
return (i);
-}
-
-/*
- * seek to an entry in a directory.
- * Only values returned by telldir should be passed to seekdir.
- */
-void
-__seekdir(DIR *dirp, long loc)
-{
- dirp-dd_loc = 0;
- dirp-dd_curpos = lseek(dirp-dd_fd, loc, SEEK_SET);
}
Index: telldir.h
===
RCS file: /cvs/src/lib/libc/gen/telldir.h,v
retrieving revision 1.6
diff -u -p -r1.6 telldir.h
--- telldir.h 13 Aug 2013 05:52:13 - 1.6
+++ telldir.h 5 Nov 2013 09:54:33 -
@@ -47,7 +47,4 @@ struct _dirdesc {
void*dd_lock; /* mutex to protect struct */
};
-long _telldir_unlocked(DIR *);
-void __seekdir(DIR *, long);
-
#endif
make ftp(1) ignore leading whitespace in URLs
Before: $ ftp ' http://localhost/snap/INSTALL.amd64' ftp: http: no address associated with name ftp: Can't connect or login to host ` http' After: $ ftp ' http://localhost/snap/INSTALL.amd64' Trying ::1... Trying 127.0.0.1... Requesting http://localhost/snap/INSTALL.amd64 100% |**| 84357 00:00 84357 bytes received in 0.00 seconds (267.27 MB/s) Do others think this useful? I hit this because I made copy/paste errors. Index: fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.110 diff -u -p -r1.110 fetch.c --- fetch.c 27 Oct 2013 18:31:24 - 1.110 +++ fetch.c 28 Oct 2013 11:57:32 - @@ -1082,6 +1082,9 @@ auto_fetch(int argc, char *argv[], char if (url == NULL) errx(1, Can't allocate memory for auto-fetch.); + while (isspace(*url)) + url++; + /* * Try HTTP URL-style arguments first. */
Re: make ftp(1) ignore leading whitespace in URLs
On 11/05/13 13:56, Stefan Sperling wrote: Before: $ ftp ' http://localhost/snap/INSTALL.amd64' ftp: http: no address associated with name ftp: Can't connect or login to host ` http' After: $ ftp ' http://localhost/snap/INSTALL.amd64' Trying ::1... Trying 127.0.0.1... Requesting http://localhost/snap/INSTALL.amd64 100% |**| 84357 00:00 84357 bytes received in 0.00 seconds (267.27 MB/s) Do others think this useful? I hit this because I made copy/paste errors. Unless you can give a reference saying an URL may start with arbitrary whitespace, I don't want this to go in. It feels like PHP. /Alexander Index: fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.110 diff -u -p -r1.110 fetch.c --- fetch.c 27 Oct 2013 18:31:24 - 1.110 +++ fetch.c 28 Oct 2013 11:57:32 - @@ -1082,6 +1082,9 @@ auto_fetch(int argc, char *argv[], char if (url == NULL) errx(1, Can't allocate memory for auto-fetch.); + while (isspace(*url)) + url++; + /* * Try HTTP URL-style arguments first. */
Re: make ftp(1) ignore leading whitespace in URLs
Hi Stefan, Stefan Sperling wrote on Tue, Nov 05, 2013 at 01:56:33PM +0100: Do others think this useful? I hit this because I made copy/paste errors. Useful? I don't know. Maybe, maybe not. But your patch is NOT OK. Try stuff like $ ftp ' foo' bar I would be surprised if you couldn't get it to segfault. 1081: url = strdup(argv[argpos]); 1086: url++; 1073: free(url); is not a great idiom, imho. Yours, Ingo Index: fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.110 diff -u -p -r1.110 fetch.c --- fetch.c 27 Oct 2013 18:31:24 - 1.110 +++ fetch.c 28 Oct 2013 11:57:32 - @@ -1082,6 +1082,9 @@ auto_fetch(int argc, char *argv[], char if (url == NULL) errx(1, Can't allocate memory for auto-fetch.); + while (isspace(*url)) + url++; + /* * Try HTTP URL-style arguments first. */
Re: make ftp(1) ignore leading whitespace in URLs
I think this would help the port yt to not crash on such urls, and I think it uses ftp to collect the youtube movies. For some reason, I paste starting spaces a lot, and since youtube urls contain and other non-shelly args, I have to paste into a pair of 's, and if there is a space to begin with, ftp acts up and yt bombs. Sure, yt could do similar whitespace removals (and perhaps should wash better?), but I don't see protocol specs beginning with spaces a lot... 2013/11/5 Alexander Hall alexan...@beard.se On 11/05/13 13:56, Stefan Sperling wrote: Before: $ ftp ' http://localhost/snap/INSTALL.amd64' ftp: http: no address associated with name ftp: Can't connect or login to host ` http' After: $ ftp ' http://localhost/snap/INSTALL.amd64' Trying ::1... Trying 127.0.0.1... Requesting http://localhost/snap/INSTALL.amd64 100% |**| 84357 00:00 84357 bytes received in 0.00 seconds (267.27 MB/s) Do others think this useful? I hit this because I made copy/paste errors. Unless you can give a reference saying an URL may start with arbitrary whitespace, I don't want this to go in. It feels like PHP. /Alexander Index: fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.110 diff -u -p -r1.110 fetch.c --- fetch.c 27 Oct 2013 18:31:24 - 1.110 +++ fetch.c 28 Oct 2013 11:57:32 - @@ -1082,6 +1082,9 @@ auto_fetch(int argc, char *argv[], char if (url == NULL) errx(1, Can't allocate memory for auto-fetch.); + while (isspace(*url)) + url++; + /* * Try HTTP URL-style arguments first. */ -- May the most significant bit of your life be positive.
Re: make ftp(1) ignore leading whitespace in URLs
On Tue, Nov 05, 2013 at 02:08:21PM +0100, Alexander Hall wrote: On 11/05/13 13:56, Stefan Sperling wrote: Before: $ ftp ' http://localhost/snap/INSTALL.amd64' ftp: http: no address associated with name ftp: Can't connect or login to host ` http' After: $ ftp ' http://localhost/snap/INSTALL.amd64' Trying ::1... Trying 127.0.0.1... Requesting http://localhost/snap/INSTALL.amd64 100% |**| 84357 00:00 84357 bytes received in 0.00 seconds (267.27 MB/s) Do others think this useful? I hit this because I made copy/paste errors. Unless you can give a reference saying an URL may start with arbitrary whitespace, I don't want this to go in. Well, I would bet anyone can easily find claims that go either way in some RFC. Here's one that's in favour of this change, FWIW. https://tools.ietf.org/html/rfc3986#appendix-C [[[ In practice, URIs are delimited in a variety of ways, but usually within double-quotes http://example.com/;, angle brackets http://example.com/, or just by using whitespace [...] For robustness, software that accepts user-typed URI should attempt to recognize and strip both delimiters and embedded whitespace. ]] It feels like PHP. But it's not PHP...
Re: make ftp(1) ignore leading whitespace in URLs
On Tue, Nov 05, 2013 at 02:18:08PM +0100, Ingo Schwarze wrote:
Hi Stefan,
Stefan Sperling wrote on Tue, Nov 05, 2013 at 01:56:33PM +0100:
Do others think this useful? I hit this because I made copy/paste errors.
Useful? I don't know. Maybe, maybe not.
But your patch is NOT OK.
Try stuff like
$ ftp ' foo' bar
I would be surprised if you couldn't get it to segfault.
1081: url = strdup(argv[argpos]);
1086: url++;
1073: free(url);
is not a great idiom, imho.
Ooops, thanks! Here's a fixed version.
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.110
diff -u -p -r1.110 fetch.c
--- fetch.c 27 Oct 2013 18:31:24 - 1.110
+++ fetch.c 5 Nov 2013 13:32:53 -
@@ -1044,7 +1044,7 @@ int
auto_fetch(int argc, char *argv[], char *outfile)
{
char *xargv[5];
- char *cp, *url, *host, *dir, *file, *portnum;
+ char *cp, *url, *scheme, *host, *dir, *file, *portnum;
char *username, *pass, *pathstart;
char *ftpproxy, *httpproxy;
int rval, xargc;
@@ -1073,7 +1073,7 @@ auto_fetch(int argc, char *argv[], char
for (rval = 0; (rval == 0) (argpos argc); free(url), argpos++) {
if (strchr(argv[argpos], ':') == NULL)
break;
- host = dir = file = portnum = username = pass = NULL;
+ scheme = host = dir = file = portnum = username = pass = NULL;
/*
* We muck with the string, so we make a copy.
@@ -1085,14 +1085,17 @@ auto_fetch(int argc, char *argv[], char
/*
* Try HTTP URL-style arguments first.
*/
- if (strncasecmp(url, HTTP_URL, sizeof(HTTP_URL) - 1) == 0 ||
+ scheme = url;
+ while (isspace(*scheme))
+ scheme++;
+ if (strncasecmp(scheme, HTTP_URL, sizeof(HTTP_URL) - 1) == 0 ||
#ifndef SMALL
/* even if we compiled without SSL, url_get will check */
- strncasecmp(url, HTTPS_URL, sizeof(HTTPS_URL) -1) == 0 ||
+ strncasecmp(scheme, HTTPS_URL, sizeof(HTTPS_URL) -1) == 0 ||
#endif /* !SMALL */
- strncasecmp(url, FILE_URL, sizeof(FILE_URL) - 1) == 0) {
+ strncasecmp(scheme, FILE_URL, sizeof(FILE_URL) - 1) == 0) {
redirect_loop = 0;
- if (url_get(url, httpproxy, outfile) == -1)
+ if (url_get(scheme, httpproxy, outfile) == -1)
rval = argpos + 1;
continue;
}
Re: make ftp(1) ignore leading whitespace in URLs
fwiw, neither curl nor wget does this.
Re: make ftp(1) ignore leading whitespace in URLs
On 11/05/13 14:44, Stefan Sperling wrote: On Tue, Nov 05, 2013 at 02:08:21PM +0100, Alexander Hall wrote: On 11/05/13 13:56, Stefan Sperling wrote: Before: $ ftp ' http://localhost/snap/INSTALL.amd64' ftp: http: no address associated with name ftp: Can't connect or login to host ` http' After: $ ftp ' http://localhost/snap/INSTALL.amd64' Trying ::1... Trying 127.0.0.1... Requesting http://localhost/snap/INSTALL.amd64 100% |**| 84357 00:00 84357 bytes received in 0.00 seconds (267.27 MB/s) Do others think this useful? I hit this because I made copy/paste errors. Unless you can give a reference saying an URL may start with arbitrary whitespace, I don't want this to go in. Well, I would bet anyone can easily find claims that go either way in some RFC. Here's one that's in favour of this change, FWIW. https://tools.ietf.org/html/rfc3986#appendix-C [[[ In practice, URIs are delimited in a variety of ways, but usually within double-quotes http://example.com/;, angle brackets http://example.com/, or just by using whitespace [...] For robustness, software that accepts user-typed URI should attempt to recognize and strip both delimiters and embedded whitespace. ]] Not sure if a program argument qualifies as user typed (the example extracts urls from a text). That said, jj@ seems to be on your line, so there are at least 2-1 votes in your favor. Just my 2 cents. /Alexander
remove useless #ifdef cruft in sppp(4)
Neither FreeBSD nor NetBSD have these #ifdef *BSD guards anymore.
It doesn't seem worthwhile to keep them in OpenBSD.
There was also one typo (__OpenBSD_ vs __OpenBSD__) which kept
the code compiling.
No binary change. ok?
Index: if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.109
diff -u -p -r1.109 if_spppsubr.c
--- if_spppsubr.c 24 Oct 2013 11:31:43 - 1.109
+++ if_spppsubr.c 5 Nov 2013 14:27:45 -
@@ -48,12 +48,8 @@
#include sys/mbuf.h
#include sys/workq.h
-#if defined (__OpenBSD__)
#include sys/timeout.h
#include crypto/md5.h
-#else
-#include sys/md5.h
-#endif
#include net/if.h
#include net/netisr.h
@@ -63,11 +59,6 @@
/* for arc4random() */
#include dev/rndvar.h
-#if defined (__FreeBSD__) || defined(__OpenBSD_) || defined(__NetBSD__)
-#include machine/random.h
-#endif
-#if defined (__NetBSD__) || defined (__OpenBSD__)
-#endif
#include sys/stdarg.h
#ifdef INET
@@ -76,25 +67,13 @@
#include netinet/in_var.h
#include netinet/ip.h
#include netinet/tcp.h
-# if defined (__FreeBSD__) || defined (__OpenBSD__)
-# include netinet/if_ether.h
-# else
-# include net/ethertypes.h
-# endif
+#include netinet/if_ether.h
#endif
#include net/if_sppp.h
-#if defined (__FreeBSD__)
-# define UNTIMEOUT(fun, arg, handle) \
- untimeout(fun, arg, handle)
-#elif defined(__OpenBSD__)
# define UNTIMEOUT(fun, arg, handle) \
timeout_del((handle))
-#else
-# define UNTIMEOUT(fun, arg, handle) \
- untimeout(fun, arg)
-#endif
#define LOOPALIVECNT 3 /* loopback detection tries */
#define MAXALIVECNT3 /* max. missed alive
packets */
@@ -252,20 +231,10 @@ struct cp {
};
static struct sppp *spppq;
-#if defined (__OpenBSD__)
static struct timeout keepalive_ch;
-#endif
-#if defined (__FreeBSD__)
-static struct callout_handle keepalive_ch;
-#endif
-#if defined (__FreeBSD__)
-#defineSPP_FMT %s%d:
-#defineSPP_ARGS(ifp) (ifp)-if_name, (ifp)-if_unit
-#else
#defineSPP_FMT %s:
#defineSPP_ARGS(ifp) (ifp)-if_xname
-#endif
/* almost every function needs these */
#define STDDCL \
@@ -461,13 +430,11 @@ static const struct cp *cps[IDX_COUNT] =
* Exported functions, comprising our interface to the lower layer.
*/
-#if defined(__OpenBSD__)
/* Workaround */
void
spppattach(struct ifnet *ifp)
{
}
-#endif
/*
* Process the received packet.
@@ -892,12 +859,8 @@ sppp_attach(struct ifnet *ifp)
/* Initialize keepalive handler. */
if (! spppq) {
-#if defined (__FreeBSD__)
- keepalive_ch = timeout(sppp_keepalive, 0, hz * 10);
-#elif defined(__OpenBSD__)
timeout_set(keepalive_ch, sppp_keepalive, NULL);
timeout_add_sec(keepalive_ch, 10);
-#endif
}
/* Insert new entry into the keepalive list. */
@@ -1197,11 +1160,7 @@ sppp_cisco_input(struct sppp *sp, struct
++sp-pp_loopcnt;
/* Generate new local sequence number */
-#if defined (__FreeBSD__) || defined (__NetBSD__) || defined(__OpenBSD__)
sp-pp_seq = arc4random();
-#else
- sp-pp_seq ^= time.tv_sec ^ time.tv_usec;
-#endif
break;
}
sp-pp_loopcnt = 0;
@@ -1890,12 +1849,7 @@ sppp_increasing_timeout (const struct cp
timo = sp-lcp.max_configure - sp-rst_counter[cp-protoidx];
if (timo 1)
timo = 1;
-#if defined(__FreeBSD__) __FreeBSD__ = 3
- sp-ch[cp-protoidx] =
- timeout(cp-TO, (void *)sp, timo * sp-lcp.timeout);
-#elif defined(__OpenBSD__)
timeout_add(sp-ch[cp-protoidx], timo * sp-lcp.timeout);
-#endif
}
HIDE void
@@ -2016,9 +1970,6 @@ sppp_lcp_init(struct sppp *sp)
sp-lcp.max_terminate = 2;
sp-lcp.max_configure = 10;
sp-lcp.max_failure = 10;
-#if defined (__FreeBSD__)
- callout_handle_init(sp-ch[IDX_LCP]);
-#endif
}
HIDE void
@@ -2606,11 +2557,7 @@ sppp_lcp_scr(struct sppp *sp)
if (sp-lcp.opts (1 LCP_OPT_MAGIC)) {
if (! sp-lcp.magic)
-#if defined (__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
sp-lcp.magic = arc4random();
-#else
- sp-lcp.magic = time.tv_sec + time.tv_usec;
-#endif
opt[i++] = LCP_OPT_MAGIC;
opt[i++] = 6;
opt[i++] = sp-lcp.magic 24;
@@ -2686,9 +2633,6 @@ sppp_ipcp_init(struct sppp *sp)
sp-ipcp.flags = 0;
sp-state[IDX_IPCP] = STATE_INITIAL;
sp-fail_counter[IDX_IPCP] = 0;
-#if defined (__FreeBSD__)
- callout_handle_init(sp-ch[IDX_IPCP]);
-#endif
}
HIDE void
@@ -3162,9 +3106,6 @@ sppp_ipv6cp_init(struct sppp *sp)
sp-ipv6cp.flags = 0;
Re: in6_leavegroup work queue
Alexander, I spent quite some time working on this problem and I found some interesting information, see below. On 31/10/13(Thu) 17:20, Alexander Bluhm wrote: On Thu, Oct 31, 2013 at 09:56:11AM +0100, Martin Pieuchot wrote: On 30/10/13(Wed) 16:48, Alexander Bluhm wrote: [...] I'm not sure to get the whole picture about this ioctl called in an interrupt context so feel free to correct me, but the only offending function is nd6_timer(), is that right? No, this code is also called from softnet interrupt context. An autoconfigured IPv6 address can deleted by a timeout or by a router advertisement. nd6_ra_input() - prelist_update() - nd6_prelist_add() - purge_detached() - in6_purgeaddr() - in6_leavegroup() By looking at the history of nd6_rtr.c, rev 1.54 tell us that this problem of adding/deleting a prefix or address in interrupt context is not new. Unfortunately the hack introduced at that time only works for a subset of prelist_update(): creating an addresses. But the story of prelist_update() does not end here. Since rev 1.55 was introduced to work around another problem caused by the fact that prelist_update() can run multiple times before the task scheduled to create an address has run. So I think that we should modify how/when prelist_update() is executed, In theory we can ignore the incomming packet, we may always loose packets and hope for a retransmit. But that would mean to avoid the sleep and do a rollback. That would be a horrible diff. I'm not sure to understand the whole picture once again, but can't we defer the (whole) processing of a router advertisement message in a task? Finally since we are moving away from workq(9) to task(9) I'd suggest you to use the latter. That is easy, but it does not solve the problems you describe. I have to call task_set() in in6_leavegroup() to set the if_index. May be calling task_set() can be moved to somewhere else but that does not look easy. I am still not happy with this diff although I think it does not cause memory corruption. Look at this call path: if_detach() - in6_ifdetach() - in6_purgeaddr() - in6_leavegroup() As I delay the SIOCDELMULTI after the interface is destroyed, the ioctl is never called. That doesn't matter ;) If the interface is gone there's no hardware filter to update. Perhaps we look at the problem from the wrong direction. Why do some network drivers sleep when changing multicast groups? Did they always do that or was something changed? Some driver sleep because that's how their author/porter wrote them since they are allowed to sleep in ioctls. Most (all) of the USB drivers are written this way, since synchronous transfer submission makes the caller sleep until the transaction is processed. But our problem here is that the nd6 code *abuse* the ioctl interface in interrupt context. So we could take another direction and design a new interface that does not allow to sleep when updating the multicast filters of an interface. But I believe it would requires much more changes and since this whole prelist_update() looks like a whole hack to me, I would prefer if someone could clean it. Martin
Re: Re : Re: Improve routing functions
On 13-11-04 05:09 PM, Claudio Jeker wrote: On Mon, Nov 04, 2013 at 10:36:39AM -0600, Adam Thompson wrote: The change I think we're both asking for is that in .../usr/sbin/bgpd/kroute.c, on line 505 (5.4-RELEASE), where we see kr-r.priority = RTP_BGP;, we need a way to override that value in (presumably) bgpd.conf. (Ditto for the IPv6 function.) Yes, this is the way to go. One thing to consider in some way is what you want to do if somebody changes the prio and the reloads the config. Or, in bgpd.conf(8) note under fib-priority that Changes to fib-priority will only take effect when bgpd(8) is restarted. Yeah, not ideal, but better than nothing. ldpd is a special beast and does not need to be changed but the other should be. (routed is dead, ripd ospfd and ospf6d all use a similar kroute.c file that also is used by bgpd so once one is done the others should be easier). ldpd does not insert new routes it only extends them with the MPLS info. Yes, it's unfortunate that routed(8) got removed from base. RIPv2 is a perfectly workable routing protocol in many situations. Wow, I didn't realize it's been missing since 4.4-release! Please someone not as overworked as me should just add the knobs to the routing deamons to allow setting a different prio. For example just a simple: fib-priority 45 would work. I can do doc changes, but I think you really, *really* don't want me writing much C code... I can perhaps do some of the initial, trivial, legwork, at best. Now, if bgpd(8) were written in Bourne/Korn shell, I'd be the guy to do this! -- -Adam Thompson athom...@athompso.net
Re: Re : Re: Improve routing functions
On 13-11-05 10:12 AM, Adam Thompson wrote:
I can do doc changes, but I think you really, *really* don't want me
writing much C code... I can perhaps do some of the initial, trivial,
legwork, at best. Now, if bgpd(8) were written in Bourne/Korn shell,
I'd be the guy to do this!
I recall now that I've never used yacc... Here's what I *can*
contribute, which is not much.
I wonder about including it as a global, per-neighbour, and per-match,
but it seems to be consistent with what's already implemented.
It would be acceptable to have it only as an attribute;
It would be more reasonable to have it as a global config item and an
attribute;
And at that point it may as well be settable on a neighbour basis, too.
--
-Adam Thompson
athom...@athompso.net
Index: bgpd.conf.5
===
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
retrieving revision 1.129
diff -u -p -r1.129 bgpd.conf.5
--- bgpd.conf.5 19 Oct 2013 15:04:25 - 1.129
+++ bgpd.conf.5 5 Nov 2013 17:45:23 -
@@ -206,6 +206,14 @@ dump all out /tmp/all-out-%H%M 300
dump updates out /tmp/updates-out-%H%M 300
.Ed
.Pp
+.It Ic fib-priority Ar priority
+If set, override the route priority value used to insert BGP routes
+into the kernel routing table. The default is
+.Ic RTP_BGP
+as defined in /usr/include/sys/net/route.h.
+.Pp
+Currently not implemented.
+.Pp
.It Xo
.Ic fib-update
.Pq Ic yes Ns | Ns Ic no
@@ -459,6 +467,14 @@ should be set to
.Ar rt
for best compatibility with other implementations.
.Pp
+.It Ic fib-priority Ar priority
+If set, override the route priority value used to insert BGP routes
+into the kernel routing table. The default is
+.Ic RTP_BGP
+as defined in /usr/include/sys/net/route.h.
+.Pp
+Currently not implemented.
+.Pp
.It Xo
.Ic fib-update
.Pq Ic yes Ns | Ns Ic no
@@ -757,6 +773,14 @@ The default value for IBGP peers is
otherwise the default is
.Ic yes .
.Pp
+.It Ic fib-priority Ar priority
+If set, override the route priority value used to insert BGP routes
+into the kernel routing table. The default is
+.Ic RTP_BGP
+as defined in /usr/include/sys/net/route.h.
+.Pp
+Currently not implemented.
+.Pp
.It Ic holdtime Ar seconds
Set the holdtime in seconds.
Inherited from the global configuration if not given.
@@ -1365,6 +1389,14 @@ bdc BGP Data Collection
.Pp
Not all type and subtype value pairs are allowed by IANA and the parser
will ensure that no invalid combination is created.
+.Pp
+.It Ic fib-priority Ar priority
+If set, override the route priority value used to insert BGP routes
+into the kernel routing table. The default is
+.Ic RTP_BGP
+as defined in /usr/include/sys/net/route.h.
+.Pp
+Currently not implemented.
.Pp
.It Ic localpref Ar number
Set the
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
retrieving revision 1.268
diff -u -p -r1.268 parse.y
--- parse.y 19 Oct 2013 15:04:25 - 1.268
+++ parse.y 5 Nov 2013 17:45:23 -
@@ -171,6 +171,7 @@ typedef struct {
%}
%token AS ROUTERID HOLDTIME YMIN LISTEN ON FIBUPDATE RTABLE
+%token FIBPRIORITY
%token RDOMAIN RD EXPORTTRGT IMPORTTRGT
%token RDE RIB EVALUATE IGNORE COMPARE
%token GROUP NEIGHBOR NETWORK
@@ -2139,6 +2140,7 @@ lookup(char *s)
{ evaluate, EVALUATE},
{ export-target, EXPORTTRGT},
{ ext-community, EXTCOMMUNITY},
+ { fib-priority, FIBPRIORITY},
{ fib-update, FIBUPDATE},
{ from, FROM},
{ group, GROUP},
Re: Amd64 relocation R_X86_64_32S in a static lib
On Tue, 5 Nov 2013, Torbjorn Granlund wrote: Philip Guenther guent...@gmail.com writes: Ah, but you are, sorta. In OpenBSD 5.3, platforms where the compiler and toolchain support were for robust for it were switched to build PIE objects and executables by default. So yes, that object _is_ expected to be position independent. c.f. the gcc-local(1) manpage. At least on OpenBSD, the processor will define __PIC__ and __pic__ when building both PIC _and_ PIE objects, so you can test those to determine whether the ASM should use position-independent code sequences. I am aware of that people have started using PIE as a novel term for PIC, sometimes with a subtle difference in meaning, sometimes with the exact same meaning. It is however evident that you're making some much larger distinction. Which one? We give it the same meaning as gcc and its documentation. For example: `-fpie' `-fPIE' These options are similar to `-fpic' and `-fPIC', but generated position independent code can be only linked into executables. Usually these options are used when `-pie' GCC option will be used during linking. R_X86_64_64 is safe for use in PIC/PIE code, though I would expect %rip relative addressing to be more efficient when it's applicable. Now I get really confused. You disallow R_X86_64_32S which is of the type S + A but allow R_X86_64_64 which is also of the type S + A. That simply makes no sense at all. If you allow R_X86_64_64 in a relocatable executable, that you will need load-time patching, which precludes any sharing. Yeah, I spoke too broadly: R_X86_64_64 is only okay for relocations in writable segments. The OpenBSD libc has some of those in its .data segment, for exmaple. You're correct that R_X86_64_64 relocations against the text segment prevent sharing. That's bad. But then why not allow R_X86_64_32S too? Because there's no guarantee that the target is within 2GB, as the reference may resolve to the symbol in a different shared object which is too far away. What was the exact decision for the change in 5.3. Is it spelled out somewhere for me to read? Exact decision for the change? I'm not sure what you mean by 'decision' there. If you're wondering about the _reason_ for the change (why we did it), the answer is so that ASLR is applied not just to the code in shared libraries but also the code in executables. If you're wondering what was done to implement the change, the part that seems to be annoying you is that in 5.3 the default for gcc is to pass -fpie to the C compiler and -pie to the linker. Is there some code which will work for all OpenBSD AMD64 releases? E.g., can I count on a PLT and GOT always? I really don't want to support many ABIs for one OS, such support tend to be fragile. Yes, you can. If you look at libc.a, for example, you'll see it contains R_X86_64_GOTPCREL and R_X86_64_PLT32 relocations. Those work both when building a PIE executable and when building a fully static non-PIE executable. In the latter case they are resolved by the linker when generating the executable. Philip Guenther
Add [-q timeout] option to netcat
Hi,
this patch adds this option to nc(1):
-q timeout
after end-of-file on stdin, wait timeout seconds and then quit.
The default is no timeout.
This should be compatible with the -q option of the netcat-openbsd
package in debian (what the heck ?)
I use this to send simple udp datagrams:
echo 'Hello World!' |obj/nc -uq0 88.88.88.88 88
ok?
Christopher
Index: nc.1
===
RCS file: /cvs/src/usr.bin/nc/nc.1,v
retrieving revision 1.65
diff -u -p -r1.65 nc.1
--- nc.120 Aug 2013 21:05:20 - 1.65
+++ nc.15 Nov 2013 20:09:32 -
@@ -172,6 +172,11 @@ should use, subject to privilege restric
It is an error to use this option in conjunction with the
.Fl l
option.
+.It Fl q Ar timeout
+after end-of-file on stdin, wait
+.Ar timeout
+seconds and then quit.
+The default is no timeout.
.It Fl r
Specifies that source and/or destination ports should be chosen
randomly instead of sequentially within a range or in the order that
the system Index: netcat.c
===
RCS file: /cvs/src/usr.bin/nc/netcat.c,v
retrieving revision 1.117
diff -u -p -r1.117 netcat.c
--- netcat.c26 Oct 2013 21:33:29 - 1.117
+++ netcat.c5 Nov 2013 20:09:33 -
@@ -90,6 +90,7 @@ int Tflag =
-1; /* IP Type of Service int
rtableid = -1;
int timeout = -1;
+int fintime = -1;
int family = AF_UNSPEC;
char *portlist[PORT_MAX+1];
char *unix_dg_tmp_socket;
@@ -136,7 +137,7 @@ main(int argc, char *argv[])
rtableid = getrtable();
while ((ch = getopt(argc, argv,
- 46DdFhI:i:klNnO:P:p:rSs:tT:UuV:vw:X:x:z)) != -1) {
+ 46DdFhI:i:klNnO:P:p:q:rSs:tT:UuV:vw:X:x:z)) != -1) {
switch (ch) {
case '4':
family = AF_INET;
@@ -216,6 +217,12 @@ main(int argc, char *argv[])
errx(1, timeout %s: %s, errstr,
optarg); timeout *= 1000;
break;
+ case 'q':
+ fintime = strtonum(optarg, 0, INT_MAX / 1000,
errstr);
+ if (errstr)
+ errx(1, timeout %s: %s, errstr,
optarg);
+ fintime *= 1000;
+ break;
case 'x':
xflag = 1;
if ((proxy = strdup(optarg)) == NULL)
@@ -782,6 +789,8 @@ readwrite(int nfd)
else if (n == 0) {
if (Nflag)
shutdown(nfd, SHUT_WR);
+ if (fintime = 0)
+ timeout = fintime;
pfd[1].fd = -1;
pfd[1].events = 0;
} else {
@@ -1090,6 +1099,7 @@ help(void)
\t-O length TCP send buffer length\n\
\t-P proxyuser\tUsername for proxy authentication\n\
\t-p port\t Specify local port for remote connects\n\
+ \t-q secs\t Timeout for final net reads after EOF is
read on stdin\n\ \t-r Randomize remote ports\n\
\t-SEnable the TCP MD5 signature option\n\
\t-s addr\t Local source address\n\
@@ -1113,7 +1123,7 @@ usage(int ret)
fprintf(stderr,
usage: nc [-46DdFhklNnrStUuvz] [-I length] [-i interval]
[-O length]\n \t [-P proxy_username] [-p source_port] [-s source]
[-T ToS]\n
- \t [-V rtable] [-w timeout] [-X proxy_protocol]\n
+ \t [-V rtable] [-w timeout] [-q timeout] [-X
proxy_protocol]\n \t [-x proxy_address[:port]] [destination]
[port]\n); if (ret)
exit(1);
--
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
1917 680A 723C BF3D 2CA3 0E44 7E24 D19F 34B8 2A2A
signature.asc
Description: PGP signature
Re: Amd64 relocation R_X86_64_32S in a static lib
Torbjorn Granlund t...@gmplib.org writes: Now we have (at least) two OpenBSD ABIs for AMD64, pre 5.3 and now 5.3, 5.4. To make sense of things, I would not be surprised to see R_X86_64_64 banned from 5.5 and on, creating a 3rd OpenBSD AMD64 ABI. I don't understand the fine details of which reloc types make sense in pic code, but if I understand Philip correctly, the main problem is not a ABI change, but changed compiler default. And then you get link errors when linking together pic objects created by the compiler, with non-pic objects created from the gmp assembly files. I think you'd get similar problems as if a user configures gmp with, e.g., --disable-shared CFLAGS=-fpic. Do you agree with this analysis? To figure out if an invocation $(CC) $(CFLAGS) generates pic code or not, one needs a configure test like AC_TRY_COMPILE( ... #if __PIC__ #error bla bla #endif ... ) Using the result of that test when deciding whether or not assembly files should use pic mode should give the correct behavior, both with compilers doing pic by default, and users enabling it explicitly. /Niels -- Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26. Internet email is subject to wholesale government surveillance.
Re: Add [-q timeout] option to netcat
I just noticed my patch's bedaviour is actually different to the behaviour of the debian version. for my diff the man page should actually say -q timeout after end-of-file on stdin, timeout the connection after timeout seconds. A timeout of 0 will close the connection after reading all pending data. debian version would be: -q after EOF on stdin, wait the specified number of seconds and then quit. If seconds is negative, wait forever. They implemented it using signal(SIGALRM, quit); alarm(timeout); I have no preference, since I just care for the -q 0 case. Christopher -- http://gmerlin.de OpenPGP: http://gmerlin.de/christopher.pub 1917 680A 723C BF3D 2CA3 0E44 7E24 D19F 34B8 2A2A signature.asc Description: PGP signature
Re: Amd64 relocation R_X86_64_32S in a static lib
From: ni...@lysator.liu.se (Niels =?iso-8859-1?Q?M=F6ller?=) Date: Tue, 05 Nov 2013 15:39:35 +0100 Torbjorn Granlund t...@gmplib.org writes: Now we have (at least) two OpenBSD ABIs for AMD64, pre 5.3 and now 5.3, 5.4. To make sense of things, I would not be surprised to see R_X86_64_64 banned from 5.5 and on, creating a 3rd OpenBSD AMD64 ABI. I don't understand the fine details of which reloc types make sense in pic code, but if I understand Philip correctly, the main problem is not a ABI change, but changed compiler default. And then you get link errors when linking together pic objects created by the compiler, with non-pic objects created from the gmp assembly files. Not really. The problem is linking non-pic objects created from the gmp assembly files into a position-independent executable. And this is in fact very similar to the problems you get when you try to link non-pic objects into a shared library. I think you'd get similar problems as if a user configures gmp with, e.g., --disable-shared CFLAGS=-fpic. Do you agree with this analysis? Well, linking pic objects into an executable will work on all common ELF platforms. So to reproduce the problem on platforms that have traditional position-dependent executables, you'd have to configure with something like: --disable-shared CFLAGS=-fpic LDFLAGS=-Wl,-pie Cheers, Mark
Re: fix seekdir(3)
Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100: Then i will send two cleanup patches to remove useless stuff and put the code into the right place, not changing any functionality. Done committed (thanks to Otto and Todd for checking). Finally, we can work out how to do the optimization. Probably, that will naturally factorize into two steps: (1) Use the information available in the userland buffer to avoid the getdents(2) syscall, *without* assuming monotonicity. That is done by the patch below; i'm asking for OKs. Note that, even though it looks superficially similar to the patch i sent originally, this one does not require monotonicity. To confirm that it is really an optimization, i did some measurements on my notebook: * The typical time required for a seekdir()/readdir() pair when the entry asked for is right at the beginning of the userland buffer is about 0.05 microseconds * The typical time required for searching through the whole userland buffer from its beginning to its end is about 5 microseconds * The typical time required for one getdents() syscall is about 100 microseconds Consequently, we have the following typical edge cases: * Best case: opendir; readdir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is near the beginning of the buffer. For that case, the following patch reduces total execution time from 10.7s to 0.005s (-99.95%). * Maximum searching case: opendir; 500 times readdir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is near the end of the buffer. For that case, the following patch reduces total execution time from 10.7s to 0.5s (-95%). * Worst case: opendir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is the very first entry in the buffer, which cannot be found, because each dirent only contains the address of the *following* dirent, so each readdir triggers getdents anyway, but we still search the whole buffer each time. For that case, the following patch increases total execution time from 10.7s to 11.3s (+5%). So in some cases, very large speedups are possible, for a relatively small cost in the (very unlikely) worst case. (2) Further optimize searching when monotonicity is available. Given the data above, i don't consider further optimization worthwhile any longer, so i consider what i'm sending here to be the final optimization patch. There are two cases where further optimization might in principle be possible based on monotonicity: * Avoid searching through the whole buffer when monotonicity allows to conclude that the entry cannot be anywhere in the buffer. But in that case, getdents() is needed anyway, which is so much more expensive than searching the buffer that the latter is practically irrelevant. * Exploit monotonicity to replace the linear search by a binary search. In the best case - the entries searched for are always near the end of the buffer - that might theoretically reduce execution times by a factor of up to 50. However, that only applies to a very narrow range of problems, specifically directories with several hundred but less than about 500 entries. For larger directories, the buffer cannot hold the whole directory at once, so execution times will be dominated by the required getdents() calls, and the search algorithm becomes irrelevant. So, in the best case, we could save up to 5 microseconds per seekdir()/readdir() pair in directories of 500 entries. That's at most a few milliseconds per iteration through the directory. So, is anybody going to have tens of thousands of directories, each just below 500 entries, and iterate all of them with seekdir()/readdir()? Not likely. I don't think this remote possibility warrants additional complication of the code, and much less introducing a new pathconf(2) value. That said, here is the optimization patch: Yours, Ingo Index: opendir.c === RCS file: /cvs/src/lib/libc/gen/opendir.c,v retrieving revision 1.25 diff -u -p -r1.25 opendir.c --- gen/opendir.c 6 Oct 2013 17:57:11 - 1.25 +++ gen/opendir.c 5 Nov 2013 22:58:40 - @@ -111,6 +111,7 @@ __fdopendir(int fd) return (NULL); } + dirp-dd_size = 0; dirp-dd_loc = 0; dirp-dd_fd = fd; dirp-dd_lock = NULL; Index: seekdir.c === RCS file: /cvs/src/lib/libc/gen/seekdir.c,v retrieving revision 1.10 diff -u -p -r1.10 seekdir.c --- gen/seekdir.c 5 Nov 2013 20:36:51 - 1.10 +++ gen/seekdir.c 5 Nov 2013 22:58:40 - @@ -1,31 +1,18 @@ /* $OpenBSD: seekdir.c,v 1.10 2013/11/05 20:36:51 schwarze Exp
Re: fix seekdir(3)
On Wed, 6 Nov 2013, Ingo Schwarze wrote: Ingo Schwarze wrote on Mon, Nov 04, 2013 at 09:51:41AM +0100: Finally, we can work out how to do the optimization. Probably, that will naturally factorize into two steps: (1) Use the information available in the userland buffer to avoid the getdents(2) syscall, *without* assuming monotonicity. That is done by the patch below; i'm asking for OKs. Note that, even though it looks superficially similar to the patch i sent originally, this one does not require monotonicity. To confirm that it is really an optimization, i did some measurements on my notebook: ... * Worst case: opendir; telldir then 100.000 times (seekdir; readdir) at that position = The entry asked for is the very first entry in the buffer, which cannot be found, because each dirent only contains the address of the *following* dirent, so each readdir triggers getdents anyway, but we still search the whole buffer each time. While caching the offset of the start of the buffer is possible, it's not obvious that that case will happen often enough to be worth it. Hmm, rewinddir() will _always_ be this worst case. Perhaps rewinddir() should not call seekdir() and just be the two assignments with lseek(), skipping the scan of the current buffer. It would be assisted by the start-o-buffer cache though. (2) Further optimize searching when monotonicity is available. Given the data above, i don't consider further optimization worthwhile any longer, so i consider what i'm sending here to be the final optimization patch. There are two cases where further optimization might in principle be possible based on monotonicity: * Avoid searching through the whole buffer when monotonicity allows to conclude that the entry cannot be anywhere in the buffer. But in that case, getdents() is needed anyway, which is so much more expensive than searching the buffer that the latter is practically irrelevant. Yeah, doesn't seem like much a win. * Exploit monotonicity to replace the linear search by a binary search. Doing a binary search would be quite tricky given the variable-size of the entries. Not worth it. That said, here is the optimization patch: Looks good. Nice analysis. ok guenther@
Re: make ftp(1) ignore leading whitespace in URLs
On Tue, Nov 05, 2013 at 08:54:00PM +0100, Marc Espie wrote: On Tue, Nov 05, 2013 at 02:20:00PM +0100, Janne Johansson wrote: I think this would help the port yt to not crash on such urls, and I think it uses ftp to collect the youtube movies. Fix yt, then. I hate this. Like others say, it makes no sense. If I pass spaces to programs, I mean space. Completely agree with Marc here. -Otto
Re: make ftp(1) ignore leading whitespace in URLs
On Tue, Nov 05, 2013 at 08:54:00PM +0100, Marc Espie wrote: On Tue, Nov 05, 2013 at 02:20:00PM +0100, Janne Johansson wrote: I think this would help the port yt to not crash on such urls, and I think it uses ftp to collect the youtube movies. Fix yt, then. I hate this. Like others say, it makes no sense. If I pass spaces to programs, I mean space. Completely agree with Marc here. the example I privately gave was ls -l
