[PATCH net-next 0/3] Add UDP tunnel support for ICMP errors in IPVS

[PATCH net-next 0/3] Add UDP tunnel support for ICMP errors in IPVS

[Julian Anastasov]

This patchset is a followup to the commit that adds UDP/GUE tunnel:
"ipvs: allow tunneling with gue encapsulation".

What we do is to put tunnel real servers in hash table (patch 1),
add function to lookup tunnels (patch 2) and use it to strip the
embedded tunnel headers from ICMP errors (patch 3).

Julian Anastasov (3):
  ipvs: allow rs_table to contain different real server types
  ipvs: add function to find tunnels
  ipvs: strip udp tunnel headers from icmp errors

 include/net/ip_vs.h             |  6 +++
 net/netfilter/ipvs/ip_vs_core.c | 66 ++++++++++++++++++++++++++++++
 net/netfilter/ipvs/ip_vs_ctl.c  | 72 +++++++++++++++++++++++++++++----
 3 files changed, 136 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH net-next 1/3] ipvs: allow rs_table to contain different real server types

[Julian Anastasov]

Before now rs_table was used only for NAT real servers.
Change it to allow TUN real severs from different types,
possibly hashed with different port key.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 include/net/ip_vs.h            |  3 +++
 net/netfilter/ipvs/ip_vs_ctl.c | 43 +++++++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 2ac40135b576..9a8ac8997e34 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1497,6 +1497,9 @@ static inline int ip_vs_todrop(struct netns_ipvs *ipvs)
 static inline int ip_vs_todrop(struct netns_ipvs *ipvs) { return 0; }
 #endif
 
+#define IP_VS_DFWD_METHOD(dest) (atomic_read(&(dest)->conn_flags) & \
+				 IP_VS_CONN_F_FWD_MASK)
+
 /* ip_vs_fwd_tag returns the forwarding tag of the connection */
 #define IP_VS_FWD_METHOD(cp)  (cp->flags & IP_VS_CONN_F_FWD_MASK)
 
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 328683452229..7de90c00c8bd 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -515,15 +515,36 @@ static inline unsigned int ip_vs_rs_hashkey(int af,
 static void ip_vs_rs_hash(struct netns_ipvs *ipvs, struct ip_vs_dest *dest)
 {
 	unsigned int hash;
+	__be16 port;
 
 	if (dest->in_rs_table)
 		return;
 
+	switch (IP_VS_DFWD_METHOD(dest)) {
+	case IP_VS_CONN_F_MASQ:
+		port = dest->port;
+		break;
+	case IP_VS_CONN_F_TUNNEL:
+		switch (dest->tun_type) {
+		case IP_VS_CONN_F_TUNNEL_TYPE_GUE:
+			port = dest->tun_port;
+			break;
+		case IP_VS_CONN_F_TUNNEL_TYPE_IPIP:
+			port = 0;
+			break;
+		default:
+			return;
+		}
+		break;
+	default:
+		return;
+	}
+
 	/*
 	 *	Hash by proto,addr,port,
 	 *	which are the parameters of the real service.
 	 */
-	hash = ip_vs_rs_hashkey(dest->af, &dest->addr, dest->port);
+	hash = ip_vs_rs_hashkey(dest->af, &dest->addr, port);
 
 	hlist_add_head_rcu(&dest->d_list, &ipvs->rs_table[hash]);
 	dest->in_rs_table = 1;
@@ -555,7 +576,8 @@ bool ip_vs_has_real_service(struct netns_ipvs *ipvs, int af, __u16 protocol,
 		if (dest->port == dport &&
 		    dest->af == af &&
 		    ip_vs_addr_equal(af, &dest->addr, daddr) &&
-		    (dest->protocol == protocol || dest->vfwmark)) {
+		    (dest->protocol == protocol || dest->vfwmark) &&
+		    (IP_VS_DFWD_METHOD(dest) == IP_VS_CONN_F_MASQ)) {
 			/* HIT */
 			return true;
 		}
@@ -585,7 +607,8 @@ struct ip_vs_dest *ip_vs_find_real_service(struct netns_ipvs *ipvs, int af,
 		if (dest->port == dport &&
 		    dest->af == af &&
 		    ip_vs_addr_equal(af, &dest->addr, daddr) &&
-			(dest->protocol == protocol || dest->vfwmark)) {
+		    (dest->protocol == protocol || dest->vfwmark) &&
+		    (IP_VS_DFWD_METHOD(dest) == IP_VS_CONN_F_MASQ)) {
 			/* HIT */
 			return dest;
 		}
@@ -831,6 +854,13 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
 	conn_flags = udest->conn_flags & IP_VS_CONN_F_DEST_MASK;
 	conn_flags |= IP_VS_CONN_F_INACTIVE;
 
+	/* Need to rehash? */
+	if ((udest->conn_flags & IP_VS_CONN_F_FWD_MASK) !=
+	    IP_VS_DFWD_METHOD(dest) ||
+	    udest->tun_type != dest->tun_type ||
+	    udest->tun_port != dest->tun_port)
+		ip_vs_rs_unhash(dest);
+
 	/* set the tunnel info */
 	dest->tun_type = udest->tun_type;
 	dest->tun_port = udest->tun_port;
@@ -839,16 +869,13 @@ __ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
 	if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ) {
 		conn_flags |= IP_VS_CONN_F_NOOUTPUT;
 	} else {
-		/*
-		 *    Put the real service in rs_table if not present.
-		 *    For now only for NAT!
-		 */
-		ip_vs_rs_hash(ipvs, dest);
 		/* FTP-NAT requires conntrack for mangling */
 		if (svc->port == FTPPORT)
 			ip_vs_register_conntrack(svc);
 	}
 	atomic_set(&dest->conn_flags, conn_flags);
+	/* Put the real service in rs_table if not present. */
+	ip_vs_rs_hash(ipvs, dest);
 
 	/* bind the service */
 	old_svc = rcu_dereference_protected(dest->svc, 1);
-- 
2.17.1

[PATCH net-next 2/3] ipvs: add function to find tunnels

[Julian Anastasov]

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;
 
 	/* 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.
  */
-- 
2.17.1

[PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Julian Anastasov]

Recognize UDP tunnels in received ICMP errors and
properly strip the tunnel headers. GUE is what we
have for now.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 net/netfilter/ipvs/ip_vs_core.c | 58 +++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 4447ee512b88..0b624e8e7982 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -39,6 +39,7 @@
 #include <net/tcp.h>
 #include <net/udp.h>
 #include <net/icmp.h>                   /* for icmp_send */
+#include <net/gue.h>
 #include <net/route.h>
 #include <net/ip6_checksum.h>
 #include <net/netns/generic.h>		/* net_generic() */
@@ -1579,6 +1580,34 @@ ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
 	return 1;
 }
 
+/* Check the UDP tunnel and return its header length */
+static int ipvs_udp_decap(struct netns_ipvs *ipvs, struct sk_buff *skb,
+			  unsigned int offset, __u16 af, __be16 dport,
+			  const union nf_inet_addr *daddr, __u8 *proto)
+{
+	struct ip_vs_dest *dest = ip_vs_find_tunnel(ipvs, af, daddr, dport);
+
+	if (!dest)
+		goto unk;
+	if (dest->tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
+		struct guehdr _gueh, *gueh;
+
+		gueh = skb_header_pointer(skb, offset, sizeof(_gueh), &_gueh);
+		if (!gueh)
+			goto unk;
+		if (gueh->control != 0 || gueh->version != 0)
+			goto unk;
+		/* Later we can support also IPPROTO_IPV6 */
+		if (gueh->proto_ctype != IPPROTO_IPIP)
+			goto unk;
+		*proto = gueh->proto_ctype;
+		return sizeof(struct guehdr) + (gueh->hlen << 2);
+	}
+
+unk:
+	return -1;
+}
+
 /*
  *	Handle ICMP messages in the outside-to-inside direction (incoming).
  *	Find any that might be relevant, check against existing connections,
@@ -1658,8 +1687,37 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
 		if (cih == NULL)
 			return NF_ACCEPT; /* The packet looks wrong, ignore */
 		ipip = true;
+	} else if (cih->protocol == IPPROTO_UDP) {	/* Can be UDP encap */
+		struct udphdr _udph, *udph;
+		__u8 iproto;
+		int ulen;
+
+		if (unlikely(cih->frag_off & htons(IP_OFFSET)))
+			return NF_ACCEPT;
+		/* Error for our tunnel must arrive at LOCAL_IN */
+		if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL))
+			return NF_ACCEPT;
+		offset2 = offset + cih->ihl * 4;
+		udph = skb_header_pointer(skb, offset2, sizeof(_udph), &_udph);
+		if (!udph)
+			goto check;
+		offset2 += sizeof(struct udphdr);
+		ulen = ipvs_udp_decap(ipvs, skb, offset2, AF_INET, udph->dest,
+				      raddr, &iproto);
+		if (ulen < 0)
+			goto check;
+		/* Skip IP + UDP + ulen */
+		offset = offset2 + ulen;
+		/* Now we should be at the original IP header */
+		cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
+		if (cih && cih->version == 4 && cih->ihl >= 5 &&
+		    iproto == IPPROTO_IPIP)
+			ipip = true;
+		else
+			return NF_ACCEPT;
 	}
 
+check:
 	pd = ip_vs_proto_data_get(ipvs, cih->protocol);
 	if (!pd)
 		return NF_ACCEPT;
-- 
2.17.1

Re: [PATCH net-next 2/3] ipvs: add function to find tunnels

[Simon Horman]

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)

...

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Simon Horman]

Hi Julian,

On Sun, Mar 31, 2019 at 01:26:21PM +0300, Julian Anastasov wrote:
> Recognize UDP tunnels in received ICMP errors and
> properly strip the tunnel headers. GUE is what we
> have for now.
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
>  net/netfilter/ipvs/ip_vs_core.c | 58 +++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index 4447ee512b88..0b624e8e7982 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -39,6 +39,7 @@
>  #include <net/tcp.h>
>  #include <net/udp.h>
>  #include <net/icmp.h>                   /* for icmp_send */
> +#include <net/gue.h>
>  #include <net/route.h>
>  #include <net/ip6_checksum.h>
>  #include <net/netns/generic.h>		/* net_generic() */
> @@ -1579,6 +1580,34 @@ ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  	return 1;
>  }
>  
> +/* Check the UDP tunnel and return its header length */
> +static int ipvs_udp_decap(struct netns_ipvs *ipvs, struct sk_buff *skb,
> +			  unsigned int offset, __u16 af, __be16 dport,
> +			  const union nf_inet_addr *daddr, __u8 *proto)
> +{
> +	struct ip_vs_dest *dest = ip_vs_find_tunnel(ipvs, af, daddr, dport);
> +
> +	if (!dest)
> +		goto unk;
> +	if (dest->tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
> +		struct guehdr _gueh, *gueh;
> +
> +		gueh = skb_header_pointer(skb, offset, sizeof(_gueh), &_gueh);
> +		if (!gueh)
> +			goto unk;
> +		if (gueh->control != 0 || gueh->version != 0)
> +			goto unk;
> +		/* Later we can support also IPPROTO_IPV6 */
> +		if (gueh->proto_ctype != IPPROTO_IPIP)
> +			goto unk;
> +		*proto = gueh->proto_ctype;
> +		return sizeof(struct guehdr) + (gueh->hlen << 2);

I think that the gue-specific portions of the above, which is most of
ipvs_udp_decap() should be a separate helper which is part of net/gue.h or
net/ipv4/fou.c - this seems to be a subset of the logic in gue_udp_recv().

> +	}
> +
> +unk:
> +	return -1;
> +}
> +
>  /*
>   *	Handle ICMP messages in the outside-to-inside direction (incoming).
>   *	Find any that might be relevant, check against existing connections,
> @@ -1658,8 +1687,37 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
>  		if (cih == NULL)
>  			return NF_ACCEPT; /* The packet looks wrong, ignore */
>  		ipip = true;
> +	} else if (cih->protocol == IPPROTO_UDP) {	/* Can be UDP encap */

Can we consider factoring out the logic in this else-if clause
out into a function to avoid the use of goto?

> +		struct udphdr _udph, *udph;
> +		__u8 iproto;
> +		int ulen;
> +
> +		if (unlikely(cih->frag_off & htons(IP_OFFSET)))
> +			return NF_ACCEPT;

I think a comment is warranted regarding the behaviour implemented
for fragments.

> +		/* Error for our tunnel must arrive at LOCAL_IN */
> +		if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL))
> +			return NF_ACCEPT;
> +		offset2 = offset + cih->ihl * 4;
> +		udph = skb_header_pointer(skb, offset2, sizeof(_udph), &_udph);
> +		if (!udph)
> +			goto check;

> +		offset2 += sizeof(struct udphdr);
> +		ulen = ipvs_udp_decap(ipvs, skb, offset2, AF_INET, udph->dest,
> +				      raddr, &iproto);
> +		if (ulen < 0)
> +			goto check;
> +		/* Skip IP + UDP + ulen */
> +		offset = offset2 + ulen;
> +		/* Now we should be at the original IP header */
> +		cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> +		if (cih && cih->version == 4 && cih->ihl >= 5 &&
> +		    iproto == IPPROTO_IPIP)
> +			ipip = true;
> +		else
> +			return NF_ACCEPT;

Skipping over tunnel headers and determining the inner IP protocol really
feels like something that should be handled code common to other UDP
encapsulation implementations.

>  	}
>  
> +check:
>  	pd = ip_vs_proto_data_get(ipvs, cih->protocol);
>  	if (!pd)
>  		return NF_ACCEPT;
> -- 
> 2.17.1
> 

Re: [PATCH net-next 1/3] ipvs: allow rs_table to contain different real server types

[Simon Horman]

Hi Julian,

On Sun, Mar 31, 2019 at 01:26:19PM +0300, Julian Anastasov wrote:
> Before now rs_table was used only for NAT real servers.
> Change it to allow TUN real severs from different types,
> possibly hashed with different port key.
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>

This looks good to me, modulo some nits below.

> ---
>  include/net/ip_vs.h            |  3 +++
>  net/netfilter/ipvs/ip_vs_ctl.c | 43 +++++++++++++++++++++++++++-------
>  2 files changed, 38 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 2ac40135b576..9a8ac8997e34 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -1497,6 +1497,9 @@ static inline int ip_vs_todrop(struct netns_ipvs *ipvs)
>  static inline int ip_vs_todrop(struct netns_ipvs *ipvs) { return 0; }
>  #endif
>  
> +#define IP_VS_DFWD_METHOD(dest) (atomic_read(&(dest)->conn_flags) & \
> +				 IP_VS_CONN_F_FWD_MASK)
> +
>  /* ip_vs_fwd_tag returns the forwarding tag of the connection */
>  #define IP_VS_FWD_METHOD(cp)  (cp->flags & IP_VS_CONN_F_FWD_MASK)
>  
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 328683452229..7de90c00c8bd 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -515,15 +515,36 @@ static inline unsigned int ip_vs_rs_hashkey(int af,
>  static void ip_vs_rs_hash(struct netns_ipvs *ipvs, struct ip_vs_dest *dest)
>  {
>  	unsigned int hash;
> +	__be16 port;
>  
>  	if (dest->in_rs_table)
>  		return;
>  
> +	switch (IP_VS_DFWD_METHOD(dest)) {
> +	case IP_VS_CONN_F_MASQ:
> +		port = dest->port;
> +		break;
> +	case IP_VS_CONN_F_TUNNEL:
> +		switch (dest->tun_type) {
> +		case IP_VS_CONN_F_TUNNEL_TYPE_GUE:
> +			port = dest->tun_port;
> +			break;
> +		case IP_VS_CONN_F_TUNNEL_TYPE_IPIP:
> +			port = 0;
> +			break;
> +		default:
> +			return;
> +		}
> +		break;
> +	default:
> +		return;
> +	}
> +
>  	/*
>  	 *	Hash by proto,addr,port,
>  	 *	which are the parameters of the real service.
>  	 */
> -	hash = ip_vs_rs_hashkey(dest->af, &dest->addr, dest->port);
> +	hash = ip_vs_rs_hashkey(dest->af, &dest->addr, port);
>  
>  	hlist_add_head_rcu(&dest->d_list, &ipvs->rs_table[hash]);
>  	dest->in_rs_table = 1;
> @@ -555,7 +576,8 @@ bool ip_vs_has_real_service(struct netns_ipvs *ipvs, int af, __u16 protocol,
>  		if (dest->port == dport &&
>  		    dest->af == af &&
  		    ip_vs_addr_equal(af, &dest->addr, daddr) &&
> -		    (dest->protocol == protocol || dest->vfwmark)) {
> +		    (dest->protocol == protocol || dest->vfwmark) &&
> +		    (IP_VS_DFWD_METHOD(dest) == IP_VS_CONN_F_MASQ)) {

nit: there seem to be unnecessary () on the line above

>  			/* HIT */
>  			return true;
>  		}
> @@ -585,7 +607,8 @@ struct ip_vs_dest *ip_vs_find_real_service(struct netns_ipvs *ipvs, int af,
>  		if (dest->port == dport &&
>  		    dest->af == af &&
>  		    ip_vs_addr_equal(af, &dest->addr, daddr) &&
> -			(dest->protocol == protocol || dest->vfwmark)) {
> +		    (dest->protocol == protocol || dest->vfwmark) &&
> +		    (IP_VS_DFWD_METHOD(dest) == IP_VS_CONN_F_MASQ)) {

ditto

>  			/* HIT */
>  			return dest;
>  		}

...

Re: [PATCH net-next 2/3] ipvs: add function to find tunnels

[Julian Anastasov]

	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>

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Julian Anastasov]

	Hello,

On Wed, 3 Apr 2019, Simon Horman wrote:

> On Sun, Mar 31, 2019 at 01:26:21PM +0300, Julian Anastasov wrote:
> > Recognize UDP tunnels in received ICMP errors and
> > properly strip the tunnel headers. GUE is what we
> > have for now.
> > 
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > ---
> >  net/netfilter/ipvs/ip_vs_core.c | 58 +++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > index 4447ee512b88..0b624e8e7982 100644
> > --- a/net/netfilter/ipvs/ip_vs_core.c
> > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > @@ -39,6 +39,7 @@
> >  #include <net/tcp.h>
> >  #include <net/udp.h>
> >  #include <net/icmp.h>                   /* for icmp_send */
> > +#include <net/gue.h>
> >  #include <net/route.h>
> >  #include <net/ip6_checksum.h>
> >  #include <net/netns/generic.h>		/* net_generic() */
> > @@ -1579,6 +1580,34 @@ ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
> >  	return 1;
> >  }
> >  
> > +/* Check the UDP tunnel and return its header length */
> > +static int ipvs_udp_decap(struct netns_ipvs *ipvs, struct sk_buff *skb,
> > +			  unsigned int offset, __u16 af, __be16 dport,
> > +			  const union nf_inet_addr *daddr, __u8 *proto)
> > +{
> > +	struct ip_vs_dest *dest = ip_vs_find_tunnel(ipvs, af, daddr, dport);
> > +
> > +	if (!dest)
> > +		goto unk;
> > +	if (dest->tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
> > +		struct guehdr _gueh, *gueh;
> > +
> > +		gueh = skb_header_pointer(skb, offset, sizeof(_gueh), &_gueh);
> > +		if (!gueh)
> > +			goto unk;
> > +		if (gueh->control != 0 || gueh->version != 0)
> > +			goto unk;
> > +		/* Later we can support also IPPROTO_IPV6 */
> > +		if (gueh->proto_ctype != IPPROTO_IPIP)
> > +			goto unk;
> > +		*proto = gueh->proto_ctype;
> > +		return sizeof(struct guehdr) + (gueh->hlen << 2);
> 
> I think that the gue-specific portions of the above, which is most of
> ipvs_udp_decap() should be a separate helper which is part of net/gue.h or
> net/ipv4/fou.c - this seems to be a subset of the logic in gue_udp_recv().

	Yes but fou.c strips the headers in all cases while
in IPVS we should do it only when connection is found because
we do not want to mess with non-IPVS traffic.

> 
> > +	}
> > +
> > +unk:
> > +	return -1;
> > +}
> > +
> >  /*
> >   *	Handle ICMP messages in the outside-to-inside direction (incoming).
> >   *	Find any that might be relevant, check against existing connections,
> > @@ -1658,8 +1687,37 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
> >  		if (cih == NULL)
> >  			return NF_ACCEPT; /* The packet looks wrong, ignore */
> >  		ipip = true;
> > +	} else if (cih->protocol == IPPROTO_UDP) {	/* Can be UDP encap */
> 
> Can we consider factoring out the logic in this else-if clause
> out into a function to avoid the use of goto?

	We can even safely return NF_ACCEPT instead of the goto

> 
> > +		struct udphdr _udph, *udph;
> > +		__u8 iproto;
> > +		int ulen;
> > +
> > +		if (unlikely(cih->frag_off & htons(IP_OFFSET)))
> > +			return NF_ACCEPT;
> 
> I think a comment is warranted regarding the behaviour implemented
> for fragments.

	I'll add in v2.

> 
> > +		/* Error for our tunnel must arrive at LOCAL_IN */
> > +		if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL))
> > +			return NF_ACCEPT;
> > +		offset2 = offset + cih->ihl * 4;
> > +		udph = skb_header_pointer(skb, offset2, sizeof(_udph), &_udph);
> > +		if (!udph)
> > +			goto check;
> 
> > +		offset2 += sizeof(struct udphdr);
> > +		ulen = ipvs_udp_decap(ipvs, skb, offset2, AF_INET, udph->dest,
> > +				      raddr, &iproto);
> > +		if (ulen < 0)
> > +			goto check;
> > +		/* Skip IP + UDP + ulen */
> > +		offset = offset2 + ulen;
> > +		/* Now we should be at the original IP header */
> > +		cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> > +		if (cih && cih->version == 4 && cih->ihl >= 5 &&
> > +		    iproto == IPPROTO_IPIP)
> > +			ipip = true;
> > +		else
> > +			return NF_ACCEPT;
> 
> Skipping over tunnel headers and determining the inner IP protocol really
> feels like something that should be handled code common to other UDP
> encapsulation implementations.

	We can easily add simple FOU in ipvs_udp_decap() by
returning 0 and correct *proto. Or you prefer to use common
code from other files to parse the headers? Currently, there
is no any GUE func that can be used for this.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net-next 2/3] ipvs: add function to find tunnels

[Simon Horman]

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>
> 

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Simon Horman]

On Thu, Apr 04, 2019 at 12:18:08AM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Wed, 3 Apr 2019, Simon Horman wrote:
> 
> > On Sun, Mar 31, 2019 at 01:26:21PM +0300, Julian Anastasov wrote:
> > > Recognize UDP tunnels in received ICMP errors and
> > > properly strip the tunnel headers. GUE is what we
> > > have for now.
> > > 
> > > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > > ---
> > >  net/netfilter/ipvs/ip_vs_core.c | 58 +++++++++++++++++++++++++++++++++
> > >  1 file changed, 58 insertions(+)
> > > 
> > > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > > index 4447ee512b88..0b624e8e7982 100644
> > > --- a/net/netfilter/ipvs/ip_vs_core.c
> > > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > > @@ -39,6 +39,7 @@
> > >  #include <net/tcp.h>
> > >  #include <net/udp.h>
> > >  #include <net/icmp.h>                   /* for icmp_send */
> > > +#include <net/gue.h>
> > >  #include <net/route.h>
> > >  #include <net/ip6_checksum.h>
> > >  #include <net/netns/generic.h>		/* net_generic() */
> > > @@ -1579,6 +1580,34 @@ ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
> > >  	return 1;
> > >  }
> > >  
> > > +/* Check the UDP tunnel and return its header length */
> > > +static int ipvs_udp_decap(struct netns_ipvs *ipvs, struct sk_buff *skb,
> > > +			  unsigned int offset, __u16 af, __be16 dport,
> > > +			  const union nf_inet_addr *daddr, __u8 *proto)
> > > +{
> > > +	struct ip_vs_dest *dest = ip_vs_find_tunnel(ipvs, af, daddr, dport);
> > > +
> > > +	if (!dest)
> > > +		goto unk;
> > > +	if (dest->tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GUE) {
> > > +		struct guehdr _gueh, *gueh;
> > > +
> > > +		gueh = skb_header_pointer(skb, offset, sizeof(_gueh), &_gueh);
> > > +		if (!gueh)
> > > +			goto unk;
> > > +		if (gueh->control != 0 || gueh->version != 0)
> > > +			goto unk;
> > > +		/* Later we can support also IPPROTO_IPV6 */
> > > +		if (gueh->proto_ctype != IPPROTO_IPIP)
> > > +			goto unk;
> > > +		*proto = gueh->proto_ctype;
> > > +		return sizeof(struct guehdr) + (gueh->hlen << 2);
> > 
> > I think that the gue-specific portions of the above, which is most of
> > ipvs_udp_decap() should be a separate helper which is part of net/gue.h or
> > net/ipv4/fou.c - this seems to be a subset of the logic in gue_udp_recv().
> 
> 	Yes but fou.c strips the headers in all cases while
> in IPVS we should do it only when connection is found because
> we do not want to mess with non-IPVS traffic.
> 
> > 
> > > +	}
> > > +
> > > +unk:
> > > +	return -1;
> > > +}
> > > +
> > >  /*
> > >   *	Handle ICMP messages in the outside-to-inside direction (incoming).
> > >   *	Find any that might be relevant, check against existing connections,
> > > @@ -1658,8 +1687,37 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
> > >  		if (cih == NULL)
> > >  			return NF_ACCEPT; /* The packet looks wrong, ignore */
> > >  		ipip = true;
> > > +	} else if (cih->protocol == IPPROTO_UDP) {	/* Can be UDP encap */
> > 
> > Can we consider factoring out the logic in this else-if clause
> > out into a function to avoid the use of goto?
> 
> 	We can even safely return NF_ACCEPT instead of the goto
> 
> > 
> > > +		struct udphdr _udph, *udph;
> > > +		__u8 iproto;
> > > +		int ulen;
> > > +
> > > +		if (unlikely(cih->frag_off & htons(IP_OFFSET)))
> > > +			return NF_ACCEPT;
> > 
> > I think a comment is warranted regarding the behaviour implemented
> > for fragments.
> 
> 	I'll add in v2.
> 
> > 
> > > +		/* Error for our tunnel must arrive at LOCAL_IN */
> > > +		if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL))
> > > +			return NF_ACCEPT;
> > > +		offset2 = offset + cih->ihl * 4;
> > > +		udph = skb_header_pointer(skb, offset2, sizeof(_udph), &_udph);
> > > +		if (!udph)
> > > +			goto check;
> > 
> > > +		offset2 += sizeof(struct udphdr);
> > > +		ulen = ipvs_udp_decap(ipvs, skb, offset2, AF_INET, udph->dest,
> > > +				      raddr, &iproto);
> > > +		if (ulen < 0)
> > > +			goto check;
> > > +		/* Skip IP + UDP + ulen */
> > > +		offset = offset2 + ulen;
> > > +		/* Now we should be at the original IP header */
> > > +		cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph);
> > > +		if (cih && cih->version == 4 && cih->ihl >= 5 &&
> > > +		    iproto == IPPROTO_IPIP)
> > > +			ipip = true;
> > > +		else
> > > +			return NF_ACCEPT;
> > 
> > Skipping over tunnel headers and determining the inner IP protocol really
> > feels like something that should be handled code common to other UDP
> > encapsulation implementations.
> 
> 	We can easily add simple FOU in ipvs_udp_decap() by
> returning 0 and correct *proto. Or you prefer to use common
> code from other files to parse the headers? Currently, there
> is no any GUE func that can be used for this.

My feeling is that using common code, even new common code, would
be better.

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Julian Anastasov]

	Hello,

On Thu, 4 Apr 2019, Simon Horman wrote:

> On Thu, Apr 04, 2019 at 12:18:08AM +0300, Julian Anastasov wrote:
> > 
> > 	We can easily add simple FOU in ipvs_udp_decap() by
> > returning 0 and correct *proto. Or you prefer to use common
> > code from other files to parse the headers? Currently, there
> > is no any GUE func that can be used for this.
> 
> My feeling is that using common code, even new common code, would
> be better.

	Let me know If you prefer to add GUE code that we can use
in this patchset, I can test it easily. I'll delay with v2 to
incorporate any needed changes.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Simon Horman]

On Sat, Apr 06, 2019 at 01:07:34PM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 4 Apr 2019, Simon Horman wrote:
> 
> > On Thu, Apr 04, 2019 at 12:18:08AM +0300, Julian Anastasov wrote:
> > > 
> > > 	We can easily add simple FOU in ipvs_udp_decap() by
> > > returning 0 and correct *proto. Or you prefer to use common
> > > code from other files to parse the headers? Currently, there
> > > is no any GUE func that can be used for this.
> > 
> > My feeling is that using common code, even new common code, would
> > be better.
> 
> 	Let me know If you prefer to add GUE code that we can use
> in this patchset, I can test it easily. I'll delay with v2 to
> incorporate any needed changes.

Thanks Julian,

yes, I would prefer that.

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Simon Horman]

On Mon, Apr 08, 2019 at 01:28:26PM +0200, Simon Horman wrote:
> On Sat, Apr 06, 2019 at 01:07:34PM +0300, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Thu, 4 Apr 2019, Simon Horman wrote:
> > 
> > > On Thu, Apr 04, 2019 at 12:18:08AM +0300, Julian Anastasov wrote:
> > > > 
> > > > 	We can easily add simple FOU in ipvs_udp_decap() by
> > > > returning 0 and correct *proto. Or you prefer to use common
> > > > code from other files to parse the headers? Currently, there
> > > > is no any GUE func that can be used for this.
> > > 
> > > My feeling is that using common code, even new common code, would
> > > be better.
> > 
> > 	Let me know If you prefer to add GUE code that we can use
> > in this patchset, I can test it easily. I'll delay with v2 to
> > incorporate any needed changes.
> 
> Thanks Julian,
> 
> yes, I would prefer that.

Thanks again Julian,

is a v2 of this series pending or am I mistaken somehow?

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Julian Anastasov]

	Hello,

On Wed, 1 May 2019, Simon Horman wrote:

> > > > > 	We can easily add simple FOU in ipvs_udp_decap() by
> > > > > returning 0 and correct *proto. Or you prefer to use common
> > > > > code from other files to parse the headers? Currently, there
> > > > > is no any GUE func that can be used for this.
> > > > 
> > > > My feeling is that using common code, even new common code, would
> > > > be better.
> > > 
> > > 	Let me know If you prefer to add GUE code that we can use
> > > in this patchset, I can test it easily. I'll delay with v2 to
> > > incorporate any needed changes.
> > 
> > Thanks Julian,
> > 
> > yes, I would prefer that.
> 
> Thanks again Julian,
> 
> is a v2 of this series pending or am I mistaken somehow?

	I thought you will have some separate patch that adds
code to be used in v2 but if you prefer I can release v2, so
that you can add followup patch[es] based on that.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net-next 3/3] ipvs: strip udp tunnel headers from icmp errors

[Simon Horman]

On Wed, May 01, 2019 at 05:07:16PM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Wed, 1 May 2019, Simon Horman wrote:
> 
> > > > > > 	We can easily add simple FOU in ipvs_udp_decap() by
> > > > > > returning 0 and correct *proto. Or you prefer to use common
> > > > > > code from other files to parse the headers? Currently, there
> > > > > > is no any GUE func that can be used for this.
> > > > > 
> > > > > My feeling is that using common code, even new common code, would
> > > > > be better.
> > > > 
> > > > 	Let me know If you prefer to add GUE code that we can use
> > > > in this patchset, I can test it easily. I'll delay with v2 to
> > > > incorporate any needed changes.
> > > 
> > > Thanks Julian,
> > > 
> > > yes, I would prefer that.
> > 
> > Thanks again Julian,
> > 
> > is a v2 of this series pending or am I mistaken somehow?
> 
> 	I thought you will have some separate patch that adds
> code to be used in v2 but if you prefer I can release v2, so
> that you can add followup patch[es] based on that.

Thanks, I think sending v2 would be best.