On Sat, May 31, 2014 at 01:53:15AM -0300, Flavio Leitner wrote:
> This patch adds generic IGMP snooping library code
> that is used in follow-up patches.
>
> Signed-off-by: Cong Wang <amw...@redhat.com>
> Signed-off-by: Daniel Borkmann <dbork...@redhat.com>
> Acked-by: Thomas Graf <tg...@redhat.com>
> Signed-off-by: Flavio Leitner <f...@redhat.com>
Thanks for the revision!
In mcast_snooping_prune_expired(), I think that we can "break" out of
the loop after we reach a bundle that has not expired (the list is
sorted on expiration time, right?).
MCAST_MROUTER_PORT_DEFAULT_IDLE_TIME is actually the fixed idle time
that is always used, so I would consider removing "default" from its
name.
My remaining comments are stylistic. I think it's easiest to just
suggest folding in the following incremental patch; let me know if
some of them don't make sense.
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index 0bf5195..8416753 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -40,7 +40,8 @@ COVERAGE_DEFINE(mcast_snooping_expired);
static struct mcast_mrouter_bundle *
mcast_snooping_mrouter_lookup(struct mcast_snooping *ms, uint16_t vlan,
- void *port);
+ void *port)
+ OVS_REQ_RDLOCK(ms->rwlock);
bool
mcast_snooping_enabled(const struct mcast_snooping *ms)
@@ -57,10 +58,7 @@ mcast_snooping_flood_unreg(const struct mcast_snooping *ms)
bool
mcast_snooping_is_query(ovs_be16 igmp_type)
{
- if (igmp_type == htons(IGMP_HOST_MEMBERSHIP_QUERY)) {
- return true;
- }
- return false;
+ return igmp_type == htons(IGMP_HOST_MEMBERSHIP_QUERY);
}
bool
@@ -75,8 +73,8 @@ mcast_snooping_is_membership(ovs_be16 igmp_type)
return false;
}
-/* Returns the number of seconds since the multicast group
- * was learned in a port */
+/* Returns the number of seconds since multicast group 'b' was learned in a
+ * port on 'ms'. */
int
mcast_bundle_age(const struct mcast_snooping *ms,
const struct mcast_group_bundle *b)
@@ -89,7 +87,7 @@ static uint32_t
mcast_table_hash(const struct mcast_snooping *ms, ovs_be32 grp_ip4,
uint16_t vlan)
{
- return hash_3words(ntohl(grp_ip4), vlan, ms->secret);
+ return hash_3words((OVS_FORCE uint32_t) grp_ip4, vlan, ms->secret);
}
static struct mcast_group_bundle *
@@ -105,7 +103,7 @@ mcast_group_from_lru_node(struct list *list)
}
/* Searches 'ms' for and returns an mcast group for destination address
- * 'dip' in 'vlan' */
+ * 'dip' in 'vlan'. */
struct mcast_group *
mcast_snooping_lookup(const struct mcast_snooping *ms, ovs_be32 dip,
uint16_t vlan)
@@ -148,7 +146,7 @@ normalize_idle_time(unsigned int idle_time)
}
/* Creates and returns a new mcast table with an initial mcast aging
- * timeout of 'idle_time' seconds and an initial maximum of
+ * timeout of MCAST_ENTRY_DEFAULT_IDLE_TIME seconds and an initial maximum of
* MCAST_DEFAULT_MAX entries. */
struct mcast_snooping *
mcast_snooping_create(void)
@@ -198,8 +196,7 @@ mcast_snooping_unref(struct mcast_snooping *ms)
/* Changes the mcast aging timeout of 'ms' to 'idle_time' seconds. */
void
-mcast_snooping_set_idle_time(struct mcast_snooping *ms,
- unsigned int idle_time)
+mcast_snooping_set_idle_time(struct mcast_snooping *ms, unsigned int idle_time)
OVS_REQ_WRLOCK(ms->rwlock)
{
struct mcast_group *grp;
@@ -233,8 +230,8 @@ mcast_snooping_set_max_entries(struct mcast_snooping *ms,
/* Sets if unregistered multicast packets should be flooded to
* all ports or only to ports connected to multicast routers
*
- * return true if previous state differs from current state
- * return false otherwise. */
+ * Returns true if previous state differs from current state,
+ * false otherwise. */
bool
mcast_snooping_set_flood_unreg(struct mcast_snooping *ms, bool enable)
OVS_REQ_WRLOCK(ms->rwlock)
@@ -282,8 +279,8 @@ mcast_group_insert_bundle(struct mcast_snooping *ms
OVS_UNUSED,
return b;
}
-/* Return true if multicast still has bundles associated
- * Return false if there is no bundles */
+/* Return true if multicast still has bundles associated.
+ * Return false if there is no bundles. */
static bool
mcast_group_has_bundles(struct mcast_group *grp)
{
@@ -291,7 +288,7 @@ mcast_group_has_bundles(struct mcast_group *grp)
}
/* Delete 'grp' from the 'ms' hash table.
- * Caller is responsible to clean bundle lru first */
+ * Caller is responsible to clean bundle lru first. */
static void
mcast_snooping_flush_group__(struct mcast_snooping *ms,
struct mcast_group *grp)
@@ -318,8 +315,8 @@ mcast_snooping_flush_group(struct mcast_snooping *ms,
struct mcast_group *grp)
}
-/* Delete bundle returning true if it succeed
- * false if it didn't find the group */
+/* Delete bundle returning true if it succeeds,
+ * false if it didn't find the group. */
static bool
mcast_group_delete_bundle(struct mcast_snooping *ms OVS_UNUSED,
struct mcast_group *grp, void *port)
@@ -337,7 +334,8 @@ mcast_group_delete_bundle(struct mcast_snooping *ms
OVS_UNUSED,
return false;
}
-/* check if any bundle has expired and delete it */
+/* If any bundle has expired, delete it. Returns the number of deleted
+ * bundles. */
static int
mcast_snooping_prune_expired(struct mcast_snooping *ms,
struct mcast_group *grp)
@@ -372,14 +370,14 @@ mcast_snooping_prune_expired(struct mcast_snooping *ms,
* move to the last position in the LRU list.
*/
bool
-mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4, uint16_t
vlan,
- void *port)
+mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock)
{
bool learned;
struct mcast_group *grp;
- /* avoid duplicate packets */
+ /* Avoid duplicate packets. */
if (mcast_snooping_mrouter_lookup(ms, vlan, port)
|| mcast_snooping_fport_lookup(ms, vlan, port)) {
return false;
@@ -407,6 +405,7 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4, uint16_t vlan,
list_remove(&grp->group_node);
}
mcast_group_insert_bundle(ms, grp, port, ms->idle_time);
+
/* Mark 'grp' as recently used. */
list_push_back(&ms->group_lru, &grp->group_node);
return learned;
@@ -428,10 +427,10 @@ mcast_snooping_leave_group(struct mcast_snooping *ms,
ovs_be32 ip4,
}
-/* Router ports */
+/* Router ports. */
/* Returns the number of seconds since the multicast router
- * was learned in a port */
+ * was learned in a port. */
int
mcast_mrouter_age(const struct mcast_snooping *ms OVS_UNUSED,
const struct mcast_mrouter_bundle *mrouter)
@@ -485,7 +484,7 @@ mcast_snooping_add_mrouter(struct mcast_snooping *ms,
uint16_t vlan,
{
struct mcast_mrouter_bundle *mrouter;
- /* avoid duplicate packets */
+ /* Avoid duplicate packets. */
if (mcast_snooping_fport_lookup(ms, vlan, port)) {
return false;
}
@@ -512,9 +511,8 @@ mcast_snooping_flush_mrouter(struct mcast_mrouter_bundle
*mrouter)
list_remove(&mrouter->mrouter_node);
free(mrouter);
}
-
-/* Flood ports */
+/* Flood ports. */
static struct mcast_fport_bundle *
mcast_fport_from_list_node(struct list *list)
@@ -582,16 +580,14 @@ mcast_snooping_set_port_flood(struct mcast_snooping *ms,
uint16_t vlan,
if (flood && !fport) {
mcast_snooping_add_fport(ms, vlan, port);
ms->need_revalidate = true;
- }
-
- if (!flood && fport) {
+ } else if (!flood && fport) {
mcast_snooping_flush_fport(fport);
ms->need_revalidate = true;
}
}
-
-/* Run and Flush */
+/* Run and flush. */
+
static void
mcast_snooping_mdb_flush__(struct mcast_snooping *ms)
OVS_REQ_WRLOCK(ms->rwlock)
@@ -599,13 +595,13 @@ mcast_snooping_mdb_flush__(struct mcast_snooping *ms)
struct mcast_group *grp;
struct mcast_mrouter_bundle *mrouter;
- while (group_get_lru(ms, &grp)){
+ while (group_get_lru(ms, &grp)) {
mcast_snooping_flush_group(ms, grp);
}
hmap_shrink(&ms->table);
- while (mrouter_get_lru(ms, &mrouter)){
+ while (mrouter_get_lru(ms, &mrouter)) {
mcast_snooping_flush_mrouter(mrouter);
}
}
@@ -622,7 +618,7 @@ mcast_snooping_mdb_flush(struct mcast_snooping *ms)
ovs_rwlock_unlock(&ms->rwlock);
}
-/* flushes mdb and flood ports */
+/* Flushes mdb and flood ports. */
static void
mcast_snooping_flush__(struct mcast_snooping *ms)
OVS_REQ_WRLOCK(ms->rwlock)
@@ -631,17 +627,17 @@ mcast_snooping_flush__(struct mcast_snooping *ms)
struct mcast_mrouter_bundle *mrouter;
struct mcast_fport_bundle *fport;
- while (group_get_lru(ms, &grp)){
+ while (group_get_lru(ms, &grp)) {
mcast_snooping_flush_group(ms, grp);
}
hmap_shrink(&ms->table);
- while (mrouter_get_lru(ms, &mrouter)){
+ while (mrouter_get_lru(ms, &mrouter)) {
mcast_snooping_flush_mrouter(mrouter);
}
- while (fport_get(ms, &fport)){
+ while (fport_get(ms, &fport)) {
mcast_snooping_flush_fport(fport);
}
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
