[ovs-dev] dpif-netlink: improve queue id error message.

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.
Related show

Commit Message

William Tu March 10, 2018, 1:35 a.m.
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(+)

Comments

Ben Pfaff April 5, 2018, 12:44 a.m. | #1
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.
William Tu April 5, 2018, 1:27 a.m. | #2
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.

Patch

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;
     }
 }