On 04/18/2018 10:43 PM, Jon Maloy wrote: > After the introduction of a 128-bit node identity it may be difficult > for a user to correlate between this identity and the generated node > hash address. > > We now try to make this easier by introducing a new ioctl() call for > fetching a node identity by using the hash value as key. This will > be particularly useful when we extend some of the commands in the > 'tipc' tool, but we also expect regular user applications to need > this feature. > > --- > v2: Fixed a crash that happened when fetching an unitialized id. > > Signed-off-by: Jon Maloy <[email protected]>
Apart from the following minor comment, other is good with me. Acked-by: Ying Xue <[email protected]> > --- > include/uapi/linux/tipc.h | 12 ++++++++---- > net/tipc/node.c | 21 +++++++++++++++++++++ > net/tipc/node.h | 1 + > net/tipc/socket.c | 13 +++++++++++-- > 4 files changed, 41 insertions(+), 6 deletions(-) > > diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h > index bf6d286..6b2fd4d 100644 > --- a/include/uapi/linux/tipc.h > +++ b/include/uapi/linux/tipc.h > @@ -209,16 +209,16 @@ struct tipc_group_req { > * The string formatting for each name element is: > * media: media > * interface: media:interface name > - * link: Z.C.N:interface-Z.C.N:interface > - * > + * link: node:interface-node:interface > */ > - > +#define TIPC_NODEID_LEN 16 > #define TIPC_MAX_MEDIA_NAME 16 > #define TIPC_MAX_IF_NAME 16 > #define TIPC_MAX_BEARER_NAME 32 > #define TIPC_MAX_LINK_NAME 68 > > -#define SIOCGETLINKNAME SIOCPROTOPRIVATE > +#define SIOCGETLINKNAME SIOCPROTOPRIVATE > +#define SIOCGETNODEID (SIOCPROTOPRIVATE + 1) > > struct tipc_sioc_ln_req { > __u32 peer; > @@ -226,6 +226,10 @@ struct tipc_sioc_ln_req { > char linkname[TIPC_MAX_LINK_NAME]; > }; > > +struct tipc_sioc_nodeid_req { > + __u32 peer; > + char node_id[TIPC_NODEID_LEN]; > +}; > > /* The macros and functions below are deprecated: > */ > diff --git a/net/tipc/node.c b/net/tipc/node.c > index c77dd2f..b0ab78f 100644 > --- a/net/tipc/node.c > +++ b/net/tipc/node.c > @@ -195,6 +195,27 @@ int tipc_node_get_mtu(struct net *net, u32 addr, u32 sel) > return mtu; > } > > +bool tipc_node_get_id(struct net *net, u32 addr, u8 *id) > +{ > + u8 *own_id = tipc_own_id(net); > + struct tipc_node *n; > + > + if (!own_id) > + return true; > + > + if (addr == tipc_own_addr(net)) { > + memcpy(id, own_id, TIPC_NODEID_LEN); > + return true; > + } > + n = tipc_node_find(net, addr); > + if (!n) > + return false; > + > + memcpy(id, &n->peer_id, TIPC_NODEID_LEN); > + tipc_node_put(n); > + return true; > +} > + > u16 tipc_node_get_capabilities(struct net *net, u32 addr) > { > struct tipc_node *n; > diff --git a/net/tipc/node.h b/net/tipc/node.h > index f24b835..22fc852 100644 > --- a/net/tipc/node.h > +++ b/net/tipc/node.h > @@ -60,6 +60,7 @@ enum { > #define INVALID_BEARER_ID -1 > > void tipc_node_stop(struct net *net); > +bool tipc_node_get_id(struct net *net, u32 addr, u8 *id); > u32 tipc_node_try_addr(struct net *net, u8 *id, u32 addr); > void tipc_node_check_dest(struct net *net, u32 onode, u8 *peer_id128, > struct tipc_bearer *bearer, > diff --git a/net/tipc/socket.c b/net/tipc/socket.c > index 1fd1c8b..61651f3 100644 > --- a/net/tipc/socket.c > +++ b/net/tipc/socket.c > @@ -2971,7 +2971,8 @@ static int tipc_getsockopt(struct socket *sock, int > lvl, int opt, > > static int tipc_ioctl(struct socket *sock, unsigned int cmd, unsigned long > arg) > { > - struct sock *sk = sock->sk; > + struct net *net = sock_net(sock->sk); > + struct tipc_sioc_nodeid_req nr = {0}; In my opinion, it seems unnecessary to initialize "nr" variable with 0. When we copy the data pointed by argp pointer to nr with copy_from_user(), its copy data length is sizeof(nr). It means the entire nr variable will be overwritten by argp. > struct tipc_sioc_ln_req lnr; > void __user *argp = (void __user *)arg; > > @@ -2979,7 +2980,7 @@ static int tipc_ioctl(struct socket *sock, unsigned int > cmd, unsigned long arg) > case SIOCGETLINKNAME: > if (copy_from_user(&lnr, argp, sizeof(lnr))) > return -EFAULT; > - if (!tipc_node_get_linkname(sock_net(sk), > + if (!tipc_node_get_linkname(net, > lnr.bearer_id & 0xffff, lnr.peer, > lnr.linkname, TIPC_MAX_LINK_NAME)) { > if (copy_to_user(argp, &lnr, sizeof(lnr))) > @@ -2987,6 +2988,14 @@ static int tipc_ioctl(struct socket *sock, unsigned > int cmd, unsigned long arg) > return 0; > } > return -EADDRNOTAVAIL; > + case SIOCGETNODEID: > + if (copy_from_user(&nr, argp, sizeof(nr))) > + return -EFAULT; > + if (!tipc_node_get_id(net, nr.peer, nr.node_id)) > + return -EADDRNOTAVAIL; > + if (copy_to_user(argp, &nr, sizeof(nr))) > + return -EFAULT; > + return 0; > default: > return -ENOIOCTLCMD; > } > ------------------------------------------------------------------------------ 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
