diff mbox

[ovs-dev] <conntrack><dpif-netdev>: ovs-appctl dump and flush command for userspace conntrack

Message ID HKXPR03MB07251A062B973A2059A6712C8BA00@HKXPR03MB0725.apcprd03.prod.outlook.com
State Not Applicable
Headers show

Commit Message

sourabh.bansal@wipro.com Feb. 19, 2016, 10:35 a.m. UTC 
Hi Joe,

Thanks for pointing out my mistake. Please find below updated one.


m 69e63a45e2773c124deb885bbc3d5deb3e032126 Mon Sep 17 00:00:00 2001
From: soumyadeep chowdhury <soumyadeep.chowdhury@wipro.com>

Date: Fri, 19 Feb 2016 07:39:34 -0500
Subject: [PATCH 4/4] Write the functions for dump-conntrack and
 flush-conntrack

Signed-off-by: Sourabh Bansal <sourabh.bansal@wipro.com>

---
 ovs-userconntrack_20151115/lib/dpif-netdev.c | 82 ++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

 dpif_netdev_init(void)
@@ -728,6 +806,10 @@ dpif_netdev_init(void)
     unixctl_command_register("dpif-netdev/pmd-stats-clear", "[dp]",
                              0, 1, dpif_netdev_pmd_info,
                              (void *)&clear_aux);
+    unixctl_command_register("dump-conntrack", "Dump the entire conntrack table",
+                              0, 0, dpif_netdev_dump_conntrack, NULL);
+    unixctl_command_register("flush-conntrack", "Flush the entire conntrack table",
+                              0, 0, dpif_netdev_flush_conntrack, NULL);
     return 0;
 }

Regards,
Sourabh Bansal

-----Original Message-----
From: Joe Stringer [mailto:joe@ovn.org]

Sent: Friday, February 19, 2016 3:08 AM
To: Sourabh Bansal (NEP) <sourabh.bansal@wipro.com>
Cc: Daniele Di Proietto <diproiettod@vmware.com>; ovs dev <dev@openvswitch.org>
Subject: Re: [ovs-dev] <conntrack><dpif-netdev>: ovs-appctl dump and flush command for userspace conntrack

On 17 February 2016 at 04:50,  <sourabh.bansal@wipro.com> wrote:
> Hi Daniele,

>

> Like to contribute the following changes to your userconntrack branch.

> Please review and provide your comments

>

> Change Description:

>

> This implementation is based on top of this branch (By Daniele Di Proietto):

> https://github.com/ddiproietto/ovs/tree/userconntrack_20151115

>

> and this provides following two commands to ovs-appctl  command -

>

>

> 1.       Ovs-appctl dump-conntrack     - To dump the conntrack table entries

>

> 2.       Ovs-appctl flush-conntrack       - To flush the conntrack table entries

>

> Corresponding function implementations are -

> dpif_netdev_dump_conntrack()

> dpif_netdev_flush_conntrack()

>

> and these functions are registered with unixctl_command_register() so

> that it can be accessed via ovs-appctl command

>

> List of change files:

>

> 1.       /lib/dpif-netdev.c


This functionality is already available here, in the same codebase:
https://github.com/ddiproietto/ovs/blob/userconntrack_20151115/lib/dpctl.c#L1246


> Change Diff:

>

> *** /home/bansal/submission_code/ovs-userconntrack_20151115/lib/dpif-netdev.c     2016-02-17 10:00:12.114559489 -0500

> --- /home/bansal/orig/ovs-userconntrack_20151115/lib/dpif-netdev.c   2015-11-16 01:07:25.000000000 -0500

> ***************

> *** 715,797 ****


The usual way to prepare and send patches for projects like this is using "git format-patch" or "git send-email".

>       unixctl_command_reply(conn, ds_cstr(&reply));

>       ds_destroy(&reply);

>   }

> -

> - /*This function will dump the entries prestent in conntrack table*/

> - static void

> - dpif_netdev_dump_conntrack(struct unixctl_conn *conn, int argc, const char *argv[],

> -                              void *aux OVS_UNUSED)

> - {

> -     struct dp_netdev *dp = NULL;

> -     struct ct_dpif_dump_state *dump;

> -     struct ct_dpif_entry cte;

> -     uint16_t *pzone = NULL;

> -     struct dpif *dpif;

> -

> -     ovs_mutex_lock(&dp_netdev_mutex);

> -

> -     if (argc == 2) {

> -         dp = shash_find_data(&dp_netdevs, argv[1]);

> -     } else if (shash_count(&dp_netdevs) == 1) {

> -                   /* There's only one datapath */

> -         dp = shash_first(&dp_netdevs)->data;

> -     }

> -

> -     if (!dp) {

> -        ovs_mutex_unlock(&dp_netdev_mutex);

> -        unixctl_command_reply_error(conn,

> -                               "please specify an existing datapath");

> -        return;

> -     }

> -     ovs_mutex_unlock(&dp_netdev_mutex);

> -     dpif = dp->dpif;

> -

> -     int verbosity = 1;

> -     int print_statistics = 0;

> -

> -     struct ds s = DS_EMPTY_INITIALIZER;

> -

> -     ct_dpif_dump_start(dpif, &dump, pzone);

> -     while (!ct_dpif_dump_next(dump, &cte)) {

> -

> -         ct_dpif_format_entry(&cte, &s, verbosity,

> -         print_statistics);

> -         ct_dpif_entry_uninit(&cte);

> -     }

> -

> -     unixctl_command_reply(conn, ds_cstr(&s));

> -     ds_destroy(&s);

> -

> -     ct_dpif_dump_done(dump);

> -     return;

> - }

> -

> - /*This function will flush the entries prestent in conntrack table*/

> - static void

> - dpif_netdev_flush_conntrack(struct unixctl_conn *conn, int argc, const char *argv[],

> -                      void *aux OVS_UNUSED)

> - {

> -     struct dp_netdev *dp = NULL;

> -     struct dpif *dpif;

> -     uint16_t *pzone = NULL;

> -

> -     ovs_mutex_lock(&dp_netdev_mutex);

> -     if (argc == 2) {

> -         dp = shash_find_data(&dp_netdevs, argv[1]);

> -     } else if (shash_count(&dp_netdevs) == 1) {

> -         dp = shash_first(&dp_netdevs)->data;

> -     }

> -

> -     ovs_mutex_unlock(&dp_netdev_mutex);

> -

> -     struct ds s = DS_EMPTY_INITIALIZER;

> -

> -     dpif = dp->dpif;

> -     ct_dpif_flush(dpif,pzone);

> -

> -     unixctl_command_reply(conn, ds_cstr(&s));

> -     ds_destroy(&s);

> -

> - }

>

>   static int

>   dpif_netdev_init(void)

> --- 715,720 ----

> ***************

> *** 805,814 ****

>       unixctl_command_register("dpif-netdev/pmd-stats-clear", "[dp]",

>                                0, 1, dpif_netdev_pmd_info,

>                                (void *)&clear_aux);

> -     unixctl_command_register("dump-conntrack", "Dump the entire conntrack table",

> -                               0, 0, dpif_netdev_dump_conntrack, NULL);

> -     unixctl_command_register("flush-conntrack", "Flush the entire conntrack table",

> -                               0, 0, dpif_netdev_flush_conntrack, NULL);

>       return 0;

>   }


It looks like your diff is in the reverse order of what you intended.

> Regards,

> Sourabh Bansal

> The information contained in this electronic message and any

> attachments to this message are intended for the exclusive use of the

> addressee(s) and may contain proprietary, confidential or privileged

> information. If you are not the intended recipient, you should not

> disseminate, distribute or copy this e-mail. Please notify the sender

> immediately and destroy all copies of this message and any

> attachments. WARNING: Computer viruses can be transmitted via email.

> The recipient should check this email and any attachments for the

> presence of viruses. The company accepts no liability for any damage

> caused by any virus transmitted by this email. www.wipro.com


This disclaimer makes entirely *no* sense on a public mailing list, please refrain from using it.
The information contained in this electronic message and any attachments to this message are intended for the exclusive use of the addressee(s) and may contain proprietary, confidential or privileged information. If you are not the intended recipient, you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately and destroy all copies of this message and any attachments. WARNING: Computer viruses can be transmitted via email. The recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email. www.wipro.com

Comments

Joe Stringer Feb. 19, 2016, 6:18 p.m. UTC | #1 
On 19 February 2016 at 02:35,  <sourabh.bansal@wipro.com> wrote:
> Hi Joe,
>
> Thanks for pointing out my mistake. Please find below updated one.
>
>
> m 69e63a45e2773c124deb885bbc3d5deb3e032126 Mon Sep 17 00:00:00 2001
> From: soumyadeep chowdhury <soumyadeep.chowdhury@wipro.com>
> Date: Fri, 19 Feb 2016 07:39:34 -0500
> Subject: [PATCH 4/4] Write the functions for dump-conntrack and
>  flush-conntrack
>
> Signed-off-by: Sourabh Bansal <sourabh.bansal@wipro.com>

While the format of the patch looks closer to correct, I think my
primary piece of feedback may have been misunderstood.

What does this patch provide which isn't already available via the
"ovs-appctl dpctl/{dump,flush}-conntrack" commands?
sourabh.bansal@wipro.com Feb. 22, 2016, 5:44 a.m. UTC | #2 
Hi Joe,

These commands will work when we are running userspace conntrack (OVS+DPDK env).
Existing commands dpctl/{dump-flush}-conntrack commands will not help here, as they are trying to get info of kernel based conntrack module.

NOTE: This implementation is over <https://github.com/ddiproietto/ovs/tree/userconntrack_20151115 > branch which is implementing userspaced conntrack module.


-----Original Message-----
From: Joe Stringer [mailto:joe@ovn.org]

Sent: Friday, February 19, 2016 11:48 PM
To: Sourabh Bansal (NEP) <sourabh.bansal@wipro.com>
Cc: Daniele Di Proietto <diproiettod@vmware.com>; ovs dev <dev@openvswitch.org>; Sanjeev Sharma (Cisco) <sanjeev.sharma15@wipro.com>; Soumyadeep Chowdhury (NEP) <soumyadeep.chowdhury@wipro.com>
Subject: Re: [ovs-dev] <conntrack><dpif-netdev>: ovs-appctl dump and flush command for userspace conntrack

On 19 February 2016 at 02:35,  <sourabh.bansal@wipro.com> wrote:
> Hi Joe,

>

> Thanks for pointing out my mistake. Please find below updated one.

>

>

> m 69e63a45e2773c124deb885bbc3d5deb3e032126 Mon Sep 17 00:00:00 2001

> From: soumyadeep chowdhury <soumyadeep.chowdhury@wipro.com>

> Date: Fri, 19 Feb 2016 07:39:34 -0500

> Subject: [PATCH 4/4] Write the functions for dump-conntrack and

> flush-conntrack

>

> Signed-off-by: Sourabh Bansal <sourabh.bansal@wipro.com>


While the format of the patch looks closer to correct, I think my primary piece of feedback may have been misunderstood.

What does this patch provide which isn't already available via the "ovs-appctl dpctl/{dump,flush}-conntrack" commands?
The information contained in this electronic message and any attachments to this message are intended for the exclusive use of the addressee(s) and may contain proprietary, confidential or privileged information. If you are not the intended recipient, you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately and destroy all copies of this message and any attachments. WARNING: Computer viruses can be transmitted via email. The recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email. www.wipro.com
sourabh.bansal@wipro.com Feb. 22, 2016, 9:51 a.m. UTC | #3 
Hi Joe,

I rechecked and ovs-appctl dpctl/dump-conntrack command works fine for me. Actually I got confused with ovs-dpctl dump-conntrack command and thought that userspace dump conntrack is not available.

Thanks for pointing it out. Withdrawing the patch.



-----Original Message-----
From: Sourabh Bansal (NEP)

Sent: Monday, February 22, 2016 11:14 AM
To: 'Joe Stringer' <joe@ovn.org>
Cc: Daniele Di Proietto <diproiettod@vmware.com>; ovs dev <dev@openvswitch.org>; Sanjeev Sharma (Cisco) <sanjeev.sharma15@wipro.com>; Soumyadeep Chowdhury (NEP) <soumyadeep.chowdhury@wipro.com>
Subject: RE: [ovs-dev] <conntrack><dpif-netdev>: ovs-appctl dump and flush command for userspace conntrack

Hi Joe,

These commands will work when we are running userspace conntrack (OVS+DPDK env).
Existing commands dpctl/{dump-flush}-conntrack commands will not help here, as they are trying to get info of kernel based conntrack module.

NOTE: This implementation is over <https://github.com/ddiproietto/ovs/tree/userconntrack_20151115 > branch which is implementing userspaced conntrack module.


-----Original Message-----
From: Joe Stringer [mailto:joe@ovn.org]

Sent: Friday, February 19, 2016 11:48 PM
To: Sourabh Bansal (NEP) <sourabh.bansal@wipro.com>
Cc: Daniele Di Proietto <diproiettod@vmware.com>; ovs dev <dev@openvswitch.org>; Sanjeev Sharma (Cisco) <sanjeev.sharma15@wipro.com>; Soumyadeep Chowdhury (NEP) <soumyadeep.chowdhury@wipro.com>
Subject: Re: [ovs-dev] <conntrack><dpif-netdev>: ovs-appctl dump and flush command for userspace conntrack

On 19 February 2016 at 02:35,  <sourabh.bansal@wipro.com> wrote:
> Hi Joe,

>

> Thanks for pointing out my mistake. Please find below updated one.

>

>

> m 69e63a45e2773c124deb885bbc3d5deb3e032126 Mon Sep 17 00:00:00 2001

> From: soumyadeep chowdhury <soumyadeep.chowdhury@wipro.com>

> Date: Fri, 19 Feb 2016 07:39:34 -0500

> Subject: [PATCH 4/4] Write the functions for dump-conntrack and

> flush-conntrack

>

> Signed-off-by: Sourabh Bansal <sourabh.bansal@wipro.com>


While the format of the patch looks closer to correct, I think my primary piece of feedback may have been misunderstood.

What does this patch provide which isn't already available via the "ovs-appctl dpctl/{dump,flush}-conntrack" commands?
The information contained in this electronic message and any attachments to this message are intended for the exclusive use of the addressee(s) and may contain proprietary, confidential or privileged information. If you are not the intended recipient, you should not disseminate, distribute or copy this e-mail. Please notify the sender immediately and destroy all copies of this message and any attachments. WARNING: Computer viruses can be transmitted via email. The recipient should check this email and any attachments for the presence of viruses. The company accepts no liability for any damage caused by any virus transmitted by this email. www.wipro.com
diff mbox

Patch

diff --git a/ovs-userconntrack_20151115/lib/dpif-netdev.c b/ovs-userconntrack_20151115/lib/dpif-netdev.c
index 340e37c..f950780 100644
--- a/ovs-userconntrack_20151115/lib/dpif-netdev.c
+++ b/ovs-userconntrack_20151115/lib/dpif-netdev.c
@@ -715,6 +715,84 @@  dpif_netdev_pmd_info(struct unixctl_conn *conn, int argc, const char *argv[],
     unixctl_command_reply(conn, ds_cstr(&reply));
     ds_destroy(&reply);
 }
+
+/*This function will dump the entries present in conntrack table*/
+static void
+dpif_netdev_dump_conntrack(struct unixctl_conn *conn, int argc, const char *argv[],
+                             void *aux OVS_UNUSED)
+{
+    struct dp_netdev *dp = NULL;
+    struct ct_dpif_dump_state *dump;
+    struct ct_dpif_entry cte;
+    uint16_t *pzone = NULL;
+    struct dpif *dpif;
+
+    ovs_mutex_lock(&dp_netdev_mutex);
+
+    if (argc == 2) {
+        dp = shash_find_data(&dp_netdevs, argv[1]);
+    } else if (shash_count(&dp_netdevs) == 1) {
+                  /* There's only one datapath */
+        dp = shash_first(&dp_netdevs)->data;
+    }
+
+    if (!dp) {
+       ovs_mutex_unlock(&dp_netdev_mutex);
+       unixctl_command_reply_error(conn,
+                              "please specify an existing datapath");
+       return;
+    }
+    ovs_mutex_unlock(&dp_netdev_mutex);
+    dpif = dp->dpif;
+
+    int verbosity = 1;
+    int print_statistics = 0;
+
+    struct ds s = DS_EMPTY_INITIALIZER;
+
+    ct_dpif_dump_start(dpif, &dump, pzone);
+    while (!ct_dpif_dump_next(dump, &cte)) {
+
+        ct_dpif_format_entry(&cte, &s, verbosity,
+        print_statistics);
+        ct_dpif_entry_uninit(&cte);
+    }
+
+    unixctl_command_reply(conn, ds_cstr(&s));
+    ds_destroy(&s);
+
+    ct_dpif_dump_done(dump);
+    return;
+}
+
+/*This function will flush the entries present in conntrack table*/
+static void
+dpif_netdev_flush_conntrack(struct unixctl_conn *conn, int argc, const char *argv[],
+                     void *aux OVS_UNUSED)
+{
+    struct dp_netdev *dp = NULL;
+    struct dpif *dpif;
+    uint16_t *pzone = NULL;
+
+    ovs_mutex_lock(&dp_netdev_mutex);
+    if (argc == 2) {
+        dp = shash_find_data(&dp_netdevs, argv[1]);
+    } else if (shash_count(&dp_netdevs) == 1) {
+        dp = shash_first(&dp_netdevs)->data;
+    }
+
+    ovs_mutex_unlock(&dp_netdev_mutex);
+
+    struct ds s = DS_EMPTY_INITIALIZER;
+
+    dpif = dp->dpif;
+    ct_dpif_flush(dpif,pzone);
+
+    unixctl_command_reply(conn, ds_cstr(&s));
+    ds_destroy(&s);
+
+}
+


 static int