On Mon, 2015-06-15 at 13:40 +0000, Madalin-Cristian Bucur wrote: > > -----Original Message----- > > From: Eric Dumazet [mailto:eric.duma...@gmail.com] > > > > On Wed, 2015-04-29 at 17:56 +0300, Madalin Bucur wrote: > > > This introduces the Freescale Data Path Acceleration Architecture > > > (DPAA) Ethernet driver (dpaa_eth) that builds upon the DPAA QMan, > > > BMan, PAMU and FMan drivers to deliver Ethernet connectivity on > > > the Freescale DPAA QorIQ platforms. > > ... > > > > > + /* We're going to store the skb backpointer at the beginning > > > + * of the data buffer, so we need a privately owned skb > > > + */ > > > + > > > + /* Code borrowed from skb_unshare(). */ > > > + if (skb_cloned(skb)) { > > > + struct sk_buff *nskb = skb_copy(skb, GFP_ATOMIC); > > > + > > > + /* Finally, create a contig FD from this skb */ > > > + skb_to_contig_fd(priv, skb, &fd, countptr, &offset); > > > + > > > + kfree_skb(skb); > > > + skb = nskb; > > > + /* skb_copy() has now linearized the skbuff. */ > > > + } > > > + > > > > You know that TCP packets are clones, right ? > > This code is killing performance. > > > > TCP allows the headers part being modified by a driver if needed. > > > > You should use skb_header_cloned() instead. > > Thank you, I'll address this. I plan to do something like this: > > + if (!nonlinear) { > + /* We're going to store the skb backpointer at the beginning > + * of the data buffer, so we need a privately owned skb > + */ > + > + /* make sure skb is not shared, skb_cow_head() assumes it's > not */ > + skb = skb_share_check(skb, GFP_ATOMIC); > + if (!skb) > + goto enomem; > + > + /* verify the skb head is not cloned */ > + if (skb_cow_head(skb, priv->tx_headroom)) > + goto enomem; > + > + nonlinear = skb_is_nonlinear(skb); > + } > > but I'm not sure the skb_share_check() is required on tx. > I'm also a bit puzzled by the aliasing between shared and cloned terms > (i.e. skb_unshare() could be named something like skb_unclone(); > the skb_share_check() not only checks but also unshares an skb so > the already taken skb_unshare() name would probably fit too).
A driver can ask tx skbs to be not shared, even if pktgen is used with "clone=X" on it. dev->priv_flags &= ~IFF_TX_SKB_SHARING; This way, you can avoid skb_share_check() completely. Only pktgen will care. Most skb_share_check() calls in drivers/net are related to input paths on virtual drivers. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
