[PATCH v2] iplink_rmnet: Allow passing IFLA_RMNET_FLAGS

Subsystems: the rest

STALE1773d

5 messages, 3 authors, 2021-09-28 · open the first message on its own page

[PATCH v2] iplink_rmnet: Allow passing IFLA_RMNET_FLAGS

From: Bjorn Andersson <hidden>
Date: 2021-09-24 03:33:21

Extend the rmnet option parser to allow enabling and disabling
IFLA_RMNET_FLAGS using ip link and add the flags to the pint_op to allow
inspecting the current settings.

Signed-off-by: Bjorn Andersson <redacted>
---

Changes since v1:
- Landed ABI change to allow setting/clearing individual bits
- Changed parser to take on/off arguments
- Added the new v5 chksum bits
- Made print_flags fancier, with some inspiration from iplink_vlan

 ip/iplink_rmnet.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
diff --git a/ip/iplink_rmnet.c b/ip/iplink_rmnet.c
index 1d16440c6900..f629ca9976d9 100644
--- a/ip/iplink_rmnet.c
+++ b/ip/iplink_rmnet.c
@@ -16,6 +16,12 @@ static void print_explain(FILE *f)
 {
 	fprintf(f,
 		"Usage: ... rmnet mux_id MUXID\n"
+		"                 [ingress-deaggregation { on | off } ]\n"
+		"                 [ingress-commands { on | off } ]\n"
+		"                 [ingress-chksumv4 { on | off } ]\n"
+		"                 [ingress-chksumv5 { on | off } ]\n"
+		"                 [egress-chksumv4 { on | off } ]\n"
+		"                 [egress-chksumv5 { on | off } ]\n"
 		"\n"
 		"MUXID := 1-254\n"
 	);
@@ -26,9 +32,16 @@ static void explain(void)
 	print_explain(stderr);
 }
 
+static int on_off(const char *msg, const char *arg)
+{
+	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n", msg, arg);
+	return -1;
+}
+
 static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
 			   struct nlmsghdr *n)
 {
+	struct ifla_rmnet_flags flags = { };
 	__u16 mux_id;
 
 	while (argc > 0) {
@@ -37,6 +50,60 @@ static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
 			if (get_u16(&mux_id, *argv, 0))
 				invarg("mux_id is invalid", *argv);
 			addattr16(n, 1024, IFLA_RMNET_MUX_ID, mux_id);
+		} else if (matches(*argv, "ingress-deaggregation") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			else
+				return on_off("ingress-deaggregation", *argv);
+		} else if (matches(*argv, "ingress-commands") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			else
+				return on_off("ingress-commands", *argv);
+		} else if (matches(*argv, "ingress-chksumv4") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			else
+				return on_off("ingress-chksumv4", *argv);
+		} else if (matches(*argv, "ingress-chksumv5") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			else
+				return on_off("ingress-chksumv5", *argv);
+		} else if (matches(*argv, "egress-chksumv4") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			else
+				return on_off("egress-chksumv4", *argv);
+		} else if (matches(*argv, "egress-chksumv5") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			else
+				return on_off("egress-chksumv5", *argv);
 		} else if (matches(*argv, "help") == 0) {
 			explain();
 			return -1;
@@ -48,11 +115,33 @@ static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
 		argc--, argv++;
 	}
 
+	if (flags.mask)
+		addattr_l(n, 1024, IFLA_RMNET_FLAGS, &flags, sizeof(flags));
+
 	return 0;
 }
 
+static void rmnet_print_flags(FILE *fp, __u32 flags)
+{
+	open_json_array(PRINT_ANY, is_json_context() ? "flags" : "<");
+#define _PF(f, s) if (flags & RMNET_FLAGS_##f) {			\
+		flags &= ~RMNET_FLAGS_##f;				\
+		print_string(PRINT_ANY, NULL, flags ? "%s," : "%s", s); \
+	}
+	_PF(INGRESS_DEAGGREGATION, "ingress-deaggregation");
+	_PF(INGRESS_MAP_COMMANDS, "ingress-commands");
+	_PF(INGRESS_MAP_CKSUMV4, "ingress-chksumv4");
+	_PF(INGRESS_MAP_CKSUMV5, "ingress-chksumv5");
+	_PF(EGRESS_MAP_CKSUMV4, "egress-chksumv4");
+	_PF(EGRESS_MAP_CKSUMV5, "egress-chksumv5");
+#undef _PF
+	close_json_array(PRINT_ANY, "> ");
+}
+
 static void rmnet_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 {
+	struct ifla_vlan_flags *flags;
+
 	if (!tb)
 		return;
 
@@ -64,6 +153,14 @@ static void rmnet_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		   "mux_id",
 		   "mux_id %u ",
 		   rta_getattr_u16(tb[IFLA_RMNET_MUX_ID]));
+
+	if (tb[IFLA_RMNET_FLAGS]) {
+		if (RTA_PAYLOAD(tb[IFLA_RMNET_FLAGS]) < sizeof(*flags))
+			return;
+		flags = RTA_DATA(tb[IFLA_RMNET_FLAGS]);
+
+		rmnet_print_flags(f, flags->flags);
+	}
 }
 
 static void rmnet_print_help(struct link_util *lu, int argc, char **argv,
-- 
2.33.0

Re: [PATCH v2] iplink_rmnet: Allow passing IFLA_RMNET_FLAGS

From: David Ahern <hidden>
Date: 2021-09-27 13:48:59

On 9/23/21 9:33 PM, Bjorn Andersson wrote:
quoted hunk
diff --git a/ip/iplink_rmnet.c b/ip/iplink_rmnet.c
index 1d16440c6900..f629ca9976d9 100644
--- a/ip/iplink_rmnet.c
+++ b/ip/iplink_rmnet.c
@@ -16,6 +16,12 @@ static void print_explain(FILE *f)
 {
 	fprintf(f,
 		"Usage: ... rmnet mux_id MUXID\n"
+		"                 [ingress-deaggregation { on | off } ]\n"
+		"                 [ingress-commands { on | off } ]\n"
+		"                 [ingress-chksumv4 { on | off } ]\n"
+		"                 [ingress-chksumv5 { on | off } ]\n"
+		"                 [egress-chksumv4 { on | off } ]\n"
+		"                 [egress-chksumv5 { on | off } ]\n"
 		"\n"
 		"MUXID := 1-254\n"
 	);
@@ -26,9 +32,16 @@ static void explain(void)
 	print_explain(stderr);
 }
 
+static int on_off(const char *msg, const char *arg)
+{
+	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n", msg, arg);
+	return -1;
+}
+
 static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
 			   struct nlmsghdr *n)
 {
+	struct ifla_rmnet_flags flags = { };
 	__u16 mux_id;
 
 	while (argc > 0) {
@@ -37,6 +50,60 @@ static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
 			if (get_u16(&mux_id, *argv, 0))
 				invarg("mux_id is invalid", *argv);
 			addattr16(n, 1024, IFLA_RMNET_MUX_ID, mux_id);
+		} else if (matches(*argv, "ingress-deaggregation") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			else
+				return on_off("ingress-deaggregation", *argv);
+		} else if (matches(*argv, "ingress-commands") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			else
+				return on_off("ingress-commands", *argv);
+		} else if (matches(*argv, "ingress-chksumv4") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			else
+				return on_off("ingress-chksumv4", *argv);
+		} else if (matches(*argv, "ingress-chksumv5") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			else
+				return on_off("ingress-chksumv5", *argv);
+		} else if (matches(*argv, "egress-chksumv4") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			else
+				return on_off("egress-chksumv4", *argv);
+		} else if (matches(*argv, "egress-chksumv5") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			else
+				return on_off("egress-chksumv5", *argv);
 		} else if (matches(*argv, "help") == 0) {
 			explain();
 			return -1;
use strcmp for new options. Also, please use 'csum' instead of 'chksum'
in the names. csum is already widely used in ip commands.

Re: [PATCH v2] iplink_rmnet: Allow passing IFLA_RMNET_FLAGS

From: Alex Elder <hidden>
Date: 2021-09-27 22:44:37

On 9/27/21 8:48 AM, David Ahern wrote:
quoted
  		} else if (matches(*argv, "help") == 0) {
  			explain();
  			return -1;
use strcmp for new options. Also, please use 'csum' instead of 'chksum'
in the names. csum is already widely used in ip commands.
On the csum remark, I agree completely.

On the other:  Are you saying to use strcmp() instead of
matches()?

That seems strange to me because matches() is used *much*
more often than strcmp(), and handles an empty *argv
differently.

I don't disagree with your suggested change, but upon
looking at the other code it surprises me a bit.  Can
you provide a little more explanation?  If you mean
something else, please clarify.  Thanks.

					-Alex

Re: [PATCH v2] iplink_rmnet: Allow passing IFLA_RMNET_FLAGS

From: Alex Elder <hidden>
Date: 2021-09-27 22:44:42

On 9/23/21 10:33 PM, Bjorn Andersson wrote:
Extend the rmnet option parser to allow enabling and disabling
IFLA_RMNET_FLAGS using ip link and add the flags to the pint_op to allow
inspecting the current settings.

Signed-off-by: Bjorn Andersson <redacted>
I think what you're doing looks OK to me, but I know
David had suggestions so I expect I'll get another shot
at review...

I have a couple very minor comments, and they're basically
unrelated.  So I guess take it or leave it.

					-Alex
quoted hunk
---

Changes since v1:
- Landed ABI change to allow setting/clearing individual bits
- Changed parser to take on/off arguments
- Added the new v5 chksum bits
- Made print_flags fancier, with some inspiration from iplink_vlan

  ip/iplink_rmnet.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 97 insertions(+)
diff --git a/ip/iplink_rmnet.c b/ip/iplink_rmnet.c
index 1d16440c6900..f629ca9976d9 100644
--- a/ip/iplink_rmnet.c
+++ b/ip/iplink_rmnet.c
@@ -16,6 +16,12 @@ static void print_explain(FILE *f)
  {
  	fprintf(f,
  		"Usage: ... rmnet mux_id MUXID\n"
+		"                 [ingress-deaggregation { on | off } ]\n"
+		"                 [ingress-commands { on | off } ]\n"
+		"                 [ingress-chksumv4 { on | off } ]\n"
+		"                 [ingress-chksumv5 { on | off } ]\n"
+		"                 [egress-chksumv4 { on | off } ]\n"
+		"                 [egress-chksumv5 { on | off } ]\n"
Looking at other usage messages, it appears that the dash
rather than underscore is preferable (though not universal).
I.e., I think you did this right.
quoted hunk
  		"\n"
  		"MUXID := 1-254\n"
  	);
@@ -26,9 +32,16 @@ static void explain(void)
  	print_explain(stderr);
  }
  
+static int on_off(const char *msg, const char *arg)
+{
+	fprintf(stderr, "Error: argument of \"%s\" must be \"on\" or \"off\", not \"%s\"\n", msg, arg);
It would be nice if this function could be defined centrally rather
than repeated.  Maybe defined in "lib/utils.c", declared in
"include/utils.h"?  Not your problem but if you were so moved,
that could be done in a separate patch.
quoted hunk
+	return -1;
+}
+
  static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
  			   struct nlmsghdr *n)
  {
+	struct ifla_rmnet_flags flags = { };
  	__u16 mux_id;
  
  	while (argc > 0) {
@@ -37,6 +50,60 @@ static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
  			if (get_u16(&mux_id, *argv, 0))
  				invarg("mux_id is invalid", *argv);
  			addattr16(n, 1024, IFLA_RMNET_MUX_ID, mux_id);
+		} else if (matches(*argv, "ingress-deaggregation") == 0) {
+			NEXT_ARG();
With these repeated blocks of code my instinct is to suggest
creating a common called function, but it gets a little ugly.
What I sketch below precludes using NEXT_ARG() where it's
needed.

static int matches_on_off(ifla_rmnet_flags *flags, u32 mask,
			  const char *name, char *arg)
{
	if (matches(arg, name))
		return 0;

	if (strcmp(arg, "on") == 0)
		flags->flag |= mask;
	else if (strcmp(arg, "off") == 0) {
		flags->flag &= ~mask;
	else
		return on_off(name, arg);

	flags->mask |= mask;

	return 1;
}

Then use:

	if (matches_on_off(&flags, RMNET_FLAGS_INGRESS_DEAGGREGATION,
			   "ingress-deaggregation", *argv) < 0)
		return -1;
	else if (matches_on_off(...))
		. . .

So I'm not sure I like how this looks, and once again,
it isn't really necessary for what you're doing here...
quoted hunk
+			flags.mask |= RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_DEAGGREGATION;
+			else
+				return on_off("ingress-deaggregation", *argv);
+		} else if (matches(*argv, "ingress-commands") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_COMMANDS;
+			else
+				return on_off("ingress-commands", *argv);
+		} else if (matches(*argv, "ingress-chksumv4") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_CKSUMV4;
+			else
+				return on_off("ingress-chksumv4", *argv);
+		} else if (matches(*argv, "ingress-chksumv5") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_INGRESS_MAP_CKSUMV5;
+			else
+				return on_off("ingress-chksumv5", *argv);
+		} else if (matches(*argv, "egress-chksumv4") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_EGRESS_MAP_CKSUMV4;
+			else
+				return on_off("egress-chksumv4", *argv);
+		} else if (matches(*argv, "egress-chksumv5") == 0) {
+			NEXT_ARG();
+			flags.mask |= RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			if (strcmp(*argv, "on") == 0)
+				flags.flags |= RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			else if (strcmp(*argv, "off") == 0)
+				flags.flags &= ~RMNET_FLAGS_EGRESS_MAP_CKSUMV5;
+			else
+				return on_off("egress-chksumv5", *argv);
  		} else if (matches(*argv, "help") == 0) {
  			explain();
  			return -1;
@@ -48,11 +115,33 @@ static int rmnet_parse_opt(struct link_util *lu, int argc, char **argv,
  		argc--, argv++;
  	}
  
+	if (flags.mask)
+		addattr_l(n, 1024, IFLA_RMNET_FLAGS, &flags, sizeof(flags));
+
  	return 0;
  }
  
+static void rmnet_print_flags(FILE *fp, __u32 flags)
+{
+	open_json_array(PRINT_ANY, is_json_context() ? "flags" : "<");
+#define _PF(f, s) if (flags & RMNET_FLAGS_##f) {			\
+		flags &= ~RMNET_FLAGS_##f;				\
+		print_string(PRINT_ANY, NULL, flags ? "%s," : "%s", s); \
+	}
+	_PF(INGRESS_DEAGGREGATION, "ingress-deaggregation");
+	_PF(INGRESS_MAP_COMMANDS, "ingress-commands");
+	_PF(INGRESS_MAP_CKSUMV4, "ingress-chksumv4");
+	_PF(INGRESS_MAP_CKSUMV5, "ingress-chksumv5");
+	_PF(EGRESS_MAP_CKSUMV4, "egress-chksumv4");
+	_PF(EGRESS_MAP_CKSUMV5, "egress-chksumv5");
+#undef _PF
+	close_json_array(PRINT_ANY, "> ");
+}
+
  static void rmnet_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
  {
+	struct ifla_vlan_flags *flags;
+
  	if (!tb)
  		return;
  
@@ -64,6 +153,14 @@ static void rmnet_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
  		   "mux_id",
  		   "mux_id %u ",
  		   rta_getattr_u16(tb[IFLA_RMNET_MUX_ID]));
+
+	if (tb[IFLA_RMNET_FLAGS]) {
+		if (RTA_PAYLOAD(tb[IFLA_RMNET_FLAGS]) < sizeof(*flags))
+			return;
+		flags = RTA_DATA(tb[IFLA_RMNET_FLAGS]);
+
+		rmnet_print_flags(f, flags->flags);
+	}
  }
  
  static void rmnet_print_help(struct link_util *lu, int argc, char **argv,

Re: [PATCH v2] iplink_rmnet: Allow passing IFLA_RMNET_FLAGS

From: David Ahern <hidden>
Date: 2021-09-28 02:09:57

On 9/27/21 4:44 PM, Alex Elder wrote:
On 9/27/21 8:48 AM, David Ahern wrote:
quoted
quoted
          } else if (matches(*argv, "help") == 0) {
              explain();
              return -1;
use strcmp for new options. Also, please use 'csum' instead of 'chksum'
in the names. csum is already widely used in ip commands.
On the csum remark, I agree completely.

On the other:  Are you saying to use strcmp() instead of
matches()?

That seems strange to me because matches() is used *much*
more often than strcmp(), and handles an empty *argv
differently.
matches is way beyond its usefulness. Its use is now deprecated. It just
leads to confusing command lines and problems maintaining which option
hits the matches.
I don't disagree with your suggested change, but upon
looking at the other code it surprises me a bit.  Can
you provide a little more explanation?  If you mean
something else, please clarify.  Thanks.

                    -Alex

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help