* [net-next PATCH] veth: don't assign a qdisc to veth
@ 2014-10-03 10:48 Jesper Dangaard Brouer
2014-10-03 16:53 ` Cong Wang
0 siblings, 1 reply; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2014-10-03 10:48 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev, David S. Miller; +Cc: Jiri Pirko, mpatel
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 <mpatel@redhat.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
drivers/net/veth.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8ad5965..3c32fcf 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -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;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [net-next PATCH] veth: don't assign a qdisc to veth
2014-10-03 10:48 [net-next PATCH] veth: don't assign a qdisc to veth Jesper Dangaard Brouer
@ 2014-10-03 16:53 ` Cong Wang
2014-10-03 20:38 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 6+ messages in thread
From: Cong Wang @ 2014-10-03 16:53 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: netdev, David S. Miller, Jiri Pirko, mpatel
On Fri, Oct 3, 2014 at 3:48 AM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> 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?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] veth: don't assign a qdisc to veth
2014-10-03 16:53 ` Cong Wang
@ 2014-10-03 20:38 ` Jesper Dangaard Brouer
2014-10-03 20:48 ` Cong Wang
2014-10-03 20:51 ` Eric Dumazet
0 siblings, 2 replies; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2014-10-03 20:38 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, David S. Miller, Jiri Pirko, mpatel, brouer
On Fri, 3 Oct 2014 09:53:16 -0700
Cong Wang <cwang@twopensource.com> wrote:
> On Fri, Oct 3, 2014 at 3:48 AM, Jesper Dangaard Brouer
> <brouer@redhat.com> wrote:
> > 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?
No, you HTB setup should still work.
--
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] veth: don't assign a qdisc to veth
2014-10-03 20:38 ` Jesper Dangaard Brouer
@ 2014-10-03 20:48 ` Cong Wang
2014-10-03 20:51 ` Eric Dumazet
1 sibling, 0 replies; 6+ messages in thread
From: Cong Wang @ 2014-10-03 20:48 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: netdev, David S. Miller, Jiri Pirko, mpatel
On Fri, Oct 3, 2014 at 1:38 PM, Jesper Dangaard Brouer
<brouer@redhat.com> wrote:
> On Fri, 3 Oct 2014 09:53:16 -0700
> Cong Wang <cwang@twopensource.com> wrote:
>
>> On Fri, Oct 3, 2014 at 3:48 AM, Jesper Dangaard Brouer
>> <brouer@redhat.com> wrote:
>> > 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?
>
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] veth: don't assign a qdisc to veth
2014-10-03 20:38 ` Jesper Dangaard Brouer
2014-10-03 20:48 ` Cong Wang
@ 2014-10-03 20:51 ` Eric Dumazet
2014-10-03 21:56 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2014-10-03 20:51 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Cong Wang, netdev, David S. Miller, Jiri Pirko, mpatel
On Fri, 2014-10-03 at 22:38 +0200, Jesper Dangaard Brouer wrote:
> On Fri, 3 Oct 2014 09:53:16 -0700
> Cong Wang <cwang@twopensource.com> wrote:
>
> > On Fri, Oct 3, 2014 at 3:48 AM, Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:
> > > 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?
>
> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [net-next PATCH] veth: don't assign a qdisc to veth
2014-10-03 20:51 ` Eric Dumazet
@ 2014-10-03 21:56 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2014-10-03 21:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: brouer, cwang, netdev, jpirko, mpatel
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 03 Oct 2014 13:51:59 -0700
> On Fri, 2014-10-03 at 22:38 +0200, Jesper Dangaard Brouer wrote:
>> On Fri, 3 Oct 2014 09:53:16 -0700
>> Cong Wang <cwang@twopensource.com> wrote:
>>
>> > On Fri, Oct 3, 2014 at 3:48 AM, Jesper Dangaard Brouer
>> > <brouer@redhat.com> wrote:
>> > > 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?
>>
>> 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.
Agreed, this change cannot be made.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-03 21:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-03 10:48 [net-next PATCH] veth: don't assign a qdisc to veth Jesper Dangaard Brouer
2014-10-03 16:53 ` Cong Wang
2014-10-03 20:38 ` Jesper Dangaard Brouer
2014-10-03 20:48 ` Cong Wang
2014-10-03 20:51 ` Eric Dumazet
2014-10-03 21:56 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).