On Monday 2009-07-27 15:46, Hannes Eder wrote:

Do the definitions (should probably be enums) exist in
very old <linux/ip_vs.h>? Then maybe you can get rid of them
from xt_ipvs.h.

+#define IP_VS_CONN_F_FWD_MASK	0x0007		/* mask for the fwd methods */
+#define IP_VS_CONN_F_MASQ	0x0000		/* masquerading/NAT */
+#define IP_VS_CONN_F_LOCALNODE	0x0001		/* local node */
+#define IP_VS_CONN_F_TUNNEL	0x0002		/* tunneling */
+#define IP_VS_CONN_F_DROUTE	0x0003		/* direct routing */
+#define IP_VS_CONN_F_BYPASS	0x0004		/* cache bypass */
+#endif
+
+#define XT_IPVS_IPVS		0x01 /* this is implied by all other options */
+#define XT_IPVS_PROTO		0x02
+#define XT_IPVS_VADDR		0x04
+#define XT_IPVS_VPORT		0x08
+#define XT_IPVS_DIR		0x10
+#define XT_IPVS_METHOD		0x20
+#define XT_IPVS_MASK		(0x40 - 1)
+#define XT_IPVS_ONCE_MASK	(XT_IPVS_MASK & ~XT_IPVS_IPVS)
+
+struct xt_ipvs {
+	union nf_inet_addr	vaddr, vmask;
+	__be16			vport;
+	u_int16_t		l4proto;
+	u_int16_t		fwd_method;
+
+	u_int8_t invert;
+	u_int8_t bitmask;
+};

As per the "Writing Netfilter modules" e-book, __u16/__u8 is required.

+config NETFILTER_XT_MATCH_IPVS
+	tristate '"ipvs" match support'
+	depends on NETFILTER_ADVANCED
+	help
+	  This option allows you to match against ipvs properties of a packet.
+
+          To compile it as a module, choos M here.  If unsure, say N.
+

IMHO the "to compile it as a module, choos[e] M" is a relict from
old times and should just be dropped. These days, I stupilate,
everyone knows that M makes modules. And if it doesnot, then
it's not allowed by Kconfig :>

+MODULE_AUTHOR("Hannes Eder [off-list ref]");
+MODULE_DESCRIPTION("Xtables: match ipvs");

"Match IPVS connection properties" is what you previously stated,
and which I think is good. Use it here, too.

+bool ipvs_mt(const struct sk_buff *skb, const struct xt_match_param *par)
+{
+	const struct xt_ipvs *data = par->matchinfo;
+	const u_int8_t family = par->family;
+	struct ip_vs_iphdr iph;
+	struct ip_vs_protocol *pp;
+	struct ip_vs_conn *cp;
+	int af;
+	bool match = true;
+
+	if (data->bitmask == XT_IPVS_IPVS) {
+		match = !!(skb->ipvs_property)
+			^ !!(data->invert & XT_IPVS_IPVS);
+		goto out;
+	}

XT_IPVS_IPVS? What's that supposed to tell me...
Anyway, this can be made much shorter, given that all following
the "out" label is a return:

	if (data->bitmask == XT_IPVS_IPVS)
		return skb->ipvs_property ^ !!(data->invert & XT_IPVS_IPVS);

+	/* other flags than XT_IPVS_IPVS are set */
+	if (!skb->ipvs_property) {
+		match = false;
+		goto out;
+	}

	if (!skb->ipvs_property)
		return false;

+	ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
+
+	if (data->bitmask & XT_IPVS_PROTO)
+		if ((iph.protocol == data->l4proto)
+		    ^ !(data->invert & XT_IPVS_PROTO)) {
+			match = false;
+			goto out;
+		}

		if (iph.protocol == data->l4proto ^
		    !(data->invert & XT_IPVS_PROTO))
			return false;

+	pp = ip_vs_proto_get(iph.protocol);
+	if (unlikely(!pp)) {
+		match = false;
+		goto out;
+	}

	if (unlikely(pp == NULL))
		return false;

Only after here we really need goto (to put pp).

+	/*
+	 * Check if the packet belongs to an existing entry
+	 */
+	/* TODO: why does it only match in inverse? */

FIXME: Figure it out :)

+	cp = pp->conn_out_get(af, skb, pp, &iph, iph.len, 1 /* inverse */);
+	if (unlikely(!cp)) {
+		match = false;
+		goto out;
+	}
+
+	/*
+	 * We found a connection, i.e. ct != 0, make sure to call
+	 * __ip_vs_conn_put before returning.  In our case jump to out_put_con.
+	 */
+
+	if (data->bitmask & XT_IPVS_VPORT)
+		if ((cp->vport == data->vport)
+		    ^ !(data->invert & XT_IPVS_VPORT)) {
+			match = false;
+			goto out_put_ct;
+		}
+
+	if (data->bitmask & XT_IPVS_DIR) {
+		/* TODO: implement */

		/* Yes please */

+	}
+
+	if (data->bitmask & XT_IPVS_METHOD)
+		if (((cp->flags & IP_VS_CONN_F_FWD_MASK) == data->fwd_method)
+		    ^ !(data->invert & XT_IPVS_METHOD)) {
+			match = false;
+			goto out_put_ct;
+		}
+
+	if (data->bitmask & XT_IPVS_VADDR) {
+		if (af != family) {
+			match = false;
+			goto out_put_ct;
+		}
+
+		if (ipvs_mt_addrcmp(&cp->vaddr, &data->vaddr, &data->vmask, af)
+		    ^ !(data->invert & XT_IPVS_VADDR)) {

I think the operator (^ in this case) always goes onto the same line as the ')'
((a) ^\n(b), not ((a)\n ^(b)), though some legacy code seems to do it
random so-so.)

+	return match;
+}
+EXPORT_SYMBOL(ipvs_mt);

What will be using ipvs_mt?

+static struct xt_match xt_ipvs_mt_reg[] __read_mostly = {
+	{
+		.name       = "ipvs",
+		.revision   = 0,
+		.family     = NFPROTO_UNSPEC,
+		.match      = ipvs_mt,
+		.matchsize  = sizeof(struct xt_ipvs),
+		.me         = THIS_MODULE,
+	},
+};

There is just one element, so no strict need for the [] array.