lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <1425049987.5130.15.camel@edumazet-glaptop2.roam.corp.google.com> Date: Fri, 27 Feb 2015 07:13:07 -0800 From: Eric Dumazet <eric.dumazet@...il.com> To: Jaedon Shin <jaedon.shin@...il.com> Cc: David Miller <davem@...emloft.net>, Florian Fainelli <f.fainelli@...il.com>, netdev@...r.kernel.org Subject: Re: [PATCH] net: bcmgenet: fix throughtput regression with TSO autosizing On Fri, 2015-02-27 at 23:33 +0900, Jaedon Shin wrote: > > On Feb 27, 2015, at 1:36 AM, David Miller <davem@...emloft.net> wrote: > > > > From: Jaedon Shin <jaedon.shin@...il.com> > > Date: Thu, 26 Feb 2015 20:05:58 +0900 > > > >> This patch prevents the performance degradation of xmit after > >> 605ad7f ("tcp: refine TSO autosizing"). > >> > >> Signed-off-by: Jaedon Shin <jaedon.shin@...il.com> > > > > I doubt this is the correct way to fix this. > > > > Also, you need to describe in detail what the actual problem > > is, how you evaluated the cause, and what made you think that > > your choosen solution is the proper one. > > The bcmgenet_tx_reclaim() of tx_ring[{0,1,2,3}] process only under 18 > descriptors is too late reclaiming transmitted skb. Therefore, > performance degradation of xmit after 605ad7f ("tcp: refine TSO autosizing"). But the real cause is that this driver disables interrupts unless a certain amount of TX descriptors are used. This is wrong and should be fixed. Only if this fix is proven to not be possible, skb_orphan() will be needed. Random guess, could you simply try this quick and dirty patch : (dirty because I am lazy and I think Florian is already working on it) diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c index ff83c46bc389..0c2e1b0cd180 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c @@ -1028,9 +1028,6 @@ static void __bcmgenet_tx_reclaim(struct net_device *dev, last_c_index &= (num_tx_bds - 1); } - if (ring->free_bds > (MAX_SKB_FRAGS + 1)) - ring->int_disable(priv, ring); - if (netif_tx_queue_stopped(txq)) netif_tx_wake_queue(txq); @@ -1302,11 +1299,13 @@ static netdev_tx_t bcmgenet_xmit(struct sk_buff *skb, struct net_device *dev) bcmgenet_tdma_ring_writel(priv, ring->index, ring->prod_index, TDMA_PROD_INDEX); - if (ring->free_bds <= (MAX_SKB_FRAGS + 1)) { + if (ring->free_bds <= (MAX_SKB_FRAGS + 1)) netif_tx_stop_queue(txq); + + if (unlikely(!ring->int_enabled)) { + ring->int_enabled = true; ring->int_enable(priv, ring); } - out: spin_unlock_irqrestore(&ring->lock, flags); diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h index b36ddec0cc0a..90d52893a1c8 100644 --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h @@ -530,6 +530,7 @@ struct bcmgenet_tx_ring { unsigned int prod_index; /* Tx ring producer index SW copy */ unsigned int cb_ptr; /* Tx ring initial CB ptr */ unsigned int end_ptr; /* Tx ring end CB ptr */ + bool int_enabled; void (*int_enable)(struct bcmgenet_priv *priv, struct bcmgenet_tx_ring *); void (*int_disable)(struct bcmgenet_priv *priv, -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists