Re: [Linuxptp-devel] [PATCH 1/8] rtnl: use ifinfomsg instead of rtgenmsg to request link info

[Hangbin L. <liu...@gm...>]

On Fri, Jun 30, 2017 at 08:44:36AM +0200, Richard Cochran wrote:
> On Fri, Jun 30, 2017 at 08:18:15AM +0200, Richard Cochran wrote:
> > At that time, I added one extra FD into clock->pollfdp[], and the
> > clock then farms out the information to the ports.  I really did not
> > to do it that way.  A better way would have been to add a new per-port
> > FD into fd.h, something like this:
> 
> I meant to say, "I really did not want to do that way."
> 
> I can't remember whether I even knew about the possibility of using
> ifinfomsg.ifi_index to bind to a particular interface.  At the time, I
> either overlooked it or I might have rejected this because of lack of
> portability to kernels before v3.7.

I guess I know why you didn't use ifinfomsg.ifi_index now. Because you didn't
need it...

After check, I find ifinfomsg.ifi_index only used to get spedific interface's
info, just as we know. But it could not get specific interface's notify.

The reason we could get interface link up/down notify is we registered sa
group with RTMGRP_LINK/RTNLGRP_LINK. So whenever a interface changed, we
will get the notify.

Before this patch set. We don't need the interface info, and only care about
the link status. So you don't need ifinfomsg when call rtnl_link_query().

Now since we need to know the interface's active slave info. I update
rtnl_link_query() to use ifinfomsg to get the interface slave info.

But for link status monitor, ifinfomsg.ifi_index doesn't do any help. We still
need to register with groupp RTNLGRP_LINK and keep receive all interfaces'
information.

> 
> 1. remove the rtnl socket from clock.c
> 
> 2. add one rtnl socket per port.
> 

Back to this question, do you still want to move rtnl socket from clock to
per port?

I think we can filter the call back if index to make sure we get correct
interface, something like

+static void port_link_status(void *ctx, int index, int linkup)
+{
+       struct port *p = ctx;
+
+       if (index != if_nametoindex(p->name) && p->link_status != linkup)
+               return;
+
+       p->link_status = linkup;
+       pr_notice("port %hu: link %s", portnum(p), linkup ? "up" : "down");
+
+       if (linkup)
+               port_dispatch(p, EV_FAULT_CLEARED, 0);
+       else
+               port_dispatch(p, EV_FAULT_DETECTED, 0);
+}

But this should have no relation with the failover patch set.

Thanks
Hangbin