On 03/21/2018 08:28 PM, Jon Maloy wrote: > For compatibility, we would need to fields. > - One in tipc_net, as now. This is still a write-once value, and is inherited > by the bearers when needed. We rename this to cluster_id. > - One per bearer, still called net_id (bearer_tag?) or similar, which can > override the central value. Inherited from tipc_net->cluster_id if not > explicitly set in the enable command, otherwise we use the value in the > command. > > Luckily, we can use the command in the tipc tool just as it is, no need for > changes there. The changes in bearer.c should be quite limited. > > Maybe a nice little task for your new guy? 😉
Let's handle this task. Thanks, Ying > > ///jon > > >> -----Original Message----- >> From: Ying Xue [mailto:[email protected]] >> Sent: Wednesday, March 21, 2018 06:58 >> To: Jon Maloy <[email protected]>; Jon Maloy >> <[email protected]> >> Cc: [email protected]; Mohan Krishna Ghanta >> Krishnamurthy <[email protected]>; >> [email protected] >> Subject: Re: [tipc-discussion] [net-next v2 3/8] tipc: remove restrictions on >> node address values >> >> On 03/21/2018 12:20 AM, Jon Maloy wrote: >>> Hi Ying, >>> It depends on what you men with 'cluster id here, but anyway it was never >> possible. >>> >>> If you mean what we so far have called net id, it was never the intention >> that it should be possible. >>> >>> If you mean the "C" part of a Z.C.N address, it was meant to become >> possible, but luckily this was never applied. >> >> My meaning about "cluster id" is the variable of net_id in tipc_net struct: >> >> struct tipc_net { >> ... >> int net_id; >> ... >> } >> >> In other words, its initial value is set to 4711, and we can change it >> through >> netlink with TIPC_NLA_NET_ID attribute. >> >>> It is currently possible (and to my big surprise a few months ago) to set a >> node address as e.g., 5.6.7, instead of just 1.1.7 as I had believed (don't >> know >> who added this), and set up clusters isolated by a 5.6.0 discovery domain. >> However, it is not possible to change the discovery domain to e.g., 0, so >> establishing links between nodes in different clusters, e.g., 1.1.7<->5.6.7 >> is >> not possible. I tested this because I was concerned about it. >>> >>> So, should we after all accept inter-cluster links? I am not sure, but if >>> there >> is a demand it would be easy to achieve. >> >> Personally, inter-cluster links are needed especially after the feature is >> merged into upstream. >> >> It would only mean that it must become possible to set the cluster id (the >> old network id) per bearer. >>> We can then set up two different clusters with different cluster ids via >>> e.g., >> Ethernet, and then enable a UDP bearer on one node in each cluster with a >> third, common id. Those two nodes will then see each other, but nobody >> else in the opposite cluster will. >>> >>> Maybe not a bad idea... >> >> Yeah, this is exactly I think of. >> >>> >>> BR >>> ///jon >>> >>> >>>> -----Original Message----- >>>> From: Ying Xue [mailto:[email protected]] >>>> Sent: Tuesday, March 20, 2018 09:27 >>>> To: Jon Maloy <[email protected]>; Jon Maloy >> <[email protected]> >>>> Cc: [email protected]; Mohan Krishna Ghanta >>>> Krishnamurthy <[email protected]>; >>>> [email protected] >>>> Subject: Re: [tipc-discussion] [net-next v2 3/8] tipc: remove >>>> restrictions on node address values >>>> >>>> One more questions about the network_id or cluster_id: >>>> >>>> Now with the network_id or cluster_id, the nodes with different >>>> cluster_id are completely isolated. But sometimes, one of the nodes >>>> with one cluster_id needs to talk with another node with another >> cluster_id. >>>> How can we make this happened? >>>> >>>> Thanks, >>>> Ying >>>> >>>> On 03/20/2018 08:22 PM, Ying Xue wrote: >>>>> On 03/19/2018 08:10 PM, Jon Maloy wrote: >>>>>> Nominally, TIPC organizes network nodes into a three-level network >>>>>> hierarchy consisting of the levels 'zone', 'cluster' and 'node'. >>>>>> This hierarchy is reflected in the node address format, - it is >>>>>> sub-divided into an 8-bit zone id, and 12 bit cluster id, and a 12-bit >>>>>> node >> id. >>>>>> >>>>>> However, the 'zone' and 'cluster' levels have in reality never been >>>>>> fully implemented,and never will be. The result of this has been >>>>>> that the first 20 bits the node identity structure have been >>>>>> wasted, and the usable node identity range within a cluster has >>>>>> been limited to >>>>>> 12 bits. This is starting to become a problem. >>>>>> >>>>>> In the following commits, we will need to be able to connect >>>>>> between nodes which are using the whole 32-bit value space of the >> node address. >>>>>> We therefore remove the restrictions on which values can be >>>>>> assigned to node identity, -it is from now on only a 32-bit integer >>>>>> with no assumed internal structure. >>>>>> >>>>>> Isolation between clusters is now achieved only by setting >>>>>> different values for the 'network id' field used during neighbor >>>>>> discovery, in practice leading to the latter becoming the new 'cluster >>>>>> id'. >>>>> >>>>> I really like this idea that we use network_id to separate a big >>>>> cluster into several small sub-clusters. It means the TIPC network >>>>> node address structure has became from the original three-level >>>>> hierarchy to a flat mode. As the network_id is very similar to vlan >>>>> ID, it's much more flexible than previous node address structure. >>>>> >>>>>> >>>>>> The rules for accepting discovery requests/responses from >>>>>> neigboring >>>>> >>>>> s/neigboring/neighboring >>>>> >>>>>> nodes now become: >>>>>> >>>>>> - If the user has configured a discovery domain different from 0 on a >>>>>> bearer, and is using legacy address format on both peers, reception >>>>>> of discovery messages is subject to the legacy lookup domain check >>>>>> in addition to the cluster id check. >>>>>> >>>>>> - Otherwise, the discovery request/response is always accepted, >>>> provided >>>>>> both peers have the same cluster id. >>>>>> >>>>>> This secures backwards compatibility for users who have been using >>>>>> zone or cluster identites as cluster separators, instead of the >>>>>> intended >>>>> >>>>> s/identites/identities >>>>> >>>>>> 'network id'. >>>>>> >>>>>> Signed-off-by: Jon Maloy <[email protected]> >>>>>> --- >>>>>> net/tipc/addr.c | 50 >>>>>> +------------------------------------------------- >>>>>> net/tipc/addr.h | 11 ----------- >>>>>> net/tipc/bearer.c | 27 ++++----------------------- >>>>>> net/tipc/discover.c | 15 +++++++-------- >>>>>> net/tipc/link.c | 6 ++---- >>>>>> net/tipc/net.c | 4 ++-- >>>>>> net/tipc/node.c | 8 ++------ >>>>>> net/tipc/node.h | 5 +++-- >>>>>> 8 files changed, 21 insertions(+), 105 deletions(-) >>>>>> >>>>>> diff --git a/net/tipc/addr.c b/net/tipc/addr.c index >>>>>> 97cd857..dfc31a7 >>>>>> 100644 >>>>>> --- a/net/tipc/addr.c >>>>>> +++ b/net/tipc/addr.c >>>>>> @@ -39,21 +39,6 @@ >>>>>> #include "core.h" >>>>>> >>>>>> /** >>>>>> - * in_own_cluster - test for cluster inclusion; <0.0.0> always >>>>>> matches >>>>>> - */ >>>>>> -int in_own_cluster(struct net *net, u32 addr) -{ >>>>>> - return in_own_cluster_exact(net, addr) || !addr; >>>>>> -} >>>>>> - >>>>>> -int in_own_cluster_exact(struct net *net, u32 addr) -{ >>>>>> - struct tipc_net *tn = net_generic(net, tipc_net_id); >>>>>> - >>>>>> - return !((addr ^ tn->own_addr) >> 12); >>>>>> -} >>>>>> - >>>>>> -/** >>>>>> * in_own_node - test for node inclusion; <0.0.0> always matches >>>>>> */ >>>>>> int in_own_node(struct net *net, u32 addr) @@ -63,46 +48,13 @@ int >>>>>> in_own_node(struct net *net, u32 addr) >>>>>> return (addr == tn->own_addr) || !addr; } >>>>>> >>>>>> -/** >>>>>> - * tipc_addr_domain_valid - validates a network domain address >>>>>> - * >>>>>> - * Accepts <Z.C.N>, <Z.C.0>, <Z.0.0>, and <0.0.0>, >>>>>> - * where Z, C, and N are non-zero. >>>>>> - * >>>>>> - * Returns 1 if domain address is valid, otherwise 0 >>>>>> - */ >>>>>> -int tipc_addr_domain_valid(u32 addr) -{ >>>>>> - u32 n = tipc_node(addr); >>>>>> - u32 c = tipc_cluster(addr); >>>>>> - u32 z = tipc_zone(addr); >>>>>> - >>>>>> - if (n && (!z || !c)) >>>>>> - return 0; >>>>>> - if (c && !z) >>>>>> - return 0; >>>>>> - return 1; >>>>>> -} >>>>>> - >>>>>> -/** >>>>>> - * tipc_addr_node_valid - validates a proposed network address for >>>>>> this node >>>>>> - * >>>>>> - * Accepts <Z.C.N>, where Z, C, and N are non-zero. >>>>>> - * >>>>>> - * Returns 1 if address can be used, otherwise 0 >>>>>> - */ >>>>>> -int tipc_addr_node_valid(u32 addr) -{ >>>>>> - return tipc_addr_domain_valid(addr) && tipc_node(addr); >>>>>> -} >>>>>> - >>>>>> int tipc_in_scope(u32 domain, u32 addr) { >>>>>> if (!domain || (domain == addr)) >>>>>> return 1; >>>>>> if (domain == tipc_cluster_mask(addr)) /* domain <Z.C.0> */ >>>>>> return 1; >>>>>> - if (domain == tipc_zone_mask(addr)) /* domain <Z.0.0> */ >>>>>> + if (domain == (addr & TIPC_ZONE_MASK)) /* domain <Z.0.0> */ >>>>>> return 1; >>>>>> return 0; >>>>>> } >>>>>> diff --git a/net/tipc/addr.h b/net/tipc/addr.h index >>>>>> 2ecf5a5..5ffde51 >>>>>> 100644 >>>>>> --- a/net/tipc/addr.h >>>>>> +++ b/net/tipc/addr.h >>>>>> @@ -50,11 +50,6 @@ static inline u32 tipc_own_addr(struct net *net) >>>>>> return tn->own_addr; >>>>>> } >>>>>> >>>>>> -static inline u32 tipc_zone_mask(u32 addr) -{ >>>>>> - return addr & TIPC_ZONE_MASK; >>>>>> -} >>>>>> - >>>>>> static inline u32 tipc_cluster_mask(u32 addr) { >>>>>> return addr & TIPC_ZONE_CLUSTER_MASK; @@ -71,14 +66,8 @@ >>>> static >>>>>> inline int tipc_scope2node(struct net *net, int sc) } >>>>>> >>>>>> u32 tipc_own_addr(struct net *net); -int in_own_cluster(struct net >>>>>> *net, u32 addr); -int in_own_cluster_exact(struct net *net, u32 >>>>>> addr); int in_own_node(struct net *net, u32 addr); >>>>>> -u32 addr_domain(struct net *net, u32 sc); -int >>>>>> tipc_addr_domain_valid(u32); -int tipc_addr_node_valid(u32 addr); >>>>>> int tipc_in_scope(u32 domain, u32 addr); -int tipc_addr_scope(u32 >>>>>> domain); char *tipc_addr_string_fill(char *string, u32 addr); >>>>>> >>>>>> #endif >>>>>> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c index >>>>>> 76340b9..a71f318 100644 >>>>>> --- a/net/tipc/bearer.c >>>>>> +++ b/net/tipc/bearer.c >>>>>> @@ -240,7 +240,6 @@ static int tipc_enable_bearer(struct net *net, >>>> const char *name, >>>>>> struct tipc_bearer *b; >>>>>> struct tipc_media *m; >>>>>> struct sk_buff *skb; >>>>>> - char addr_string[16]; >>>>>> int bearer_id = 0; >>>>>> int res = -EINVAL; >>>>>> char *errstr = ""; >>>>>> @@ -256,21 +255,6 @@ static int tipc_enable_bearer(struct net *net, >>>> const char *name, >>>>>> goto rejected; >>>>>> } >>>>>> >>>>>> - if (tipc_addr_domain_valid(disc_domain) && disc_domain != self) >>>>>> { >>>>>> - if (tipc_in_scope(disc_domain, self)) { >>>>>> - /* Accept any node in own cluster */ >>>>>> - disc_domain = self & TIPC_ZONE_CLUSTER_MASK; >>>>>> - res = 0; >>>>>> - } else if (in_own_cluster_exact(net, disc_domain)) { >>>>>> - /* Accept specified node in own cluster */ >>>>>> - res = 0; >>>>>> - } >>>>>> - } >>>>>> - if (res) { >>>>>> - errstr = "illegal discovery domain"; >>>>>> - goto rejected; >>>>>> - } >>>>>> - >>>>>> if (prio > TIPC_MAX_LINK_PRI && prio != TIPC_MEDIA_LINK_PRI) { >>>>>> errstr = "illegal priority"; >>>>>> goto rejected; >>>>>> @@ -354,12 +338,11 @@ static int tipc_enable_bearer(struct net >>>>>> *net, >>>> const char *name, >>>>>> return -ENOMEM; >>>>>> } >>>>>> >>>>>> - tipc_addr_string_fill(addr_string, disc_domain); >>>>>> - pr_info("Enabled bearer <%s>, discovery scope %s, priority >>>>>> %u\n", >>>>>> - name, addr_string, prio); >>>>>> + pr_info("Enabled bearer <%s>, priority %u\n", name, prio); >>>>>> + >>>>>> return res; >>>>>> rejected: >>>>>> - pr_warn("Bearer <%s> rejected, %s\n", name, errstr); >>>>>> + pr_warn("Enabling of bearer <%s> rejected, %s\n", name, errstr); >>>>>> return res; >>>>>> } >>>>>> >>>>>> @@ -865,12 +848,10 @@ int __tipc_nl_bearer_enable(struct sk_buff >>>> *skb, struct genl_info *info) >>>>>> char *bearer; >>>>>> struct nlattr *attrs[TIPC_NLA_BEARER_MAX + 1]; >>>>>> struct net *net = sock_net(skb->sk); >>>>>> - struct tipc_net *tn = net_generic(net, tipc_net_id); >>>>>> - u32 domain; >>>>>> + u32 domain = 0; >>>>>> u32 prio; >>>>>> >>>>>> prio = TIPC_MEDIA_LINK_PRI; >>>>>> - domain = tn->own_addr & TIPC_ZONE_CLUSTER_MASK; >>>>> >>>>> >>>>>> >>>>>> if (!info->attrs[TIPC_NLA_BEARER]) >>>>>> return -EINVAL; >>>>>> diff --git a/net/tipc/discover.c b/net/tipc/discover.c index >>>>>> a737870..ba4abb1 100644 >>>>>> --- a/net/tipc/discover.c >>>>>> +++ b/net/tipc/discover.c >>>>>> @@ -160,18 +160,17 @@ void tipc_disc_rcv(struct net *net, struct >>>>>> sk_buff >>>> *skb, >>>>>> return; >>>>>> if (!memcmp(&maddr, &b->addr, sizeof(maddr))) >>>>>> return; >>>>>> - if (!tipc_addr_domain_valid(dst)) >>>>>> - return; >>>>>> - if (!tipc_addr_node_valid(src)) >>>>>> - return; >>>>>> if (in_own_node(net, src)) { >>>>>> disc_dupl_alert(b, self, &maddr); >>>>>> return; >>>>>> } >>>>>> - if (!tipc_in_scope(dst, self)) >>>>>> - return; >>>>>> - if (!tipc_in_scope(b->domain, src)) >>>>>> - return; >>>>>> + /* Domain filter only works if both peers use legacy address >>>>>> +format >>>> */ >>>>>> + if (b->domain) { >>>>>> + if (!tipc_in_scope(dst, self)) >>>>>> + return; >>>>>> + if (!tipc_in_scope(b->domain, src)) >>>>>> + return; >>>>>> + } >>>>>> tipc_node_check_dest(net, src, b, caps, signature, >>>>>> &maddr, &respond, &dupl_addr); >>>>>> if (dupl_addr) >>>>>> diff --git a/net/tipc/link.c b/net/tipc/link.c index >>>>>> 3c23046..86fde00 >>>>>> 100644 >>>>>> --- a/net/tipc/link.c >>>>>> +++ b/net/tipc/link.c >>>>>> @@ -434,7 +434,7 @@ char *tipc_link_name(struct tipc_link *l) >>>>>> */ >>>>>> bool tipc_link_create(struct net *net, char *if_name, int bearer_id, >>>>>> int tolerance, char net_plane, u32 mtu, int >>>>>> priority, >>>>>> - int window, u32 session, u32 ownnode, u32 peer, >>>>>> + int window, u32 session, u32 self, u32 peer, >>>>>> u16 peer_caps, >>>>>> struct tipc_link *bc_sndlink, >>>>>> struct tipc_link *bc_rcvlink, @@ -451,9 +451,7 @@ >>>>>> bool >>>>>> tipc_link_create(struct net *net, char *if_name, int bearer_id, >>>>>> l->session = session; >>>>>> >>>>>> /* Note: peer i/f name is completed by reset/activate message */ >>>>>> - sprintf(l->name, "%u.%u.%u:%s-%u.%u.%u:unknown", >>>>>> - tipc_zone(ownnode), tipc_cluster(ownnode), >>>> tipc_node(ownnode), >>>>>> - if_name, tipc_zone(peer), tipc_cluster(peer), >>>> tipc_node(peer)); >>>>>> + sprintf(l->name, "%x:%s-%x:unknown", self, if_name, peer); >>>>>> strcpy(l->if_name, if_name); >>>>>> l->addr = peer; >>>>>> l->peer_caps = peer_caps; >>>>>> diff --git a/net/tipc/net.c b/net/tipc/net.c index 5c4c440..a074f28 >>>>>> 100644 >>>>>> --- a/net/tipc/net.c >>>>>> +++ b/net/tipc/net.c >>>>>> @@ -121,7 +121,7 @@ int tipc_net_start(struct net *net, u32 addr) >>>>>> TIPC_CLUSTER_SCOPE, 0, tn->own_addr); >>>>>> >>>>>> pr_info("Started in network mode\n"); >>>>>> - pr_info("Own node address %s, network identity %u\n", >>>>>> + pr_info("Own node address %s, cluster identity %u\n", >>>>>> tipc_addr_string_fill(addr_string, tn->own_addr), >>>>>> tn->net_id); >>>>>> return 0; >>>>>> @@ -238,7 +238,7 @@ int __tipc_nl_net_set(struct sk_buff *skb, >>>>>> struct >>>> genl_info *info) >>>>>> return -EPERM; >>>>>> >>>>>> addr = nla_get_u32(attrs[TIPC_NLA_NET_ADDR]); >>>>>> - if (!tipc_addr_node_valid(addr)) >>>>>> + if (!addr) >>>>>> return -EINVAL; >>>>>> >>>>>> tipc_net_start(net, addr); >>>>>> diff --git a/net/tipc/node.c b/net/tipc/node.c index >>>>>> 389193d..8a4b049 >>>>>> 100644 >>>>>> --- a/net/tipc/node.c >>>>>> +++ b/net/tipc/node.c >>>>>> @@ -233,9 +233,6 @@ static struct tipc_node *tipc_node_find(struct >>>>>> net >>>> *net, u32 addr) >>>>>> struct tipc_node *node; >>>>>> unsigned int thash = tipc_hashfn(addr); >>>>>> >>>>>> - if (unlikely(!in_own_cluster_exact(net, addr))) >>>>>> - return NULL; >>>>>> - >>>>>> rcu_read_lock(); >>>>>> hlist_for_each_entry_rcu(node, &tn->node_htable[thash], hash) { >>>>>> if (node->addr != addr) >>>>>> @@ -836,10 +833,9 @@ void tipc_node_check_dest(struct net *net, >> u32 >>>>>> onode, >>>>>> >>>>>> /* Now create new link if not already existing */ >>>>>> if (!l) { >>>>>> - if (n->link_cnt == 2) { >>>>>> - pr_warn("Cannot establish 3rd link to %x\n", n- >>>>> addr); >>>>> >>>>> I think the warning message is still useful for users especially >>>>> when they meet some errors. >>>>> >>>>>> + if (n->link_cnt == 2) >>>>>> goto exit; >>>>>> - } >>>>>> + >>>>>> if_name = strchr(b->name, ':') + 1; >>>>>> if (!tipc_link_create(net, if_name, b->identity, >>>>>> b->tolerance, >>>>>> b->net_plane, b->mtu, >>>>>> b->priority, diff -- >>>> git >>>>>> a/net/tipc/node.h b/net/tipc/node.h index 4ce5e3a..5fb38cf 100644 >>>>>> --- a/net/tipc/node.h >>>>>> +++ b/net/tipc/node.h >>>>>> @@ -49,13 +49,14 @@ enum { >>>>>> TIPC_BCAST_STATE_NACK = (1 << 2), >>>>>> TIPC_BLOCK_FLOWCTL = (1 << 3), >>>>>> TIPC_BCAST_RCAST = (1 << 4), >>>>>> - TIPC_MCAST_GROUPS = (1 << 5) >>>>>> + TIPC_NODE_ID32 = (1 << 5) >>>>>> }; >>>>>> >>>>>> #define TIPC_NODE_CAPABILITIES (TIPC_BCAST_SYNCH | \ >>>>>> TIPC_BCAST_STATE_NACK | \ >>>>>> TIPC_BCAST_RCAST | \ >>>>>> - TIPC_BLOCK_FLOWCTL) >>>>>> + TIPC_BLOCK_FLOWCTL | \ >>>>>> + TIPC_NODE_ID32) >>>>>> #define INVALID_BEARER_ID -1 >>>>>> >>>>>> void tipc_node_stop(struct net *net); >>>>>> >>>>> >>>>> -------------------------------------------------------------------- >>>>> -- >>>>> -------- Check out the vibrant tech community on one of the world's >>>>> most engaging tech sites, Slashdot.org! http://sdm.link/slashdot >>>>> _______________________________________________ >>>>> tipc-discussion mailing list >>>>> [email protected] >>>>> https://lists.sourceforge.net/lists/listinfo/tipc-discussion >>>>> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tipc-discussion mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tipc-discussion
