Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Mon, 2015-08-17 at 08:51 +0200, Jesper Dangaard Brouer wrote: On Fri, 14 Aug 2015 10:41:53 +0200 Phil Sutter p...@nwl.cc wrote: On Thu, Aug 13, 2015 at 12:11:57PM -0700, Stephen Hemminger wrote: [...] But adding a flag risks breaking external scripts. Could you please elaborate on this? As far as I can tell, introducing a separate flag is the only solution *not* breaking existing scripts. So if you see the rub, I would like to know where exactly it is. I agree with Phil. AFAIC see this approach does not break existing scripts. Acked-by: Jesper Dangaard Brouer bro...@redhat.com But, what is the long term plan ? If long term plan is to change veth txqueuelen to 0, we said no. (because it is too late and this change will break some setups) If not, this flag wont help the case you want to optimize anyway. (ie : veth with no qdisc) So really, _new_ user scripts should either: make sure a qdisc is not there, or is there. Relying on a new flag wont help. There is no point adding this flag as such if we do not take proper decisions. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Mon, 17 Aug 2015 06:44:51 -0700 Eric Dumazet eric.duma...@gmail.com wrote: On Mon, 2015-08-17 at 08:51 +0200, Jesper Dangaard Brouer wrote: On Fri, 14 Aug 2015 10:41:53 +0200 Phil Sutter p...@nwl.cc wrote: On Thu, Aug 13, 2015 at 12:11:57PM -0700, Stephen Hemminger wrote: [...] But adding a flag risks breaking external scripts. Could you please elaborate on this? As far as I can tell, introducing a separate flag is the only solution *not* breaking existing scripts. So if you see the rub, I would like to know where exactly it is. I agree with Phil. AFAIC see this approach does not break existing scripts. Acked-by: Jesper Dangaard Brouer bro...@redhat.com But, what is the long term plan ? If long term plan is to change veth txqueuelen to 0, we said no. (because it is too late and this change will break some setups) No, veth keep txqueuelen at the default 1000. We simply add the flag IFF_NO_QUEUE to veth. That result in (1) veth gets no-qdisc when using the default qdisc, and (2) if anyone assigns another qdisc it still works. This is backward compatible with existing scripts. Long term plan is to change e.g. vlan to be marked with IFF_NO_QUEUE and don't set txqueuelen to zero (automatic falls back to default setting). This avoids any gotchas for users assigning a qdisc. This is also backward compatible, with users who know to change the txqueuelen before assigning a qdisc to a vlan. -- 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 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
From: Phil Sutter p...@nwl.cc Date: Thu, 13 Aug 2015 19:01:05 +0200 This series adds a new private net_device flag indicating that a device may (and probably should) be used without a queueing discipline attached to it. This is already common practice for many virtual device types like e.g. loopback, VLAN (802.1Q) or bridges (802.1D). The reason for this is that these devices lack an underlying layer which could impose back pressure and therefore making a TX queue necessary to not slow down senders. Up to now, drivers being aware of the above applying to them set dev-tx_queue_len to zero to indicate no qdisc should be attached to the interface they drive and the kernel reacts upon this by assigning the noop qdisc instead of the default pfifo_fast. This implicit agreement though leads to an inconvenient situation once a user tries to attach a real qdisc to these devices, as the formerly special tx_queue_len value becomes a regular one, limiting the queue to zero packets and thus prevents any TX from happening. To overcome this, practically all qdisc implementations intercept and sanitize the malicious value. With this series applied, drivers may signal the lack of need for a qdisc without having to tamper with tx_queue_len, making fallbacks in qdiscs and caveats in userspace unnecessary. Upon upstream acceptance, this series will be followed up by a set of patches converting device drivers, adding a warning so out-of-tree driver authors get aware of this change and dropping all special handling of tx_queue_len in net/sched/. Series applied, thanks. Since the VRF changes went in right before this, I had to bump the IFF_NO_QUEUE value to use bit 26 instead of bit 25. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Fri, 14 Aug 2015 10:41:53 +0200 Phil Sutter p...@nwl.cc wrote: On Thu, Aug 13, 2015 at 12:11:57PM -0700, Stephen Hemminger wrote: [...] But adding a flag risks breaking external scripts. Could you please elaborate on this? As far as I can tell, introducing a separate flag is the only solution *not* breaking existing scripts. So if you see the rub, I would like to know where exactly it is. I agree with Phil. AFAIC see this approach does not break existing scripts. Acked-by: Jesper Dangaard Brouer bro...@redhat.com -- 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 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Thu, Aug 13, 2015 at 12:11:57PM -0700, Stephen Hemminger wrote: On Thu, 13 Aug 2015 20:40:37 +0200 Jesper Dangaard Brouer bro...@redhat.com wrote: On Thu, 13 Aug 2015 10:49:50 -0700 Stephen Hemminger step...@networkplumber.org wrote: On Thu, 13 Aug 2015 19:01:05 +0200 Phil Sutter p...@nwl.cc wrote: Up to now, drivers being aware of the above applying to them set dev-tx_queue_len to zero to indicate no qdisc should be attached to the interface they drive and the kernel reacts upon this by assigning the noop qdisc instead of the default pfifo_fast. This implicit agreement though leads to an inconvenient situation once a user tries to attach a real qdisc to these devices, as the formerly special tx_queue_len value becomes a regular one, So this is a workaround for user ignorance by introducing kernel API complexity. Before user sets qdisc, why don't they set tx queue length? Please don't insist on keeping this broke interface... how should users know that BEFORE adding a qdisc they MUST change the _device_ tx queue length (not zero). Before setting any qdisc, they should set queue length anyway. Probably, yes. But if they don't, it depends on the interface driver whether they're screwed or not. In my opinion, this inconsistency alone is worth fixing. Getting back to the original state, they MUST change the device tx queue len back to zero BEFORE deleting the qdisc, such that when assigning the default queue qdisc the system detects this device can work without a qdisc. Changing the tx queue len to zero after the qdisc is deleted will have not effect. Listen to the description, that interface is broken. The kernel really needs to hide these details from userspace. It even allows you to misconfigure the kernel, by tricking the kernel into assigning noqueue to physical devices that really need it. But adding a flag risks breaking external scripts. Could you please elaborate on this? As far as I can tell, introducing a separate flag is the only solution *not* breaking existing scripts. So if you see the rub, I would like to know where exactly it is. Cheers, Phil -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
This series adds a new private net_device flag indicating that a device may (and probably should) be used without a queueing discipline attached to it. This is already common practice for many virtual device types like e.g. loopback, VLAN (802.1Q) or bridges (802.1D). The reason for this is that these devices lack an underlying layer which could impose back pressure and therefore making a TX queue necessary to not slow down senders. Up to now, drivers being aware of the above applying to them set dev-tx_queue_len to zero to indicate no qdisc should be attached to the interface they drive and the kernel reacts upon this by assigning the noop qdisc instead of the default pfifo_fast. This implicit agreement though leads to an inconvenient situation once a user tries to attach a real qdisc to these devices, as the formerly special tx_queue_len value becomes a regular one, limiting the queue to zero packets and thus prevents any TX from happening. To overcome this, practically all qdisc implementations intercept and sanitize the malicious value. With this series applied, drivers may signal the lack of need for a qdisc without having to tamper with tx_queue_len, making fallbacks in qdiscs and caveats in userspace unnecessary. Upon upstream acceptance, this series will be followed up by a set of patches converting device drivers, adding a warning so out-of-tree driver authors get aware of this change and dropping all special handling of tx_queue_len in net/sched/. Phil Sutter (2): net: declare new net_device priv_flag IFF_NO_QUEUE net: sch_generic: react upon IFF_NO_QUEUE flag include/linux/netdevice.h | 3 +++ net/sched/sch_generic.c | 6 -- 2 files changed, 7 insertions(+), 2 deletions(-) -- 2.1.2 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Thu, 13 Aug 2015 19:01:05 +0200 Phil Sutter p...@nwl.cc wrote: Up to now, drivers being aware of the above applying to them set dev-tx_queue_len to zero to indicate no qdisc should be attached to the interface they drive and the kernel reacts upon this by assigning the noop qdisc instead of the default pfifo_fast. This implicit agreement though leads to an inconvenient situation once a user tries to attach a real qdisc to these devices, as the formerly special tx_queue_len value becomes a regular one, So this is a workaround for user ignorance by introducing kernel API complexity. Before user sets qdisc, why don't they set tx queue length? -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Thu, 13 Aug 2015 10:49:50 -0700 Stephen Hemminger step...@networkplumber.org wrote: On Thu, 13 Aug 2015 19:01:05 +0200 Phil Sutter p...@nwl.cc wrote: Up to now, drivers being aware of the above applying to them set dev-tx_queue_len to zero to indicate no qdisc should be attached to the interface they drive and the kernel reacts upon this by assigning the noop qdisc instead of the default pfifo_fast. This implicit agreement though leads to an inconvenient situation once a user tries to attach a real qdisc to these devices, as the formerly special tx_queue_len value becomes a regular one, So this is a workaround for user ignorance by introducing kernel API complexity. Before user sets qdisc, why don't they set tx queue length? Please don't insist on keeping this broke interface... how should users know that BEFORE adding a qdisc they MUST change the _device_ tx queue length (not zero). Getting back to the original state, they MUST change the device tx queue len back to zero BEFORE deleting the qdisc, such that when assigning the default queue qdisc the system detects this device can work without a qdisc. Changing the tx queue len to zero after the qdisc is deleted will have not effect. Listen to the description, that interface is broken. The kernel really needs to hide these details from userspace. It even allows you to misconfigure the kernel, by tricking the kernel into assigning noqueue to physical devices that really need it. -- 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 -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] net: introduce IFF_NO_QUEUE as successor of zero tx_queue_len
On Thu, 13 Aug 2015 20:40:37 +0200 Jesper Dangaard Brouer bro...@redhat.com wrote: On Thu, 13 Aug 2015 10:49:50 -0700 Stephen Hemminger step...@networkplumber.org wrote: On Thu, 13 Aug 2015 19:01:05 +0200 Phil Sutter p...@nwl.cc wrote: Up to now, drivers being aware of the above applying to them set dev-tx_queue_len to zero to indicate no qdisc should be attached to the interface they drive and the kernel reacts upon this by assigning the noop qdisc instead of the default pfifo_fast. This implicit agreement though leads to an inconvenient situation once a user tries to attach a real qdisc to these devices, as the formerly special tx_queue_len value becomes a regular one, So this is a workaround for user ignorance by introducing kernel API complexity. Before user sets qdisc, why don't they set tx queue length? Please don't insist on keeping this broke interface... how should users know that BEFORE adding a qdisc they MUST change the _device_ tx queue length (not zero). Before setting any qdisc, they should set queue length anyway. Getting back to the original state, they MUST change the device tx queue len back to zero BEFORE deleting the qdisc, such that when assigning the default queue qdisc the system detects this device can work without a qdisc. Changing the tx queue len to zero after the qdisc is deleted will have not effect. Listen to the description, that interface is broken. The kernel really needs to hide these details from userspace. It even allows you to misconfigure the kernel, by tricking the kernel into assigning noqueue to physical devices that really need it. But adding a flag risks breaking external scripts. -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
