On Tue, Oct 01, 2013 at 01:36:14PM -0700, Alex Wang wrote:
> This commit adds a new module ofproto-dpif-monitor in ofproto
> directory. This module is in charge of executing the periodic
> functions of monitoring code (e.g. bfd and cfm).
>
> Signed-off-by: Alex Wang <[email protected]>
The comment here might more specifically say that it "owns" references
to structs (it's obvious that it contains them):
> +/* Monitored port. It contains references to ofport, bfd, cfm structs. */
> +struct mport {
This is in 'monitor_hmap' specifically, right? Then I'd make the
comment say that instead of the vaguer "In monitor's hmap":
> + struct hmap_node hmap_node; /* In monitor's hmap. */
> + const struct ofport_dpif *ofport; /* The corresponding ofport. */
> +
> + struct cfm *cfm; /* Reference to cfm. */
> + struct bfd *bfd; /* Reference to bfd. */
> + uint8_t hw_addr[OFP_ETH_ALEN]; /* Hardware address. */
> +};
This comment might say that the hmap contains "struct mport"s:
> +/* hmap that contains all monitored port. */
> +static struct hmap monitor_hmap = HMAP_INITIALIZER(&monitor_hmap);
The new comments in xlate_ofport_set() and xlate_ofport_remove() seem
to me to not really be that helpful. I would consider removing them.
Thanks,
Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
