Re: [ovs-dev] [v12 06/11] dpif-netdev: Add packet count and core id paramters for study

[Eelco Chaudron]

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 <echau...@redhat.com>
Sent: Thursday, July 15, 2021 11:08 AM
To: Van Haaren, Harry <harry.van.haa...@intel.com>
Cc: Amber, Kumar <kumar.am...@intel.com>; ovs-dev@openvswitch.org;

f...@sysclose.org; i.maxim...@ovn.org; Ferriter, Cian <cian.ferri...@intel.com>;

Stokes, Ian <ian.sto...@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.

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev