Re: [ovs-dev] [PATCH v2 09/11] dpctl: Refactor opt_dpif_open().
On Wed, Aug 1, 2018 at 9:53 PM, Darrell Ball wrote:
> Thanks for the patch Yi-hung
>
> This patch does not seem to fit with this series somehow; it seems more
> related
> to flushing conntrack by zone and tuple ?
>
My apologies; after I reached the details of patch 10, I see why you wanted
to add
patch 9 as part of the series.
>
> I had a generic change in my queue. I think it is more straightforward,
> since it
> keeps opt_dpif_open() simple and improves the error handling and also
> simplifies
> dpctl_flush_conntrack() and any potential similar functions.
>
> https://patchwork.ozlabs.org/patch/952580/
>
>
>
>
> On 8/1/18, 5:45 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yi-Hung
> Wei"
> wrote:
>
> Make opt_dpif_open() to support mulitple optional arguments. It will
> be useful for the following patches.
>
> Signed-off-by: Yi-Hung Wei
> ---
> lib/dpctl.c | 85 ++
> ++-
> 1 file changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index 4f1e443f2662..35733774b331 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -191,14 +191,40 @@ parsed_dpif_open(const char *arg_, bool create,
> struct dpif **dpifp)
> *
> * The datapath name is not a mandatory parameter for this command.
> If
> * it is not specified -- so 'argc' < 'max_args' -- we retrieve it
> from
> - * the current setup, assuming only one exists. On success stores the
> - * opened dpif in '*dpifp'. */
> + * the current setup, assuming only one exists.
> + * For commands with multiple optional arguments, we try to use the
> first
> + * argument as the dpif name. If it is failed, then we fallback to
> + * retrieve it form the current setup.
> + * On success stores the opened dpif in '*dpifp', and the next
> arugment
> + * to be parsed in '*indexp'. */
> static int
> opt_dpif_open(int argc, const char *argv[], struct dpctl_params
> *dpctl_p,
> - uint8_t max_args, struct dpif **dpifp)
> + uint8_t max_args, struct dpif **dpifp, bool multi_opt,
> + int *indexp)
> {
> +char *dpname;
> int error = 0;
> -char *dpname = argc >= max_args ? xstrdup(argv[1]) :
> get_one_dp(dpctl_p);
> +
> +if (indexp) {
> +*indexp = 1;
> +}
> +
> +if (multi_opt && argc > 1) {
> +error = parsed_dpif_open(argv[1], false, dpifp);
> +if (!error) {
> +if (indexp) {
> +*indexp = 2;
> +}
> +return 0;
> +} else if (argc == max_args) {
> +dpctl_error(dpctl_p, error, "invalid datapath");
> +return error;
> +}
> +dpname = get_one_dp(dpctl_p);
> +} else {
> +dpname = argc >= max_args ? xstrdup(argv[1]) :
> get_one_dp(dpctl_p);
> +}
> +
> if (!dpname) {
> error = EINVAL;
> dpctl_error(dpctl_p, error, "datapath not found");
> @@ -863,7 +889,7 @@ dpctl_dump_flows(int argc, const char *argv[],
> struct dpctl_params *dpctl_p)
> }
> }
>
> -error = opt_dpif_open(argc, argv, dpctl_p, 2, );
> +error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
> if (error) {
> goto out_free;
> }
> @@ -990,7 +1016,7 @@ dpctl_put_flow(int argc, const char *argv[], enum
> dpif_flow_put_flags flags,
> struct simap port_names;
> int n, error;
>
> -error = opt_dpif_open(argc, argv, dpctl_p, 4, );
> +error = opt_dpif_open(argc, argv, dpctl_p, 4, , false, NULL);
> if (error) {
> return error;
> }
> @@ -1092,7 +1118,7 @@ dpctl_get_flow(int argc, const char *argv[],
> struct dpctl_params *dpctl_p)
> struct ds ds;
> int n, error;
>
> -error = opt_dpif_open(argc, argv, dpctl_p, 3, );
> +error = opt_dpif_open(argc, argv, dpctl_p, 3, , false, NULL);
> if (error) {
> return error;
> }
> @@ -1141,7 +1167,7 @@ dpctl_del_flow(int argc, const char *argv[],
> struct dpctl_params *dpctl_p)
> struct simap port_names;
> int n, error;
>
> -error = opt_dpif_open(argc, argv, dpctl_p, 3, );
> +error = opt_dpif_open(argc, argv, dpctl_p, 3, , false, NULL);
> if (error) {
> return error;
> }
> @@ -1210,7 +1236,7 @@ dpctl_del_flows(int argc, const char *argv[],
> struct dpctl_params *dpctl_p)
> {
> struct dpif *dpif;
>
> -int error = opt_dpif_open(argc, argv, dpctl_p, 2, );
> +int error = opt_dpif_open(argc, argv, dpctl_p, 2, , false,
> NULL);
> if (error) {
> return error;
> }
> @@ -1271,7 +1297,7 @@ dpctl_dump_conntrack(int argc, const
Re: [ovs-dev] [PATCH v2 09/11] dpctl: Refactor opt_dpif_open().
Thanks for the patch Yi-hung
This patch does not seem to fit with this series somehow; it seems more related
to flushing conntrack by zone and tuple ?
I had a generic change in my queue. I think it is more straightforward, since it
keeps opt_dpif_open() simple and improves the error handling and also simplifies
dpctl_flush_conntrack() and any potential similar functions.
https://patchwork.ozlabs.org/patch/952580/
On 8/1/18, 5:45 PM, "ovs-dev-boun...@openvswitch.org on behalf of Yi-Hung Wei"
wrote:
Make opt_dpif_open() to support mulitple optional arguments. It will
be useful for the following patches.
Signed-off-by: Yi-Hung Wei
---
lib/dpctl.c | 85
-
1 file changed, 44 insertions(+), 41 deletions(-)
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 4f1e443f2662..35733774b331 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -191,14 +191,40 @@ parsed_dpif_open(const char *arg_, bool create,
struct dpif **dpifp)
*
* The datapath name is not a mandatory parameter for this command. If
* it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists. On success stores the
- * opened dpif in '*dpifp'. */
+ * the current setup, assuming only one exists.
+ * For commands with multiple optional arguments, we try to use the first
+ * argument as the dpif name. If it is failed, then we fallback to
+ * retrieve it form the current setup.
+ * On success stores the opened dpif in '*dpifp', and the next arugment
+ * to be parsed in '*indexp'. */
static int
opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
- uint8_t max_args, struct dpif **dpifp)
+ uint8_t max_args, struct dpif **dpifp, bool multi_opt,
+ int *indexp)
{
+char *dpname;
int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) :
get_one_dp(dpctl_p);
+
+if (indexp) {
+*indexp = 1;
+}
+
+if (multi_opt && argc > 1) {
+error = parsed_dpif_open(argv[1], false, dpifp);
+if (!error) {
+if (indexp) {
+*indexp = 2;
+}
+return 0;
+} else if (argc == max_args) {
+dpctl_error(dpctl_p, error, "invalid datapath");
+return error;
+}
+dpname = get_one_dp(dpctl_p);
+} else {
+dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
+}
+
if (!dpname) {
error = EINVAL;
dpctl_error(dpctl_p, error, "datapath not found");
@@ -863,7 +889,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
}
}
-error = opt_dpif_open(argc, argv, dpctl_p, 2, );
+error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
if (error) {
goto out_free;
}
@@ -990,7 +1016,7 @@ dpctl_put_flow(int argc, const char *argv[], enum
dpif_flow_put_flags flags,
struct simap port_names;
int n, error;
-error = opt_dpif_open(argc, argv, dpctl_p, 4, );
+error = opt_dpif_open(argc, argv, dpctl_p, 4, , false, NULL);
if (error) {
return error;
}
@@ -1092,7 +1118,7 @@ dpctl_get_flow(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
struct ds ds;
int n, error;
-error = opt_dpif_open(argc, argv, dpctl_p, 3, );
+error = opt_dpif_open(argc, argv, dpctl_p, 3, , false, NULL);
if (error) {
return error;
}
@@ -1141,7 +1167,7 @@ dpctl_del_flow(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
struct simap port_names;
int n, error;
-error = opt_dpif_open(argc, argv, dpctl_p, 3, );
+error = opt_dpif_open(argc, argv, dpctl_p, 3, , false, NULL);
if (error) {
return error;
}
@@ -1210,7 +1236,7 @@ dpctl_del_flows(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
{
struct dpif *dpif;
-int error = opt_dpif_open(argc, argv, dpctl_p, 2, );
+int error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
if (error) {
return error;
}
@@ -1271,7 +1297,7 @@ dpctl_dump_conntrack(int argc, const char *argv[],
argc--;
}
-error = opt_dpif_open(argc, argv, dpctl_p, 2, );
+error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
if (error) {
return error;
}
@@ -1313,34 +1339,11 @@ dpctl_flush_conntrack(int argc, const char *argv[],
struct ct_dpif_tuple tuple, *ptuple = NULL;
struct ds ds
[ovs-dev] [PATCH v2 09/11] dpctl: Refactor opt_dpif_open().
Make opt_dpif_open() to support mulitple optional arguments. It will
be useful for the following patches.
Signed-off-by: Yi-Hung Wei
---
lib/dpctl.c | 85 -
1 file changed, 44 insertions(+), 41 deletions(-)
diff --git a/lib/dpctl.c b/lib/dpctl.c
index 4f1e443f2662..35733774b331 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -191,14 +191,40 @@ parsed_dpif_open(const char *arg_, bool create, struct
dpif **dpifp)
*
* The datapath name is not a mandatory parameter for this command. If
* it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists. On success stores the
- * opened dpif in '*dpifp'. */
+ * the current setup, assuming only one exists.
+ * For commands with multiple optional arguments, we try to use the first
+ * argument as the dpif name. If it is failed, then we fallback to
+ * retrieve it form the current setup.
+ * On success stores the opened dpif in '*dpifp', and the next arugment
+ * to be parsed in '*indexp'. */
static int
opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
- uint8_t max_args, struct dpif **dpifp)
+ uint8_t max_args, struct dpif **dpifp, bool multi_opt,
+ int *indexp)
{
+char *dpname;
int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
+
+if (indexp) {
+*indexp = 1;
+}
+
+if (multi_opt && argc > 1) {
+error = parsed_dpif_open(argv[1], false, dpifp);
+if (!error) {
+if (indexp) {
+*indexp = 2;
+}
+return 0;
+} else if (argc == max_args) {
+dpctl_error(dpctl_p, error, "invalid datapath");
+return error;
+}
+dpname = get_one_dp(dpctl_p);
+} else {
+dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
+}
+
if (!dpname) {
error = EINVAL;
dpctl_error(dpctl_p, error, "datapath not found");
@@ -863,7 +889,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
}
}
-error = opt_dpif_open(argc, argv, dpctl_p, 2, );
+error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
if (error) {
goto out_free;
}
@@ -990,7 +1016,7 @@ dpctl_put_flow(int argc, const char *argv[], enum
dpif_flow_put_flags flags,
struct simap port_names;
int n, error;
-error = opt_dpif_open(argc, argv, dpctl_p, 4, );
+error = opt_dpif_open(argc, argv, dpctl_p, 4, , false, NULL);
if (error) {
return error;
}
@@ -1092,7 +1118,7 @@ dpctl_get_flow(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
struct ds ds;
int n, error;
-error = opt_dpif_open(argc, argv, dpctl_p, 3, );
+error = opt_dpif_open(argc, argv, dpctl_p, 3, , false, NULL);
if (error) {
return error;
}
@@ -1141,7 +1167,7 @@ dpctl_del_flow(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
struct simap port_names;
int n, error;
-error = opt_dpif_open(argc, argv, dpctl_p, 3, );
+error = opt_dpif_open(argc, argv, dpctl_p, 3, , false, NULL);
if (error) {
return error;
}
@@ -1210,7 +1236,7 @@ dpctl_del_flows(int argc, const char *argv[], struct
dpctl_params *dpctl_p)
{
struct dpif *dpif;
-int error = opt_dpif_open(argc, argv, dpctl_p, 2, );
+int error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
if (error) {
return error;
}
@@ -1271,7 +1297,7 @@ dpctl_dump_conntrack(int argc, const char *argv[],
argc--;
}
-error = opt_dpif_open(argc, argv, dpctl_p, 2, );
+error = opt_dpif_open(argc, argv, dpctl_p, 2, , false, NULL);
if (error) {
return error;
}
@@ -1313,34 +1339,11 @@ dpctl_flush_conntrack(int argc, const char *argv[],
struct ct_dpif_tuple tuple, *ptuple = NULL;
struct ds ds = DS_EMPTY_INITIALIZER;
uint16_t zone, *pzone = NULL;
-char *name;
int error, i = 1;
-bool got_dpif = false;
-/* Parse datapath name. It is not a mandatory parameter for this command.
- * If it is not specified, we retrieve it from the current setup,
- * assuming only one exists. */
-if (argc >= 2) {
-error = parsed_dpif_open(argv[i], false, );
-if (!error) {
-got_dpif = true;
-i++;
-} else if (argc == 4) {
-dpctl_error(dpctl_p, error, "invalid datapath");
-return error;
-}
-}
-if (!got_dpif) {
-name = get_one_dp(dpctl_p);
-if (!name) {
-return EINVAL;
-}
-error = parsed_dpif_open(name, false, );
-free(name);
-if (error) {
-dpctl_error(dpctl_p, error, "opening datapath");
-return error;
-}
+error = opt_dpif_open(argc, argv, dpctl_p, 4, ,
