Hi Alex,
On Wed, 2014-03-05 at 16:34 -0800, Alex Wang wrote:
> @@ -352,17 +358,45 @@ struct dpif_class {
> /* Enables or disables receiving packets with dpif_recv() for 'dpif'.
> * Turning packet receive off and then back on is allowed to change
> Netlink
> * PID assignments (see ->port_get_pid()). The client is responsible for
> - * updating flows as necessary if it does this. */
> - int (*recv_set)(struct dpif *dpif, bool enable);
> + * updating flows as necessary if it does this.
> + *
> + * The 'n_handlers' specifies the number of upcall handlers there are.
> + * Since multiple upcall handlers can read upcalls simultaneously from
> + * 'dpif', each port can have multiple Netlink sockets, one per upcall
> + * handler. And recv_set() is responsible for the following tasks:
> + *
> + * When receiving is enabled:
> + *
> + * - create 'n_handlers' Netlink sockets for each port.
> + *
> + * - create 'n_handlers' poll loops, one for each upcall handler.
> + *
> + * - registering the Netlink sockets for the same upcall handler to
> + * the corresponding poll loop.
> + *
> + * When receiving is disabled:
> + *
> + * - the opposite of above.
> + *
> + * */
> + int (*recv_set)(struct dpif *dpif, bool enable, uint32_t n_handlers);
> +
> + /* Refreshes the poll loops and Netlink sockets associated to each port,
> + * when the number of upcall handlers is changed to 'n_handlers' and
> + * receiving packets is enabled for 'dpif'. */
> + int (*handlers_set)(struct dpif *dpif, uint32_t n_handlers);
Now since you already have a separate interface handlers_set() to update
n_handlers, why still keep the n_handlers parameter in recv_set()? This
makes the meaning of recv_set() unclear when it is called with "enabled"
state equal to current recv state. See the implementation of
dpif_linux_recv_set__() in dpif-linux.c in patch 4/5.
In my opinion it would be more clear if:
- handlers_set() is responsible for setting/updating handler number and
refreshing channels if recv is already enabled.
- recv_set() only enable/disables receiving. When enabling, use
current/default n_handlers to create channels.
Best regards,
Han
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
