[systemd-devel] [PATCH v3] Added support for Uplink Failure Detection using BindCarrier

[Rauta, Alin]

> Well, one option could be to keep a set of bound_by and bound_to links
> around for each link, and then just update that each time an interface
> comes, goes, or changes names.

Yes, but the updates need to be done for all links and I'm not sure adding this is a good thing.
I'm now having 64 links on the switch and I need the failure detection in networkd to be quite fast because however even now it's probably slower due to evaluating dynamically the BindCarrier strings when comparing this with the previous solution with an UFD group monitoring some interfaces and with some internal counters knowing exactly when to issue "link_down" for an interface. So adding "bound_by" and "bound_to" makes the solution even slower.

Besides this, having only one function "sd_network_link_get_carrier_bound_to" makes also sense because only the behavior of "bond_to" links is controlled by this feature. "bound_by" means almost nothing for an interface. A tool like "networkctl" may take into account to display only the "bound_to" links because that's what's relevant. The fact that "networkctl" displays both "bound_to" and "bound_by" it's a good thing, but it doesn't mean each tool should do that.

/Alin

-----Original Message-----
From: Lennart Poettering [mailto:lennart at poettering.net] 
Sent: Wednesday, February 11, 2015 6:03 PM
To: Rauta, Alin
Cc: zbyszek at in.waw.pl; teg at jklm.no; systemd-devel at lists.freedesktop.org; Kinsella, Ray
Subject: Re: [PATCH v3] Added support for Uplink Failure Detection using BindCarrier

On Wed, 11.02.15 17:44, Rauta, Alin (alin.rauta at intel.com) wrote:

> Hi Lennart,
> 
> > +_public_ int sd_network_link_get_carriers(int ifindex, char ***ret) {
> > +        return network_get_link_strv("CARRIERS", ifindex, ret); }
> > +
> 
> >  I think it would be better to have two calls here:
> >
> >   int sd_network_link_get_carrier_bound_to(int ifindex, int **others);
> >   int sd_network_link_get_carrier_bound_by(int ifindex, int 
> > **others);
> 
> In terms of functionality, " sd_network_link_get_carriers " is 
> actually " sd_network_link_get_carrier_bound_to" and is applicable to 
> "bound to" links just like "BindCarrier=" is available only for "bound 
> to" links. I wanted to save to systemd "run" files just minimum info.
> 
> If I add "sd_network_link_get_carrier_bound_by", then each time 
> "link_save" is called for a link I should query "BindCarrier="s for 
> all interfaces to print each link that bounds the current interface.
> Then, if I rename a link I should call "link_save" for all available 
> links because the "BindCarrier=" can be interpreted in another way due 
> to wildcards.

Well, one option could be to keep a set of bound_by and bound_to links around for each link, and then just update that each time an interface comes, goes, or changes names. That way you always have direct access to the bound links, and don't have to resolve the globs each time you need them, but only when they actually change?

The reason why I figured having two calls for this would be useful is simply to make it easy to write tools like networkctl, which wants to show this information in both directions for each interface...

> > +/* get the links that are bound to this port. */ static int 
> > +get_downlinks(const char *name,
> > +                         sd_rtnl_message *m,
> > +                         LinkInfo **downlinks,
> > +                         int *down_count) {
> 
> Regarding "get_uplinks" and "get_downlinks" I can rename them to 
> "get_links_bound_to" and "get_links_bound_by". Would this be fine ?

Sure!

But before you rework any of this, let's see what Tom has to say, he's the networkd maintainer...

Lennart

--
Lennart Poettering, Red Hat