Thanks for the patch
I have a few comments inline
On 08/02/2016 07:30, "Ilya Maximets" <i.maxim...@samsung.com> wrote:
>Since 5f2ccb1c0d3b ("dpif: Allow adding ukeys for same flow by
>different pmds.") there is the possibility to reassign queues among
>pmd threads without restarting them and deleting the megaflow cache.
>
>So, reconfiguration can be performed without destroying of PMD threads
>if pmd-cpu-mask not changed.
>
>Reassignment of all rx queues done to achieve fair distribution.
>
>Signed-off-by: Ilya Maximets <i.maxim...@samsung.com>
>---
> lib/dpif-netdev.c | 62
>+++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 42 insertions(+), 20 deletions(-)
>
>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>index e62f5f6..8069163 100644
>--- a/lib/dpif-netdev.c
>+++ b/lib/dpif-netdev.c
>@@ -2373,27 +2373,31 @@ 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 of rx queues 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, &dp->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;
> }
> }
>+ return false;
>+}
>
>+/* Returns true if cpu mask changed. */
>+static bool
>+pmd_cpu_mask_changed(const struct dp_netdev *dp, const char *cmask)
>+{
> if (dp->pmd_cmask != NULL && cmask != NULL) {
> return strcmp(dp->pmd_cmask, cmask);
>- } else {
>- return (dp->pmd_cmask != NULL || cmask != NULL);
> }
>+
>+ return (dp->pmd_cmask != NULL || cmask != NULL);
> }
>
> /* Resets pmd threads if the configuration for 'rxq's or cpu mask
>changes. */
>@@ -2401,11 +2405,20 @@ static int
> dpif_netdev_pmd_set(struct dpif *dpif, const char *cmask)
> {
> struct dp_netdev *dp = get_dp_netdev(dpif);
>+ bool cmask_changed = pmd_cpu_mask_changed(dp, cmask);
>+ struct dp_netdev_port *port;
>
>- if (pmd_config_changed(dp, cmask)) {
>- struct dp_netdev_port *port;
>+ if (cmask_changed || pmd_n_rxq_changed(dp)) {
We could just return here if nothing is changed. That will save us
one level of indentation.
>+ if (cmask_changed) {
>+ dp_netdev_destroy_all_pmds(dp);
>+ } else {
>+ struct dp_netdev_pmd_thread *pmd;
>
>- dp_netdev_destroy_all_pmds(dp);
>+ CMAP_FOR_EACH (pmd, node, &dp->poll_threads) {
>+ dp_netdev_pmd_clear_poll_list(pmd);
>+ dp_netdev_reload_pmd__(pmd);
I guess this is still more than necessary. Have you thought about
reloading just touching the pmd threads involved with the ports that
changed configuration?
>+ }
>+ }
>
> CMAP_FOR_EACH (port, node, &dp->ports) {
> struct netdev *netdev = port->netdev;
>@@ -2439,15 +2452,24 @@ dpif_netdev_pmd_set(struct dpif *dpif, const char
>*cmask)
> }
> }
> }
>- /* 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_changed) {
>+ /* 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);
>+ } else {
>+ CMAP_FOR_EACH (port, node, &dp->ports) {
>+ if (netdev_is_pmd(port->netdev)) {
>+ dp_netdev_add_port_to_pmds(dp, port);
>+ }
>+ }
>+ }
> }
>
> return 0;
>--
>2.5.0
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
