| Message ID | 20190331102621.7460-3-ja@ssi.bg |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | Add UDP tunnel support for ICMP errors in IPVS | expand |
On Sun, Mar 31, 2019 at 01:26:20PM +0300, Julian Anastasov wrote: > Add ip_vs_find_tunnel() to match tunnel headers > by family, address and optional port. Use it to > properly find the tunnel real server used in > received ICMP errors. > > Signed-off-by: Julian Anastasov <ja@ssi.bg> > --- > include/net/ip_vs.h | 3 +++ > net/netfilter/ipvs/ip_vs_core.c | 8 ++++++++ > net/netfilter/ipvs/ip_vs_ctl.c | 29 +++++++++++++++++++++++++++++ > 3 files changed, 40 insertions(+) > > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h > index 9a8ac8997e34..b01a94ebfc0e 100644 > --- a/include/net/ip_vs.h > +++ b/include/net/ip_vs.h > @@ -1404,6 +1404,9 @@ bool ip_vs_has_real_service(struct netns_ipvs *ipvs, int af, __u16 protocol, > struct ip_vs_dest * > ip_vs_find_real_service(struct netns_ipvs *ipvs, int af, __u16 protocol, > const union nf_inet_addr *daddr, __be16 dport); > +struct ip_vs_dest *ip_vs_find_tunnel(struct netns_ipvs *ipvs, int af, > + const union nf_inet_addr *daddr, > + __be16 tun_port); > > int ip_vs_use_count_inc(void); > void ip_vs_use_count_dec(void); > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c > index 14457551bcb4..4447ee512b88 100644 > --- a/net/netfilter/ipvs/ip_vs_core.c > +++ b/net/netfilter/ipvs/ip_vs_core.c > @@ -1598,6 +1598,7 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, > struct ip_vs_proto_data *pd; > unsigned int offset, offset2, ihl, verdict; > bool ipip, new_cp = false; > + union nf_inet_addr *raddr; > > *related = 1; > > @@ -1636,15 +1637,22 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, > cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); > if (cih == NULL) > return NF_ACCEPT; /* The packet looks wrong, ignore */ > + raddr = (union nf_inet_addr *)&cih->daddr; Hi Julian, Could we consider the following instead of casting? union nf_inet_addr raddr; ... raddr.ip = cih->daddr; ... est = ip_vs_find_tunnel(ipvs, AF_INET, &raddr, 0); > > /* Special case for errors for IPIP packets */ > ipip = false; > if (cih->protocol == IPPROTO_IPIP) { > + struct ip_vs_dest *dest; > + > if (unlikely(cih->frag_off & htons(IP_OFFSET))) > return NF_ACCEPT; > /* Error for our IPIP must arrive at LOCAL_IN */ > if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL)) > return NF_ACCEPT; > + dest = ip_vs_find_tunnel(ipvs, AF_INET, raddr, 0); > + /* Only for known tunnel */ > + if (!dest || dest->tun_type != IP_VS_CONN_F_TUNNEL_TYPE_IPIP) > + return NF_ACCEPT; > offset += cih->ihl * 4; > cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); > if (cih == NULL) ...
Hello, On Wed, 3 Apr 2019, Simon Horman wrote: > On Sun, Mar 31, 2019 at 01:26:20PM +0300, Julian Anastasov wrote: > > > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c > > index 14457551bcb4..4447ee512b88 100644 > > --- a/net/netfilter/ipvs/ip_vs_core.c > > +++ b/net/netfilter/ipvs/ip_vs_core.c > > @@ -1598,6 +1598,7 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, > > struct ip_vs_proto_data *pd; > > unsigned int offset, offset2, ihl, verdict; > > bool ipip, new_cp = false; > > + union nf_inet_addr *raddr; > > > > *related = 1; > > > > @@ -1636,15 +1637,22 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, > > cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); > > if (cih == NULL) > > return NF_ACCEPT; /* The packet looks wrong, ignore */ > > + raddr = (union nf_inet_addr *)&cih->daddr; > > Hi Julian, > > Could we consider the following instead of casting? > > union nf_inet_addr raddr; > > ... > > raddr.ip = cih->daddr; It was my initial option but then I decided to reduce the stack usage Regards -- Julian Anastasov <ja@ssi.bg>
On Wed, Apr 03, 2019 at 11:52:37PM +0300, Julian Anastasov wrote: > > Hello, > > On Wed, 3 Apr 2019, Simon Horman wrote: > > > On Sun, Mar 31, 2019 at 01:26:20PM +0300, Julian Anastasov wrote: > > > > > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c > > > index 14457551bcb4..4447ee512b88 100644 > > > --- a/net/netfilter/ipvs/ip_vs_core.c > > > +++ b/net/netfilter/ipvs/ip_vs_core.c > > > @@ -1598,6 +1598,7 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, > > > struct ip_vs_proto_data *pd; > > > unsigned int offset, offset2, ihl, verdict; > > > bool ipip, new_cp = false; > > > + union nf_inet_addr *raddr; > > > > > > *related = 1; > > > > > > @@ -1636,15 +1637,22 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, > > > cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); > > > if (cih == NULL) > > > return NF_ACCEPT; /* The packet looks wrong, ignore */ > > > + raddr = (union nf_inet_addr *)&cih->daddr; > > > > Hi Julian, > > > > Could we consider the following instead of casting? > > > > union nf_inet_addr raddr; > > > > ... > > > > raddr.ip = cih->daddr; > > It was my initial option but then I decided to reduce the > stack usage Understood, I guess that minimising stack usage wins. > > Regards > > -- > Julian Anastasov <ja@ssi.bg> >
diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index 9a8ac8997e34..b01a94ebfc0e 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -1404,6 +1404,9 @@ bool ip_vs_has_real_service(struct netns_ipvs *ipvs, int af, __u16 protocol, struct ip_vs_dest * ip_vs_find_real_service(struct netns_ipvs *ipvs, int af, __u16 protocol, const union nf_inet_addr *daddr, __be16 dport); +struct ip_vs_dest *ip_vs_find_tunnel(struct netns_ipvs *ipvs, int af, + const union nf_inet_addr *daddr, + __be16 tun_port); int ip_vs_use_count_inc(void); void ip_vs_use_count_dec(void); diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c index 14457551bcb4..4447ee512b88 100644 --- a/net/netfilter/ipvs/ip_vs_core.c +++ b/net/netfilter/ipvs/ip_vs_core.c @@ -1598,6 +1598,7 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, struct ip_vs_proto_data *pd; unsigned int offset, offset2, ihl, verdict; bool ipip, new_cp = false; + union nf_inet_addr *raddr; *related = 1; @@ -1636,15 +1637,22 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); if (cih == NULL) return NF_ACCEPT; /* The packet looks wrong, ignore */ + raddr = (union nf_inet_addr *)&cih->daddr; /* Special case for errors for IPIP packets */ ipip = false; if (cih->protocol == IPPROTO_IPIP) { + struct ip_vs_dest *dest; + if (unlikely(cih->frag_off & htons(IP_OFFSET))) return NF_ACCEPT; /* Error for our IPIP must arrive at LOCAL_IN */ if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL)) return NF_ACCEPT; + dest = ip_vs_find_tunnel(ipvs, AF_INET, raddr, 0); + /* Only for known tunnel */ + if (!dest || dest->tun_type != IP_VS_CONN_F_TUNNEL_TYPE_IPIP) + return NF_ACCEPT; offset += cih->ihl * 4; cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); if (cih == NULL) diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c index 7de90c00c8bd..9852c63ec4d8 100644 --- a/net/netfilter/ipvs/ip_vs_ctl.c +++ b/net/netfilter/ipvs/ip_vs_ctl.c @@ -617,6 +617,35 @@ struct ip_vs_dest *ip_vs_find_real_service(struct netns_ipvs *ipvs, int af, return NULL; } +/* Find real service record by <af,addr,tun_port>. + * In case of multiple records with the same <af,addr,tun_port>, only + * the first found record is returned. + * + * To be called under RCU lock. + */ +struct ip_vs_dest *ip_vs_find_tunnel(struct netns_ipvs *ipvs, int af, + const union nf_inet_addr *daddr, + __be16 tun_port) +{ + struct ip_vs_dest *dest; + unsigned int hash; + + /* Check for "full" addressed entries */ + hash = ip_vs_rs_hashkey(af, daddr, tun_port); + + hlist_for_each_entry_rcu(dest, &ipvs->rs_table[hash], d_list) { + if (dest->tun_port == tun_port && + dest->af == af && + ip_vs_addr_equal(af, &dest->addr, daddr) && + (IP_VS_DFWD_METHOD(dest) == IP_VS_CONN_F_TUNNEL)) { + /* HIT */ + return dest; + } + } + + return NULL; +} + /* Lookup destination by {addr,port} in the given service * Called under RCU lock. */
Add ip_vs_find_tunnel() to match tunnel headers by family, address and optional port. Use it to properly find the tunnel real server used in received ICMP errors. Signed-off-by: Julian Anastasov <ja@ssi.bg> --- include/net/ip_vs.h | 3 +++ net/netfilter/ipvs/ip_vs_core.c | 8 ++++++++ net/netfilter/ipvs/ip_vs_ctl.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)