Re: [PATCH net-next 3/4 v4] amt: add multicast(IGMP) report message handler

Hi David,
Thank you for your review!

On 10/28/21 12:08 AM, David Ahern wrote:

On 10/26/21 9:10 AM, Taehee Yoo wrote:

quoted

+static bool amt_status_filter(struct amt_source_node *snode,
+			      enum amt_filter filter)
+{

How about:
	bool rc = false;

and then

quoted

+	switch (filter) {
+	case AMT_FILTER_FWD:
+		if (snode->status == AMT_SOURCE_STATUS_FWD &&
+		    snode->flags == AMT_SOURCE_OLD)
+			rc = true;
+		break;

similar change for the rest of the cases.

Thanks, I will use it.

quoted

+	case AMT_FILTER_D_FWD:
+		if (snode->status == AMT_SOURCE_STATUS_D_FWD &&
+		    snode->flags == AMT_SOURCE_OLD)
+			return true;
+		else
+			return false;
+	case AMT_FILTER_FWD_NEW:
+		if (snode->status == AMT_SOURCE_STATUS_FWD &&
+		    snode->flags == AMT_SOURCE_NEW)
+			return true;
+		else
+			return false;
+	case AMT_FILTER_D_FWD_NEW:
+		if (snode->status == AMT_SOURCE_STATUS_D_FWD &&
+		    snode->flags == AMT_SOURCE_NEW)
+			return true;
+		else
+			return false;
+	case AMT_FILTER_ALL:
+		return true;
+	case AMT_FILTER_NONE_NEW:
+		if (snode->status == AMT_SOURCE_STATUS_NONE &&
+		    snode->flags == AMT_SOURCE_NEW)
+			return true;
+		else
+			return false;
+	case AMT_FILTER_BOTH:
+		if ((snode->status == AMT_SOURCE_STATUS_D_FWD ||
+		     snode->status == AMT_SOURCE_STATUS_FWD) &&
+		    snode->flags == AMT_SOURCE_OLD)
+			return true;
+		else
+			return false;
+	case AMT_FILTER_BOTH_NEW:
+		if ((snode->status == AMT_SOURCE_STATUS_D_FWD ||
+		     snode->status == AMT_SOURCE_STATUS_FWD) &&
+		    snode->flags == AMT_SOURCE_NEW)
+			return true;
+		else
+			return false;
+	default:
+		return false;
+	}
+
+	return false;
+}
+

quoted

+
+/* If a source timer expires with a router filter-mode for the group of
+ * INCLUDE, the router concludes that traffic from this particular
+ * source is no longer desired on the attached network, and deletes the
+ * associated source record.
+ */
+static void amt_source_work(struct work_struct *work)
+{
+	struct amt_source_node *snode = container_of(to_delayed_work(work),
+						     struct amt_source_node,
+						     source_timer);
+	struct amt_group_node *gnode = snode->gnode;
+	struct amt_dev *amt = gnode->amt;
+	struct amt_tunnel_list *tunnel;
+
+	tunnel = gnode->tunnel_list;
+	spin_lock_bh(&tunnel->lock);
+	rcu_read_lock();
+	if (gnode->filter_mode == MCAST_INCLUDE) {
+		amt_destroy_source(snode);
+		if (!gnode->nr_sources)
+			amt_del_group(amt, gnode);
+	} else {
+/* When a router filter-mode for a group is EXCLUDE, source records are
+ * only deleted when the group timer expires
+ */

comment needs to be indented.

Okay, I will fix it.

quoted

+		snode->status = AMT_SOURCE_STATUS_D_FWD;
+	}
+	rcu_read_unlock();
+	spin_unlock_bh(&tunnel->lock);
+}
+

Thanks a lot for your reviews.
I will fix these things and send v2 patch.

Thanks,
Taehee