Re: [patch net-next 07/12] mlxsw: spectrum: Add the multicast routing offloading logic

On 09/22/2017 04:21 PM, Andrew Lunn wrote:

On Fri, Sep 22, 2017 at 11:36:59AM +0300, Yotam Gigi wrote:

quoted

On 09/21/2017 06:26 PM, Andrew Lunn wrote:

quoted

quoted

+static void mlxsw_sp_mr_route_stats_update(struct mlxsw_sp *mlxsw_sp,
+					   struct mlxsw_sp_mr_route *mr_route)
+{
+	struct mlxsw_sp_mr *mr = mlxsw_sp->mr;
+	u64 packets, bytes;
+
+	if (mr_route->route_action == MLXSW_SP_MR_ROUTE_ACTION_TRAP)
+		return;
+
+	mr->mr_ops->route_stats(mlxsw_sp, mr_route->route_priv, &packets,
+				&bytes);
+
+	switch (mr_route->mr_table->proto) {
+	case MLXSW_SP_L3_PROTO_IPV4:
+		mr_route->mfc4->mfc_un.res.pkt = packets;
+		mr_route->mfc4->mfc_un.res.bytes = bytes;

What about wrong_if and lastuse? 

Hi Yotam

quoted

wronf_if is updated by ipmr as it is trapped to the CPU.

Great.

quoted

We did not address lastuse currently, though it can be easily
addressed here.

Please do. I've written multicast routing daemons, where i use it to
flush out MFCs which are no longer in use. Having it always 0 is going
to break daemons.

I will. Thanks for the feedback!

quoted

quoted

Is an mfc with iif on the host, not the switch, not offloaded?

I am not sure I followed. What do you mean MFC with iif on the host? you mean
MFC with iif that is an external NIC which is not part of the spectrum ASIC?

Yes. We probably have different perspectives on the world. To
Mellanox, everything is a switch in a box. In the DSA world, we tend
to think of having a general purpose machine which also has a switch
connected. Think of a wireless access point, set top box, passenger
entertainment system. We have applications on the general purpose
computer, we have wifi interfaces, cable modems, etc. Think about all
the different packages in LEDE. We might have a multicast video
stream, coming from the cable modem being sent over ports of the
switch to clients.

So when i look at these patches, i try to make sure the general use
cases works, not just the plain boring Ethernet switch box use cases
:-)

So when doing it, we did think about multi-ASIC situations, so I think it should
fit :)

quoted

in this case, the route will not be offloaded and all traffic will
pass in slowpath.

O.K. I was just thinking if those counters need to be +=, not =.  But
either the iif is on the host, or it is in the switch. It cannot be
both. So = is O.K.

Thanks
	Andrew