RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
-Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] On Wed, 2015-06-10 at 07:38 +, Madalin-Cristian Bucur wrote: Hi Eric, Can you please tell us if this change would be for the better? I was about to say yes to this request but checked and no other Ethernet driver seems to use the queue trans_start. I was able to find your patch net: tx scalability works : trans_start [ http://patchwork.ozlabs.org/patch/27104/ ] but did not find more about this topic. Drivers no longer have to worry about updating trans_start themselves. Core networking stack handles this. Check txq_trans_update() done from netdev_start_xmit() I'll remove the updates, thank you. Madalin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
-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).
Madalin
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
On Mon, 2015-06-15 at 13:40 +, 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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
Hi Eric,
Can you please tell us if this change would be for the better?
I was about to say yes to this request but checked and no other Ethernet driver
seems to use the queue trans_start.
I was able to find your patch net: tx scalability works : trans_start [
http://patchwork.ozlabs.org/patch/27104/ ] but did not find more about this
topic.
Thank you,
Madalin
-Original Message-
From: Xie Jianhua-B29408
Sent: Wednesday, June 10, 2015 9:00 AM
To: Bucur Madalin-Cristian-B32716; net...@vger.kernel.org; linuxppc-
d...@lists.ozlabs.org
Cc: linux-ker...@vger.kernel.org
Subject: RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+jianhua.xie=freescale@lists.ozlabs.org] On Behalf Of
Madalin
Bucur
Sent: Wednesday, April 29, 2015 10:57 PM
To: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
Cc: linux-ker...@vger.kernel.org; Bucur Madalin-Cristian-B32716
Subject: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
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.
Snip..
+
+ if (unlikely(dpa_xmit(priv, percpu_stats, queue_mapping, fd) 0))
+ goto xmit_failed;
+
+ net_dev-trans_start = jiffies;
It is probably better to use netdev_queue-trans_start to instead of
net_dev-trans_start on SMP.
Best Regards,
Jianhua
+ return NETDEV_TX_OK;
+
+xmit_failed:
+ if (fd.cmd FM_FD_CMD_FCO) {
+ (*countptr)--;
+ dpa_fd_release(net_dev, fd);
+ percpu_stats-tx_errors++;
+ return NETDEV_TX_OK;
+ }
+ _dpa_cleanup_tx_fd(priv, fd);
+ percpu_stats-tx_errors++;
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
+}
--
1.7.11.7
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
On Wed, 2015-06-10 at 07:38 +, Madalin-Cristian Bucur wrote: Hi Eric, Can you please tell us if this change would be for the better? I was about to say yes to this request but checked and no other Ethernet driver seems to use the queue trans_start. I was able to find your patch net: tx scalability works : trans_start [ http://patchwork.ozlabs.org/patch/27104/ ] but did not find more about this topic. Drivers no longer have to worry about updating trans_start themselves. Core networking stack handles this. Check txq_trans_update() done from netdev_start_xmit() ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
-Original Message-
From: Linuxppc-dev [mailto:linuxppc-dev-
bounces+jianhua.xie=freescale@lists.ozlabs.org] On Behalf Of Madalin
Bucur
Sent: Wednesday, April 29, 2015 10:57 PM
To: net...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
Cc: linux-ker...@vger.kernel.org; Bucur Madalin-Cristian-B32716
Subject: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
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.
Snip..
+
+ if (unlikely(dpa_xmit(priv, percpu_stats, queue_mapping, fd) 0))
+ goto xmit_failed;
+
+ net_dev-trans_start = jiffies;
It is probably better to use netdev_queue-trans_start to instead of
net_dev-trans_start on SMP.
Best Regards,
Jianhua
+ return NETDEV_TX_OK;
+
+xmit_failed:
+ if (fd.cmd FM_FD_CMD_FCO) {
+ (*countptr)--;
+ dpa_fd_release(net_dev, fd);
+ percpu_stats-tx_errors++;
+ return NETDEV_TX_OK;
+ }
+ _dpa_cleanup_tx_fd(priv, fd);
+ percpu_stats-tx_errors++;
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
+}
--
1.7.11.7
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC,v3 02/10] dpaa_eth: add support for DPAA Ethernet
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.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
