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.

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.

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