| Message ID | 1412024209-25468-1-git-send-email-fw@strlen.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2014-09-29 at 22:56 +0200, Florian Westphal wrote: > tested on RTL8168d/8111d. > > Cc: Francois Romieu <romieu@fr.zoreil.com> > Cc: Hayes Wang <hayeswang@realtek.com> > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) Thanks Florian, this looks good to me. Acked-by: Eric Dumazet <edumazet@google.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2014-09-29 at 22:56 +0200, Florian Westphal wrote: > tested on RTL8168d/8111d. One small nit : > @@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev) > static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) > { > unsigned int dirty_tx, tx_left; > + unsigned bytes_compl = 0, pkts_compl = 0; unsigned int bytes_compl = 0, pkts_compl = 0; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Florian, Thanks for doing this! Can you add a little description in commit log on test results, something that would indicate accounting is correct. Thanks, Tom On Mon, Sep 29, 2014 at 1:56 PM, Florian Westphal <fw@strlen.de> wrote: > tested on RTL8168d/8111d. > > Cc: Francois Romieu <romieu@fr.zoreil.com> > Cc: Hayes Wang <hayeswang@realtek.com> > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c > index 1d81238..73c6ad1 100644 > --- a/drivers/net/ethernet/realtek/r8169.c > +++ b/drivers/net/ethernet/realtek/r8169.c > @@ -4712,6 +4712,8 @@ static void rtl_hw_reset(struct rtl8169_private *tp) > RTL_W8(ChipCmd, CmdReset); > > rtl_udelay_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100); > + > + netdev_reset_queue(tp->dev); > } > > static void rtl_request_uncached_firmware(struct rtl8169_private *tp) > @@ -6592,6 +6594,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb, > > txd->opts2 = cpu_to_le32(opts[1]); > > + netdev_sent_queue(dev, skb->len); > + > skb_tx_timestamp(skb); > > wmb(); > @@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev) > static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) > { > unsigned int dirty_tx, tx_left; > + unsigned bytes_compl = 0, pkts_compl = 0; > > dirty_tx = tp->dirty_tx; > smp_rmb(); > @@ -6709,10 +6714,8 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) > rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb, > tp->TxDescArray + entry); > if (status & LastFrag) { > - u64_stats_update_begin(&tp->tx_stats.syncp); > - tp->tx_stats.packets++; > - tp->tx_stats.bytes += tx_skb->skb->len; > - u64_stats_update_end(&tp->tx_stats.syncp); > + pkts_compl++; > + bytes_compl += tx_skb->skb->len; > dev_kfree_skb_any(tx_skb->skb); > tx_skb->skb = NULL; > } > @@ -6721,6 +6724,13 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) > } > > if (tp->dirty_tx != dirty_tx) { > + netdev_completed_queue(tp->dev, pkts_compl, bytes_compl); > + > + u64_stats_update_begin(&tp->tx_stats.syncp); > + tp->tx_stats.packets += pkts_compl; > + tp->tx_stats.bytes += bytes_compl; > + u64_stats_update_end(&tp->tx_stats.syncp); > + > tp->dirty_tx = dirty_tx; > /* Sync with rtl8169_start_xmit: > * - publish dirty_tx ring index (write barrier) > -- > 1.8.1.5 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tom Herbert <therbert@google.com> wrote: > Thanks for doing this! Can you add a little description in commit log > on test results, something that would indicate accounting is correct. Do you have any specific test in mind? I ran 'super_netperf 40' for 20 minutes without stalls/hangups and bql monitor seemed to look ok. Anything else I should use/test with? Thanks, Florian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Sep 29, 2014 at 2:11 PM, Florian Westphal <fw@strlen.de> wrote: > Tom Herbert <therbert@google.com> wrote: >> Thanks for doing this! Can you add a little description in commit log >> on test results, something that would indicate accounting is correct. > > Do you have any specific test in mind? > > I ran 'super_netperf 40' for 20 minutes without stalls/hangups and > bql monitor seemed to look ok. > > Anything else I should use/test with? > Watch inflight and limit in the byte_queue_limits for the queue. inflight must always go back to zero when link goes idle. > Thanks, > Florian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tom Herbert <therbert@google.com> wrote: > Watch inflight and limit in the byte_queue_limits for the queue. > inflight must always go back to zero when link goes idle. Yes, inflight goes to 0 when link is idle. Output of while true; do for n in inflight limit; do echo -n $n\ ; cat $n; done; sleep 1; done during netperf run, 100mbit peer: inflight 0 limit 3028 inflight 6056 limit 4542 [ no changes during test ] limit 4542 inflight 3028 limit 6122 inflight 0 limit 6122 [ changed cable to 1gbit peer, restart netperf ] inflight 37850 limit 36336 inflight 33308 limit 31794 inflight 33308 limit 31794 inflight 27252 limit 25738 [ no changes during test ] inflight 27252 limit 25738 inflight 0 limit 28766 [ change cable to 100mbit peer, restart netperf ] limit 28766 inflight 27370 limit 28766 inflight 4542 limit 5990 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 6056 limit 4542 inflight 0 [ end of test ] I think thats what its supposed to look like :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 30, 2014 at 12:42 AM, Florian Westphal <fw@strlen.de> wrote: > Tom Herbert <therbert@google.com> wrote: >> Watch inflight and limit in the byte_queue_limits for the queue. >> inflight must always go back to zero when link goes idle. > > Yes, inflight goes to 0 when link is idle. > > Output of > while true; do > for n in inflight limit; do > echo -n $n\ ; cat $n; > done; sleep 1; > done > > during netperf run, 100mbit peer: > > inflight 0 > limit 3028 > inflight 6056 > limit 4542 > [ no changes during test ] > limit 4542 > inflight 3028 > limit 6122 > inflight 0 > limit 6122 > [ changed cable to 1gbit peer, restart netperf ] > inflight 37850 > limit 36336 > inflight 33308 > limit 31794 > inflight 33308 > limit 31794 > inflight 27252 > limit 25738 > [ no changes during test ] > inflight 27252 > limit 25738 > inflight 0 > limit 28766 > [ change cable to 100mbit peer, restart netperf ] > limit 28766 > inflight 27370 > limit 28766 > inflight 4542 > limit 5990 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 6056 > limit 4542 > inflight 0 > > [ end of test ] > > I think thats what its supposed to look like :-) Looks good! Acked-by: Tom Herbert <therbert@google.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c index 1d81238..73c6ad1 100644 --- a/drivers/net/ethernet/realtek/r8169.c +++ b/drivers/net/ethernet/realtek/r8169.c @@ -4712,6 +4712,8 @@ static void rtl_hw_reset(struct rtl8169_private *tp) RTL_W8(ChipCmd, CmdReset); rtl_udelay_loop_wait_low(tp, &rtl_chipcmd_cond, 100, 100); + + netdev_reset_queue(tp->dev); } static void rtl_request_uncached_firmware(struct rtl8169_private *tp) @@ -6592,6 +6594,8 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb, txd->opts2 = cpu_to_le32(opts[1]); + netdev_sent_queue(dev, skb->len); + skb_tx_timestamp(skb); wmb(); @@ -6691,6 +6695,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev) static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) { unsigned int dirty_tx, tx_left; + unsigned bytes_compl = 0, pkts_compl = 0; dirty_tx = tp->dirty_tx; smp_rmb(); @@ -6709,10 +6714,8 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) rtl8169_unmap_tx_skb(&tp->pci_dev->dev, tx_skb, tp->TxDescArray + entry); if (status & LastFrag) { - u64_stats_update_begin(&tp->tx_stats.syncp); - tp->tx_stats.packets++; - tp->tx_stats.bytes += tx_skb->skb->len; - u64_stats_update_end(&tp->tx_stats.syncp); + pkts_compl++; + bytes_compl += tx_skb->skb->len; dev_kfree_skb_any(tx_skb->skb); tx_skb->skb = NULL; } @@ -6721,6 +6724,13 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp) } if (tp->dirty_tx != dirty_tx) { + netdev_completed_queue(tp->dev, pkts_compl, bytes_compl); + + u64_stats_update_begin(&tp->tx_stats.syncp); + tp->tx_stats.packets += pkts_compl; + tp->tx_stats.bytes += bytes_compl; + u64_stats_update_end(&tp->tx_stats.syncp); + tp->dirty_tx = dirty_tx; /* Sync with rtl8169_start_xmit: * - publish dirty_tx ring index (write barrier)
tested on RTL8168d/8111d. Cc: Francois Romieu <romieu@fr.zoreil.com> Cc: Hayes Wang <hayeswang@realtek.com> Signed-off-by: Florian Westphal <fw@strlen.de> --- drivers/net/ethernet/realtek/r8169.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)