Adam Nielsen wrote:
+static unsigned int
+led_tg(struct sk_buff *skb, const struct xt_target_param *par)
+{
+ const struct xt_led_info *ledinfo = par->targinfo;
+ /*noconst*/struct xt_led_info_internal *ledinternal =
ledinfo->internal_data;
+
+ /*
+ * Make sure the timer callback doesn't go switching the LED off while
+ * we're figuring out what to do
+ */
+ if (ledinfo->delay > 0) {
+ mutex_lock(&ledinternal->led_changing_state);
You can't use a mutex here, this might be running in softirq context.
Is locking necessary at all? Whats the worst that might happen? The
LED might forget to blink once? :)
+static bool led_tg_check(const struct xt_tgchk_param *par)
+{
+ /*noconst*/ struct xt_led_info *ledinfo = par->targinfo;
+ struct xt_led_info_internal *ledinternal;
+ int err;
+
+ if (ledinfo->id[0] == '\0') {
+ printk(KERN_CRIT KBUILD_MODNAME ": No 'id' parameter given.\n");
+ return false;
+ }
I guess KERN_ERR or default level is enough.
+ if (!(ledinternal = kzalloc(sizeof(struct xt_led_info_internal),
GFP_KERNEL))) {
+ printk(KERN_CRIT KBUILD_MODNAME ": out of memory\n");
+ return false;
+ }
Please seperate assignments from comparisons.
+
+ ledinternal->netfilter_led_trigger.name = ledinfo->id;
+ mutex_init(&ledinternal->led_changing_state);
+
+ printk(KERN_NOTICE KBUILD_MODNAME ": Adding led trigger \"%s\"\n",
+ ledinfo->id);
Too noisy, no printks except errors please.
+
+ err = led_trigger_register(&ledinternal->netfilter_led_trigger);
+ if (err) {
+ printk(KERN_CRIT KBUILD_MODNAME
+ ": led_trigger_register() failed\n");
+ if (err == -EEXIST) {
+ printk(KERN_CRIT KBUILD_MODNAME
+ ": Trigger name is already in use.\n");
+ }
+ goto exit_alloc;
+ }
+
+ /* See if we need to set up a timer */
+ if (ledinfo->delay > 0)
+ setup_timer(&ledinternal->timer, led_timeout_callback,
+ (unsigned long) ledinfo);
+
+ ledinfo->internal_data = ledinternal;
+
+ return true;
+
+exit_alloc:
+ kfree(ledinternal);
+
+ return false;
+}
+
+static void led_tg_destroy(const struct xt_tgdtor_param *par)
+{
+ const struct xt_led_info *ledinfo = par->targinfo;
+ /*noconst*/struct xt_led_info_internal *ledinternal =
ledinfo->internal_data;
+
+ printk(KERN_NOTICE KBUILD_MODNAME ": Removing led trigger \"%s\"\n",
+ ledinternal->netfilter_led_trigger.name);
+
+ if (ledinfo->delay > 0)
+ del_timer_sync(&ledinternal->timer);
+
+ led_trigger_unregister(&ledinternal->netfilter_led_trigger);
+ kfree(ledinternal);
+}
+
+static int __init led_tg_init(void)
+{
+ printk(KERN_NOTICE KBUILD_MODNAME ": Registering LED netfilter
target\n");
This is too noisy, please remove this.
+ return xt_register_target(&led_tg_reg);
+}
+
+static void __exit led_tg_exit(void)
+{
+ printk(KERN_NOTICE KBUILD_MODNAME ": Unregistering LED netfilter
target\n");
+ xt_unregister_target(&led_tg_reg);
+}
--
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
