[RFC] ipvs: allow transmit of GRO aggregated skbs

[RFC] ipvs: allow transmit of GRO aggregated skbs

[Simon Horman]

This is a first attempt at allowing LVS to transmit
skbs of greater than MTU length that have been aggregated by GRO.

I have lightly tested the ip_vs_dr_xmit() portion of this patch and
although it seems to work I am unsure that netif_needs_gso() is the correct
test to use.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

LRO is still an outstanding issue, but as its deprecated in favour
of GRO perhaps it doesn't need to be solved.

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 93c15a1..93a782e 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -246,7 +246,8 @@ ip_vs_bypass_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF)) &&
+	    !netif_needs_gso(rt->u.dst.dev, skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -310,7 +311,7 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !netif_needs_gso(rt->u.dst.dev, skb)) {
 		dst_release(&rt->u.dst);
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -377,7 +378,8 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF)) &&
+	    !netif_needs_gso(rt->u.dst.dev, skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL_PKT(0, pp, skb, 0, "ip_vs_nat_xmit(): frag needed for");
@@ -453,7 +455,7 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !netif_needs_gso(rt->u.dst.dev, skb)) {
 		dst_release(&rt->u.dst);
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP_VS_DBG_RL_PKT(0, pp, skb, 0,
@@ -562,8 +564,9 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	df |= (old_iph->frag_off & htons(IP_DF));
 
-	if ((old_iph->frag_off & htons(IP_DF))
-	    && mtu < ntohs(old_iph->tot_len)) {
+	if ((old_iph->frag_off & htons(IP_DF) &&
+	    mtu < ntohs(old_iph->tot_len) &&
+	    !netif_needs_gso(rt->u.dst.dev, skb))) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		ip_rt_put(rt);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -672,7 +675,8 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 	if (skb_dst(skb))
 		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), mtu);
 
-	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr)) {
+	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
+	    !netif_needs_gso(rt->u.dst.dev, skb)) {
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		dst_release(&rt->u.dst);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -761,7 +765,8 @@ ip_vs_dr_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu) {
+	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu &&
+	    !netif_needs_gso(rt->u.dst.dev, skb)) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		ip_rt_put(rt);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -889,7 +894,8 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if ((skb->len > mtu) && (ip_hdr(skb)->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (ip_hdr(skb)->frag_off & htons(IP_DF)) &&
+	    !netif_needs_gso(rt->u.dst.dev, skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -964,7 +970,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb, struct ip_vs_conn *cp,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->u.dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !netif_needs_gso(rt->u.dst.dev, skb)) {
 		dst_release(&rt->u.dst);
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Simon Horman]

This is a first attempt at allowing LVS to transmit
skbs of greater than MTU length that have been aggregated by GRO.

I have lightly tested the ip_vs_dr_xmit() portion of this patch and
although it seems to work I am unsure that netif_needs_gso() is the correct
test to use.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 

* LRO is still an outstanding issue, but as its deprecated in favour
  of GRO perhaps it doesn't need to be solved.

* v1
  - Based on 2.6.35

* v2
  - Rebase on current nf-next-2.6 tree (~2.6.37-rc1)

Index: lvs-test-2.6/net/netfilter/ipvs/ip_vs_xmit.c
===================================================================
--- lvs-test-2.6.orig/net/netfilter/ipvs/ip_vs_xmit.c	2010-10-30 11:43:37.000000000 +0900
+++ lvs-test-2.6/net/netfilter/ipvs/ip_vs_xmit.c	2010-11-06 08:09:17.000000000 +0900
@@ -408,7 +408,8 @@ ip_vs_bypass_xmit(struct sk_buff *skb, s
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF)) &&
+	    !netif_needs_gso(rt->dst.dev, skb)) {
 		ip_rt_put(rt);
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -461,7 +462,7 @@ ip_vs_bypass_xmit_v6(struct sk_buff *skb
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !netif_needs_gso(rt->dst.dev, skb)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -560,7 +561,8 @@ ip_vs_nat_xmit(struct sk_buff *skb, stru
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (iph->frag_off & htons(IP_DF)) &&
+	    !netif_needs_gso(rt->dst.dev, skb)) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL_PKT(0, AF_INET, pp, skb, 0,
 				 "ip_vs_nat_xmit(): frag needed for");
@@ -675,7 +677,7 @@ ip_vs_nat_xmit_v6(struct sk_buff *skb, s
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !netif_needs_gso(rt->dst.dev, skb)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -790,8 +792,9 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, s
 
 	df |= (old_iph->frag_off & htons(IP_DF));
 
-	if ((old_iph->frag_off & htons(IP_DF))
-	    && mtu < ntohs(old_iph->tot_len)) {
+	if ((old_iph->frag_off & htons(IP_DF) &&
+	    mtu < ntohs(old_iph->tot_len) &&
+	    !netif_needs_gso(rt->dst.dev, skb))) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
 		goto tx_error_put;
@@ -903,7 +906,8 @@ ip_vs_tunnel_xmit_v6(struct sk_buff *skb
 	if (skb_dst(skb))
 		skb_dst(skb)->ops->update_pmtu(skb_dst(skb), mtu);
 
-	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr)) {
+	if (mtu < ntohs(old_iph->payload_len) + sizeof(struct ipv6hdr) &&
+	    !netif_needs_gso(rt->dst.dev, skb)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 
@@ -1008,7 +1012,8 @@ ip_vs_dr_xmit(struct sk_buff *skb, struc
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu) {
+	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu &&
+	    !netif_needs_gso(rt->dst.dev, skb)) {
 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
 		ip_rt_put(rt);
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
@@ -1174,7 +1179,8 @@ ip_vs_icmp_xmit(struct sk_buff *skb, str
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if ((skb->len > mtu) && (ip_hdr(skb)->frag_off & htons(IP_DF))) {
+	if ((skb->len > mtu) && (ip_hdr(skb)->frag_off & htons(IP_DF)) &&
+	    !netif_needs_gso(rt->dst.dev, skb)) {
 		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED, htonl(mtu));
 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);
 		goto tx_error_put;
@@ -1288,7 +1294,7 @@ ip_vs_icmp_xmit_v6(struct sk_buff *skb,
 
 	/* MTU checking */
 	mtu = dst_mtu(&rt->dst);
-	if (skb->len > mtu) {
+	if (skb->len > mtu && !netif_needs_gso(rt->dst.dev, skb)) {
 		if (!skb->dev) {
 			struct net *net = dev_net(skb_dst(skb)->dev);
 

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Julian Anastasov]

 	Hello,

On Sat, 6 Nov 2010, Simon Horman wrote:

> This is a first attempt at allowing LVS to transmit
> skbs of greater than MTU length that have been aggregated by GRO.
>
> I have lightly tested the ip_vs_dr_xmit() portion of this patch and
> although it seems to work I am unsure that netif_needs_gso() is the correct
> test to use.

 	ip_forward() uses !skb_is_gso(skb), so may be it is
enough to check for GRO instead of using netif_needs_gso?

> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
> * LRO is still an outstanding issue, but as its deprecated in favour
>  of GRO perhaps it doesn't need to be solved.
>
> * v1
>  - Based on 2.6.35
>
> * v2
>  - Rebase on current nf-next-2.6 tree (~2.6.37-rc1)

> --- lvs-test-2.6.orig/net/netfilter/ipvs/ip_vs_xmit.c	2010-10-30 11:43:37.000000000 +0900
> +++ lvs-test-2.6/net/netfilter/ipvs/ip_vs_xmit.c	2010-11-06 08:09:17.000000000 +0900

> @@ -1008,7 +1012,8 @@ ip_vs_dr_xmit(struct sk_buff *skb, struc
>
> 	/* MTU checking */
> 	mtu = dst_mtu(&rt->dst);
> -	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu) {
> +	if ((iph->frag_off & htons(IP_DF)) && skb->len > mtu &&
> +	    !netif_needs_gso(rt->dst.dev, skb)) {
> 		icmp_send(skb, ICMP_DEST_UNREACH,ICMP_FRAG_NEEDED, htonl(mtu));
> 		ip_rt_put(rt);
> 		IP_VS_DBG_RL("%s(): frag needed\n", __func__);

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Simon Horman]

On Sat, Nov 06, 2010 at 04:18:21PM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Sat, 6 Nov 2010, Simon Horman wrote:
> 
> >This is a first attempt at allowing LVS to transmit
> >skbs of greater than MTU length that have been aggregated by GRO.
> >
> >I have lightly tested the ip_vs_dr_xmit() portion of this patch and
> >although it seems to work I am unsure that netif_needs_gso() is the correct
> >test to use.
> 
> 	ip_forward() uses !skb_is_gso(skb), so may be it is
> enough to check for GRO instead of using netif_needs_gso?

Thanks, I'll look into that.

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Simon Horman]

[ CCing Herbet Xu ]

On Sat, Nov 06, 2010 at 11:28:21PM +0900, Simon Horman wrote:
> On Sat, Nov 06, 2010 at 04:18:21PM +0200, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Sat, 6 Nov 2010, Simon Horman wrote:
> > 
> > >This is a first attempt at allowing LVS to transmit
> > >skbs of greater than MTU length that have been aggregated by GRO.
> > >
> > >I have lightly tested the ip_vs_dr_xmit() portion of this patch and
> > >although it seems to work I am unsure that netif_needs_gso() is the correct
> > >test to use.
> > 
> > 	ip_forward() uses !skb_is_gso(skb), so may be it is
> > enough to check for GRO instead of using netif_needs_gso?
> 
> Thanks, I'll look into that.

Hi Julian,

just to clarify, you think that !skb_is_gso(skb) should be
used in ip_vs_xmit.c? If so, yes I think that makes sense
and I'll re-spin my patch accordingly.

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Julian Anastasov]

 	Hello,

On Mon, 8 Nov 2010, Simon Horman wrote:

> [ CCing Herbet Xu ]
>
>>>> This is a first attempt at allowing LVS to transmit
>>>> skbs of greater than MTU length that have been aggregated by GRO.
>>>>
>>>> I have lightly tested the ip_vs_dr_xmit() portion of this patch and
>>>> although it seems to work I am unsure that netif_needs_gso() is the correct
>>>> test to use.
>>>
>>> 	ip_forward() uses !skb_is_gso(skb), so may be it is
>>> enough to check for GRO instead of using netif_needs_gso?
>>
>> Thanks, I'll look into that.
>
> Hi Julian,
>
> just to clarify, you think that !skb_is_gso(skb) should be
> used in ip_vs_xmit.c? If so, yes I think that makes sense
> and I'll re-spin my patch accordingly.

 	Yes, I think we should check for !skb_is_gso(skb)
as it looks as correct check to avoid FRAG_NEEDED after GRO
but lets wait for confirmation from Herbert Xu.
Also, !skb->local_df check should help for local IPVS clients
that set local_df.

 	If you prefer you can create such helper in ip_vs_xmit.c:

/* Check if packet exceeds MTU */
static inline int ip_vs_mtu_exceeded(struct sk_buff *skb, unsigned int mtu)
{
 	return skb->len > mtu && !skb_is_gso(skb) && !skb->local_df;
}

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Simon Horman]

On Mon, Nov 08, 2010 at 11:36:21AM +0200, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Mon, 8 Nov 2010, Simon Horman wrote:
> 
> >[ CCing Herbet Xu ]
> >
> >>>>This is a first attempt at allowing LVS to transmit
> >>>>skbs of greater than MTU length that have been aggregated by GRO.
> >>>>
> >>>>I have lightly tested the ip_vs_dr_xmit() portion of this patch and
> >>>>although it seems to work I am unsure that netif_needs_gso() is the correct
> >>>>test to use.
> >>>
> >>>	ip_forward() uses !skb_is_gso(skb), so may be it is
> >>>enough to check for GRO instead of using netif_needs_gso?
> >>
> >>Thanks, I'll look into that.
> >
> >Hi Julian,
> >
> >just to clarify, you think that !skb_is_gso(skb) should be
> >used in ip_vs_xmit.c? If so, yes I think that makes sense
> >and I'll re-spin my patch accordingly.
> 
> 	Yes, I think we should check for !skb_is_gso(skb)
> as it looks as correct check to avoid FRAG_NEEDED after GRO
> but lets wait for confirmation from Herbert Xu.

Yes, lets try and get a word from Herbert Xu on this.

> Also, !skb->local_df check should help for local IPVS clients
> that set local_df.
> 
> 	If you prefer you can create such helper in ip_vs_xmit.c:
> 
> /* Check if packet exceeds MTU */
> static inline int ip_vs_mtu_exceeded(struct sk_buff *skb, unsigned int mtu)
> {
> 	return skb->len > mtu && !skb_is_gso(skb) && !skb->local_df;
> }

Good thinking.

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Herbert Xu]

On Mon, Nov 08, 2010 at 08:45:42PM +0900, Simon Horman wrote:
>
> > 	Yes, I think we should check for !skb_is_gso(skb)
> > as it looks as correct check to avoid FRAG_NEEDED after GRO
> > but lets wait for confirmation from Herbert Xu.
> 
> Yes, lets try and get a word from Herbert Xu on this.

Yes skb_is_gso(skb) is the best for now.  It isn't 100% correct
as we should still be checking that the gso_size is less than
the MTU but we need to fix that elsewhere first.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC v2] ipvs: allow transmit of GRO aggregated skbs

[Simon Horman]

On Mon, Nov 08, 2010 at 10:19:55PM +0800, Herbert Xu wrote:
> On Mon, Nov 08, 2010 at 08:45:42PM +0900, Simon Horman wrote:
> >
> > > 	Yes, I think we should check for !skb_is_gso(skb)
> > > as it looks as correct check to avoid FRAG_NEEDED after GRO
> > > but lets wait for confirmation from Herbert Xu.
> > 
> > Yes, lets try and get a word from Herbert Xu on this.
> 
> Yes skb_is_gso(skb) is the best for now.  It isn't 100% correct
> as we should still be checking that the gso_size is less than
> the MTU but we need to fix that elsewhere first.

Thanks Herbert.