On Sunday 2008-11-09 14:19, Adam Nielsen wrote:
> Firstly, I've declared xt_led_info in both the iptables library and the
> kernel module. This probably isn't ideal, but I wanted to avoid having
> to create another header file, which presumably needs to be installed on
> the user's system before iptables can be compiled. Please let me know
> what the preferred solution to this might be.
One solution is to have a file <linux/netfilter/xt_LED.h>
in the kernel tree that is copied to the iptables tree
at iptables/include/linux/netfilter/xt_LED.h.
But you just gave me something to consider, so keep it as it is right now.
> Secondly, I'm not sure whether setting xtables_target.family to
> PF_UNSPEC is correct. Given that the process of blinking lights isn't
> related to any particular network protocol, I wanted the LED target to
> be available in as many cases as possible. I think PF_UNSPEC does this,
> but please advise if there is a more suitable alternative.
It is correct.
> @@ -0,0 +1,182 @@
> +/*
> + * libxt_LED.c - shared library add-on to iptables to add customized LED
> + * trigger support.
> + *
> + * (C) 2008 Adam Nielsen <a.nielsen@xxxxxxxxxxx>
> + *
> + * This allows you to blink LEDs on your system in response to incoming
> + * network packets.
> + *
> + * Create a LED trigger for incoming SSH traffic:
> + * iptables -A INPUT -p tcp --dport 22 -j LED --led-trigger-id ssh
> + *
> + * Then attach the new trigger to a LED on your system:
> + * echo netfilter-ssh > /sys/class/leds/<ledname>/trigger
> + *
> + * See /usr/src/linux/Documentation/leds-class.txt for LED details.
I do not think the help text is needed in code (either iptables
or kernel). It should be in the iptables manpage, i.e.
libxt_LED.man.
> +struct xt_led_info {
> + char id[26]; /* Unique ID for this trigger in the LED class */
> + int delay; /* Delay until LED is switched off again after trigger */
> +
> + void *internal_data; /* Kernel data used in the module */
> +};
Move it to xt_LED.h.
> +static void LED_init(struct xt_entry_target *t)
> +{
> + struct xt_led_info *led = (struct xt_led_info *)t->data;
> +
> + /* defaults */
> + led->id[0] = 0;
> + led->delay = 0;
> +
> + return;
> +}
It is by default initialized to zero, no need to do it yourself again.
(As such the entire function cen be gotten rid of.)
> +static int LED_parse(int c, char **argv, int invert, unsigned int *flags,
> + const void *entry, struct xt_entry_target **target)
> +{
> + struct xt_led_info *led = (struct xt_led_info *)(*target)->data;
> +
> + switch (c) {
> + case 'i':
> + if (check_inverse(optarg, &invert, NULL, 0))
> + exit_error(PARAMETER_PROBLEM,
> + "Unexpected `!' after --led-trigger-id");
Just remove check_inverse - no more intraposition negation support.
Inversion is indicated by the "invert" variable already.
> + if (strlen(optarg) > 15)
> + exit_error(PARAMETER_PROBLEM,
> + "--led-trigger-id must be 16 chars or less");
Why 15, if struct xt_led_info->name is 26 in size? Oh 'tis confusing!
Try using
strlen("netfilter-") + strlen(optarg) >= sizeof(led->name) instead.
> + case 'd':
> + if (check_inverse(optarg, &invert, NULL, 0))
> + exit_error(PARAMETER_PROBLEM,
> + "Unexpected `!' after --led-delay");
Remove check_inverse.
> + if (strncasecmp(optarg, "inf", 3) == 0)
> + led->delay = -1;
> + else
> + led->delay = atoi(optarg);
strto(u)l, for great justice :)
> + default:
> + break;
> +
No need for a blank default.
> +static void LED_final_check(unsigned int flags)
> +{
> + if (!flags)
> + exit_error(PARAMETER_PROBLEM, "--led-trigger-id must be
> specified");
> + return;
> +}
-return
> +/* Print our config data, e.g. with iptables -L -v */
> +static void LED_print(const void *ip, const struct xt_entry_target *target,
> + int numeric)
> +{
> + const struct xt_led_info *led = (const struct xt_led_info
> *)target->data;
> + const char *id = led->id + 10; /* trim off "netfilter-" prefix */
> + printf("led-trigger-id:'");
> + /* Some not so great code to escape single quotes in the ID */
iptables-restore does not recognize single-quoted words, you will
have to use double quotes.
Or just forbid quote characters altogether?
> + while (*id) {
> + if (*id == '\'')
> + printf("\\'");
> + else
> + printf("%c", *id);
> + id++;
> + }
> + if (led->delay == -1) printf("' led-delay:inf ");
> + else printf("' led-delay:%dms ", led->delay);
> + return;
> +}
> +
> +static void LED_save(const void *ip, const struct xt_entry_target *target)
> +{
> + const struct xt_led_info *led = (const struct xt_led_info
> *)target->data;
> + const char *id = led->id + 10; /* trim off "netfilter-" prefix */
strlen("netfilter-"); Compiler is smart enough to optimize it.
> +
> + /* The same not so great code to escape single quotes in the ID */
> + printf("--led-trigger-id '");
> + while (*id) {
> + if (*id == '\'')
> + printf("\\'");
> + else
> + printf("%c", *id);
> + id++;
> + }
> + printf("' ");
> +
> + /* Only print the delay if it's not zero (the default) */
> + if (led->delay > 0)
> + printf("--led-delay %d ", led->delay);
> + else if (led->delay == -1)
> + printf("--led-delay inf ");
> +
> + return;
> +}
> +
> +static struct xtables_target led_tg_reg = {
> + .family = PF_UNSPEC,
> + .name = "LED",
> + .version = XTABLES_VERSION,
> + .size = XT_ALIGN(sizeof(struct xt_led_info)),
> + .userspacesize = XT_ALIGN(sizeof(struct xt_led_info)),
Wow did you actually try this? Since you need...
.userspacesize = offsetof(struct xt_led_info, internal_data)
> + .help = LED_help,
> + .init = LED_init,
> + .parse = LED_parse,
> + .final_check = LED_final_check,
> + .extra_opts = LED_opts,
> + .print = LED_print,
> + .save = LED_save,
> +};
> +
> +void _init(void)
> +{
> + xtables_register_target(&led_tg_reg);
> +}
> diff --git a/extensions/libxt_LED.man b/extensions/libxt_LED.man
> new file mode 100644
> index 0000000..96dd17c
> --- /dev/null
> +++ b/extensions/libxt_LED.man
> @@ -0,0 +1,19 @@
> +This creates an LED-trigger that can then be attached to system indicator
> +lights, to blink or illuminate them when certain packets pass through the
> +system. One example might be to light up an LED for a few minutes every time
> +an SSH connection is made to the local machine. The following options control
> +the trigger behaviour:
> +.TP
> +.BI "--led-trigger-id " "name"
> +This is the name given to the LED trigger. The actual name of the trigger
> +will have the prefix "netfilter-".
> +.TP
> +.BI "--led-delay " "ms"
> +This indicates how long (in milliseconds) the LED should be left illuminated
> +when a packet arrives before being switched off again. The default is 0
> +(blink as fast as possible.) The special value
> +.I inf
> +can be given to leave the LED on permanently once activated. (In this case
> +the trigger will need to be manually detached and reattached to the LED device
> +to switch it off again.)
> +.PP
I prefer \fB/\fI over .B/.I because they read more like inline HTML,
but that's me.
No trailing .PP.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
