[RFC] tulip: Support for byte queue limits

The New Hotness of fq_codel wants bql support, but my WAN link is on my
Old And Busted tulip card, which lacks it.

It's just a few lines, but the important thing is knowing where to
put them, and I've sort of guessed.  In particular, it seems like the
netdev_sent_queue call could be almost anywhere in tulip_start_xmit and
I'm not sure if there are reasons to prefer any particular position.

(You may have my S-o-b on copyright grounds, but I'd like to test it
some more before declaring this patch ready to be merged.)


--
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 Fri, 2013-07-12 at 09:20 -0400, George Spelvin wrote:
> The New Hotness of fq_codel wants bql support, but my WAN link is on my
> Old And Busted tulip card, which lacks it.
> 
> It's just a few lines, but the important thing is knowing where to
> put them, and I've sort of guessed.  In particular, it seems like the
> netdev_sent_queue call could be almost anywhere in tulip_start_xmit and
> I'm not sure if there are reasons to prefer any particular position.
> 
> (You may have my S-o-b on copyright grounds, but I'd like to test it
> some more before declaring this patch ready to be merged.)

Seems fine to me.

You might wait before David Miller re-opens net-next for new
submissions, a bit after linux-3.11-rc1 is released, before
sending an official patch.

Thanks




--
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 Fri, Jul 12, 2013 at 6:20 AM, George Spelvin <linux@horizon.com> wrote:
> The New Hotness of fq_codel wants bql support, but my WAN link is on my
> Old And Busted tulip card, which lacks it.
>
> It's just a few lines, but the important thing is knowing where to
> put them, and I've sort of guessed.  In particular, it seems like the
> netdev_sent_queue call could be almost anywhere in tulip_start_xmit and
> I'm not sure if there are reasons to prefer any particular position.

Hi George,
While you are right that functionally it doesn't matter, my preference
would be to have nothing between the wmb() and iowrite() that kicks
off the TX. This marginally helps kick off the TX process consistently
slightly sooner. On modern HW, probably irrelevant, but not on the HW
these chips are used on.

I don't have a clue about fq_codel and trust Eric thinks the change is good.

Lastly, given I haven't powered up a system in two years which has
tulip, any one want to take over maintainer for tulip driver?
It's basically obsolete with a few rare patches like this one coming in.

cheers,
grant

>
> (You may have my S-o-b on copyright grounds, but I'd like to test it
> some more before declaring this patch ready to be merged.)
>
>
> diff --git a/drivers/net/ethernet/dec/tulip/interrupt.c b/drivers/net/ethernet/dec/tulip/interrupt.c
> index 92306b3..d74426e 100644
> --- a/drivers/net/ethernet/dec/tulip/interrupt.c
> +++ b/drivers/net/ethernet/dec/tulip/interrupt.c
> @@ -532,6 +532,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
>  #endif
>         unsigned int work_count = tulip_max_interrupt_work;
>         unsigned int handled = 0;
> +       unsigned int bytes_compl = 0;
>
>         /* Let's see whether the interrupt really is for us */
>         csr5 = ioread32(ioaddr + CSR5);
> @@ -634,6 +635,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
>                                                  PCI_DMA_TODEVICE);
>
>                                 /* Free the original skb. */
> +                               bytes_compl += tp->tx_buffers[entry].skb->len;
>                                 dev_kfree_skb_irq(tp->tx_buffers[entry].skb);
>                                 tp->tx_buffers[entry].skb = NULL;
>                                 tp->tx_buffers[entry].mapping = 0;
> @@ -802,6 +804,7 @@ irqreturn_t tulip_interrupt(int irq, void *dev_instance)
>         }
>  #endif /* CONFIG_TULIP_NAPI */
>
> +       netdev_completed_queue(dev, tx, bytes_compl);
>         if ((missed = ioread32(ioaddr + CSR8) & 0x1ffff)) {
>                 dev->stats.rx_dropped += missed & 0x10000 ? 0x10000 : missed;
>         }
> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index 1e9443d..b4249f3 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -703,6 +703,7 @@ tulip_start_xmit(struct sk_buff *skb, struct net_device *dev)
>         wmb();
>
>         tp->cur_tx++;
> +       netdev_sent_queue(dev, skb->len);
>
>         /* Trigger an immediate transmit demand. */
>         iowrite32(0, tp->base_addr + CSR1);
> @@ -746,6 +747,7 @@ static void tulip_clean_tx_ring(struct tulip_private *tp)
>                 tp->tx_buffers[entry].skb = NULL;
>                 tp->tx_buffers[entry].mapping = 0;
>         }
> +       netdev_reset_queue(tp->dev);
>  }
>
>  static void tulip_down (struct net_device *dev)
--
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