netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netfilter: add CHECKSUM target
@ 2010-07-08 22:29 Michael S. Tsirkin
  2010-07-08 22:35 ` [PATCH] extensions: libipt_CHECKSUM extension Michael S. Tsirkin
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-08 22:29 UTC (permalink / raw)
  To: Michael S. Tsirkin, Patrick McHardy, David S. Miller,
	Alexey Kuznetsov, "Pekka Savola (ipv6)" <pek

This adds a `CHECKSUM' target, which can be used in the iptables mangle
table.

You can use this target to compute and fill in the checksum in
an IP packet that lacks a checksum.  This is particularly useful,
if you need to work around old applications such as dhcp clients,
that do not work well with checksum offloads, but don't want to
disable checksum offload in your device.

The problem happens in the field with virtualized applications.
For reference, see Red Hat bz 605555, as well as
http://www.spinics.net/lists/kvm/msg37660.html

Typical expected use (helps old dhclient binary running in a VM):
iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
--checksum-fill

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Lightly tested. Please review for 2.6.36. Thanks!

 include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
 net/ipv4/netfilter/Kconfig                  |   16 ++++++
 net/ipv4/netfilter/Makefile                 |    1 +
 net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++
 4 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/netfilter_ipv4/ipt_CHECKSUM.h
 create mode 100644 net/ipv4/netfilter/ipt_CHECKSUM.c

diff --git a/include/linux/netfilter_ipv4/ipt_CHECKSUM.h b/include/linux/netfilter_ipv4/ipt_CHECKSUM.h
new file mode 100644
index 0000000..0b0f660
--- /dev/null
+++ b/include/linux/netfilter_ipv4/ipt_CHECKSUM.h
@@ -0,0 +1,18 @@
+/* Header file for iptables ipt_CHECKSUM target
+ *
+ * (C) 2002 by Harald Welte <laforge@gnumonks.org>
+ * (C) 2010 Red Hat Inc
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This software is distributed under GNU GPL v2, 1991
+*/
+#ifndef _IPT_CHECKSUM_TARGET_H
+#define _IPT_CHECKSUM_TARGET_H
+
+#define IPT_CHECKSUM_OP_FILL	0x01	/* fill in checksum in IPv4 header */
+
+struct ipt_CHECKSUM_info {
+	u_int8_t operation;	/* bitset of operations */
+};
+
+#endif /* _IPT_CHECKSUM_TARGET_H */
diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
index 1833bdb..2fb979b 100644
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -322,6 +322,22 @@ config IP_NF_TARGET_ECN
 
 	  To compile it as a module, choose M here.  If unsure, say N.
 
+config IP_NF_TARGET_CHECKSUM
+	tristate "CHECKSUM target support"
+	depends on IP_NF_MANGLE
+	depends on NETFILTER_ADVANCED
+	---help---
+	  This option adds a `CHECKSUM' target, which can be used in the iptables mangle
+	  table.  
+
+	  You can use this target to compute and fill in the checksum in
+	  an IP packet that lacks a checksum.  This is particularly useful,
+	  if you need to work around old applications such as dhcp clients,
+	  that do not work well with checksum offloads, but don't want to disable
+	  checksum offload in your device.
+
+	  To compile it as a module, choose M here.  If unsure, say N.
+
 config IP_NF_TARGET_TTL
 	tristate '"TTL" target support'
 	depends on NETFILTER_ADVANCED
diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
index 4811159..d7047bb 100644
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_IP_NF_MATCH_ECN) += ipt_ecn.o
 # targets
 obj-$(CONFIG_IP_NF_TARGET_CLUSTERIP) += ipt_CLUSTERIP.o
 obj-$(CONFIG_IP_NF_TARGET_ECN) += ipt_ECN.o
+obj-$(CONFIG_IP_NF_TARGET_CHECKSUM) += ipt_CHECKSUM.o
 obj-$(CONFIG_IP_NF_TARGET_LOG) += ipt_LOG.o
 obj-$(CONFIG_IP_NF_TARGET_MASQUERADE) += ipt_MASQUERADE.o
 obj-$(CONFIG_IP_NF_TARGET_NETMAP) += ipt_NETMAP.o
diff --git a/net/ipv4/netfilter/ipt_CHECKSUM.c b/net/ipv4/netfilter/ipt_CHECKSUM.c
new file mode 100644
index 0000000..b8346a1
--- /dev/null
+++ b/net/ipv4/netfilter/ipt_CHECKSUM.c
@@ -0,0 +1,72 @@
+/* iptables module for the IPv4 CHECKSUM
+ *
+ * (C) 2002 by Harald Welte <laforge@netfilter.org>
+ * (C) 2010 Red Hat, Inc.
+ *
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/in.h>
+#include <linux/module.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+
+#include <linux/netfilter/x_tables.h>
+#include <linux/netfilter_ipv4/ip_tables.h>
+#include <linux/netfilter_ipv4/ipt_CHECKSUM.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Michael S. Tsirkin <mst@redhat.com>");
+MODULE_DESCRIPTION("Xtables: checksum modification");
+
+static unsigned int
+checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
+{
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		skb_checksum_help(skb);
+	}
+
+	return XT_CONTINUE;
+}
+
+static int checksum_tg_check(const struct xt_tgchk_param *par)
+{
+	const struct ipt_CHECKSUM_info *einfo = par->targinfo;
+
+	if (einfo->operation & ~IPT_CHECKSUM_OP_FILL) {
+		pr_info("unsupported CHECKSUM operation %x\n", einfo->operation);
+		return -EINVAL;
+	}
+	if (!einfo->operation) {
+		pr_info("no CHECKSUM operation enabled\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct xt_target checksum_tg_reg __read_mostly = {
+	.name		= "CHECKSUM",
+	.family		= NFPROTO_IPV4,
+	.target		= checksum_tg,
+	.targetsize	= sizeof(struct ipt_CHECKSUM_info),
+	.table		= "mangle",
+	.checkentry	= checksum_tg_check,
+	.me		= THIS_MODULE,
+};
+
+static int __init checksum_tg_init(void)
+{
+	return xt_register_target(&checksum_tg_reg);
+}
+
+static void __exit checksum_tg_exit(void)
+{
+	xt_unregister_target(&checksum_tg_reg);
+}
+
+module_init(checksum_tg_init);
+module_exit(checksum_tg_exit);
-- 
1.7.2.rc0.14.g41c1c

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH] extensions: libipt_CHECKSUM extension
  2010-07-08 22:29 [PATCH] netfilter: add CHECKSUM target Michael S. Tsirkin
@ 2010-07-08 22:35 ` Michael S. Tsirkin
  2010-07-09  7:18 ` [PATCH] netfilter: add CHECKSUM target Herbert Xu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-08 22:35 UTC (permalink / raw)
  To: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris <jmorris@

This adds a `CHECKSUM' target, which can be used in the iptables mangle
table.

You can use this target to compute and fill in the checksum in
an IP packet that lacks a checksum.  This is particularly useful,
if you need to work around old applications such as dhcp clients,
that do not work well with checksum offloads, but don't want to disable
checksum offload in your device.

The problem happens in the field with virtualized applications.
For reference, see Red Hat bz 605555, as well as
http://www.spinics.net/lists/kvm/msg37660.html

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

This patch adds userspace support for the CHECKSUM kernel module I sent previously.

 extensions/libipt_CHECKSUM.c   |  100 ++++++++++++++++++++++++++++++++++++++++
 extensions/libipt_CHECKSUM.man |    8 +++
 2 files changed, 108 insertions(+), 0 deletions(-)
 create mode 100644 extensions/libipt_CHECKSUM.c
 create mode 100644 extensions/libipt_CHECKSUM.man

diff --git a/extensions/libipt_CHECKSUM.c b/extensions/libipt_CHECKSUM.c
new file mode 100644
index 0000000..93d3e3f
--- /dev/null
+++ b/extensions/libipt_CHECKSUM.c
@@ -0,0 +1,100 @@
+/* Shared library add-on to iptables for CHECKSUM
+ *
+ * (C) 2002 by Harald Welte <laforge@gnumonks.org>
+ * (C) 2010 by Red Hat, Inc
+ * Author: Michael S. Tsirkin <mst@redhat.com>
+ *
+ * This program is distributed under the terms of GNU GPL v2, 1991
+ *
+ * libipt_CHECKSUM.c borrowed heavily from libipt_ECN.c
+ *
+ * $Id$
+ */
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <getopt.h>
+
+#include <xtables.h>
+#include <linux/netfilter_ipv4/ipt_CHECKSUM.h>
+
+static void CHECKSUM_help(void)
+{
+	printf(
+"CHECKSUM target options\n"
+"  --checksum-fill			Fill in packet checksum.\n");
+}
+
+static const struct option CHECKSUM_opts[] = {
+	{ "checksum-fill", 0, NULL, 'F' },
+	{ .name = NULL }
+};
+
+static int CHECKSUM_parse(int c, char **argv, int invert, unsigned int *flags,
+                     const void *entry, struct xt_entry_target **target)
+{
+	unsigned int result;
+	struct ipt_CHECKSUM_info *einfo
+		= (struct ipt_CHECKSUM_info *)(*target)->data;
+
+	switch (c) {
+	case 'F':
+		if (*flags)
+			xtables_error(PARAMETER_PROBLEM,
+			        "CHECKSUM target: Only use --checksum-fill ONCE!");
+		einfo->operation = IPT_CHECKSUM_OP_FILL;
+		*flags |= IPT_CHECKSUM_OP_FILL;
+		break;
+	default:
+		return 0;
+	}
+
+	return 1;
+}
+
+static void CHECKSUM_check(unsigned int flags)
+{
+	if (!flags)
+		xtables_error(PARAMETER_PROBLEM,
+		           "CHECKSUM target: Parameter --checksum-fill is required");
+}
+
+static void CHECKSUM_print(const void *ip, const struct xt_entry_target *target,
+                      int numeric)
+{
+	const struct ipt_CHECKSUM_info *einfo =
+		(const struct ipt_CHECKSUM_info *)target->data;
+
+	printf("CHECKSUM ");
+
+	if (einfo->operation & IPT_CHECKSUM_OP_FILL)
+		printf("fill ");
+}
+
+static void CHECKSUM_save(const void *ip, const struct xt_entry_target *target)
+{
+	const struct ipt_CHECKSUM_info *einfo =
+		(const struct ipt_CHECKSUM_info *)target->data;
+
+	if (einfo->operation & IPT_CHECKSUM_OP_FILL)
+		printf("--checksum-fill ");
+}
+
+static struct xtables_target checksum_tg_reg = {
+	.name		= "CHECKSUM",
+	.version	= XTABLES_VERSION,
+	.family		= NFPROTO_IPV4,
+	.size		= XT_ALIGN(sizeof(struct ipt_CHECKSUM_info)),
+	.userspacesize	= XT_ALIGN(sizeof(struct ipt_CHECKSUM_info)),
+	.help		= CHECKSUM_help,
+	.parse		= CHECKSUM_parse,
+	.final_check	= CHECKSUM_check,
+	.print		= CHECKSUM_print,
+	.save		= CHECKSUM_save,
+	.extra_opts	= CHECKSUM_opts,
+};
+
+void _init(void)
+{
+	xtables_register_target(&checksum_tg_reg);
+}
diff --git a/extensions/libipt_CHECKSUM.man b/extensions/libipt_CHECKSUM.man
new file mode 100644
index 0000000..4f83335
--- /dev/null
+++ b/extensions/libipt_CHECKSUM.man
@@ -0,0 +1,8 @@
+This target allows to selectively work around broken/old applications.
+It can only be used in the mangle table.
+.TP
+\fB\-\-checksum\-fill\fP
+Compute and fill in the checksum in an IP packet that lacks a checksum.
+This is particularly useful, if you need to work around old applications
+such as dhcp clients, that do not work well with checksum offloads,
+but don't want to disable checksum offload in your device.
-- 
1.7.2.rc0.14.g41c1c

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-08 22:29 [PATCH] netfilter: add CHECKSUM target Michael S. Tsirkin
  2010-07-08 22:35 ` [PATCH] extensions: libipt_CHECKSUM extension Michael S. Tsirkin
@ 2010-07-09  7:18 ` Herbert Xu
  2010-07-11  9:21   ` Michael S. Tsirkin
  2010-07-09  9:56 ` Jan Engelhardt
  2010-07-09 15:17 ` Patrick McHardy
  3 siblings, 1 reply; 16+ messages in thread
From: Herbert Xu @ 2010-07-09  7:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev, kvm

On Fri, Jul 09, 2010 at 01:29:13AM +0300, Michael S. Tsirkin wrote:
> This adds a `CHECKSUM' target, which can be used in the iptables mangle
> table.
> 
> You can use this target to compute and fill in the checksum in
> an IP packet that lacks a checksum.  This is particularly useful,
> if you need to work around old applications such as dhcp clients,
> that do not work well with checksum offloads, but don't want to
> disable checksum offload in your device.
> 
> The problem happens in the field with virtualized applications.
> For reference, see Red Hat bz 605555, as well as
> http://www.spinics.net/lists/kvm/msg37660.html
> 
> Typical expected use (helps old dhclient binary running in a VM):
> iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
> --checksum-fill
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

I'd think that this target would be protocol-agnostic, no?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-08 22:29 [PATCH] netfilter: add CHECKSUM target Michael S. Tsirkin
  2010-07-08 22:35 ` [PATCH] extensions: libipt_CHECKSUM extension Michael S. Tsirkin
  2010-07-09  7:18 ` [PATCH] netfilter: add CHECKSUM target Herbert Xu
@ 2010-07-09  9:56 ` Jan Engelhardt
  2010-07-11 12:27   ` Michael S. Tsirkin
  2010-07-09 15:17 ` Patrick McHardy
  3 siblings, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2010-07-09  9:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
	herbert.xu, kvm


On Friday 2010-07-09 00:29, Michael S. Tsirkin wrote:
>
> include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
> net/ipv4/netfilter/Kconfig                  |   16 ++++++
> net/ipv4/netfilter/Makefile                 |    1 +
> net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++

New modules should use xt.

>+static unsigned int
>+checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
>+{
>+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
>+		skb_checksum_help(skb);
>+	}

- {}

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-08 22:29 [PATCH] netfilter: add CHECKSUM target Michael S. Tsirkin
                   ` (2 preceding siblings ...)
  2010-07-09  9:56 ` Jan Engelhardt
@ 2010-07-09 15:17 ` Patrick McHardy
  2010-07-09 16:26   ` Jan Engelhardt
  2010-07-11 10:47   ` Michael S. Tsirkin
  3 siblings, 2 replies; 16+ messages in thread
From: Patrick McHardy @ 2010-07-09 15:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, linux-kernel, netfilter-devel,
	netfilter, coreteam, netdev, herbert.xu, kvm

Am 09.07.2010 00:29, schrieb Michael S. Tsirkin:
> This adds a `CHECKSUM' target, which can be used in the iptables mangle
> table.
> 
> You can use this target to compute and fill in the checksum in
> an IP packet that lacks a checksum.  This is particularly useful,
> if you need to work around old applications such as dhcp clients,
> that do not work well with checksum offloads, but don't want to
> disable checksum offload in your device.
> 
> The problem happens in the field with virtualized applications.
> For reference, see Red Hat bz 605555, as well as
> http://www.spinics.net/lists/kvm/msg37660.html
> 
> Typical expected use (helps old dhclient binary running in a VM):
> iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
> --checksum-fill

I'm not sure this is something we want to merge upstream and
support indefinitely. Dave suggested this as a temporary
out-of-tree workaround until the majority of guest dhcp clients
are fixed. Has anything changed that makes this course of
action impractical?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-09 15:17 ` Patrick McHardy
@ 2010-07-09 16:26   ` Jan Engelhardt
  2010-07-09 16:36     ` Patrick McHardy
  2010-07-11 10:47   ` Michael S. Tsirkin
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2010-07-09 16:26 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Michael S. Tsirkin, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
	herbert.xu, kvm


On Friday 2010-07-09 17:17, Patrick McHardy wrote:
>
>> This adds a `CHECKSUM' target, which can be used in the iptables mangle
>> table.
>> 
>> You can use this target to compute and fill in the checksum in
>> an IP packet that lacks a checksum.  This is particularly useful,
>> if you need to work around old applications such as dhcp clients,
>> that do not work well with checksum offloads, but don't want to
>> disable checksum offload in your device.
>
>I'm not sure this is something we want to merge upstream and
>support indefinitely.

We could put it into Xtables-addons. That would also be consistent
with Dave's suggestion.

>Dave suggested this as a temporary
>out-of-tree workaround until the majority of guest dhcp clients
>are fixed. Has anything changed that makes this course of
>action impractical?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-09 16:26   ` Jan Engelhardt
@ 2010-07-09 16:36     ` Patrick McHardy
  0 siblings, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2010-07-09 16:36 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Michael S. Tsirkin, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
	herbert.xu, kvm

Am 09.07.2010 18:26, schrieb Jan Engelhardt:
> 
> On Friday 2010-07-09 17:17, Patrick McHardy wrote:
>>
>>> This adds a `CHECKSUM' target, which can be used in the iptables mangle
>>> table.
>>>
>>> You can use this target to compute and fill in the checksum in
>>> an IP packet that lacks a checksum.  This is particularly useful,
>>> if you need to work around old applications such as dhcp clients,
>>> that do not work well with checksum offloads, but don't want to
>>> disable checksum offload in your device.
>>
>> I'm not sure this is something we want to merge upstream and
>> support indefinitely.
> 
> We could put it into Xtables-addons. That would also be consistent
> with Dave's suggestion.

Sure, that would be fine with me.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-09  7:18 ` [PATCH] netfilter: add CHECKSUM target Herbert Xu
@ 2010-07-11  9:21   ` Michael S. Tsirkin
  2010-07-11 10:24     ` Herbert Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-11  9:21 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev, kvm

On Fri, Jul 09, 2010 at 03:18:14PM +0800, Herbert Xu wrote:
> On Fri, Jul 09, 2010 at 01:29:13AM +0300, Michael S. Tsirkin wrote:
> > This adds a `CHECKSUM' target, which can be used in the iptables mangle
> > table.
> > 
> > You can use this target to compute and fill in the checksum in
> > an IP packet that lacks a checksum.  This is particularly useful,
> > if you need to work around old applications such as dhcp clients,
> > that do not work well with checksum offloads, but don't want to
> > disable checksum offload in your device.
> > 
> > The problem happens in the field with virtualized applications.
> > For reference, see Red Hat bz 605555, as well as
> > http://www.spinics.net/lists/kvm/msg37660.html
> > 
> > Typical expected use (helps old dhclient binary running in a VM):
> > iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
> > --checksum-fill
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> I'd think that this target would be protocol-agnostic, no?

Meaning it should go into net/netfilter/? Will do.

> Cheers,
> -- 
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-11  9:21   ` Michael S. Tsirkin
@ 2010-07-11 10:24     ` Herbert Xu
  0 siblings, 0 replies; 16+ messages in thread
From: Herbert Xu @ 2010-07-11 10:24 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev, kvm

On Sun, Jul 11, 2010 at 12:21:17PM +0300, Michael S. Tsirkin wrote:
>
> > I'd think that this target would be protocol-agnostic, no?
> 
> Meaning it should go into net/netfilter/? Will do.

Right, and you should get rid of any IPv4 dependencies.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-09 15:17 ` Patrick McHardy
  2010-07-09 16:26   ` Jan Engelhardt
@ 2010-07-11 10:47   ` Michael S. Tsirkin
  2010-07-15 15:17     ` Patrick McHardy
  1 sibling, 1 reply; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-11 10:47 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, linux-kernel, netfilter-devel,
	netfilter, coreteam, netdev, herbert.xu, kvm

On Fri, Jul 09, 2010 at 05:17:36PM +0200, Patrick McHardy wrote:
> Am 09.07.2010 00:29, schrieb Michael S. Tsirkin:
> > This adds a `CHECKSUM' target, which can be used in the iptables mangle
> > table.
> > 
> > You can use this target to compute and fill in the checksum in
> > an IP packet that lacks a checksum.  This is particularly useful,
> > if you need to work around old applications such as dhcp clients,
> > that do not work well with checksum offloads, but don't want to
> > disable checksum offload in your device.
> > 
> > The problem happens in the field with virtualized applications.
> > For reference, see Red Hat bz 605555, as well as
> > http://www.spinics.net/lists/kvm/msg37660.html
> > 
> > Typical expected use (helps old dhclient binary running in a VM):
> > iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
> > --checksum-fill
> 
> I'm not sure this is something we want to merge upstream and
> support indefinitely. Dave suggested this as a temporary
> out-of-tree workaround until the majority of guest dhcp clients
> are fixed. Has anything changed that makes this course of
> action impractical?

If I understand what Dave said correctly, it's up to you ...

The arguments for putting this upstream are:

Given the track record, I wouldn't hope for quick fix in the majority of
guest dhcp clients, unfortunately :(.  We are talking years here.
Even after that, one of the uses of virtualization is
to keep old guests running. So yes, I think we'll
keep using work-arounds for this for a very long time.

Further, since we have to add the module and we have to teach management
to program it, it will be much less painful for everyone
involved if we can put the code upstream, rather than forking
management code.

-- 
MST

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-09  9:56 ` Jan Engelhardt
@ 2010-07-11 12:27   ` Michael S. Tsirkin
  2010-07-11 12:45     ` Changli Gao
  2010-07-11 12:45     ` Jan Engelhardt
  0 siblings, 2 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-11 12:27 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
	herbert.xu, kvm

On Fri, Jul 09, 2010 at 11:56:26AM +0200, Jan Engelhardt wrote:
> 
> On Friday 2010-07-09 00:29, Michael S. Tsirkin wrote:
> >
> > include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
> > net/ipv4/netfilter/Kconfig                  |   16 ++++++
> > net/ipv4/netfilter/Makefile                 |    1 +
> > net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++
> 
> New modules should use xt.

I tried moving the module to xt_CHECKSUM but now
	iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM --checksum-fill
does not to load the module.
It seems that xt_request_find_target in x_tables uses the prefix 'ip' for a module.
What am I missing?

> >+static unsigned int
> >+checksum_tg(struct sk_buff *skb, const struct xt_action_param *par)
> >+{
> >+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >+		skb_checksum_help(skb);
> >+	}
> 
> - {}

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-11 12:27   ` Michael S. Tsirkin
@ 2010-07-11 12:45     ` Changli Gao
  2010-07-11 13:19       ` Michael S. Tsirkin
  2010-07-11 12:45     ` Jan Engelhardt
  1 sibling, 1 reply; 16+ messages in thread
From: Changli Gao @ 2010-07-11 12:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jan Engelhardt, Patrick McHardy, David S. Miller,
	Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
	Hideaki YOSHIFUJI, linux-kernel, netfilter-devel, netfilter,
	coreteam, netdev, herbert.xu, kvm

On Sun, Jul 11, 2010 at 8:27 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Jul 09, 2010 at 11:56:26AM +0200, Jan Engelhardt wrote:
>>
>> On Friday 2010-07-09 00:29, Michael S. Tsirkin wrote:
>> >
>> > include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
>> > net/ipv4/netfilter/Kconfig                  |   16 ++++++
>> > net/ipv4/netfilter/Makefile                 |    1 +
>> > net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++
>>
>> New modules should use xt.
>
> I tried moving the module to xt_CHECKSUM but now
>        iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM --checksum-fill
> does not to load the module.
> It seems that xt_request_find_target in x_tables uses the prefix 'ip' for a module.
> What am I missing?
>

You should add MODULE_ALIAS clause, i.e.

MODULE_ALIAS("ipt_CHECKSUM");

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-11 12:27   ` Michael S. Tsirkin
  2010-07-11 12:45     ` Changli Gao
@ 2010-07-11 12:45     ` Jan Engelhardt
  2010-07-11 13:19       ` Michael S. Tsirkin
  1 sibling, 1 reply; 16+ messages in thread
From: Jan Engelhardt @ 2010-07-11 12:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
	herbert.xu, kvm


On Sunday 2010-07-11 14:27, Michael S. Tsirkin wrote:
>> >
>> > include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
>> > net/ipv4/netfilter/Kconfig                  |   16 ++++++
>> > net/ipv4/netfilter/Makefile                 |    1 +
>> > net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++
>> 
>> New modules should use xt.
>
>I tried moving the module to xt_CHECKSUM but now
>	iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM --checksum-fill
>does not to load the module.
>It seems that xt_request_find_target in x_tables uses the prefix 'ip' for a module.
>What am I missing?

MODULE_ALIAS like the others have.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-11 12:45     ` Jan Engelhardt
@ 2010-07-11 13:19       ` Michael S. Tsirkin
  0 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-11 13:19 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Patrick McHardy, David S. Miller, Alexey Kuznetsov,
	Pekka Savola (ipv6), James Morris, Hideaki YOSHIFUJI,
	linux-kernel, netfilter-devel, netfilter, coreteam, netdev,
	herbert.xu, kvm

On Sun, Jul 11, 2010 at 02:45:49PM +0200, Jan Engelhardt wrote:
> 
> On Sunday 2010-07-11 14:27, Michael S. Tsirkin wrote:
> >> >
> >> > include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
> >> > net/ipv4/netfilter/Kconfig                  |   16 ++++++
> >> > net/ipv4/netfilter/Makefile                 |    1 +
> >> > net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++
> >> 
> >> New modules should use xt.
> >
> >I tried moving the module to xt_CHECKSUM but now
> >	iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM --checksum-fill
> >does not to load the module.
> >It seems that xt_request_find_target in x_tables uses the prefix 'ip' for a module.
> >What am I missing?
> 
> MODULE_ALIAS like the others have.

Worked for me, thanks!

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-11 12:45     ` Changli Gao
@ 2010-07-11 13:19       ` Michael S. Tsirkin
  0 siblings, 0 replies; 16+ messages in thread
From: Michael S. Tsirkin @ 2010-07-11 13:19 UTC (permalink / raw)
  To: Changli Gao
  Cc: Jan Engelhardt, Patrick McHardy, David S. Miller,
	Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
	Hideaki YOSHIFUJI, linux-kernel, netfilter-devel, netfilter,
	coreteam, netdev, herbert.xu, kvm

On Sun, Jul 11, 2010 at 08:45:01PM +0800, Changli Gao wrote:
> On Sun, Jul 11, 2010 at 8:27 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Jul 09, 2010 at 11:56:26AM +0200, Jan Engelhardt wrote:
> >>
> >> On Friday 2010-07-09 00:29, Michael S. Tsirkin wrote:
> >> >
> >> > include/linux/netfilter_ipv4/ipt_CHECKSUM.h |   18 +++++++
> >> > net/ipv4/netfilter/Kconfig                  |   16 ++++++
> >> > net/ipv4/netfilter/Makefile                 |    1 +
> >> > net/ipv4/netfilter/ipt_CHECKSUM.c           |   72 +++++++++++++++++++++++++++
> >>
> >> New modules should use xt.
> >
> > I tried moving the module to xt_CHECKSUM but now
> >        iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM --checksum-fill
> > does not to load the module.
> > It seems that xt_request_find_target in x_tables uses the prefix 'ip' for a module.
> > What am I missing?
> >
> 
> You should add MODULE_ALIAS clause, i.e.
> 
> MODULE_ALIAS("ipt_CHECKSUM");

This worked, thanks!

> -- 
> Regards,
> Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] netfilter: add CHECKSUM target
  2010-07-11 10:47   ` Michael S. Tsirkin
@ 2010-07-15 15:17     ` Patrick McHardy
  0 siblings, 0 replies; 16+ messages in thread
From: Patrick McHardy @ 2010-07-15 15:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI, linux-kernel, netfilter-devel,
	netfilter, coreteam, netdev, herbert.xu, kvm

Am 11.07.2010 12:47, schrieb Michael S. Tsirkin:
> On Fri, Jul 09, 2010 at 05:17:36PM +0200, Patrick McHardy wrote:
>> Am 09.07.2010 00:29, schrieb Michael S. Tsirkin:
>>> This adds a `CHECKSUM' target, which can be used in the iptables mangle
>>> table.
>>>
>>> You can use this target to compute and fill in the checksum in
>>> an IP packet that lacks a checksum.  This is particularly useful,
>>> if you need to work around old applications such as dhcp clients,
>>> that do not work well with checksum offloads, but don't want to
>>> disable checksum offload in your device.
>>>
>>> The problem happens in the field with virtualized applications.
>>> For reference, see Red Hat bz 605555, as well as
>>> http://www.spinics.net/lists/kvm/msg37660.html
>>>
>>> Typical expected use (helps old dhclient binary running in a VM):
>>> iptables -A POSTROUTING -t mangle -p udp --dport 68 -j CHECKSUM
>>> --checksum-fill
>>
>> I'm not sure this is something we want to merge upstream and
>> support indefinitely. Dave suggested this as a temporary
>> out-of-tree workaround until the majority of guest dhcp clients
>> are fixed. Has anything changed that makes this course of
>> action impractical?
> 
> If I understand what Dave said correctly, it's up to you ...
> 
> The arguments for putting this upstream are:
> 
> Given the track record, I wouldn't hope for quick fix in the majority of
> guest dhcp clients, unfortunately :(.  We are talking years here.
> Even after that, one of the uses of virtualization is
> to keep old guests running. So yes, I think we'll
> keep using work-arounds for this for a very long time.
> 
> Further, since we have to add the module and we have to teach management
> to program it, it will be much less painful for everyone
> involved if we can put the code upstream, rather than forking
> management code.

Fair enough, its simple enough that I don't expect much maintenance
overhead.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2010-07-15 15:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-08 22:29 [PATCH] netfilter: add CHECKSUM target Michael S. Tsirkin
2010-07-08 22:35 ` [PATCH] extensions: libipt_CHECKSUM extension Michael S. Tsirkin
2010-07-09  7:18 ` [PATCH] netfilter: add CHECKSUM target Herbert Xu
2010-07-11  9:21   ` Michael S. Tsirkin
2010-07-11 10:24     ` Herbert Xu
2010-07-09  9:56 ` Jan Engelhardt
2010-07-11 12:27   ` Michael S. Tsirkin
2010-07-11 12:45     ` Changli Gao
2010-07-11 13:19       ` Michael S. Tsirkin
2010-07-11 12:45     ` Jan Engelhardt
2010-07-11 13:19       ` Michael S. Tsirkin
2010-07-09 15:17 ` Patrick McHardy
2010-07-09 16:26   ` Jan Engelhardt
2010-07-09 16:36     ` Patrick McHardy
2010-07-11 10:47   ` Michael S. Tsirkin
2010-07-15 15:17     ` Patrick McHardy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).