[patch net REPOST 0/3] UFO fixes

[patch net REPOST 0/3] UFO fixes

[Jiri Pirko]

Couple of patches fixing UFO functionality in different situations.

Jiri Pirko (3):
  udp6: respect IPV6_DONTFRAG sockopt in case there are pending frames
  ip6_output: do skb ufo init for peeked non ufo skb as well
  ip_output: do skb ufo init for peeked non ufo skb as well

 net/ipv4/ip_output.c  | 14 +++++++++-----
 net/ipv6/ip6_output.c | 26 ++++++++++++++------------
 net/ipv6/udp.c        |  5 ++---
 3 files changed, 25 insertions(+), 20 deletions(-)

-- 
1.8.3.1

[patch net REPOST 1/3] udp6: respect IPV6_DONTFRAG sockopt in case there are pending frames

[Jiri Pirko]

if up->pending != 0 dontfrag is left with default value -1. That
causes that application that do:
sendto len>mtu flag MSG_MORE
sendto len>mtu flag 0
will receive EMSGSIZE errno as the result of the second sendto.

This patch fixes it by respecting IPV6_DONTFRAG socket option.

introduced by:
commit 4b340ae20d0e2366792abe70f46629e576adaf5e "IPv6: Complete IPV6_DONTFRAG support"

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/ipv6/udp.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 72b7eaa..1878609 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1225,9 +1225,6 @@ do_udp_sendmsg:
 	if (tclass < 0)
 		tclass = np->tclass;
 
-	if (dontfrag < 0)
-		dontfrag = np->dontfrag;
-
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;
 back_from_confirm:
@@ -1246,6 +1243,8 @@ back_from_confirm:
 	up->pending = AF_INET6;
 
 do_append_data:
+	if (dontfrag < 0)
+		dontfrag = np->dontfrag;
 	up->len += ulen;
 	getfrag  =  is_udplite ?  udplite_getfrag : ip_generic_getfrag;
 	err = ip6_append_data(sk, getfrag, msg->msg_iov, ulen,
-- 
1.8.3.1

[patch net REPOST 2/3] ip6_output: do skb ufo init for peeked non ufo skb as well

[Jiri Pirko]

Now, if user application does:
sendto len<mtu flag MSG_MORE
sendto len>mtu flag 0
The skb is not treated as fragmented one because it is not initialized
that way. So move the initialization to fix this.

introduced by:
commit e89e9cf539a28df7d0eb1d0a545368e9920b34ac "[IPv4/IPv6]: UFO Scatter-gather approach"

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/ipv6/ip6_output.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index a54c45c..c6cfa2f 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1008,6 +1008,7 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 
 {
 	struct sk_buff *skb;
+	struct frag_hdr fhdr;
 	int err;
 
 	/* There is support for UDP large send offload by network
@@ -1015,8 +1016,6 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 	 * udp datagram
 	 */
 	if ((skb = skb_peek_tail(&sk->sk_write_queue)) == NULL) {
-		struct frag_hdr fhdr;
-
 		skb = sock_alloc_send_skb(sk,
 			hh_len + fragheaderlen + transhdrlen + 20,
 			(flags & MSG_DONTWAIT), &err);
@@ -1036,20 +1035,23 @@ static inline int ip6_ufo_append_data(struct sock *sk,
 		skb->transport_header = skb->network_header + fragheaderlen;
 
 		skb->protocol = htons(ETH_P_IPV6);
-		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum = 0;
 
-		/* Specify the length of each IPv6 datagram fragment.
-		 * It has to be a multiple of 8.
-		 */
-		skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
-					     sizeof(struct frag_hdr)) & ~7;
-		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
-		ipv6_select_ident(&fhdr, rt);
-		skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
 		__skb_queue_tail(&sk->sk_write_queue, skb);
-	}
+	} else if (skb_is_gso(skb))
+		goto append;
+
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	/* Specify the length of each IPv6 datagram fragment.
+	 * It has to be a multiple of 8.
+	 */
+	skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
+				     sizeof(struct frag_hdr)) & ~7;
+	skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
+	ipv6_select_ident(&fhdr, rt);
+	skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
 
+append:
 	return skb_append_datato_frags(sk, skb, getfrag, from,
 				       (length - transhdrlen));
 }
-- 
1.8.3.1

[patch net REPOST 3/3] ip_output: do skb ufo init for peeked non ufo skb as well

[Jiri Pirko]

Now, if user application does:
sendto len<mtu flag MSG_MORE
sendto len>mtu flag 0
The skb is not treated as fragmented one because it is not initialized
that way. So move the initialization to fix this.

introduced by:
commit e89e9cf539a28df7d0eb1d0a545368e9920b34ac "[IPv4/IPv6]: UFO Scatter-gather approach"

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/ipv4/ip_output.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index a04d872..bd21c5d 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -772,15 +772,19 @@ static inline int ip_ufo_append_data(struct sock *sk,
 		/* initialize protocol header pointer */
 		skb->transport_header = skb->network_header + fragheaderlen;
 
-		skb->ip_summed = CHECKSUM_PARTIAL;
 		skb->csum = 0;
 
-		/* specify the length of each IP datagram fragment */
-		skb_shinfo(skb)->gso_size = maxfraglen - fragheaderlen;
-		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
+
 		__skb_queue_tail(queue, skb);
-	}
+	} else if (skb_is_gso(skb))
+		goto append;
+
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	/* specify the length of each IP datagram fragment */
+	skb_shinfo(skb)->gso_size = maxfraglen - fragheaderlen;
+	skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
 
+append:
 	return skb_append_datato_frags(sk, skb, getfrag, from,
 				       (length - transhdrlen));
 }
-- 
1.8.3.1

Re: [patch net REPOST 0/3] UFO fixes

[Jiri Pirko]

Hannes, sorry for messed up email address :/

Fri, Oct 18, 2013 at 07:13:48PM CEST, jiri@resnulli.us wrote:
>Couple of patches fixing UFO functionality in different situations.
>
>Jiri Pirko (3):
>  udp6: respect IPV6_DONTFRAG sockopt in case there are pending frames
>  ip6_output: do skb ufo init for peeked non ufo skb as well
>  ip_output: do skb ufo init for peeked non ufo skb as well
>
> net/ipv4/ip_output.c  | 14 +++++++++-----
> net/ipv6/ip6_output.c | 26 ++++++++++++++------------
> net/ipv6/udp.c        |  5 ++---
> 3 files changed, 25 insertions(+), 20 deletions(-)
>
>-- 
>1.8.3.1
>

Re: [patch net REPOST 0/3] UFO fixes

[Hannes Frederic Sowa]

On Fri, Oct 18, 2013 at 07:15:24PM +0200, Jiri Pirko wrote:
> Hannes, sorry for messed up email address :/

No problem, I got them. ;)

I'll review them again this weekend. Also Jon mailed me that he will
look at the neterion driver this weekend, so this gets finally sorted out.

Thanks,

  Hannes

Re: [patch net REPOST 2/3] ip6_output: do skb ufo init for peeked non ufo skb as well

[Sergei Shtylyov]

Hello.

On 10/18/2013 09:13 PM, Jiri Pirko wrote:

> Now, if user application does:
> sendto len<mtu flag MSG_MORE
> sendto len>mtu flag 0
> The skb is not treated as fragmented one because it is not initialized
> that way. So move the initialization to fix this.

> introduced by:
> commit e89e9cf539a28df7d0eb1d0a545368e9920b34ac "[IPv4/IPv6]: UFO Scatter-gather approach"

> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>   net/ipv6/ip6_output.c | 26 ++++++++++++++------------
>   1 file changed, 14 insertions(+), 12 deletions(-)

> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index a54c45c..c6cfa2f 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
[...]
> @@ -1036,20 +1035,23 @@ static inline int ip6_ufo_append_data(struct sock *sk,
>   		skb->transport_header = skb->network_header + fragheaderlen;
>
>   		skb->protocol = htons(ETH_P_IPV6);
> -		skb->ip_summed = CHECKSUM_PARTIAL;
>   		skb->csum = 0;
>
> -		/* Specify the length of each IPv6 datagram fragment.
> -		 * It has to be a multiple of 8.
> -		 */
> -		skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
> -					     sizeof(struct frag_hdr)) & ~7;
> -		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> -		ipv6_select_ident(&fhdr, rt);
> -		skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
>   		__skb_queue_tail(&sk->sk_write_queue, skb);
> -	}
> +	} else if (skb_is_gso(skb))
> +		goto append;

    According to Documentation/CodingStyle, there should be {} in both arms of 
*if* statement if there's {} in one of them.

WBR, Sergei

Re: [patch net REPOST 3/3] ip_output: do skb ufo init for peeked non ufo skb as well

[Sergei Shtylyov]

On 10/18/2013 09:13 PM, Jiri Pirko wrote:

> Now, if user application does:
> sendto len<mtu flag MSG_MORE
> sendto len>mtu flag 0
> The skb is not treated as fragmented one because it is not initialized
> that way. So move the initialization to fix this.

> introduced by:
> commit e89e9cf539a28df7d0eb1d0a545368e9920b34ac "[IPv4/IPv6]: UFO Scatter-gather approach"

> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>   net/ipv4/ip_output.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)

> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index a04d872..bd21c5d 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -772,15 +772,19 @@ static inline int ip_ufo_append_data(struct sock *sk,
>   		/* initialize protocol header pointer */
>   		skb->transport_header = skb->network_header + fragheaderlen;
>
> -		skb->ip_summed = CHECKSUM_PARTIAL;
>   		skb->csum = 0;
>
> -		/* specify the length of each IP datagram fragment */
> -		skb_shinfo(skb)->gso_size = maxfraglen - fragheaderlen;
> -		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> +
>   		__skb_queue_tail(queue, skb);
> -	}
> +	} else if (skb_is_gso(skb))
> +		goto append;

    Same comment on the necessity of {} here.

WBR, Sergei

Re: [patch net REPOST 2/3] ip6_output: do skb ufo init for peeked non ufo skb as well

[Jiri Pirko]

Fri, Oct 18, 2013 at 08:44:43PM CEST, sergei.shtylyov@cogentembedded.com wrote:
>Hello.
>
>On 10/18/2013 09:13 PM, Jiri Pirko wrote:
>
>>Now, if user application does:
>>sendto len<mtu flag MSG_MORE
>>sendto len>mtu flag 0
>>The skb is not treated as fragmented one because it is not initialized
>>that way. So move the initialization to fix this.
>
>>introduced by:
>>commit e89e9cf539a28df7d0eb1d0a545368e9920b34ac "[IPv4/IPv6]: UFO Scatter-gather approach"
>
>>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>---
>>  net/ipv6/ip6_output.c | 26 ++++++++++++++------------
>>  1 file changed, 14 insertions(+), 12 deletions(-)
>
>>diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>>index a54c45c..c6cfa2f 100644
>>--- a/net/ipv6/ip6_output.c
>>+++ b/net/ipv6/ip6_output.c
>[...]
>>@@ -1036,20 +1035,23 @@ static inline int ip6_ufo_append_data(struct sock *sk,
>>  		skb->transport_header = skb->network_header + fragheaderlen;
>>
>>  		skb->protocol = htons(ETH_P_IPV6);
>>-		skb->ip_summed = CHECKSUM_PARTIAL;
>>  		skb->csum = 0;
>>
>>-		/* Specify the length of each IPv6 datagram fragment.
>>-		 * It has to be a multiple of 8.
>>-		 */
>>-		skb_shinfo(skb)->gso_size = (mtu - fragheaderlen -
>>-					     sizeof(struct frag_hdr)) & ~7;
>>-		skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
>>-		ipv6_select_ident(&fhdr, rt);
>>-		skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
>>  		__skb_queue_tail(&sk->sk_write_queue, skb);
>>-	}
>>+	} else if (skb_is_gso(skb))
>>+		goto append;
>
>   According to Documentation/CodingStyle, there should be {} in both
>arms of *if* statement if there's {} in one of them.

Yep, you are right. I wonder why scripts/checkpatch.pl do not warn about
this. I will send v2. Thanks


>
>WBR, Sergei
>