After discussing with Pravin offline, it is good to use first get the
reference
and then access the 'port->netdev' for pmd check.

So, use a nested if statement:

@@ -1864,20 +1874,27 @@ pmd_load_queues(struct pmd_thread *f,
     index = 0;

     CMAP_FOR_EACH (port, node, &f->dp->ports) {
-        if (netdev_is_pmd(port->netdev)) {
-            int i;
-
-            for (i = 0; i < netdev_n_rxq(port->netdev); i++) {
-                if ((index % dp->n_pmd_threads) == id) {
-                    poll_list = xrealloc(poll_list, sizeof *poll
-
-                    port_ref(port);
-                    poll_list[poll_cnt].port = port;
-                    poll_list[poll_cnt].rx = port->rxq[i];
-                    poll_cnt++;
+        /* Calls port_try_ref() to prevent the main thread
+         * from deleting the port. */
+        if (port_try_ref(port)) {
+            if (netdev_is_pmd(port->netdev)) {
+                int i;
+
+                for (i = 0; i < netdev_n_rxq(port->netdev); i++)
+                    if ((index % dp->n_pmd_threads) == id) {
+                        poll_list = xrealloc(poll_list,
+                                        sizeof *poll_list * (pol
+
+                        port_ref(port);
+                        poll_list[poll_cnt].port = port;
+                        poll_list[poll_cnt].rx = port->rxq[i];
+                        poll_cnt++;
+                    }
+                    index++;
                 }
-                index++;
             }
+            /* Unrefs the port_try_ref(). */
+            port_unref(port);
         }

Will apply to master soon,


On Tue, Sep 2, 2014 at 9:39 PM, Alex Wang <al...@nicira.com> wrote:

> > @@ -1864,7 +1874,10 @@ pmd_load_queues(struct pmd_thread *f,
>>  >      index = 0;
>> >
>> >      CMAP_FOR_EACH (port, node, &f->dp->ports) {
>> > -        if (netdev_is_pmd(port->netdev)) {
>> > +        /* Calls port_try_ref() to prevent the main thread
>> > +         * from deleting the port. */
>> > +        if (netdev_is_pmd(port->netdev)
>> > +            && port_try_ref(port)) {
>> >              int i;
>> >
>> port_try_ref() check should be first in the condition.
>>
>> Otherwise looks good.
>> Acked-by: Pravin B Shelar <pshe...@nicira.com>
>>
>>
>
> Could you explain more about why?  if port_try_ref() is called first,
> then we will try reference the both dpdk and non_dpdk port.  And
> we need to un-reference the port outside the if statement.
>
> However, when we try to un-reference the port, we dont know if the
> reference succeeds.
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to