Re: [tipc-discussion] [PATCH net-next 8/8] tipc: add hardware gso

[Jon M. <jm...@re...>]

On 06/07/2021 14:22, Xin Long wrote:
> Since there's no enough bit in netdev_features_t for
> NETIF_F_GSO_TIPC_BIT, and tipc is using the simliar
> code as sctp, this patch will reuse SKB_GSO_SCTP and
> NETIF_F_GSO_SCTP_BIT for tipc.
> 
> Signed-off-by: Xin Long <luc...@gm...>
> ---
>   include/linux/skbuff.h |  2 --
>   net/tipc/node.c        | 15 ++++++++++++++-
>   net/tipc/offload.c     |  4 ++--
>   3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 148bf0ed7336..b2db9cd9a73f 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -599,8 +599,6 @@ enum {
>   	SKB_GSO_UDP_L4 = 1 << 17,
>   
>   	SKB_GSO_FRAGLIST = 1 << 18,
> -
> -	SKB_GSO_TIPC = 1 << 19,
>   };
>   
>   #if BITS_PER_LONG > 32
> diff --git a/net/tipc/node.c b/net/tipc/node.c
> index 9947b7dfe1d2..17e59c8dac31 100644
> --- a/net/tipc/node.c
> +++ b/net/tipc/node.c
> @@ -2068,7 +2068,7 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
>    * Invoked with no locks held. Bearer pointer must point to a valid bearer
>    * structure (i.e. cannot be NULL), but bearer can be inactive.
>    */
> -void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
> +static void __tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
>   {
>   	struct sk_buff_head xmitq;
>   	struct tipc_link_entry *le;
> @@ -2189,6 +2189,19 @@ void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
>   	kfree_skb(skb);
>   }
>   
> +void tipc_rcv(struct net *net, struct sk_buff *skb, struct tipc_bearer *b)
> +{
> +	struct sk_buff *seg, *next;
> +
> +	if (!skb_is_gso(skb) || !skb_is_gso_sctp(skb))
> +		return __tipc_rcv(net, skb, b);
> +
> +	skb_list_walk_safe(skb_shinfo(skb)->frag_list, seg, next)
> +		__tipc_rcv(net, seg, b);
> +	skb_shinfo(skb)->frag_list = NULL;
> +	consume_skb(skb);
> +}
> +
>   void tipc_node_apply_property(struct net *net, struct tipc_bearer *b,
>   			      int prop)
>   {
> diff --git a/net/tipc/offload.c b/net/tipc/offload.c
> index d137679f4db0..26e372178635 100644
> --- a/net/tipc/offload.c
> +++ b/net/tipc/offload.c
> @@ -5,7 +5,7 @@
>   static struct sk_buff *tipc_gso_segment(struct sk_buff *skb,
>   					netdev_features_t features)
>   {
> -	if (!(skb_shinfo(skb)->gso_type & SKB_GSO_TIPC))
> +	if (!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP))
>   		return ERR_PTR(-EINVAL);
>   
>   	return skb_segment(skb, (features | NETIF_F_HW_CSUM) & ~NETIF_F_SG);
> @@ -39,7 +39,7 @@ bool tipc_msg_gso_append(struct sk_buff **p, struct sk_buff *skb, u16 segs)
>   
>   		skb_shinfo(nskb)->frag_list = head;
>   		skb_shinfo(nskb)->gso_segs = 1;
> -		skb_shinfo(nskb)->gso_type = SKB_GSO_TIPC;
> +		skb_shinfo(nskb)->gso_type = SKB_GSO_SCTP;
>   		skb_shinfo(nskb)->gso_size = GSO_BY_FRAGS;
>   		skb_reset_network_header(head);
>   
> 

I don´t have much more to add, -it looks good to me, and way simpler 
than what I was trying a couple of years ago.

If you fix the checkpatch issues and, maybe if possible, split it into 
two series, you have my ack.

PS. Did you test this with crypto?

///jon