Skip to contentSkip to conversation

veth: don't assign a qdisc to veth

PATCH In review Jesper Dangaard Brouer 4 participants 6 messages Oct 3, 2014

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Jesper Dangaard Brouer

author
#0 lore raw

PATCH 1/1 veth: don't assign a qdisc to veth

The veth driver is a virtual device, and should not have assigned the default qdisc. Verified (ndo_start_xmit) veth_xmit can only return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Reported-by Mrunal Patel <[email protected]>
Signed-off-by Jesper Dangaard Brouer <[email protected]>

---

 drivers/net/veth.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/net/veth.c +1 -0
index 8ad5965..3c32fcf 100644
@@ -287,6 +287,7 @@ static const struct net_device_ops veth_netdev_ops = {
 static void veth_setup(struct net_device *dev)
 {
 	ether_setup(dev);
+	dev->tx_queue_len = 0;
 
 	dev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
#1 lore raw
Quoted 6 lines · Jesper Dangaard Brouer

The veth driver is a virtual device, and should not have assigned the default qdisc. Verified (ndo_start_xmit) veth_xmit can only return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Huh?? Maybe your $subject is too misleading, but we do use HTB on veth, this will break our code since we will have to set tx_queue_len after your patch, no?

author
#2 lore raw
Quoted 6 lines · On Fri, 3 Oct 2014 09:53:16 -0700 Cong Wang <[email protected]> wrote:
Quoted 4 lines · Jesper Dangaard Brouer

The veth driver is a virtual device, and should not have assigned the default qdisc. Verified (ndo_start_xmit) veth_xmit can only return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Quoted 6 lines
Quoted 2 lines

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Huh?? Maybe your $subject is too misleading, but we do use HTB on veth, this will break our code since we will have to set tx_queue_len after your patch, no?

No, you HTB setup should still work.

Signature · 5 lines
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer
#3 lore raw
Quoted 9 lines · Jesper Dangaard Brouer
Quoted 6 lines · On Fri, 3 Oct 2014 09:53:16 -0700 Cong Wang <[email protected]> wrote:
Quoted 4 lines · Jesper Dangaard Brouer

The veth driver is a virtual device, and should not have assigned the default qdisc. Verified (ndo_start_xmit) veth_xmit can only return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Huh?? Maybe your $subject is too misleading, but we do use HTB on veth, this will break our code since we will have to set tx_queue_len after your patch, no?

No, you HTB setup should still work.

I wish you are right, but this really worries me...

http://marc.info/?l=linux-netdev&m=137516888117465&w=2

#4 lore raw
In reply to Jesper Dangaard Brouer
Quoted 9 lines · Jesper Dangaard Brouer
Quoted 6 lines · On Fri, 3 Oct 2014 09:53:16 -0700 Cong Wang <[email protected]> wrote:
Quoted 4 lines · Jesper Dangaard Brouer

The veth driver is a virtual device, and should not have assigned the default qdisc. Verified (ndo_start_xmit) veth_xmit can only return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Huh?? Maybe your $subject is too misleading, but we do use HTB on veth, this will break our code since we will have to set tx_queue_len after your patch, no?

No, you HTB setup should still work.

Unfortunately no....

Default htb classes are pfifo, and this uses device txqueuelen as default limit.

So your change should have been done years ago.

Now its too late as it can break existing user scripts.

maintainer
#5 lore raw

From: Eric Dumazet <[email protected]>
Date: Fri, 03 Oct 2014 13:51:59 -0700

Quoted 19 lines · Jesper Dangaard Brouer
Quoted 9 lines
Quoted 6 lines · On Fri, 3 Oct 2014 09:53:16 -0700 Cong Wang <[email protected]> wrote:
Quoted 4 lines · Jesper Dangaard Brouer

The veth driver is a virtual device, and should not have assigned the default qdisc. Verified (ndo_start_xmit) veth_xmit can only return NETDEV_TX_OK, thus this should be safe to bypass qdisc.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

Not assigning a qdisc is subtly done by setting tx_queue_len to zero.

Huh?? Maybe your $subject is too misleading, but we do use HTB on veth, this will break our code since we will have to set tx_queue_len after your patch, no?

No, you HTB setup should still work.

Quoted 8 lines

Unfortunately no....

Default htb classes are pfifo, and this uses device txqueuelen as default limit.

So your change should have been done years ago.

Now its too late as it can break existing user scripts.

Agreed, this change cannot be made.