[ovs-dev] [v12 06/11] dpif-netdev: Add packet count and core id paramters for study
Eelco Chaudron
echaudro at redhat.com
Thu Jul 15 10:45:53 UTC 2021
On 15 Jul 2021, at 12:31, Eelco Chaudron wrote:
> On 15 Jul 2021, at 12:10, Van Haaren, Harry wrote:
>
>>> -----Original Message-----
>>> From: Eelco Chaudron <echaudro at redhat.com>
>>> Sent: Thursday, July 15, 2021 11:08 AM
>>> To: Van Haaren, Harry <harry.van.haaren at intel.com>
>>> Cc: Amber, Kumar <kumar.amber at intel.com>; ovs-dev at openvswitch.org;
>>> fbl at sysclose.org; i.maximets at ovn.org; Ferriter, Cian
>>> <cian.ferriter at intel.com>;
>>> Stokes, Ian <ian.stokes at intel.com>
>>> Subject: Re: [v12 06/11] dpif-netdev: Add packet count and core id
>>> paramters for
>>> study
>>
>> <snip>
>>
>>>> Aha, yes OK I see what you're suggesting clearly now.
>>>>
>>>> Personally I liked the "only process extra args if study" trick (by
>>>> "indenting" it a
>>> level),
>>>> but I'll rework to your suggestion here for simplicity/consistency.
>>>
>>> Thanks looking forward to v13, a nice number to sign off on ;)
>>
>> Hah yeah. To keep you interested, below the diff to fixup the new
>> behaviour.
>> Note that study_count must be initialized to the default value, as it
>> is an optional
>> argument and there is no "} else {" clause to handle the default
>> value anymore.
>>
>> v13 on its way, -Harry
>>
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 14ccca7a03..476678e9fa 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -1088,7 +1088,7 @@ dpif_miniflow_extract_impl_set(struct
>> unixctl_conn *conn, int argc,
>> const char *mfex_name = NULL;
>> const char *reply_str = NULL;
>> struct ds reply = DS_EMPTY_INITIALIZER;
>> - unsigned int study_count = 0;
>> + unsigned int study_count = MFEX_MAX_PKT_COUNT;
>> int err;
>> struct shash_node *node;
>>
>> @@ -1122,23 +1122,17 @@ dpif_miniflow_extract_impl_set(struct
>> unixctl_conn *conn, int argc,
>> argc -= 1;
>> argv += 1;
>>
>> - /* If name is study and more args, parse study_count
>> value. */
>> - if (strcmp("study", mfex_name) == 0) {
>> - if (argc >= 2) {
>> - if (!str_to_uint(argv[1], 10, &study_count) ||
>> - (study_count == 0)) {
>> - ds_put_format(&reply,
>> - "Error: Invalid study_pkt_cnt value:
>> %s.\n",
>> - argv[1]);
>> - goto error;
>> - }
>> -
>> - argc -= 1;
>> - argv += 1;
>> - } else {
>> - study_count = MFEX_MAX_PKT_COUNT;
>> - }
Now I remember what I was thinking ;)
You do the compare here:
if (strcmp("study", mfex_name))
study_count = MFEX_MAX_PKT_COUNT;
This way you can use !study_count in the rest of the code.
>> + /* If name is study and more args exist, parse study_count
>> value. */
>> + } else if (mfex_name && strcmp("study", mfex_name) == 0) {
>> + if (!str_to_uint(argv[1], 10, &study_count) ||
>> + (study_count == 0)) {
>> + ds_put_format(&reply,
>> + "Error: Invalid study_pkt_cnt value: %s.\n",
>> argv[1]);
>> + goto error;
>> }
>> +
>> + argc -= 1;
>> + argv += 1;
>> } else {
>> ds_put_format(&reply, "Error: unknown argument %s.\n",
>> argv[1]);
>> goto error;
>
> Note that that might mess up the output at the end on successful
> completion, and setting the value globally.
More information about the dev
mailing list