On 8/28/19 1:02 PM, Srinivas Neeli wrote:
> Case 1:
> can_put_echo_skb(); -> skb = can_create_echo_skb(skb); -> return skb;
> 
> In can_create_echo_skb() not using the shared_skb, so we are returning the old skb.
> Storing the return value in "skb". But it's a pointer, for storing that need double pointer.
> Instead of double-pointer using a single pointer. In this scenario it's ok , we are returning the same SKB.
> 
> Case 2:
> can_put_echo_skb(skb, ndev, priv->tx_head % priv->tx_max); -> skb = can_create_echo_skb(skb); -> can_skb_set_owner(nskb, skb->sk); - Returning nskb;
> 
> shared_skb scenario:
> In share-skb case “can_create_echo_skb(skb);”  returning "new skb". For storing new skb need a double pointer.
> 
> Providing an example for overcoming above issue.
> Example:
> can_put_echo_skb(struct sk_buff **skb,struct net_device *dev,unsigned int idx);

Now I get what you mean.

> If you ok with this change, I will send a patch.

I think the can_put_echo_skb() API needs clarification. The driver is
not allowed to touch the skb any more after can_put_echo_skb(). We
should review the driver for this.

thanks,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |