> -----Original Message-----
> From: Partha [mailto:[email protected]]
> Sent: Wednesday, April 04, 2018 03:01
> 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 1/1] tipc: introduce ioctl for 
> fetching
> node identity
> 
> 
> On 2018-04-03 12:22, Jon Maloy wrote:
> > Hi Partha,
> > Actually, I didn't consider that option. What I did consider, was to add new
> commands for 'tipc node list' and 'tipc nametable show' along the existing
> ones, but I found it would be too much duplicated code. I also thought that
> regular user programs (others than the 'tipc' tool) might want to
> programmatically know this mapping, so I first considered a getsockopt, but
> those aren't really made for this.  I know that ioctl() used to be considered
> deprecated(), but that is not the case any more AFAIK, and I feel it is 
> simpler
> to use than netlink (i.e., no callback function) for a program which only 
> needs
> to quickly check the id of a node, given the hash address.
> Yeah, the programs can use getsockname() to get their own hash and
> getpeername() for peer hash.
> 
> But these come with restrictions that the peername can be fetched only for
> connected sockets etc. For dgram, you need to get the peer hash via netlink
> only. This makes their usage inconsistent.
> 
> > Can you see nay obvious advantages with using netlink here?
> Its not about advantages but more about design consistency. With the use of
> a single tool nltrace (~tcpdump for netlink), i have debugged most of the
> netlink configuration errors.
> 
> Especially we in tipc community should avoid introducing different
> mechanism to fetch different attributes, otherwise we will give way for
> others to follow.

The point is, I see this as a part of the runtime API, not the maintenance API.
But I will give this a second thought. Net-next is closed now, so there is some 
time.

> 
> If its done for the ease of use, then we should provide a library for tipc
> netlink api's in tipcutils.
> 
> I dont have any other opinion, feel free to proceed with the series.
> 
> I see sudden burst of recent changes in TIPC, seems like you are in full flow
> reducing the backlog.

I think we are almost done. 
I put my idea about an all-128-bit address on hold for now, since it seems to 
be very intrusive. I may come back to it later if I see a demand, and if I find 
a way of 
The remaining part now is the link layer bypass, which is still only at the 
experimental level. That will probably take some time.

Regards
///jon


> 
> regards
> Partha
> >
> > ///jon
> >
> >
> >> -----Original Message-----
> >> From: Partha [mailto:[email protected]]
> >> Sent: Tuesday, April 03, 2018 02:39
> >> 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 1/1] tipc: introduce ioctl
> >> for fetching node identity
> >>
> >> On 2018-03-30 01:15, 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.
> >>>
> >> Why don't we extend the Node info to include a new type
> >> TIPC_NLA_NODE_FULL_IDENTITY, or something like that.
> >> If the user space understand this new type, he processes it else discards
> it.
> >> The user doesn't need to open a different socket (family
> >> TIPC) for ioctl().
> >>
> >> I would prefer that we continue to use the scalable netlink instead of
> ioctl().
> >>
> >> regards
> >> Partha
> >>
> >>> Signed-off-by: Jon Maloy <[email protected]>
> >>> ---
> >>>    include/uapi/linux/tipc.h |  9 ++++++---
> >>>    net/tipc/node.c           | 16 ++++++++++++++++
> >>>    net/tipc/node.h           |  1 +
> >>>    net/tipc/socket.c         | 13 +++++++++++--
> >>>    4 files changed, 34 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h
> >>> index bf6d286..e24e068 100644
> >>> --- a/include/uapi/linux/tipc.h
> >>> +++ b/include/uapi/linux/tipc.h
> >>> @@ -209,16 +209,15 @@ 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
> >>> - *
> >>>     */
> >>> -
> >>> +#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 SIOCGETNODEID          (SIOCPROTOPRIVATE + 1)
> >>>
> >>>    struct tipc_sioc_ln_req {
> >>>           __u32 peer;
> >>> @@ -226,6 +225,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..45a2d86
> >>> 100644
> >>> --- a/net/tipc/node.c
> >>> +++ b/net/tipc/node.c
> >>> @@ -195,6 +195,22 @@ 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) {
> >>> + struct tipc_node *n;
> >>> +
> >>> + if (!addr || addr == tipc_own_addr(net)) {
> >>> +         memcpy(id, tipc_own_id(net), 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
> >>> 3e5eba3..468c2dc 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;
> >>>           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

Reply via email to