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 <al...@nicira.com>

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
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to