| Message ID | 20090727134621.12897.75558.stgit@jazzy.zrh.corp.google.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Monday 2009-07-27 15:46, Hannes Eder wrote: >--- /dev/null >+++ b/include/linux/netfilter/xt_ipvs.h >@@ -0,0 +1,32 @@ >+#ifndef _XT_IPVS_H >+#define _XT_IPVS_H 1 >+ >+#ifndef _IP_VS_H 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 <heder@google.com>"); >+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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 27, 2009 at 20:35, Jan Engelhardt<jengelh@medozas.de> wrote: > > On Monday 2009-07-27 15:46, Hannes Eder wrote: >>--- /dev/null >>+++ b/include/linux/netfilter/xt_ipvs.h >>@@ -0,0 +1,32 @@ >>+#ifndef _XT_IPVS_H >>+#define _XT_IPVS_H 1 >>+ >>+#ifndef _IP_VS_H > > 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. Definitions removed from xt_ipvs.h. For xt_ipvs.c I just included linux/ip_vs.h. For libxt_ipvs (user space library) I added the entire linux/ip_vs.h. Do you think this is better? >>+#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. Done. >>+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 :> Done. >>+MODULE_AUTHOR("Hannes Eder <heder@google.com>"); >>+MODULE_DESCRIPTION("Xtables: match ipvs"); > > "Match IPVS connection properties" is what you previously stated, > and which I think is good. Use it here, too. Done. >>+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... Just matching aginst the skb->ipvs_property, I changed to name to XT_IPVS_IPVS_PROPERTY. > Anyway, this can be made much shorter, given that all following > the "out" label is a return: You are right. For the moment, I'll keep it as is. Having a single entry/exit point makes it easier to instrument the function for debugging. > > 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 :) Still working on that ;) >>+ 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 */ Here we go: if (data->bitmask & XT_IPVS_DIR) { enum ip_conntrack_info ctinfo; struct nf_conn *ct = nf_ct_get(skb, &ctinfo); if (ct == NULL || ct == &nf_conntrack_untracked) { match = false; goto out_put_cp; } if ((ctinfo >= IP_CT_IS_REPLY) ^ !!(data->invert & XT_IPVS_DIR)) { match = false; goto out_put_cp; } } >>+ } >>+ >>+ 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.) I changed it as suggested, though I could not find anything in Documentation/CodingStyle about that. >>+ return match; >>+} >>+EXPORT_SYMBOL(ipvs_mt); > > What will be using ipvs_mt? Nobody, EXPORT_SYMBOL removed. >>+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. You are right. Done. Thanks for the review. Cheers, -Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/netfilter/xt_ipvs.h b/include/linux/netfilter/xt_ipvs.h new file mode 100644 index 0000000..3a70289 --- /dev/null +++ b/include/linux/netfilter/xt_ipvs.h @@ -0,0 +1,32 @@ +#ifndef _XT_IPVS_H +#define _XT_IPVS_H 1 + +#ifndef _IP_VS_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; +}; + +#endif /* _XT_IPVS_H */ diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig index cb3ad74..677a880 100644 --- a/net/netfilter/Kconfig +++ b/net/netfilter/Kconfig @@ -678,6 +678,14 @@ config NETFILTER_XT_MATCH_IPRANGE If unsure, say M. +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. + config NETFILTER_XT_MATCH_LENGTH tristate '"length" match support' depends on NETFILTER_ADVANCED diff --git a/net/netfilter/Makefile b/net/netfilter/Makefile index 6282060..1c18e80 100644 --- a/net/netfilter/Makefile +++ b/net/netfilter/Makefile @@ -72,6 +72,7 @@ obj-$(CONFIG_NETFILTER_XT_MATCH_HASHLIMIT) += xt_hashlimit.o obj-$(CONFIG_NETFILTER_XT_MATCH_HELPER) += xt_helper.o obj-$(CONFIG_NETFILTER_XT_MATCH_HL) += xt_hl.o obj-$(CONFIG_NETFILTER_XT_MATCH_IPRANGE) += xt_iprange.o +obj-$(CONFIG_NETFILTER_XT_MATCH_IPVS) += xt_ipvs.o obj-$(CONFIG_NETFILTER_XT_MATCH_LENGTH) += xt_length.o obj-$(CONFIG_NETFILTER_XT_MATCH_LIMIT) += xt_limit.o obj-$(CONFIG_NETFILTER_XT_MATCH_MAC) += xt_mac.o diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c index a01520e..84bf4e6 100644 --- a/net/netfilter/ipvs/ip_vs_proto.c +++ b/net/netfilter/ipvs/ip_vs_proto.c @@ -94,6 +94,7 @@ struct ip_vs_protocol * ip_vs_proto_get(unsigned short proto) return NULL; } +EXPORT_SYMBOL(ip_vs_proto_get); /* diff --git a/net/netfilter/xt_ipvs.c b/net/netfilter/xt_ipvs.c new file mode 100644 index 0000000..00373d9 --- /dev/null +++ b/net/netfilter/xt_ipvs.c @@ -0,0 +1,171 @@ +/* + * xt_ipvs - kernel module to match ipvs properties of a packet + * + * Author: Hannes Eder <heder@google.com> + */ + +#include <linux/module.h> +#include <linux/moduleparam.h> +#include <linux/spinlock.h> +#include <linux/skbuff.h> +#ifdef CONFIG_IP_VS_IPV6 +#include <net/ipv6.h> +#endif +#include <linux/types.h> +#include <linux/netfilter/x_tables.h> +#include <linux/netfilter/xt_ipvs.h> + +#include <net/ip_vs.h> + +MODULE_AUTHOR("Hannes Eder <heder@google.com>"); +MODULE_DESCRIPTION("Xtables: match ipvs"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("ipt_ipvs"); +MODULE_ALIAS("ip6t_ipvs"); + +/* borrowed from xt_conntrack */ +static bool ipvs_mt_addrcmp(const union nf_inet_addr *kaddr, + const union nf_inet_addr *uaddr, + const union nf_inet_addr *umask, + unsigned int l3proto) +{ + if (l3proto == NFPROTO_IPV4) + return ((kaddr->ip ^ uaddr->ip) & umask->ip) == 0; +#ifdef CONFIG_IP_VS_IPV6 + else if (l3proto == NFPROTO_IPV6) + return ipv6_masked_addr_cmp(&kaddr->in6, &umask->in6, + &uaddr->in6) == 0; +#endif + else + return false; +} + +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; + } + + /* other flags than XT_IPVS_IPVS are set */ + if (!skb->ipvs_property) { + match = false; + goto out; + } + + switch (skb->protocol) { + case htons(ETH_P_IP): + af = AF_INET; + break; +#ifdef CONFIG_IP_VS_IPV6 + case htons(ETH_P_IPV6): + af = AF_INET6; + break; +#endif + default: + match = false; + goto out; + } + + 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; + } + + pp = ip_vs_proto_get(iph.protocol); + if (unlikely(!pp)) { + match = false; + goto out; + } + + /* + * Check if the packet belongs to an existing entry + */ + /* TODO: why does it only match in inverse? */ + 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 */ + } + + 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)) { + match = false; + goto out_put_ct; + } + } + +out_put_ct: + __ip_vs_conn_put(cp); +out: + return match; +} +EXPORT_SYMBOL(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, + }, +}; + +static int __init ipvs_mt_init(void) +{ + return xt_register_matches(xt_ipvs_mt_reg, + ARRAY_SIZE(xt_ipvs_mt_reg)); +} + +static void __exit ipvs_mt_exit(void) +{ + xt_unregister_matches(xt_ipvs_mt_reg, + ARRAY_SIZE(xt_ipvs_mt_reg)); +} + +module_init(ipvs_mt_init); +module_exit(ipvs_mt_exit);