On 02/12/2016 18:21, "Daniele Di Proietto" <[email protected]> wrote:
>
>
>
>
>
>On 02/12/2016 06:48, "Ilya Maximets" <[email protected]> wrote:
>
>>This reduces code duplication. Additionally dp_netdev_free
>>will not reconfigure datapath on each removed port.
>>
>>Signed-off-by: Ilya Maximets <[email protected]>
>
>I like reducing code duplication, but I'm not sure I like that
>do_del_port() calls reconfigure_datapath() and reconfigure_datapath()
>calls do_del_port(). Do you think we could avoid this "recursion"?
>
>Thanks,
>
>Daniele
Also, I forgot to mention that dp_netdev_free() is only called by
dpctl/del-dp, so I don't think is superimportant to optimize it
>
>>---
>> lib/dpif-netdev.c | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>>diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>index f6552da..8e9a623 100644
>>--- a/lib/dpif-netdev.c
>>+++ b/lib/dpif-netdev.c
>>@@ -561,7 +561,8 @@ static void dp_netdev_free(struct dp_netdev *)
>> static int do_add_port(struct dp_netdev *dp, const char *devname,
>> const char *type, odp_port_t port_no)
>> OVS_REQUIRES(dp->port_mutex);
>>-static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *)
>>+static void do_del_port(struct dp_netdev *dp, struct dp_netdev_port *,
>>+ bool port_in_datapath)
>> OVS_REQUIRES(dp->port_mutex);
>> static int dpif_netdev_open(const struct dpif_class *, const char *name,
>> bool create, struct dpif **);
>>@@ -1150,12 +1151,13 @@ dp_netdev_free(struct dp_netdev *dp)
>>
>> shash_find_and_delete(&dp_netdevs, dp->name);
>>
>>+ dp_netdev_destroy_all_pmds(dp, true);
>>+
>> ovs_mutex_lock(&dp->port_mutex);
>> HMAP_FOR_EACH_SAFE (port, next, node, &dp->ports) {
>>- do_del_port(dp, port);
>>+ do_del_port(dp, port, false);
>> }
>> ovs_mutex_unlock(&dp->port_mutex);
>>- dp_netdev_destroy_all_pmds(dp, true);
>> cmap_destroy(&dp->poll_threads);
>>
>> ovs_mutex_destroy(&dp->non_pmd_mutex);
>>@@ -1399,7 +1401,7 @@ dpif_netdev_port_del(struct dpif *dpif, odp_port_t
>>port_no)
>>
>> error = get_port_by_number(dp, port_no, &port);
>> if (!error) {
>>- do_del_port(dp, port);
>>+ do_del_port(dp, port, true);
>> }
>> }
>> ovs_mutex_unlock(&dp->port_mutex);
>>@@ -1495,13 +1497,16 @@ has_pmd_port(struct dp_netdev *dp)
>> }
>>
>> static void
>>-do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port)
>>+do_del_port(struct dp_netdev *dp, struct dp_netdev_port *port,
>>+ bool port_in_datapath)
>> OVS_REQUIRES(dp->port_mutex)
>> {
>> hmap_remove(&dp->ports, &port->node);
>> seq_change(dp->port_seq);
>>
>>- reconfigure_datapath(dp);
>>+ if (port_in_datapath) {
>>+ reconfigure_datapath(dp);
>>+ }
>>
>> port_destroy(port);
>> }
>>@@ -3243,17 +3248,12 @@ reconfigure_datapath(struct dp_netdev *dp)
>> * not being used by any pmd thread at the moment. If a port fails to
>> * reconfigure we remove it from the datapath. */
>> HMAP_FOR_EACH(port, node, &dp->ports) {
>>- int err;
>>-
>> if (!port->need_reconfigure) {
>> continue;
>> }
>>
>>- err = port_reconfigure(port);
>>- if (err) {
>>- hmap_remove(&dp->ports, &port->node);
>>- seq_change(dp->port_seq);
>>- port_destroy(port);
>>+ if (port_reconfigure(port)) {
>>+ do_del_port(dp, port, false);
>> } else {
>> port->dynamic_txqs = netdev_n_txq(port->netdev) < wanted_txqs;
>> }
>>--
>>2.7.4
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
