| Message ID | 20171013060107.23592-1-ligs@dtdream.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] netdev-linux: do not remove ingress qdisc when disable policing. | expand |
On Fri, Oct 13, 2017 at 02:01:07PM +0800, Guoshuai Li wrote: > rate limiting may be implemented in other ways (such as nova), > this time need to disable rate limiting. > I think it should not remove tc qdisk when ingress_policing_burst > is disabled. > > Signed-off-by: Guoshuai Li <ligs@dtdream.com> > Co-authored-by: huweihua <huwh@dtdream.com> After this patch is applied, it looks to me like removing policing becomes a complete no-op. That cannot be right; it means that policing, once enabled, cannot be disabled. Can you help me understand better? Thanks, Ben.
I'm sorry for my mistake. I did not think so much I actually want to express the meaning is tc qdisc should not be operated when never enabled policing. Because my question is this: step1. Configure the tc command at the ovs-port # tc qdisc add dev ovs-port ingress # tc qdisc show dev ovs-port qdisc noqueue 0: root refcnt 2 qdisc ingress ffff: parent ffff:fff1 ---------------- step2. Then refresh the ovs-port state, qdisc ingress is removed. # ip link set ovs-port down # ip link set ovs-port up # tc qdisc show dev ovs-port qdisc noqueue 0: root refcnt 2 I want to fix this problem, but accidentally broke the original function. I'm very sorry. I sent a new version to fix it: https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/340971.html > After this patch is applied, it looks to me like removing policing > becomes a complete no-op. That cannot be right; it means that policing, > once enabled, cannot be disabled. > > Can you help me understand better? > > Thanks, > > Ben.
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c index 2ff3e2bcc..eafe38257 100644 --- a/lib/netdev-linux.c +++ b/lib/netdev-linux.c @@ -2127,16 +2127,16 @@ netdev_linux_set_policing(struct netdev *netdev_, goto out; } - COVERAGE_INC(netdev_set_policing); - /* Remove any existing ingress qdisc. */ - error = tc_add_del_ingress_qdisc(ifindex, false); - if (error) { - VLOG_WARN_RL(&rl, "%s: removing policing failed: %s", - netdev_name, ovs_strerror(error)); - goto out; - } - if (kbits_rate) { + COVERAGE_INC(netdev_set_policing); + /* Remove any existing ingress qdisc. */ + error = tc_add_del_ingress_qdisc(ifindex, false); + if (error) { + VLOG_WARN_RL(&rl, "%s: removing policing failed: %s", + netdev_name, ovs_strerror(error)); + goto out; + } + error = tc_add_del_ingress_qdisc(ifindex, true); if (error) { VLOG_WARN_RL(&rl, "%s: adding policing qdisc failed: %s",
rate limiting may be implemented in other ways (such as nova), this time need to disable rate limiting. I think it should not remove tc qdisk when ingress_policing_burst is disabled. Signed-off-by: Guoshuai Li <ligs@dtdream.com> Co-authored-by: huweihua <huwh@dtdream.com> --- lib/netdev-linux.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)