| Message ID | 1389788704-21079-1-git-send-email-pablo@netfilter.org |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Jan 15, 2014 at 01:25:04PM +0100, Pablo Neira Ayuso wrote: > Currently, nft displays the debugging information if it's compiled with > --enable-debug (which seems a good idea) and when intervals are used > in maps. Add a new option to enable debugging to segtree, so we only > get this information when explicitly requested. > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > We need this in master to avoid inconditional display of the > debugging information when --enable-debug is passed at configuration > / compilation time. I'm aware this patch conflicts with next-3.14, > that conflict should be easy to resolve. Looks good, thanks, I wanted to change this myself. A few more things about debugging, some are easy, some might be a bit more work: - we should use stdout for netlink rule debugging - netlink rule debugging is very very verbose since it dumps the entire netlink message. I'd prefer to only have the netlink expressions ([meta load ...]) and have the netlink message as another option. It keeps scrolling everything off the screen. - netlink message debugging colors break in less. We should check for isatty(). -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 15, 2014 at 02:48:10PM +0000, Patrick McHardy wrote: > On Wed, Jan 15, 2014 at 01:25:04PM +0100, Pablo Neira Ayuso wrote: > > Currently, nft displays the debugging information if it's compiled with > > --enable-debug (which seems a good idea) and when intervals are used > > in maps. Add a new option to enable debugging to segtree, so we only > > get this information when explicitly requested. > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > --- > > We need this in master to avoid inconditional display of the > > debugging information when --enable-debug is passed at configuration > > / compilation time. I'm aware this patch conflicts with next-3.14, > > that conflict should be easy to resolve. > > Looks good, thanks, I wanted to change this myself. Just pushed it, thanks. > A few more things about debugging, some are easy, some might be a bit more > work: > > - we should use stdout for netlink rule debugging OK, pushed another small patch for this. > - netlink rule debugging is very very verbose since it dumps the entire > netlink message. I'd prefer to only have the netlink expressions > ([meta load ...]) and have the netlink message as another option. > It keeps scrolling everything off the screen. Fine with me. I'd like to keep that there as option, it has helped me to debug some issues in the past. Will check this. > - netlink message debugging colors break in less. We should check for > isatty(). less -R can interpret them here. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 15, 2014 at 04:03:00PM +0100, Pablo Neira Ayuso wrote: > On Wed, Jan 15, 2014 at 02:48:10PM +0000, Patrick McHardy wrote: > > On Wed, Jan 15, 2014 at 01:25:04PM +0100, Pablo Neira Ayuso wrote: > > > Currently, nft displays the debugging information if it's compiled with > > > --enable-debug (which seems a good idea) and when intervals are used > > > in maps. Add a new option to enable debugging to segtree, so we only > > > get this information when explicitly requested. > > > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > --- > > > We need this in master to avoid inconditional display of the > > > debugging information when --enable-debug is passed at configuration > > > / compilation time. I'm aware this patch conflicts with next-3.14, > > > that conflict should be easy to resolve. > > > > Looks good, thanks, I wanted to change this myself. > > Just pushed it, thanks. > > > A few more things about debugging, some are easy, some might be a bit more > > work: > > > > - we should use stdout for netlink rule debugging > > OK, pushed another small patch for this. > > > - netlink rule debugging is very very verbose since it dumps the entire > > netlink message. I'd prefer to only have the netlink expressions > > ([meta load ...]) and have the netlink message as another option. > > It keeps scrolling everything off the screen. > > Fine with me. I'd like to keep that there as option, it has helped me > to debug some issues in the past. Will check this. > > > - netlink message debugging colors break in less. We should check for > > isatty(). > > less -R can interpret them here. Ok that's good enough for now, though I would usually rather surpress them completely. Still I'd suggest to use isatty() for redirections to files, bugtracker entries etc. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 15, 2014 at 03:06:24PM +0000, Patrick McHardy wrote: > On Wed, Jan 15, 2014 at 04:03:00PM +0100, Pablo Neira Ayuso wrote: > > On Wed, Jan 15, 2014 at 02:48:10PM +0000, Patrick McHardy wrote: > > > On Wed, Jan 15, 2014 at 01:25:04PM +0100, Pablo Neira Ayuso wrote: > > > > Currently, nft displays the debugging information if it's compiled with > > > > --enable-debug (which seems a good idea) and when intervals are used > > > > in maps. Add a new option to enable debugging to segtree, so we only > > > > get this information when explicitly requested. > > > > > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > > --- > > > > We need this in master to avoid inconditional display of the > > > > debugging information when --enable-debug is passed at configuration > > > > / compilation time. I'm aware this patch conflicts with next-3.14, > > > > that conflict should be easy to resolve. > > > > > > Looks good, thanks, I wanted to change this myself. > > > > Just pushed it, thanks. > > > > > A few more things about debugging, some are easy, some might be a bit more > > > work: > > > > > > - we should use stdout for netlink rule debugging > > > > OK, pushed another small patch for this. > > > > > - netlink rule debugging is very very verbose since it dumps the entire > > > netlink message. I'd prefer to only have the netlink expressions > > > ([meta load ...]) and have the netlink message as another option. > > > It keeps scrolling everything off the screen. > > > > Fine with me. I'd like to keep that there as option, it has helped me > > to debug some issues in the past. Will check this. > > > > > - netlink message debugging colors break in less. We should check for > > > isatty(). > > > > less -R can interpret them here. > > Ok that's good enough for now, though I would usually rather surpress > them completely. Still I'd suggest to use isatty() for redirections to > files, bugtracker entries etc. I'll add a new interface to libmnl that accepts flags to decide if we want colors or not. But it would take some time until distributors pack that new version of the library and I would like to avoid that. I'll add this to my notepad so we can revisit this later. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/nftables.h b/include/nftables.h index 12f3c49..f989161 100644 --- a/include/nftables.h +++ b/include/nftables.h @@ -17,6 +17,7 @@ enum debug_level { DEBUG_PARSER = 0x2, DEBUG_EVALUATION = 0x4, DEBUG_NETLINK = 0x8, + DEBUG_SEGTREE = 0x10, }; #define INCLUDE_PATHS_MAX 16 diff --git a/src/main.c b/src/main.c index e8be423..d78eea7 100644 --- a/src/main.c +++ b/src/main.c @@ -111,7 +111,7 @@ static void show_help(const char *name) " -a/--handle Output rule handle.\n" " -I/--includepath <directory> Add <directory> to the paths searched for include files.\n" #ifdef DEBUG -" --debug <level [,level...]> Specify debugging level (scanner, parser, eval, netlink, all)\n" +" --debug <level [,level...]> Specify debugging level (scanner, parser, eval, netlink, segtree, all)\n" #endif "\n", name); @@ -139,6 +139,10 @@ static const struct { .level = DEBUG_NETLINK, }, { + .name = "segtree", + .level = DEBUG_SEGTREE, + }, + { .name = "all", .level = ~0, }, diff --git a/src/segtree.c b/src/segtree.c index 356a8b7..5426e24 100644 --- a/src/segtree.c +++ b/src/segtree.c @@ -157,6 +157,15 @@ static void __ei_insert(struct seg_tree *tree, struct elementary_interval *new) rb_insert_color(&new->rb_node, &tree->root); } +static bool segtree_debug(void) +{ +#ifdef DEBUG + if (debug_level & DEBUG_SEGTREE) + return true; +#endif + return false; +} + /** * ei_insert - insert an elementary interval into the tree * @@ -180,7 +189,8 @@ static void ei_insert(struct seg_tree *tree, struct elementary_interval *new) lei = ei_lookup(tree, new->left); rei = ei_lookup(tree, new->right); - pr_debug("insert: [%Zx %Zx]\n", new->left, new->right); + if (segtree_debug()) + pr_debug("insert: [%Zx %Zx]\n", new->left, new->right); if (lei != NULL && rei != NULL && lei == rei) { /* @@ -189,7 +199,8 @@ static void ei_insert(struct seg_tree *tree, struct elementary_interval *new) * * [lei_left, new_left) and (new_right, rei_right] */ - pr_debug("split [%Zx %Zx]\n", lei->left, lei->right); + if (segtree_debug()) + pr_debug("split [%Zx %Zx]\n", lei->left, lei->right); ei_remove(tree, lei); @@ -208,7 +219,10 @@ static void ei_insert(struct seg_tree *tree, struct elementary_interval *new) * * [lei_left, new_left)[new_left, new_right] */ - pr_debug("adjust left [%Zx %Zx]\n", lei->left, lei->right); + if (segtree_debug()) { + pr_debug("adjust left [%Zx %Zx]\n", + lei->left, lei->right); + } mpz_sub_ui(lei->right, new->left, 1); mpz_sub(lei->size, lei->right, lei->left); @@ -223,7 +237,10 @@ static void ei_insert(struct seg_tree *tree, struct elementary_interval *new) * * [new_left, new_right](new_right, rei_right] */ - pr_debug("adjust right [%Zx %Zx]\n", rei->left, rei->right); + if (segtree_debug()) { + pr_debug("adjust right [%Zx %Zx]\n", + rei->left, rei->right); + } mpz_add_ui(rei->left, new->right, 1); mpz_sub(rei->size, rei->right, rei->left); @@ -350,7 +367,7 @@ static void set_to_segtree(struct expr *set, struct seg_tree *tree) for (n = 0; n < set->size; n++) { if (n < set->size - 1 && interval_conflict(intervals[n], intervals[n+1])) - printf("conflict\n"); + pr_debug("conflict\n"); ei_insert(tree, intervals[n]); } @@ -371,7 +388,8 @@ static void segtree_linearize(struct list_head *list, struct seg_tree *tree) * Convert the tree of open intervals to half-closed map expressions. */ rb_for_each_entry_safe(ei, node, next, &tree->root, rb_node) { - pr_debug("iter: [%Zx %Zx]\n", ei->left, ei->right); + if (segtree_debug()) + pr_debug("iter: [%Zx %Zx]\n", ei->left, ei->right); if (prev == NULL) { /* @@ -454,14 +472,19 @@ void set_to_intervals(struct set *set) segtree_linearize(&list, &tree); list_for_each_entry_safe(ei, next, &list, list) { - pr_debug("list: [%.*Zx %.*Zx]\n", - 2 * tree.keylen / BITS_PER_BYTE, ei->left, - 2 * tree.keylen / BITS_PER_BYTE, ei->right); + if (segtree_debug()) { + pr_debug("list: [%.*Zx %.*Zx]\n", + 2 * tree.keylen / BITS_PER_BYTE, ei->left, + 2 * tree.keylen / BITS_PER_BYTE, ei->right); + } set_insert_interval(set->init, &tree, ei); ei_destroy(ei); } - expr_print(set->init); printf("\n"); + if (segtree_debug()) { + expr_print(set->init); + pr_debug("\n"); + } } static bool range_is_prefix(const mpz_t range)
Currently, nft displays the debugging information if it's compiled with --enable-debug (which seems a good idea) and when intervals are used in maps. Add a new option to enable debugging to segtree, so we only get this information when explicitly requested. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- We need this in master to avoid inconditional display of the debugging information when --enable-debug is passed at configuration / compilation time. I'm aware this patch conflicts with next-3.14, that conflict should be easy to resolve. include/nftables.h | 1 + src/main.c | 6 +++++- src/segtree.c | 43 +++++++++++++++++++++++++++++++++---------- 3 files changed, 39 insertions(+), 11 deletions(-)