On 8 Jul 2021, at 12:34, Amber, Kumar wrote:

> Hi Eelco,
>
> Pls find replies inline.
>
>> -----Original Message-----
>> From: Eelco Chaudron <[email protected]>
>> Sent: Thursday, July 8, 2021 2:14 PM
>> To: Amber, Kumar <[email protected]>
>> Cc: Ferriter, Cian <[email protected]>; [email protected];
>> [email protected]; [email protected]; Van Haaren, Harry
>> <[email protected]>; Stokes, Ian <[email protected]>
>> Subject: Re: [v6 06/11] dpif-netdev: Add packet count and core id paramters 
>> for
>> study
>>
>>
>>
>> On 8 Jul 2021, at 9:41, Amber, Kumar wrote:
>>
>>> Hi Eelco,
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Eelco Chaudron <[email protected]>
>>>> Sent: Thursday, July 8, 2021 1:00 PM
>>>> To: Amber, Kumar <[email protected]>
>>>> Cc: Ferriter, Cian <[email protected]>;
>>>> [email protected]; [email protected]; [email protected]; Van
>>>> Haaren, Harry <[email protected]>; Stokes, Ian
>>>> <[email protected]>
>>>> Subject: Re: [v6 06/11] dpif-netdev: Add packet count and core id
>>>> paramters for study
>>>>
>>>>
>>>>
>>>> On 7 Jul 2021, at 17:16, Amber, Kumar wrote:
>>>>
>>>>> Hi Eelco,
>>>>>
>>>>> Don’t know the formatting keeps breaking . replies are inline.
>>>>>
>>>>
>>>> <Snip>
>>>>> + /* argv[2] is optional packet count, which user can provide along
>>>>> + with
>>>>> + * study function to set the minimum packet that must be matched in
>>>>> + order
>>>>> + * to choose the optimal function. */ uint32_t pkt_cmp_count = 0;
>>>>> + uint32_t study_ret = 0;
>>>>> +
>>>>> + if ((argc == 3) || (argc == 4)) {
>>>>> + if (str_to_uint(argv[2], 10, &pkt_cmp_count)) { study_ret =
>>>>> + mfex_set_study_pkt_cnt(pkt_cmp_count, mfex_name); } else {
>>>>> + study_ret = -EINVAL;
>>>>>
>>>>> An invalid input was given so we should error out.
>>>>>
>>>>> The error is handled later and since we already have a fallback to
>>>>> default value
>>>> we just fall-back.
>>>>
>>>> Don't think we should fallback to default, because if someone types:
>>>>
>>>>   $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator
>>>> THIS_PACKET_COUNT_PLEASE
>>>>   Miniflow implementation set to autovalidator
>>>>
>>>> We accept it and use the default value, it’s better to error out…
>>>>
>>>
>>> For any other function other than study we simply set the request and ignore
>> the study_cnt in that case :
>>>
>>> root@npg-ngf-wlpr-srv06:~/amber# $OVS_DIR/utilities/ovs-appctl
>>> dpif-netdev/miniflow-parser-set scalar 2056 2021-07-
>> 08T07:26:36Z|00101|dpif_netdev|INFO|Miniflow implementation set to scalar.
>>> Miniflow implementation set to scalar.
>>>
>>> For Study only if someone doesn’t prove a valid number we set the study-cnt
>> to default value .
>>>
>>> root@npg-ngf-wlpr-srv06:~/amber# $OVS_DIR/utilities/ovs-appctl
>>> dpif-netdev/miniflow-parser-set -pmd 3  study abc 2021-07-
>> 08T07:29:04Z|00103|dpif_netdev|INFO|Miniflow implementation set to study,
>> on pmd thread 3.
>>> 2021-07-08T07:29:04Z|00007|dpif_mfex_extract_study(pmd-c03/id:7)|INFO|
>>> MFEX study chose impl avx512_dot1q_ipv4_udp: (hits 128/128 pkts) Miniflow
>> implementation set to study, on pmd thread 3.
>>
>> I think we should inform the user of any mistake he makes, and not assume a
>> default value. So in case, the count is supplied for a function not 
>> supporting it
>> we should error.
>>
>> For example:
>>
>>   $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator 13
>>   The study_pkt_cnt option is not valid for the autovalidator implementation.
>>   ovs-appctl: ovs-vswitchd: server returned an error
>>
>> And for an invalid value to the study option:
>>
>>   $ ovs-appctl dpif-netdev/miniflow-parser-set study WEWERWER
>>   Invalid value, “WEWERWER”, supplied for the study_pkt_cnt option.
>>   ovs-appctl: ovs-vswitchd: server returned an error
>>
>> Or something along these lines.
>>
>
> Sure :
>
> root@npg-ngf-wlpr-srv06:~/amber/ovs# $OVS_DIR/utilities/ovs-appctl 
> dpif-netdev/miniflow-parser-set -pmd 0 scalar abcd
> 2021-07-08T09:51:01Z|00100|dpif_netdev|INFO|Invalid value: abcd.
> Invalid value: abcd.

As you know the parameter type, I would include it in the error message: 
“Invalid study_pkt_cnt value: abcd.”

> root@npg-ngf-wlpr-srv06:~/amber/ovs# $OVS_DIR/utilities/ovs-appctl 
> dpif-netdev/miniflow-parser-set -pmd 3 scalar 2048
> 2021-07-08T10:22:32Z|00110|dpif_netdev|INFO|The study_pkt_cnt option is not 
> validfor the scalar implementation.
> The study_pkt_cnt option is not validfor the scalar implementation.

Missing a space “valid for”


Thanks for fixing.

//Eelco

> ovs-appctl: ovs-vswitchd: server returned an error
>>
>>>>
>>>> <snip>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to