Re: [PATCH net-next] sctp: define sctp_packet_gso_append to build GSO frames

[Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>]

On Thu, Jun 14, 2018 at 07:37:02AM +0800, Xin Long wrote:
> Now sctp GSO uses skb_gro_receive() to append the data into head
> skb frag_list. However it actually only needs very few code from
> skb_gro_receive(). Besides, NAPI_GRO_CB has to be set while most
> of its members are not needed here.
> 
> This patch is to add sctp_packet_gso_append() to build GSO frames
> instead of skb_gro_receive(), and it would avoid many unnecessary
> checks and make the code clearer.
> 
> Note that sctp will use page frags instead of frag_list to build
> GSO frames in another patch. But it may take time, as sctp's GSO
> frames may have different size. skb_segment() can only split it
> into the frags with the same size, which would break the border
> of sctp chunks.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

I cannot test it, but it looks good to me. Thanks Xin

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  include/net/sctp/structs.h |  5 +++++
>  net/sctp/output.c          | 28 ++++++++++++++++++----------
>  2 files changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index ebf809e..dbe1b91 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1133,6 +1133,11 @@ struct sctp_input_cb {
>  };
>  #define SCTP_INPUT_CB(__skb)	((struct sctp_input_cb *)&((__skb)->cb[0]))
>  
> +struct sctp_output_cb {
> +	struct sk_buff *last;
> +};
> +#define SCTP_OUTPUT_CB(__skb)	((struct sctp_output_cb *)&((__skb)->cb[0]))
> +
>  static inline const struct sk_buff *sctp_gso_headskb(const struct sk_buff *skb)
>  {
>  	const struct sctp_chunk *chunk = SCTP_INPUT_CB(skb)->chunk;
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index e672dee..7f849b0 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -409,6 +409,21 @@ static void sctp_packet_set_owner_w(struct sk_buff *skb, struct sock *sk)
>  	refcount_inc(&sk->sk_wmem_alloc);
>  }
>  
> +static void sctp_packet_gso_append(struct sk_buff *head, struct sk_buff *skb)
> +{
> +	if (SCTP_OUTPUT_CB(head)->last == head)
> +		skb_shinfo(head)->frag_list = skb;
> +	else
> +		SCTP_OUTPUT_CB(head)->last->next = skb;
> +	SCTP_OUTPUT_CB(head)->last = skb;
> +
> +	head->truesize += skb->truesize;
> +	head->data_len += skb->len;
> +	head->len += skb->len;
> +
> +	__skb_header_release(skb);
> +}
> +
>  static int sctp_packet_pack(struct sctp_packet *packet,
>  			    struct sk_buff *head, int gso, gfp_t gfp)
>  {
> @@ -422,7 +437,7 @@ static int sctp_packet_pack(struct sctp_packet *packet,
>  
>  	if (gso) {
>  		skb_shinfo(head)->gso_type = sk->sk_gso_type;
> -		NAPI_GRO_CB(head)->last = head;
> +		SCTP_OUTPUT_CB(head)->last = head;
>  	} else {
>  		nskb = head;
>  		pkt_size = packet->size;
> @@ -503,15 +518,8 @@ static int sctp_packet_pack(struct sctp_packet *packet,
>  					 &packet->chunk_list);
>  		}
>  
> -		if (gso) {
> -			if (skb_gro_receive(&head, nskb)) {
> -				kfree_skb(nskb);
> -				return 0;
> -			}
> -			if (WARN_ON_ONCE(skb_shinfo(head)->gso_segs >=
> -					 sk->sk_gso_max_segs))
> -				return 0;
> -		}
> +		if (gso)
> +			sctp_packet_gso_append(head, nskb);
>  
>  		pkt_count++;
>  	} while (!list_empty(&packet->chunk_list));
> -- 
> 2.1.0
>