| Message ID | 1520645745-3785-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] dpif-netlink: improve queue id error message. | expand |
On Fri, Mar 09, 2018 at 05:35:45PM -0800, William Tu wrote: > When users set queue id larger than the 61440 limit, > ex: set_queue: 65500, print the value in wanring message. > > VMWare-BZ: #2071111 > Signed-off-by: William Tu <u9012063@gmail.com> > --- > lib/dpif-netlink.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index acf1a6debcf7..c171795710df 100644 > --- a/lib/dpif-netlink.c > +++ b/lib/dpif-netlink.c > @@ -2511,6 +2511,7 @@ dpif_netlink_queue_to_priority(const struct dpif *dpif OVS_UNUSED, > *priority = TC_H_MAKE(1 << 16, queue_id + 1); > return 0; > } else { > + VLOG_WARN("queue id %u is over limit (61440)", queue_id); > return EINVAL; > } > } Thank you for the improvement. It seems risky to log this without a rate limit. Would you mind adding a rate limit? With this change, the limit appears in two places, in two different forms. Would you mind adding an enum or #define for it and using that instead? Thanks, Ben.
On Wed, Apr 4, 2018 at 5:44 PM, Ben Pfaff <blp@ovn.org> wrote: > On Fri, Mar 09, 2018 at 05:35:45PM -0800, William Tu wrote: >> When users set queue id larger than the 61440 limit, >> ex: set_queue: 65500, print the value in wanring message. >> >> VMWare-BZ: #2071111 >> Signed-off-by: William Tu <u9012063@gmail.com> >> --- >> lib/dpif-netlink.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c >> index acf1a6debcf7..c171795710df 100644 >> --- a/lib/dpif-netlink.c >> +++ b/lib/dpif-netlink.c >> @@ -2511,6 +2511,7 @@ dpif_netlink_queue_to_priority(const struct dpif *dpif OVS_UNUSED, >> *priority = TC_H_MAKE(1 << 16, queue_id + 1); >> return 0; >> } else { >> + VLOG_WARN("queue id %u is over limit (61440)", queue_id); >> return EINVAL; >> } >> } > > Thank you for the improvement. > > It seems risky to log this without a rate limit. Would you mind adding > a rate limit? > > With this change, the limit appears in two places, in two different > forms. Would you mind adding an enum or #define for it and using that > instead? > > Thanks, > > Ben. thanks, I will send out v2.
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index acf1a6debcf7..c171795710df 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -2511,6 +2511,7 @@ dpif_netlink_queue_to_priority(const struct dpif *dpif OVS_UNUSED, *priority = TC_H_MAKE(1 << 16, queue_id + 1); return 0; } else { + VLOG_WARN("queue id %u is over limit (61440)", queue_id); return EINVAL; } }
When users set queue id larger than the 61440 limit, ex: set_queue: 65500, print the value in wanring message. VMWare-BZ: #2071111 Signed-off-by: William Tu <u9012063@gmail.com> --- lib/dpif-netlink.c | 1 + 1 file changed, 1 insertion(+)