Re: [ovs-dev] [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration in dpif_netdev_run().
> >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().
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().
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
[ovs-dev] [PATCH v5 08/12] dpif-netdev: Change pmd thread configuration in dpif_netdev_run().
Signed-off-by: Daniele Di ProiettoTested-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 +reconfigure_pmd_threads(struct dp_netdev *dp) +{ +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 < port->n_rxq; i++)