Re: PATCH: netstat '-p' optional feature
On Mon, Jul 28, 2008 at 09:54:35AM +0200, Denys Vlasenko wrote: > Applied, with 1k of shrinkage on top of it. Please try current svn. Tried it, and it works like a champ ! Thanks again, Gabriel ___ busybox mailing list busybox@busybox.net http://busybox.net/cgi-bin/mailman/listinfo/busybox
Re: PATCH: netstat '-p' optional feature
On Sun, Jul 27, 2008 at 02:07:29PM +0200, Denys Vlasenko wrote:
>
> See attached patch where these (and some more) are fixed.
>
That looks great to me. Only question is about the following section:
> #define PROGNAME_WIDTH 20
> #define PROGNAME_WIDTH_STR "20"
> /* PROGNAME_WIDTH chars: 12345678901234567890 */
> #define PROGNAME_BANNER "PID/Program name"
>
> struct prg_node {
> struct prg_node *next;
> long inode;
> char name[PROGNAME_WIDTH];
> };
>
> #define PRG_HASH_SIZE 211
Don't you want to wrap it in #if ENABLE_FEATURE_NETSTAT_PRG ? I don't
think the compiler would care either way, but everything else -p
related is wrapped like that... :)
Otherwise, if you'd apply it, I'd be very happy :)
Thanks,
Gabriel
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
Re: PATCH: netstat '-p' optional feature
On Wed, Jul 23, 2008 at 09:38:41AM +0200, Bernhard Fischer wrote:
> bloat-o-meter output? size(1) before and after your patch, with and
> without -p support?
The following numbers are for the newly reworked patch
(please see attached).
Before:
$ size networking/netstat.o
textdata bss dec hex filename
3426 5 03431 d67 networking/netstat.o
After, without -p support compiled in (but with the nice, fixed,
standard options and globals code):
$ size networking/netstat.o
textdata bss dec hex filename
3487 0 03487 d9f networking/netstat.o
After, with -p support built in:
$ size networking/netstat.o
textdata bss dec hex filename
4642 0 046421222 networking/netstat.o
> >+#define PROGNAME_WIDTH 20
> >+#define PROGNAME_WIDTHs "%-20s"
>
> Just curious why you don't use PROGNAME_WIDTH here?
The original, which Denys didn't like on account of being too
complicated, looked like this:
#define PROGNAME_WIDTH 20
#define PROGNAME_WIDTHs PROGNAME_WIDTH1(PROGNAME_WIDTH)
#define PROGNAME_WIDTH1(s) PROGNAME_WIDTH2(s)
#define PROGNAME_WIDTH2(s) #s
This allowed PROGNAME_WIDTHs to be used inside a printf format string:
printf("%-" PROGNAME_WIDTHs "s", some_string);
You can't use PROGNAME_WIDTH directly, since that would substitute to:
printf("%-" 20 "s", some_string);
which is a syntax error. Also,
printf("%-PROGNAME_WIDTHs", some_string);
is obviously broken, too.
You can't
#define PROGNAME_WIDTHs "PROGNAME_WIDTH"
either, because you'd end up with the literal string instead of the
desired \"20\"
I'm not an expert on C preprocessor voodoo by any means, but the
original form which Denys didn't like did accomplish creating a preproc.
variable PROGNAME_WIDTHs which automatically takes the string form of
whatever PROGNAME_WIDTH is set to ("20" for 20, "30" for 30, etc).
The (more readable) alternative is to have to modify both by hand. If
you know how to automatically get PROGNAME_WIDTHs to adjust when
modifying PROGNAME_WIDTH, any simpler than the WIDTH1() WIDTH2() trick
above, let me know... Or, how to jam PROGNAME_WIDTH into a printf
format string directly -- I tried and it didn't get substituted :(
>
> >+static void prg_cache_add(int inode, char *name)
...
> >+}
> >+*pnp = xmalloc(sizeof(struct prg_node));
> >+pn = *pnp;
> >+pn->next = NULL;
>
> an xzalloc would have rendered nullifying next moot.
Done.
> >+static const char *prg_cache_get(int inode)
> >+{
> >+unsigned hi = PRG_HASHIT(inode);
> >+struct prg_node *pn;
> >+
> >+for (pn = prg_hash[hi]; pn; pn = pn->next)
> >+if (pn->inode == inode)
> >+return pn->name;
>
> argh, that's exactly what index_in_strings should have been used for.
Not sure I get that -- index_in_strings searches a contiguous
concatenation of null-terminated strings; netstat has a hash array
with linked lists hanging off each entry (and the list elements have a
string *and* an int member)...
> nah. Use recursive_action() instead of growing your own (huge!) impl
> for it.
Done. However, this requires the addition of an ACTION_QUIET flag to
recursive_action() (also included in the attached patch). We do NOT
want recursive_action() to print to stderr each time it gets a
permission error when trying to open /proc entries, should the user
run netstat as non-root !!!
> The preferred way look like:
>
> ->+#if ENABLE_FEATURE_NETSTAT_PRG
> ->+ if (prg_flag)
> +>+ if (ENABLE_FEATURE_NETSTAT_PRG && prg_flag)
Respectfully disagree. If ENABLE_FEATURE_NETSTAT_PRG is not enabled,
prg_flag is undeclared, and we get a compile-time error.
Am I missing anything ?
> >+printf(PROGNAME_WIDTHs, prg_cache_get(inode));
> >+#endif
> >+printf("\n");
>
> Is that \n here on purpose?
Yes. I want to print the string "foo bar\n" with the following
restrictions:
if ENABLE_FEATURE_NETSTAT_PRG is undefined, I only want "foo\n",
always.
if ENABLE_FEATURE_NETSTAT_PRG is true, I want "foo bar\n" if the -p flag
was supplied on the command line, otherwise I still only want "foo\n".
My solution to this was to first print "foo" without \n
then, #if ENABLE_FEATURE_NETSTAT_PRG to check for prg_flag, and if
true to print "bar"
then, last thing, I always print "\n".
You don't *always* want to print the program name and pid if
ENABLE_FEATURE_NETSTAT_PRG is true, so just ifdef-ing the format
string from the get-go isn't a solution.
>
> Sounds a bit like you are too newline friendly ;)
Maybe, and maybe I've been staring at this thing for too long and the
obvious simple and clean solution escapes me... How would you do it ? :)
> This sounds way too big, please provide the bloat-o-meter and size(1)
> output i mentioned above, and please take my comments into account.
Done -- please let me know your thoughts !
--Gabriel
> TIA,
> Bernhard
>
diff -NarU5
Re: PATCH: netstat '-p' optional feature
On Tue, Jul 22, 2008 at 11:00:57PM +0200, Denys Vlasenko wrote:
> Can you do it?
Allright, how about this one ?
--Gabriel
diff -NarU5 busybox-svn-22735/include/usage.h
busybox-svn-22735-netstatp/include/usage.h
--- busybox-svn-22735/include/usage.h 2008-07-09 23:04:08.0 -0400
+++ busybox-svn-22735-netstatp/include/usage.h 2008-07-16 20:37:41.0
-0400
@@ -2813,11 +2813,11 @@
/* "\nport numbers can be individual or ranges: lo-hi [inclusive]" */
#endif
#define netstat_trivial_usage \
- "[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")"]"
+
"[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")USE_FEATURE_NETSTAT_PRG("p")"]"
#define netstat_full_usage "\n\n" \
"Display networking information\n" \
"\nOptions:" \
"\n -l Display listening server sockets" \
"\n -a Display all sockets (default: connected)" \
@@ -2828,10 +2828,13 @@
"\n -w Raw sockets" \
"\n -x Unix sockets" \
"\n -r Display routing table" \
USE_FEATURE_NETSTAT_WIDE( \
"\n -W Display with no column truncation" \
+ ) \
+ USE_FEATURE_NETSTAT_PRG( \
+ "\n -p Display PID/Program name for sockets" \
)
#define nice_trivial_usage \
"[-n ADJUST] [COMMAND [ARG]...]"
#define nice_full_usage "\n\n" \
diff -NarU5 busybox-svn-22735/networking/Config.in
busybox-svn-22735-netstatp/networking/Config.in
--- busybox-svn-22735/networking/Config.in 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735-netstatp/networking/Config.in 2008-07-16
20:37:41.0 -0400
@@ -630,10 +630,17 @@
depends on NETSTAT
help
Add support for wide columns. Useful when displaying IPv6 addresses
(-W option).
+config FEATURE_NETSTAT_PRG
+ bool "Enable PID/Program name output"
+ default n
+ depends on NETSTAT
+ help
+ Add support for -p flag to print out PID and program name.
+
config NSLOOKUP
bool "nslookup"
default n
help
nslookup is a tool to query Internet name servers.
diff -NarU5 busybox-svn-22735/networking/netstat.c
busybox-svn-22735-netstatp/networking/netstat.c
--- busybox-svn-22735/networking/netstat.c 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735-netstatp/networking/netstat.c 2008-07-22
22:32:52.0 -0400
@@ -6,22 +6,41 @@
* Copyright (C) 2002 by Bart Visscher <[EMAIL PROTECTED]>
*
* 2002-04-20
* IPV6 support added by Bart Visscher <[EMAIL PROTECTED]>
*
+ * 2008-07-10
+ * optional '-p' flag support ported from net-tools by G. Somlo <[EMAIL
PROTECTED]>
+ *
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
#include "libbb.h"
#include "inet_common.h"
+#define NETSTAT_OPTS "laentuwx" \
+ USE_ROUTE( "r") \
+ USE_FEATURE_NETSTAT_WIDE("W") \
+ USE_FEATURE_NETSTAT_PRG( "p")
+
enum {
- OPT_extended = 0x4,
- OPT_showroute = 0x100,
- OPT_widedisplay = 0x200 * ENABLE_FEATURE_NETSTAT_WIDE,
+ OPTBIT_KEEP_OLD = 7,
+ USE_ROUTE( OPTBIT_ROUTE,)
+ USE_FEATURE_NETSTAT_WIDE(OPTBIT_WIDE ,)
+ USE_FEATURE_NETSTAT_PRG( OPTBIT_PRG ,)
+ OPT_sock_listen = 1 << 0, // l
+ OPT_sock_all= 1 << 1, // a
+ OPT_extended= 1 << 2, // e
+ OPT_noresolve = 1 << 3, // n
+ OPT_sock_tcp= 1 << 4, // t
+ OPT_sock_udp= 1 << 5, // u
+ OPT_sock_raw= 1 << 6, // w
+ OPT_sock_unix = 1 << 7, // x
+ OPT_route = USE_ROUTE( (1inode == inode) {
+ /* Some warning should be appropriate here
+ as we got multiple processes for one i-node */
+ return;
+ }
+ }
+ *pnp = xmalloc(sizeof(struct prg_node));
+ pn = *pnp;
+ pn->next = NULL;
+ pn->inode = inode;
+ safe_strncpy(pn->name, name, PROGNAME_WIDTH);
+}
+
+static const char *prg_cache_get(int inode)
+{
+ unsigned hi = PRG_HASHIT(inode);
+ struct prg_node *pn;
+
+ for (pn = prg_hash[hi]; pn; pn = pn->next)
+ if (pn->inode == inode)
+ return pn->name;
+ return "-";
+}
+
+static void prg_cache_clear(void)
+{
+ struct prg_node **pnp, *pn;
+
+ if (prg_cache_loaded == 2)
+ for (pnp = prg_hash; pnp < prg_hash + PRG_HASH_SIZE; pnp++)
+ while ((pn = *pnp)) {
+ *pnp = pn->next;
+ free(pn);
+ }
+ prg_cache_loaded = 0;
+}
+
+static long extract_socket_inode(const char *lname) {
+
+ size_t llen = strlen(lname);
+ long inode = -1;
+
+ if (llen >= PRG_SOCKET_PFXl + 3 &&
+ memcmp(lname, PRG_SOCKET_PFX, PRG_SOCKET_PFXl) == 0 &&
+ lname[llen
Re: PATCH: netstat '-p' optional feature
On Tue, Jul 22, 2008 at 11:00:57PM +0200, Denys Vlasenko wrote:
> Hm.
>
> +static struct prg_node {
> + struct prg_node *next;
> + int inode;
> + char name[PROGNAME_WIDTH];
> +} *prg_list = NULL;
> +
> +static int flag_prg = 0;
>
>
> It still has these statics. I explained how to get rid of them:
...
>
> Can you do it?
It has those two, plus a bunch of others it had before I started
messing with it:
static smallint flags = NETSTAT_CONNECTED | NETSTAT_ALLPROTO;
static const char *const tcp_state[] = {
...
};
typedef enum {
...
} socket_state;
static const char *net_conn_line = PRINT_NET_CONN;
I'll get them all into a struct globals thingie, if
that's the price of getting 'netstat -p' from busybox... :)
BRB,
Gabriel
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
Re: PATCH: netstat '-p' optional feature
On Sun, Jul 20, 2008 at 08:55:46PM +0200, Denys Vlasenko wrote:
> You need to just minimize usage of globals. IOW: use them
> when it is _noticeably_ more difficult to not do so.
turning the hash into a linked list wasn't that hard, and while in
theory it could be slower, in practice that shouldn't make a real
difference.
> +#define PROGNAME_WIDTH 20
> +
> +#define PROGNAME_WIDTHs PROGNAME_WIDTH1(PROGNAME_WIDTH)
> +#define PROGNAME_WIDTH1(s) PROGNAME_WIDTH2(s)
> +#define PROGNAME_WIDTH2(s) #s
>
> Well. Why not just #define PROGNAME_WIDTHs "20" so than reader
> doesn't get confused?
Guess they were going for the whole "one place to modify
PROGNAME_WIDTH" thing -- turning it into a quoted string to glue
together with "%-" and "s" to get a printf format string. I turned it
into
#define PROGNAME_WIDTH 20
#define PROGNAME_WIDTHs "%-20s"
at the expense of having to modify in two places if we ever want to
change 20 to something else (which will happen like, NEVER, so it
shouldn't be a problem)...
> My faith in gcc optimizing strlen is not so high.
> Can you use sizeof("str")-1 instead?
> (and something wrong with indentation).
Done.
> +static long extract_socket_inode(const char lname[]) {
>
> Should be
>
> static long extract_socket_inode(const char *lname)
Done.
New version is attached.
Thanks,
Gabriel
diff -NarU5 busybox-svn-22735.orig/include/usage.h
busybox-svn-22735/include/usage.h
--- busybox-svn-22735.orig/include/usage.h 2008-07-09 23:04:08.0
-0400
+++ busybox-svn-22735/include/usage.h 2008-07-16 20:37:41.0 -0400
@@ -2813,11 +2813,11 @@
/* "\nport numbers can be individual or ranges: lo-hi [inclusive]" */
#endif
#define netstat_trivial_usage \
- "[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")"]"
+
"[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")USE_FEATURE_NETSTAT_PRG("p")"]"
#define netstat_full_usage "\n\n" \
"Display networking information\n" \
"\nOptions:" \
"\n -l Display listening server sockets" \
"\n -a Display all sockets (default: connected)" \
@@ -2828,10 +2828,13 @@
"\n -w Raw sockets" \
"\n -x Unix sockets" \
"\n -r Display routing table" \
USE_FEATURE_NETSTAT_WIDE( \
"\n -W Display with no column truncation" \
+ ) \
+ USE_FEATURE_NETSTAT_PRG( \
+ "\n -p Display PID/Program name for sockets" \
)
#define nice_trivial_usage \
"[-n ADJUST] [COMMAND [ARG]...]"
#define nice_full_usage "\n\n" \
diff -NarU5 busybox-svn-22735.orig/networking/Config.in
busybox-svn-22735/networking/Config.in
--- busybox-svn-22735.orig/networking/Config.in 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/Config.in 2008-07-16 20:37:41.0
-0400
@@ -630,10 +630,17 @@
depends on NETSTAT
help
Add support for wide columns. Useful when displaying IPv6 addresses
(-W option).
+config FEATURE_NETSTAT_PRG
+ bool "Enable PID/Program name output"
+ default n
+ depends on NETSTAT
+ help
+ Add support for -p flag to print out PID and program name.
+
config NSLOOKUP
bool "nslookup"
default n
help
nslookup is a tool to query Internet name servers.
diff -NarU5 busybox-svn-22735.orig/networking/netstat.c
busybox-svn-22735/networking/netstat.c
--- busybox-svn-22735.orig/networking/netstat.c 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/netstat.c 2008-07-22 15:53:04.0
-0400
@@ -6,22 +6,41 @@
* Copyright (C) 2002 by Bart Visscher <[EMAIL PROTECTED]>
*
* 2002-04-20
* IPV6 support added by Bart Visscher <[EMAIL PROTECTED]>
*
+ * 2008-07-10
+ * optional '-p' flag support ported from net-tools by G. Somlo <[EMAIL
PROTECTED]>
+ *
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
#include "libbb.h"
#include "inet_common.h"
+#define NETSTAT_OPTS "laentuwx" \
+ USE_ROUTE( "r") \
+ USE_FEATURE_NETSTAT_WIDE("W") \
+ USE_FEATURE_NETSTAT_PRG( "p")
+
enum {
- OPT_extended = 0x4,
- OPT_showroute = 0x100,
- OPT_widedisplay = 0x200 * ENABLE_FEATURE_NETSTAT_WIDE,
+ OPTBIT_KEEP_OLD = 7,
+ USE_ROUTE( OPTBIT_ROUTE,)
+ USE_FEATURE_NETSTAT_WIDE(OPTBIT_WIDE ,)
+ USE_FEATURE_NETSTAT_PRG( OPTBIT_PRG ,)
+ OPT_sock_listen = 1 << 0, // l
+ OPT_sock_all= 1 << 1, // a
+ OPT_extended= 1 << 2, // e
+ OPT_noresolve = 1 << 3, // n
+ OPT_sock_tcp= 1 << 4, // t
+ OPT_sock_udp= 1 << 5, // u
+ OPT_sock_raw= 1 << 6, // w
+ OPT_sock_unix = 1 << 7, // x
+ OPT_route = USE_ROUTE( (1inode = inode;
+ safe_strncpy(pn->name, name, PROGNAME_WIDTH);
+ prg_list = pn;
+}
+
+static const char *prg_cache_get(int inode)
+
Re: PATCH: netstat '-p' optional feature
On Mon, Jul 14, 2008 at 11:21:39PM +0200, Denys Vlasenko wrote:
> Cool eh? ;) Can you do the same?
Done, options are now taken care of.
> Lately there is a trend to have all new applets to be like this:
>
> # size mdev.o
>textdata bss dec hex filename
>2176 0 02176 880 mdev.o
>
> zero zero :)
Allright, so I rewrote the whole prg_cache_* function set to
essentially use a linked list instead of a hash (slower, but doesn't
require global data beyond one single pointer for the list head, and
we don't care because we don't expect there to be *that* many
processes where linear lookup times would become noticeable).
So, what do you think about this latest version of the patch ?
Thanks,
Gabriel
diff -NarU5 busybox-svn-22735.orig/include/usage.h
busybox-svn-22735/include/usage.h
--- busybox-svn-22735.orig/include/usage.h 2008-07-09 23:04:08.0
-0400
+++ busybox-svn-22735/include/usage.h 2008-07-16 20:37:41.0 -0400
@@ -2813,11 +2813,11 @@
/* "\nport numbers can be individual or ranges: lo-hi [inclusive]" */
#endif
#define netstat_trivial_usage \
- "[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")"]"
+
"[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")USE_FEATURE_NETSTAT_PRG("p")"]"
#define netstat_full_usage "\n\n" \
"Display networking information\n" \
"\nOptions:" \
"\n -l Display listening server sockets" \
"\n -a Display all sockets (default: connected)" \
@@ -2828,10 +2828,13 @@
"\n -w Raw sockets" \
"\n -x Unix sockets" \
"\n -r Display routing table" \
USE_FEATURE_NETSTAT_WIDE( \
"\n -W Display with no column truncation" \
+ ) \
+ USE_FEATURE_NETSTAT_PRG( \
+ "\n -p Display PID/Program name for sockets" \
)
#define nice_trivial_usage \
"[-n ADJUST] [COMMAND [ARG]...]"
#define nice_full_usage "\n\n" \
diff -NarU5 busybox-svn-22735.orig/networking/Config.in
busybox-svn-22735/networking/Config.in
--- busybox-svn-22735.orig/networking/Config.in 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/Config.in 2008-07-16 20:37:41.0
-0400
@@ -630,10 +630,17 @@
depends on NETSTAT
help
Add support for wide columns. Useful when displaying IPv6 addresses
(-W option).
+config FEATURE_NETSTAT_PRG
+ bool "Enable PID/Program name output"
+ default n
+ depends on NETSTAT
+ help
+ Add support for -p flag to print out PID and program name.
+
config NSLOOKUP
bool "nslookup"
default n
help
nslookup is a tool to query Internet name servers.
diff -NarU5 busybox-svn-22735.orig/networking/netstat.c
busybox-svn-22735/networking/netstat.c
--- busybox-svn-22735.orig/networking/netstat.c 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/netstat.c 2008-07-17 13:42:32.0
-0400
@@ -6,22 +6,41 @@
* Copyright (C) 2002 by Bart Visscher <[EMAIL PROTECTED]>
*
* 2002-04-20
* IPV6 support added by Bart Visscher <[EMAIL PROTECTED]>
*
+ * 2008-07-10
+ * optional '-p' flag support ported from net-tools by G. Somlo <[EMAIL
PROTECTED]>
+ *
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
#include "libbb.h"
#include "inet_common.h"
+#define NETSTAT_OPTS "laentuwx" \
+ USE_ROUTE( "r") \
+ USE_FEATURE_NETSTAT_WIDE("W") \
+ USE_FEATURE_NETSTAT_PRG( "p")
+
enum {
- OPT_extended = 0x4,
- OPT_showroute = 0x100,
- OPT_widedisplay = 0x200 * ENABLE_FEATURE_NETSTAT_WIDE,
+ OPTBIT_KEEP_OLD = 7,
+ USE_ROUTE( OPTBIT_ROUTE,)
+ USE_FEATURE_NETSTAT_WIDE(OPTBIT_WIDE ,)
+ USE_FEATURE_NETSTAT_PRG( OPTBIT_PRG ,)
+ OPT_sock_listen = 1 << 0, // l
+ OPT_sock_all= 1 << 1, // a
+ OPT_extended= 1 << 2, // e
+ OPT_noresolve = 1 << 3, // n
+ OPT_sock_tcp= 1 << 4, // t
+ OPT_sock_udp= 1 << 5, // u
+ OPT_sock_raw= 1 << 6, // w
+ OPT_sock_unix = 1 << 7, // x
+ OPT_route = USE_ROUTE( (1inode = inode;
+ safe_strncpy(pn->name, name, PROGNAME_WIDTH);
+ prg_list = pn;
+}
+
+static const char *prg_cache_get(int inode)
+{
+ struct prg_node *pn;
+
+ for (pn = prg_list; pn; pn = pn->next)
+ if (pn->inode == inode)
+ return pn->name;
+ return "-";
+}
+
+static void prg_cache_clear(void)
+{
+ struct prg_node *pn;
+
+ while (prg_list) {
+ pn = prg_list;
+ prg_list = prg_list->next;
+ free(pn);
+ }
+}
+
+static long extract_socket_inode(const char lname[]) {
+
+ size_t llen = strlen(lname);
+ long inode = -1;
+
+ if (llen >= PRG_SOCKET_PFXl + 3 &&
+ memcmp(ln
Re: PATCH: netstat '-p' optional feature
On Sun, Jul 13, 2008 at 02:13:56AM +0200, Denys Vlasenko wrote:
> enum {
> OPT_extended = 0x4,
> OPT_showroute = 0x100,
> - OPT_widedisplay = 0x200 * ENABLE_FEATURE_NETSTAT_WIDE,
> + OPT_widedisplay = 0x200,
> + OPT_showprog = 0x400,
>
> Why this change?
Mainly for uniform handling of *optional* options. Originally,
NETSTAT_OPTS had a conditional USE_FEATURE_NETSTAT_WIDE("W"). Couldn't
add "p" the same way, or else we'd never know which one of them is
0x200 and which one is 0x400 :) Once I decided to put them both in
there regardless of whether support for them was compiled in or not, I
wanted to allow usage to be printed out if -W was selected on the
command line w/o support for it being compiled in:
if (opt & OPT_widedisplay) { // -W
#if ENABLE_FEATURE_NETSTAT_WIDE
net_conn_line = PRINT_NET_CONN_WIDE;
net_conn_line_header = PRINT_NET_CONN_HEADER_WIDE;
#else
bb_show_usage();
#endif
}
This is the same as behavior for -r and my own addition, -p. You get
usage if the command line option you just gave isn't supported, rather
than being silently ignored.
> +#define PRG_HASH_SIZE 211
> +
> +static struct prg_node {
> + struct prg_node *next;
> + int inode;
> + char name[PROGNAME_WIDTH];
> +} *prg_hash[PRG_HASH_SIZE];
> +
> +static char prg_cache_loaded = 0;
> +
> +int flag_prg = 0;
>
> This is a lot of global data. This hurts NOMMU systems.
> I suspect flag_prg can be substituted for
> (option_mask32 & OPT_showprog)?
flag_prg is checked in all the [tcp|udp|raw|unix]_do_one() functions
to decide whether to print the PID and program name. Unless we want to
add a parameter to do_info(), which is gonna pass it along to the
*_do_one() functions, I guess we'd better let it stay. Besides, it's
one lonely integer, nothing compared to the prg_hash table itself...
We could go crazy and rewrite the prg_cache_load() function to return
the hash as a completely dynamically allocated structure, and pass it
around to all other functions, but that feels a bit extreme...
> + if (!(*pnp = malloc(sizeof(**pnp
> + return;
>
> Pull assignment out of if(), please.
Done.
> + if (pn->inode == inode) return(pn->name);
>
> busybox uniformly uses return xxx, not return(xxx).
Fixed.
> Have mercy. Do strlen just once.
Fixed.
> + *inode_p = strtol(inode_str, &serr, 0);
> + if (!serr || *serr || *inode_p < 0 || *inode_p >= INT_MAX)
> + *inode_p = -1;
>
> bb_strtou is easier to use - you need to check just errno after it.
You mean bb_strtol, but yeah, fixed :)
>
> + while (errno = 0, direproc = readdir(dirproc)) {
>
> This is even worse than assignment in if().
True, and it's redundant too, errno isn't being checked for anyway after
the readdir call... Let's just get rid of the 'errno = 0' part and
forget it ever happened :) :)
> + if (direproc->d_type != DT_DIR) continue;
>
> This is not realiable. See the last sentence:
In truth, this was ifdef-ed out in the net-tools code, so I'm going to
just remove it from here as well. My bad...
> + for (cs = direproc->d_name; *cs && isdigit(*cs); cs++);
>
> Put dummy "continue" in the loop body, so that everyone will be sure
> you intended the loop to be empty.
Done.
> Something went wrong with indentation.
Fixed as well (tab vs. spaces thing).
New version of the patch is attached. Please let me know what you
think.
Thanks,
Gabriel
diff -NarU5 busybox-svn-22735.orig/include/usage.h
busybox-svn-22735/include/usage.h
--- busybox-svn-22735.orig/include/usage.h 2008-07-09 23:04:08.0
-0400
+++ busybox-svn-22735/include/usage.h 2008-07-13 14:02:58.0 -0400
@@ -2813,11 +2813,11 @@
/* "\nport numbers can be individual or ranges: lo-hi [inclusive]" */
#endif
#define netstat_trivial_usage \
- "[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")"]"
+
"[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")USE_FEATURE_NETSTAT_PRG("p")"]"
#define netstat_full_usage "\n\n" \
"Display networking information\n" \
"\nOptions:" \
"\n -l Display listening server sockets" \
"\n -a Display all sockets (default: connected)" \
@@ -2828,10 +2828,13 @@
"\n -w Raw sockets" \
"\n -x Unix sockets" \
"\n -r Display routing table" \
USE_FEATURE_NETSTAT_WIDE( \
"\n -W Display with no column truncation" \
+ ) \
+ USE_FEATURE_NETSTAT_PRG( \
+ "\n -p Display PID/Program name for sockets" \
)
#define nice_trivial_usage \
"[-n ADJUST] [COMMAND [ARG]...]"
#define nice_full_usage "\n\n" \
diff -NarU5 busybox-svn-22735.orig/networking/Config.in
busybox-svn-22735/networking/Config.in
--- busybox-svn-22735.orig/networking/Config.in 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/Config.in 2008-07-13 14:02:58.0
-0
PATCH: netstat '-p' optional feature
Denys & All,
I needed -p support in busybox's netstat for a project I'm working
on, so I ported the functionality over from net-tools.
The attached patch (against svn 22735) adds optional -p support
allowing netstat to print out process IDs and command names.
Please let me know what you think, and apply if you agree.
Thanks,
Gabriel
diff -NarU5 busybox-svn-22735.orig/include/usage.h
busybox-svn-22735/include/usage.h
--- busybox-svn-22735.orig/include/usage.h 2008-07-09 23:04:08.0
-0400
+++ busybox-svn-22735/include/usage.h 2008-07-10 17:24:22.0 -0400
@@ -2813,11 +2813,11 @@
/* "\nport numbers can be individual or ranges: lo-hi [inclusive]" */
#endif
#define netstat_trivial_usage \
- "[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")"]"
+
"[-laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")USE_FEATURE_NETSTAT_PRG("p")"]"
#define netstat_full_usage "\n\n" \
"Display networking information\n" \
"\nOptions:" \
"\n -l Display listening server sockets" \
"\n -a Display all sockets (default: connected)" \
@@ -2828,10 +2828,13 @@
"\n -w Raw sockets" \
"\n -x Unix sockets" \
"\n -r Display routing table" \
USE_FEATURE_NETSTAT_WIDE( \
"\n -W Display with no column truncation" \
+ ) \
+ USE_FEATURE_NETSTAT_PRG( \
+ "\n -p Display PID/Program name for sockets" \
)
#define nice_trivial_usage \
"[-n ADJUST] [COMMAND [ARG]...]"
#define nice_full_usage "\n\n" \
diff -NarU5 busybox-svn-22735.orig/networking/Config.in
busybox-svn-22735/networking/Config.in
--- busybox-svn-22735.orig/networking/Config.in 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/Config.in 2008-07-10 17:24:22.0
-0400
@@ -630,10 +630,17 @@
depends on NETSTAT
help
Add support for wide columns. Useful when displaying IPv6 addresses
(-W option).
+config FEATURE_NETSTAT_PRG
+ bool "Enable PID/Program name output"
+ default n
+ depends on NETSTAT
+ help
+ Add support for -p flag to print out PID and program name.
+
config NSLOOKUP
bool "nslookup"
default n
help
nslookup is a tool to query Internet name servers.
diff -NarU5 busybox-svn-22735.orig/networking/netstat.c
busybox-svn-22735/networking/netstat.c
--- busybox-svn-22735.orig/networking/netstat.c 2008-07-09 23:04:03.0
-0400
+++ busybox-svn-22735/networking/netstat.c 2008-07-10 17:39:35.0
-0400
@@ -6,22 +6,26 @@
* Copyright (C) 2002 by Bart Visscher <[EMAIL PROTECTED]>
*
* 2002-04-20
* IPV6 support added by Bart Visscher <[EMAIL PROTECTED]>
*
+ * 2008-07-10
+ * optional '-p' flag support ported from net-tools by G. Somlo <[EMAIL
PROTECTED]>
+ *
* Licensed under GPLv2 or later, see file LICENSE in this tarball for details.
*/
#include "libbb.h"
#include "inet_common.h"
enum {
OPT_extended = 0x4,
OPT_showroute = 0x100,
- OPT_widedisplay = 0x200 * ENABLE_FEATURE_NETSTAT_WIDE,
+ OPT_widedisplay = 0x200,
+ OPT_showprog = 0x400,
};
-# define NETSTAT_OPTS "laentuwxr"USE_FEATURE_NETSTAT_WIDE("W")
+# define NETSTAT_OPTS "laentuwxrWp"
#define NETSTAT_CONNECTED 0x01
#define NETSTAT_LISTENING 0x02
#define NETSTAT_NUMERIC 0x04
/* Must match getopt32 option string */
@@ -74,25 +78,235 @@
#define SO_WAITDATA (1<<17) /* wait data to read*/
#define SO_NOSPACE (1<<18) /* no space to write*/
/* Standard printout size */
#define PRINT_IP_MAX_SIZE 23
-#define PRINT_NET_CONN "%s %6ld %6ld %-23s %-23s %-12s\n"
-#define PRINT_NET_CONN_HEADER "\nProto Recv-Q Send-Q %-23s %-23s State\n"
+#define PRINT_NET_CONN "%s %6ld %6ld %-23s %-23s %-12s"
+#define PRINT_NET_CONN_HEADER "\nProto Recv-Q Send-Q %-23s %-23s State
"
/* When there are IPv6 connections the IPv6 addresses will be
* truncated to none-recognition. The '-W' option makes the
* address columns wide enough to accomodate for longest possible
* IPv6 addresses, i.e. addresses of the form
* ::::::ddd.ddd.ddd.ddd
*/
#define PRINT_IP_MAX_SIZE_WIDE 51 /* INET6_ADDRSTRLEN + 5 for the port
number */
-#define PRINT_NET_CONN_WIDE "%s %6ld %6ld %-51s %-51s %-12s\n"
-#define PRINT_NET_CONN_HEADER_WIDE "\nProto Recv-Q Send-Q %-51s %-51s State\n"
+#define PRINT_NET_CONN_WIDE "%s %6ld %6ld %-51s %-51s %-12s"
+#define PRINT_NET_CONN_HEADER_WIDE "\nProto Recv-Q Send-Q %-51s %-51s State
"
static const char *net_conn_line = PRINT_NET_CONN;
+#if ENABLE_FEATURE_NETSTAT_PRG
+
+#define PROGNAME_WIDTH 20
+
+#define PROGNAME_WIDTHs PROGNAME_WIDTH1(PROGNAME_WIDTH)
+#define PROGNAME_WIDTH1(s) PROGNAME_WIDTH2(s)
+#define PROGNAME_WIDTH2(s) #s
+
+#define PRG_HASH_SIZE 211
+
+static struct prg_node {
+
Re: VRRP advice
On Wed, Apr 09, 2008 at 03:02:57AM +0200, Denys Vlasenko wrote: > > This one? > > http://en.wikipedia.org/wiki/Virtual_Router_Redundancy_Protocol Yeah, that one :) > > I just learned from you that there is the beast called VRRP... Looking at that page again (been a while), I now have a third possibility: - ucarp, BSD's alternative to hsrp / vrrp Anyone ever used any of the three (vrrpd and keepalived offering standard VRRP, or ucarp offering BSD's own way to accomplish the same thing) ? Any endorsements, warnings, etc. most welcome ! Thanks, Gabriel ___ busybox mailing list busybox@busybox.net http://busybox.net/cgi-bin/mailman/listinfo/busybox
VRRP advice
Hi, I have to add VRRP capability to my embedded box, and I'm looking for experiences and advice on how to accomplish that: The two choices I found so far are - vrrpd from sourceforge, which seems compact and lightweight enough, but hasn't been updated since 2002; - keepalived, actively maintained, but even when I leave out all sorts of extraneous features, it still pulls in a ton of libraries I'm not sure are absolutely necessary for just plain old VRRP. Does anyone else implement VRRP on a busybox-based system, and, if so, what software do they use ? Thanks, Gabriel ___ busybox mailing list busybox@busybox.net http://busybox.net/cgi-bin/mailman/listinfo/busybox
Re: PATCH: brctl show
On Sun, Apr 06, 2008 at 09:19:24AM +0200, Denys Vlasenko wrote: > On Saturday 05 April 2008 23:10, L. Gabriel Somlo wrote: > > NB: "standard" brctl talks to kernel via /sys, not ioctls. > brctl ioctls are deprecated - for one, they don't work > in 32/64 mixed environment, and will never work - > kernel people decided to let it rust. (I agree with them). I noticed that, but then, in the header of the busybox brctl.c file, there's this: /* This applet currently uses only the ioctl interface and no sysfs at * all. * At the time of this writing this was considered a feature. */ So I figured, when in Rome, act like the Romans :) So, is the consensus that brctl should be rewritten to use /sys exclusively ? Thanks, Gabriel ___ busybox mailing list busybox@busybox.net http://busybox.net/cgi-bin/mailman/listinfo/busybox
PATCH: brctl show
Hi,
I need to be able to see the list of ports currently configured on a
bridge, so I took a shot at implementing 'brctl show'. Might follow up
with 'showmacs' later, but for now, let me know what you think, and
please apply...
Thanks,
Gabriel
diff -NarU5 busybox-svn-21645.orig/include/usage.h
busybox-svn-21645/include/usage.h
--- busybox-svn-21645.orig/include/usage.h 2008-04-05 16:55:33.0
-0400
+++ busybox-svn-21645/include/usage.h 2008-04-05 17:05:36.0 -0400
@@ -149,10 +149,13 @@
"\n setmaxage BRIDGE TIME Set max message age" \
"\n setpathcost BRIDGE COST Set path cost" \
"\n setportprio BRIDGE PRIO Set port priority" \
"\n setbridgeprio BRIDGE PRIO Set bridge priority" \
"\n stp BRIDGE [1|0]STP on/off" \
+ ) \
+ USE_FEATURE_BRCTL_SHOW( \
+ "\n showShow a list of bridges" \
)
#define bunzip2_trivial_usage \
"[OPTION]... [FILE]"
#define bunzip2_full_usage \
"Uncompress FILE (or standard input if FILE is '-' or omitted)\n" \
diff -NarU5 busybox-svn-21645.orig/networking/brctl.c
busybox-svn-21645/networking/brctl.c
--- busybox-svn-21645.orig/networking/brctl.c 2008-04-05 16:55:29.0
-0400
+++ busybox-svn-21645/networking/brctl.c2008-04-05 17:05:36.0
-0400
@@ -23,16 +23,10 @@
/* Use internal number parsing and not the "exact" conversion. */
/* #define BRCTL_USE_INTERNAL 0 */ /* use exact conversion */
#define BRCTL_USE_INTERNAL 1
-#ifdef ENABLE_FEATURE_BRCTL_SHOW
-#error Remove these
-#endif
-#define ENABLE_FEATURE_BRCTL_SHOW 0
-#define USE_FEATURE_BRCTL_SHOW(...)
-
#if ENABLE_FEATURE_BRCTL_FANCY
#include
/* FIXME: These 4 funcs are not really clean and could be improved */
static ALWAYS_INLINE void strtotimeval(struct timeval *tv,
@@ -121,26 +115,88 @@
while (*argv) {
key = index_in_strings(keywords, *argv);
if (key == -1) /* no match found in keywords array, bail out. */
bb_error_msg_and_die(bb_msg_invalid_arg, *argv,
applet_name);
argv++;
+ fd = xsocket(AF_INET, SOCK_STREAM, 0);
+
#if ENABLE_FEATURE_BRCTL_SHOW
if (key == ARG_show) { /* show */
- goto out; /* FIXME: implement me! :) */
+ char brname[IFNAMSIZ];
+ int bridx[MAX_PORTS];
+ int i, num;
+ arm_ioctl(args, BRCTL_GET_BRIDGES,
+ (unsigned long) bridx,
MAX_PORTS);
+ num = ioctl_or_warn(fd, SIOCGIFBR, args);
+ if (num < 0)
+ bb_simple_perror_msg_and_die("can't get bridge
indices");
+ printf("bridge name\tbridge id\t\tSTP
enabled\tinterfaces\n");
+ for (i = 0; i < num; i++) {
+ char ifname[IFNAMSIZ];
+ int ifidx[MAX_PORTS];
+ int j, tabs;
+ struct __bridge_info bi;
+ unsigned char *x;
+
+ ifr.ifr_data = (char *) &args;
+ if (!if_indextoname(bridx[i], brname))
+ bb_perror_msg_and_die("can't get bridge
name for index %d", i);
+ safe_strncpy(ifr.ifr_name, brname, IFNAMSIZ);
+
+ arm_ioctl(args, BRCTL_GET_BRIDGE_INFO,
+ (unsigned long) &bi, 0);
+ xioctl(fd, SIOCDEVPRIVATE, &ifr);
+ printf("%s\t\t", brname);
+
+
+ /* print bridge id */
+ x = (unsigned char *) &bi.bridge_id;
+ for (j = 0; j < 8; j++) {
+ printf("%.2x", x[j]);
+ if (j == 1)
+ printf(".");
+ }
+ printf("\t%s", bi.stp_enabled? "yes" : "no");
+
+ /* print interface list */
+ arm_ioctl(args, BRCTL_GET_PORT_LIST,
+ (unsigned long) ifidx,
MAX_PORTS);
+ xioctl(fd, SIOCDEVPRIVATE, &ifr);
+ tabs = 0;
+ for (j = 0; j < MAX_PORTS; j++) {
+ if (!ifidx[j])
+ continue;
+ if (!if_indextoname(ifidx[j], ifname))
+ bb_perror_msg_and_die("ca
Re: PATCH: udhcpc -- don't request set of options by default
> I am thinking - maybe we should just junk the idea of "default"
> options to ask for? We can ask user to always provide explicit
> list of -O OPTs to ask. What do you think?
I agree. Although others might yell at us if we change default
behavior :)
> Please take a look at attached patch - will this work for you?
>
...
>
> + if (client_config.no_default_options)
> + return;
> +
This returns before -O explicit options have a chance to be added,
doesn't it ?
> packet->options[end + OPT_CODE] = DHCP_PARAM_REQ;
> for (i = 0; (c = dhcp_options[i].code) != 0; i++) {
> if ((dhcp_options[i].flags & OPTION_REQ)
> @@ -107,7 +110,9 @@ int send_discover(uint32_t xid, uint32_t
> /* Explicitly saying that we want RFC-compliant packets helps
>* some buggy DHCP servers to NOT send bigger packets */
> add_simple_option(packet.options, DHCP_MAX_SIZE, htons(576));
Junking default options altogether definitely feels cleaner, no need
to monkey around with all this '-o' stuff... :) Would anyone be truly
horrified to see that happen ?
BTW, I really did like the [[ %udhcpc_opts%]] idea, I'd like that at
least to stay... :)
Thanks,
--Gabriel
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
Re: PATCH: udhcpc -- don't request set of options by default
On Wed, Apr 02, 2008 at 12:14:14AM +0200, Denys Vlasenko wrote:
> On Tuesday 01 April 2008 23:09, L. Gabriel Somlo wrote:
> > Last, but not least, a slightly more complex sample udhcpc.script,
> > which attempts to react to changes in parameters returned by the dhcp
> > server in a minimal-effort sort of way (i.e. don't reconfigure the
> > interface if the only change was in the list of ntp servers, etc.)
>
> Imagine that I have *two* interfaces, both sit on DHCP configured
> networks (*different* networks).
> Here one "deconfig"-ed iface will happily nuke /etc/resolv.conf
> created by another interface:
I agree with your no_ifup blurb in principle. However, this is not
about ifupdown at all (adding a way to pass no-default-options to
udhcpc from ifupdown was an afterthought since I happen to use
ifupdown, but totally unrelated to the udhcpc sample script).
The real question is, who "wins" w.r.t. /etc/resolv.conf when multiple
dhcp-configured interfaces are up simultaneously ? This is already a
fun problem for laptops that connect both on wireless and on wired
ethernet, and it's a trainwreck, ifupdown or not...
> +case "$1" in
> + deconfig)
> +if [ "$PEERDNS_IF" == "$interface" ] ; then
> + [ -f ${RESOLV_CONF}.dhcsave ] && mv -f ${RESOLV_CONF}.dhcsave
> $RESOLV_CONF
> +fi
> +if [ "$PEERNTP_IF" == "$interface" ] ; then
> + [ -f ${NTP_CONF}.dhcsave ] && mv -f ${NTP_CONF}.dhcsave $NTP_CONF
> +fi
In my udhcpc.script sample I declare an interface to be the "owner" of
resolv.conf (e.g. PEERDNS_IF=eth0), and only when eth0 goes down does
resolv.conf get nuked. Deconfig-ing eth1 doesn't do anything to
resolv.conf.
This does have its own drawbacks, for sure, but we first need to have
a rule about what what happens in this scenario:
- eth0 comes up
- we request a dhcp lease, and receive back a bunch of stuff (e.g.
nameservers)
- we populate /etc/resolv.conf with these nameservers
- now, while eth0 stays up, eth1 comes up also
- we request a dhcp lease for eth1, and receive back a bunch of stuff
(including a *different* set of nameservers)
- ???
Do we overwrite resolv.conf, or do we add the new nameservers to the
list (and if we add them, where, at the front or back of the list ?)
If we add them to the front, it's almost like we overwrote resolv.conf
(the new nameserver gets queried for *everything* with precendence).
If we add to the back, it's like we didn't overwrite resolv.conf at
all (eth0's nameserver gets queried for *everything* with precedence).
So, what I'm trying to say, this is not really an ifupdown is
bad/good/ugly problem, but rather "how do we reconcile information
received via dhcp on multiple interfaces?"; My (yes, possibly flawed)
answer was "hardcode a winner", but what's the right thing to do ?
Thanks,
Gabriel
PS. Here's a simplified patch that only addresses the '-o' or
'--no-default-options' change to udhcpc, leaving out ifupdown and the
sample udhcpc.script, since they are really totally separate issues.
Please apply this one while we figure out what to do with resolv.conf
:)
diff -NarU5 busybox-svn-21617.orig/include/usage.h
busybox-svn-21617/include/usage.h
--- busybox-svn-21617.orig/include/usage.h 2008-04-01 14:47:29.0
-0400
+++ busybox-svn-21617/include/usage.h 2008-04-01 15:20:40.0 -0400
@@ -4114,11 +4114,11 @@
"[-T last-check-time] [-U UUID] device"
#define tune2fs_full_usage \
"Adjust filesystem options on ext[23] filesystems"
#define udhcpc_trivial_usage \
- "[-Cfbnqtv] [-c CID] [-V VCLS] [-H HOSTNAME] [-i INTERFACE]\n" \
+ "[-Cfbnqtvo] [-c CID] [-V VCLS] [-H HOSTNAME] [-i INTERFACE]\n" \
" [-p pidfile] [-r IP] [-s script] [-O dhcp-option]..."
USE_FEATURE_UDHCP_PORT(" [-P N]")
#define udhcpc_full_usage \
USE_GETOPT_LONG( \
" -V,--vendorclass=CLASSIDVendor class identifier" \
"\n -i,--interface=INTERFACEInterface to use (default
eth0)" \
@@ -4142,10 +4142,11 @@
"\n -P,--client-port N Use port N instead of default 68" \
) \
USE_FEATURE_UDHCPC_ARPING( \
"\n -a,--arping Use arping to validate offered address" \
) \
+ "\n -o,--no-default-options Do not request options by default" \
) \
SKIP_GETOPT_LONG( \
" -V CLASSID Vendor class identifier" \
"\n -i INTERFACEInterface to use (default: eth0)" \
"\n -H,-h HOSTNAME Client hostname" \
diff -NarU5 busybox-svn-21617.orig/networking/udhcp/clientpacket.c
busybox-svn
PATCH: udhcpc -- don't request set of options by default
Denys & All,
I noticed that udhcpc sends a default list of options in its request
(i.e., all options listed with flag OPTION_REQ in options.c)
That's bad, at least when used with some versions of isc dhcpd, which
will only include the requested options in a reply.
I know about the -O flag that can get udhcpc to request extra options,
but what if I want to see options I didn't think about asking for ?
Including a default option list in the request will always preclude me
from seeing options I didn't know to ask for.
I've added the '-o' command line flag to udhcpc, which inhibits
inclusion of default options in the request. Options can still be
explicitly requested with -O.
I have also added a mechanism to request '-o' (actually, its long form
--no-default-options) from ifupdown's /etc/network/interfaces:
auto eth0
iface eth0 inet dhcp
nodefaultopts --no-default-options
Last, but not least, a slightly more complex sample udhcpc.script,
which attempts to react to changes in parameters returned by the dhcp
server in a minimal-effort sort of way (i.e. don't reconfigure the
interface if the only change was in the list of ntp servers, etc.)
Please note that I did not change anything in the default behavior,
although I'd like to propose that udhcpc should not ask for a default
list of options unless we explicitly tell it to do so...
Please let me know what you think, and apply if you agree.
Regards,
--Gabriel
diff -NarU5 busybox-svn-21617.orig/examples/udhcp/udhcpc.script
busybox-svn-21617/examples/udhcp/udhcpc.script
--- busybox-svn-21617.orig/examples/udhcp/udhcpc.script 1969-12-31
19:00:00.0 -0500
+++ busybox-svn-21617/examples/udhcp/udhcpc.script 2008-04-01
16:25:24.0 -0400
@@ -0,0 +1,88 @@
+#!/bin/busybox sh
+
+# a more "complex" udhcpc script that attempts to handle option changes with
minimal work
+# Gabriel Somlo <[EMAIL PROTECTED]>, 04/01/2008
+
+[ -z "$1" ] && echo 'Error: should be called from udhcpc' && exit 1
+
+CFG="/var/run/udhcpc.${interface}.cfg"
+RESOLV_CONF='/etc/resolv.conf'
+NTP_CONF='/etc/ntp.conf'
+# which interface configures DNS and NTP ? Comment out if none:
+PEERDNS_IF=eth0
+PEERNTP_IF=eth0
+
+case "$1" in
+ deconfig)
+ip addr flush dev $interface
+ip link set up dev $interface
+rm -f $CFG
+if [ "$PEERDNS_IF" == "$interface" ] ; then
+ [ -f ${RESOLV_CONF}.dhcsave ] && mv -f ${RESOLV_CONF}.dhcsave
$RESOLV_CONF
+fi
+if [ "$PEERNTP_IF" == "$interface" ] ; then
+ [ -f ${NTP_CONF}.dhcsave ] && mv -f ${NTP_CONF}.dhcsave $NTP_CONF
+fi
+;;
+ bound)
+set > $CFG
+ip addr flush dev $interface
+[ -n "$mtu" ] && ip link set mtu $mtu dev $interface
+ip addr add ${ip}/${mask} dev $interface
+[ -n "$router" ] && ip route add default via ${router%% *} dev $interface
+if [ "$PEERDNS_IF" == "$interface" ] ; then
+ [ -f $RESOLV_CONF ] && mv -f $RESOLV_CONF ${RESOLV_CONF}.dhcsave
+ [ -n "$domain" ] && echo search $domain > $RESOLV_CONF
+ for i in $dns ; do
+echo nameserver $i >> $RESOLV_CONF
+ done
+fi
+if [ "$PEERNTP_IF" == "$interface" ] ; then
+ [ -f $NTP_CONF ] && mv -f $NTP_CONF ${NTP_CONF}.dhcsave
+ > $NTP_CONF
+ for i in $ntpsrv ; do
+echo server $i >> $NTP_CONF
+ done
+fi
+;;
+ renew)
+set > ${CFG}.new
+for i in $(diff -U1 $CFG ${CFG}.new | grep -E ^[+-] \
+| tail +3 \
+| awk -F[+-=] '{print $2}') ; do
+ case "$i" in
+ip|mask|router|mtu)
+ REDO_NET='yes'
+ ;;
+domain|dns)
+ REDO_DNS='yes'
+ ;;
+ntpsrv)
+ REDO_NTP='yes'
+ ;;
+ esac
+done
+mv -f ${CFG}.new $CFG
+if [ -n "$REDO_NET" ] ; then
+ ip addr flush dev $interface
+ [ -n "$mtu" ] && ip link set mtu $mtu dev $interface
+ ip addr add ${ip}/${mask} dev $interface
+ [ -n "$router" ] && ip route add default via ${router%% *} dev $interface
+fi
+if [ -n "$REDO_DNS" -a "$PEERDNS_IF" == "$interface" ] ; then
+ [ -n "$domain" ] && echo search $domain > $RESOLV_CONF
+ for i in $dns ; do
+echo nameserver $i >> $RESOLV_CONF
+ done
+fi
+if [ -n "$REDO_NTP" -a "$PEERNTP_IF" == "$interface" ] ; then
+ > $NTP_CONF
+ for i in $ntpsrv ; do
+echo server $i >> $NTP_CONF
+ done
+ # FIXME: RELOAD NTP DAEMON HERE
+fi
+;;
+esac
+
+exit 0
diff -NarU5 busybox-svn-21617.orig/include/usage.h
busybox-svn-21617/include/usage.h
--- busybox-svn-21617.orig/include/usage.h 2008-04-01 14:47:29.0
-0400
+++ busybox-svn-21617/include/usage.h 2008-04-01 15:20:40.0 -0400
@@ -4114,11 +4114,11 @@
"[-T last-check-time] [-U UUID] device"
#define tune2fs_full_usage \
"Adjust filesystem options on ext[23] filesystems"
#define udhcpc_trivial_usage \
- "[-Cfbnqtv] [
PATCH: ifupdown and more udhcpc command-line options
Denis & all,
I'd like to make a few more of udhcpcd's command line options
available to ifupdown via the /etc/network/interfaces file.
For instance, the attached patch allows access to the
'-t ' option by using the following setup
in /etc/network/interfaces:
auto eth0
iface eth0 inet dhcp
retries 10
Please consider applying, if you agree.
Thanks,
Gabriel
diff -NarU5 busybox-svn-21473.orig/networking/ifupdown.c
busybox-svn-21473/networking/ifupdown.c
--- busybox-svn-21473.orig/networking/ifupdown.c2008-03-24
15:07:47.0 -0400
+++ busybox-svn-21473/networking/ifupdown.c 2008-03-24 16:02:04.0
-0400
@@ -474,11 +474,11 @@
{ "pump",
"pump -i %iface%[[ -h %hostname%]][[ -l %leasehours%]]",
"pump -i %iface% -k",
},
{ "udhcpc",
- "udhcpc -R -n -p /var/run/udhcpc.%iface%.pid -i %iface%[[ -H
%hostname%]][[ -c %clientid%]][[ -s %script%]]",
+ "udhcpc -R -n -p /var/run/udhcpc.%iface%.pid -i %iface%[[ -H
%hostname%]][[ -c %clientid%]][[ -s %script%]][[ -t %retries%]]",
"kill `cat /var/run/udhcpc.%iface%.pid` 2>/dev/null",
},
};
#endif /* ENABLE_FEATURE_IFUPDOWN_EXTERNAL_DHCPC */
@@ -505,11 +505,11 @@
/* ip doesn't up iface when it configures it (unlike ifconfig) */
if (!execute("ip link set %iface% up", ifd, exec))
return 0;
#endif
return execute("udhcpc -R -n -p /var/run/udhcpc.%iface%.pid "
- "-i %iface%[[ -H %hostname%]][[ -c %clientid%]][[ -s
%script%]]",
+ "-i %iface%[[ -H %hostname%]][[ -c %clientid%]][[ -s
%script%]][[ -t %retries%]]",
ifd, exec);
}
#else
static int dhcp_up(struct interface_defn_t *ifd ATTRIBUTE_UNUSED,
execfn *exec ATTRIBUTE_UNUSED)
___
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox
