Re: [ovs-dev] [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration in dpif_netdev_run().

2016-03-29 Thread Kavanagh, Mark B 
>
>Hi Mark,
>
>thanks for your comment, I replied inline
>
>On 24/03/2016 10:17, "Kavanagh, Mark B"  wrote:
>
>>Hi Daniele,
>>
>>One comment inline.
>>
>>Cheers,
>>Mark
>>
>>>-Original Message-
>>>From: Daniele Di Proietto [mailto:diproiet...@vmware.com]
>>>Sent: Wednesday, March 23, 2016 6:37 PM
>>>To: dev@openvswitch.org
>>>Cc: Ben Pfaff; Kavanagh, Mark B; Ilya Maximets; Daniele Di Proietto
>>>Subject: [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration
>>>in dpif_netdev_run().
>>>
>>>Signed-off-by: Daniele Di Proietto 
>>>Tested-by: Ilya Maximets 
>>>Acked-by: Ilya Maximets 
>>>---
>>> lib/dpif-netdev.c   | 140
>>>++--
>>> lib/dpif-provider.h |   3 +-
>>> 2 files changed, 83 insertions(+), 60 deletions(-)
>>>
>>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>>index 66c0b19..6aaeaeb 100644
>>>--- a/lib/dpif-netdev.c
>>>+++ b/lib/dpif-netdev.c
>>>@@ -223,7 +223,9 @@ struct dp_netdev {
>>> ovsthread_key_t per_pmd_key;
>>>
>>> /* Cpu mask for pin of pmd threads. */
>>>+char *requested_pmd_cmask;
>>> char *pmd_cmask;
>>>+
>>> uint64_t last_tnl_conf_seq;
>>> };
>>>
>>>@@ -2447,82 +2449,44 @@ dpif_netdev_operate(struct dpif *dpif, struct
>>>dpif_op **ops, size_t
>>>n_ops)
>>> }
>>> }
>>>
>>>-/* Returns true if the configuration for rx queues or cpu mask
>>>- * is changed. */
>>>+/* Returns true if the configuration for rx queues is changed. */
>>> static bool
>>>-pmd_config_changed(const struct dp_netdev *dp, const char *cmask)
>>>+pmd_n_rxq_changed(const struct dp_netdev *dp)
>>> {
>>> struct dp_netdev_port *port;
>>>
>>> CMAP_FOR_EACH (port, node, >ports) {
>>>-struct netdev *netdev = port->netdev;
>>>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>>>-if (netdev_is_pmd(netdev)
>>>+int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>>>+
>>>+if (netdev_is_pmd(port->netdev)
>>> && port->latest_requested_n_rxq != requested_n_rxq) {
>>> return true;
>>> }
>>> }
>>>
>>>-if (dp->pmd_cmask != NULL && cmask != NULL) {
>>>-return strcmp(dp->pmd_cmask, cmask);
>>>-} else {
>>>-return (dp->pmd_cmask != NULL || cmask != NULL);
>>>+return false;
>>>+}
>>>+
>>>+static bool
>>>+cmask_equals(const char *a, const char *b)
>>>+{
>>>+if (a && b) {
>>>+return !strcmp(a, b);
>>> }
>>>+
>>>+return a == NULL && b == NULL;
>>> }
>>>
>>>-/* Resets pmd threads if the configuration for 'rxq's or cpu mask
>>>changes. */
>>>+/* Changes the number or the affinity of pmd threads.  The changes are
>>>actually
>>>+ * applied in dpif_netdev_run(). */
>>> static int
>>> dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask)
>>> {
>>> struct dp_netdev *dp = get_dp_netdev(dpif);
>>>
>>>-if (pmd_config_changed(dp, cmask)) {
>>>-struct dp_netdev_port *port;
>>>-
>>>-dp_netdev_destroy_all_pmds(dp);
>>>-
>>>-CMAP_FOR_EACH (port, node, >ports) {
>>>-struct netdev *netdev = port->netdev;
>>>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>>>-if (netdev_is_pmd(port->netdev)
>>>-&& port->latest_requested_n_rxq != requested_n_rxq) {
>>>-int i, err;
>>>-
>>>-/* Closes the existing 'rxq's. */
>>>-for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
>>>-netdev_rxq_close(port->rxq[i]);
>>>-port->rxq[i] = NULL;
>>>-}
>>>-port->n_rxq = 0;
>>>-
>>>-/* Sets the new rx queue config.  */
>>>-err = netdev_set_multiq(port->netdev,
>>>-ovs_numa_get_n_cores() + 1,
>>>-requested_n_rxq);
>>>-if (err && (err != EOPNOTSUPP)) {
>>>-VLOG_ERR("Failed to set dpdk interface %s rx_queue
>>>to:"
>>>- " %u", netdev_get_name(port->netdev),
>>>- requested_n_rxq);
>>>-return err;
>>>-}
>>>-port->latest_requested_n_rxq = requested_n_rxq;
>>>-/* If the set_multiq() above succeeds, reopens the
>>>'rxq's. */
>>>-port->n_rxq = netdev_n_rxq(port->netdev);
>>>-port->rxq = xrealloc(port->rxq, sizeof *port->rxq *
>>>port->n_rxq);
>>>-for (i = 0; i < port->n_rxq; i++) {
>>>-netdev_rxq_open(port->netdev, >rxq[i], i);
>>>-}
>>>-}
>>>-}
>>>-/* Reconfigures the cpu mask. */
>>>-ovs_numa_set_cpu_mask(cmask);
>>>-free(dp->pmd_cmask);
>>>-dp->pmd_cmask = cmask ? xstrdup(cmask) : NULL;
>>>-
>>>-/* Restores the non-pmd. */
>>>-

Re: [ovs-dev] [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration in dpif_netdev_run().

2016-03-28 Thread Daniele Di Proietto 
Hi Mark,

thanks for your comment, I replied inline

On 24/03/2016 10:17, "Kavanagh, Mark B"  wrote:

>Hi Daniele,
>
>One comment inline.
>
>Cheers,
>Mark
>
>>-Original Message-
>>From: Daniele Di Proietto [mailto:diproiet...@vmware.com]
>>Sent: Wednesday, March 23, 2016 6:37 PM
>>To: dev@openvswitch.org
>>Cc: Ben Pfaff; Kavanagh, Mark B; Ilya Maximets; Daniele Di Proietto
>>Subject: [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration
>>in dpif_netdev_run().
>>
>>Signed-off-by: Daniele Di Proietto 
>>Tested-by: Ilya Maximets 
>>Acked-by: Ilya Maximets 
>>---
>> lib/dpif-netdev.c   | 140
>>++--
>> lib/dpif-provider.h |   3 +-
>> 2 files changed, 83 insertions(+), 60 deletions(-)
>>
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>index 66c0b19..6aaeaeb 100644
>>--- a/lib/dpif-netdev.c
>>+++ b/lib/dpif-netdev.c
>>@@ -223,7 +223,9 @@ struct dp_netdev {
>> ovsthread_key_t per_pmd_key;
>>
>> /* Cpu mask for pin of pmd threads. */
>>+char *requested_pmd_cmask;
>> char *pmd_cmask;
>>+
>> uint64_t last_tnl_conf_seq;
>> };
>>
>>@@ -2447,82 +2449,44 @@ dpif_netdev_operate(struct dpif *dpif, struct
>>dpif_op **ops, size_t
>>n_ops)
>> }
>> }
>>
>>-/* Returns true if the configuration for rx queues or cpu mask
>>- * is changed. */
>>+/* Returns true if the configuration for rx queues is changed. */
>> static bool
>>-pmd_config_changed(const struct dp_netdev *dp, const char *cmask)
>>+pmd_n_rxq_changed(const struct dp_netdev *dp)
>> {
>> struct dp_netdev_port *port;
>>
>> CMAP_FOR_EACH (port, node, >ports) {
>>-struct netdev *netdev = port->netdev;
>>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>>-if (netdev_is_pmd(netdev)
>>+int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>>+
>>+if (netdev_is_pmd(port->netdev)
>> && port->latest_requested_n_rxq != requested_n_rxq) {
>> return true;
>> }
>> }
>>
>>-if (dp->pmd_cmask != NULL && cmask != NULL) {
>>-return strcmp(dp->pmd_cmask, cmask);
>>-} else {
>>-return (dp->pmd_cmask != NULL || cmask != NULL);
>>+return false;
>>+}
>>+
>>+static bool
>>+cmask_equals(const char *a, const char *b)
>>+{
>>+if (a && b) {
>>+return !strcmp(a, b);
>> }
>>+
>>+return a == NULL && b == NULL;
>> }
>>
>>-/* Resets pmd threads if the configuration for 'rxq's or cpu mask
>>changes. */
>>+/* Changes the number or the affinity of pmd threads.  The changes are
>>actually
>>+ * applied in dpif_netdev_run(). */
>> static int
>> dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask)
>> {
>> struct dp_netdev *dp = get_dp_netdev(dpif);
>>
>>-if (pmd_config_changed(dp, cmask)) {
>>-struct dp_netdev_port *port;
>>-
>>-dp_netdev_destroy_all_pmds(dp);
>>-
>>-CMAP_FOR_EACH (port, node, >ports) {
>>-struct netdev *netdev = port->netdev;
>>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>>-if (netdev_is_pmd(port->netdev)
>>-&& port->latest_requested_n_rxq != requested_n_rxq) {
>>-int i, err;
>>-
>>-/* Closes the existing 'rxq's. */
>>-for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
>>-netdev_rxq_close(port->rxq[i]);
>>-port->rxq[i] = NULL;
>>-}
>>-port->n_rxq = 0;
>>-
>>-/* Sets the new rx queue config.  */
>>-err = netdev_set_multiq(port->netdev,
>>-ovs_numa_get_n_cores() + 1,
>>-requested_n_rxq);
>>-if (err && (err != EOPNOTSUPP)) {
>>-VLOG_ERR("Failed to set dpdk interface %s rx_queue
>>to:"
>>- " %u", netdev_get_name(port->netdev),
>>- requested_n_rxq);
>>-return err;
>>-}
>>-port->latest_requested_n_rxq = requested_n_rxq;
>>-/* If the set_multiq() above succeeds, reopens the
>>'rxq's. */
>>-port->n_rxq = netdev_n_rxq(port->netdev);
>>-port->rxq = xrealloc(port->rxq, sizeof *port->rxq *
>>port->n_rxq);
>>-for (i = 0; i < port->n_rxq; i++) {
>>-netdev_rxq_open(port->netdev, >rxq[i], i);
>>-}
>>-}
>>-}
>>-/* Reconfigures the cpu mask. */
>>-ovs_numa_set_cpu_mask(cmask);
>>-free(dp->pmd_cmask);
>>-dp->pmd_cmask = cmask ? xstrdup(cmask) : NULL;
>>-
>>-/* Restores the non-pmd. */
>>-dp_netdev_set_nonpmd(dp);
>>-/* Restores all pmd threads. */
>>-dp_netdev_reset_pmd_threads(dp);
>>+if

Re: [ovs-dev] [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration in dpif_netdev_run().

2016-03-24 Thread Kavanagh, Mark B 
Hi Daniele,

One comment inline.

Cheers,
Mark

>-Original Message-
>From: Daniele Di Proietto [mailto:diproiet...@vmware.com]
>Sent: Wednesday, March 23, 2016 6:37 PM
>To: dev@openvswitch.org
>Cc: Ben Pfaff; Kavanagh, Mark B; Ilya Maximets; Daniele Di Proietto
>Subject: [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration in 
>dpif_netdev_run().
>
>Signed-off-by: Daniele Di Proietto 
>Tested-by: Ilya Maximets 
>Acked-by: Ilya Maximets 
>---
> lib/dpif-netdev.c   | 140 ++--
> lib/dpif-provider.h |   3 +-
> 2 files changed, 83 insertions(+), 60 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index 66c0b19..6aaeaeb 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -223,7 +223,9 @@ struct dp_netdev {
> ovsthread_key_t per_pmd_key;
>
> /* Cpu mask for pin of pmd threads. */
>+char *requested_pmd_cmask;
> char *pmd_cmask;
>+
> uint64_t last_tnl_conf_seq;
> };
>
>@@ -2447,82 +2449,44 @@ dpif_netdev_operate(struct dpif *dpif, struct dpif_op 
>**ops, size_t
>n_ops)
> }
> }
>
>-/* Returns true if the configuration for rx queues or cpu mask
>- * is changed. */
>+/* Returns true if the configuration for rx queues is changed. */
> static bool
>-pmd_config_changed(const struct dp_netdev *dp, const char *cmask)
>+pmd_n_rxq_changed(const struct dp_netdev *dp)
> {
> struct dp_netdev_port *port;
>
> CMAP_FOR_EACH (port, node, >ports) {
>-struct netdev *netdev = port->netdev;
>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>-if (netdev_is_pmd(netdev)
>+int requested_n_rxq = netdev_requested_n_rxq(port->netdev);
>+
>+if (netdev_is_pmd(port->netdev)
> && port->latest_requested_n_rxq != requested_n_rxq) {
> return true;
> }
> }
>
>-if (dp->pmd_cmask != NULL && cmask != NULL) {
>-return strcmp(dp->pmd_cmask, cmask);
>-} else {
>-return (dp->pmd_cmask != NULL || cmask != NULL);
>+return false;
>+}
>+
>+static bool
>+cmask_equals(const char *a, const char *b)
>+{
>+if (a && b) {
>+return !strcmp(a, b);
> }
>+
>+return a == NULL && b == NULL;
> }
>
>-/* Resets pmd threads if the configuration for 'rxq's or cpu mask changes. */
>+/* Changes the number or the affinity of pmd threads.  The changes are 
>actually
>+ * applied in dpif_netdev_run(). */
> static int
> dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask)
> {
> struct dp_netdev *dp = get_dp_netdev(dpif);
>
>-if (pmd_config_changed(dp, cmask)) {
>-struct dp_netdev_port *port;
>-
>-dp_netdev_destroy_all_pmds(dp);
>-
>-CMAP_FOR_EACH (port, node, >ports) {
>-struct netdev *netdev = port->netdev;
>-int requested_n_rxq = netdev_requested_n_rxq(netdev);
>-if (netdev_is_pmd(port->netdev)
>-&& port->latest_requested_n_rxq != requested_n_rxq) {
>-int i, err;
>-
>-/* Closes the existing 'rxq's. */
>-for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
>-netdev_rxq_close(port->rxq[i]);
>-port->rxq[i] = NULL;
>-}
>-port->n_rxq = 0;
>-
>-/* Sets the new rx queue config.  */
>-err = netdev_set_multiq(port->netdev,
>-ovs_numa_get_n_cores() + 1,
>-requested_n_rxq);
>-if (err && (err != EOPNOTSUPP)) {
>-VLOG_ERR("Failed to set dpdk interface %s rx_queue to:"
>- " %u", netdev_get_name(port->netdev),
>- requested_n_rxq);
>-return err;
>-}
>-port->latest_requested_n_rxq = requested_n_rxq;
>-/* If the set_multiq() above succeeds, reopens the 'rxq's. */
>-port->n_rxq = netdev_n_rxq(port->netdev);
>-port->rxq = xrealloc(port->rxq, sizeof *port->rxq * 
>port->n_rxq);
>-for (i = 0; i < port->n_rxq; i++) {
>-netdev_rxq_open(port->netdev, >rxq[i], i);
>-}
>-}
>-}
>-/* Reconfigures the cpu mask. */
>-ovs_numa_set_cpu_mask(cmask);
>-free(dp->pmd_cmask);
>-dp->pmd_cmask = cmask ? xstrdup(cmask) : NULL;
>-
>-/* Restores the non-pmd. */
>-dp_netdev_set_nonpmd(dp);
>-/* Restores all pmd threads. */
>-dp_netdev_reset_pmd_threads(dp);
>+if (!cmask_equals(dp->requested_pmd_cmask, cmask)) {
>+free(dp->requested_pmd_cmask);
>+dp->requested_pmd_cmask = cmask ? xstrdup(cmask) : NULL;
> }
>
> return 0;
>@@ -2622,6 +2586,58 @@ dp_netdev_process_rxq_port(struct dp_netdev_pmd_thread 
>*pmd,
> }
> }
>
>+static void