Re: [PATCH v2] ipvs: allow tunneling with gre encapsulation

[Vadim Fedorenko <vfedorenko@xxxxxxxxxxxxxx>]

Hi Simon,

Thanks for review. I'll send v3 patch with 'else if' statement.
Converting it to case statement seems to be overkill until adding
another type of encapsulation

On 01.07.2019 18:29, Simon Horman wrote:
> On Mon, Jul 01, 2019 at 01:28:49AM +0300, Vadim Fedorenko wrote:
> > windows real servers can handle gre tunnels, this patch allows
> > gre encapsulation with the tunneling method, thereby letting ipvs
> > be load balancer for windows-based services
> >
> > Signed-off-by: Vadim Fedorenko <vfedorenko@xxxxxxxxxxxxxx>
> > ---
> >   include/uapi/linux/ip_vs.h      |  1 +
> >   net/netfilter/ipvs/ip_vs_ctl.c  |  1 +
> >   net/netfilter/ipvs/ip_vs_xmit.c | 76 +++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 78 insertions(+)
> >
> > diff --git a/include/uapi/linux/ip_vs.h b/include/uapi/linux/ip_vs.h
> > index e4f1806..4102ddc 100644
> > --- a/include/uapi/linux/ip_vs.h
> > +++ b/include/uapi/linux/ip_vs.h
> > @@ -128,6 +128,7 @@
> >   enum {
> >   	IP_VS_CONN_F_TUNNEL_TYPE_IPIP = 0,	/* IPIP */
> >   	IP_VS_CONN_F_TUNNEL_TYPE_GUE,		/* GUE */
> > +	IP_VS_CONN_F_TUNNEL_TYPE_GRE,		/* GRE */
> >   	IP_VS_CONN_F_TUNNEL_TYPE_MAX,
> >   };
> >   
> > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> > index 84384d8..998353b 100644
> > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> > @@ -525,6 +525,7 @@ static void ip_vs_rs_hash(struct netns_ipvs *ipvs, struct ip_vs_dest *dest)
> >   			port = dest->tun_port;
> >   			break;
> >   		case IP_VS_CONN_F_TUNNEL_TYPE_IPIP:
> > +		case IP_VS_CONN_F_TUNNEL_TYPE_GRE:
> >   			port = 0;
> >   			break;
> >   		default:
> > diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
> > index 71fc6d6..37cc674 100644
> > --- a/net/netfilter/ipvs/ip_vs_xmit.c
> > +++ b/net/netfilter/ipvs/ip_vs_xmit.c
> > @@ -29,6 +29,7 @@
> >   #include <linux/tcp.h>                  /* for tcphdr */
> >   #include <net/ip.h>
> >   #include <net/gue.h>
> > +#include <net/gre.h>
> >   #include <net/tcp.h>                    /* for csum_tcpudp_magic */
> >   #include <net/udp.h>
> >   #include <net/icmp.h>                   /* for icmp_send */
> > @@ -389,6 +390,13 @@ static inline bool decrement_ttl(struct netns_ipvs *ipvs,
> >   			    skb->ip_summed == CHECKSUM_PARTIAL)
> >   				mtu -= GUE_PLEN_REMCSUM + GUE_LEN_PRIV;
> >   		}
> > +		if (dest->tun_type == IP_VS_CONN_F_TUNNEL_TYPE_GRE) {
>
> Hi Vadim,
>
> The previous conditional also checks the value of dest->tun_type,
> so I think that it would be nicer if this was changed to either else if or
> to use a case statement. Likewise elsewhere in this patch.
>
> > +			__be16 tflags = 0;
> > +
> > +			if (dest->tun_flags & IP_VS_TUNNEL_ENCAP_FLAG_CSUM)
> > +				tflags |= TUNNEL_CSUM;
> > +			mtu -= gre_calc_hlen(tflags);
> > +		}
> >   		if (mtu < 68) {
> >   			IP_VS_DBG_RL("%s(): mtu less than 68\n", __func__);
> >   			goto err_put;
>
> ...