On 2016-06-08 21:40, Jon Maloy wrote:
> 
> 
>> -----Original Message-----
>> From: Parthasarathy Bhuvaragan
>> Sent: Thursday, 02 June, 2016 10:10
>> To: tipc-discussion@lists.sourceforge.net; Jon Maloy; ma...@donjonn.com; Ying
>> Xue; Richard Alpe
>> Subject: [PATCH iproute2 v3 09/10] tipc: add link monitor list
>>
>> In this commit, we list the monitor attributes for the specified
>> bearer.
>> A sample usage is shown below:
>> $ tipc link monitor list media eth dev data0-br
>> Node          S D Dom Applied Node Status [Non-Applied Node:Status]
> 
> Despite being the originator of this algorithm, I was unable to understand 
> this list until I looked at the man page. That is a bad sign.
> I understand your wish to keep the left half of the table as narrow as 
> possible, to leave more space for the "applied" column, but I think this can 
> be done better.
> 
> -  Instead of a column with the header "D" and values "Y/N", I would suggest 
> "M" (for "Montored" and "D/I" (for "Direct/Indirect"). Or better, use the 
> indentation method, as I  did. I suspect Richard might have objections to 
> this because it makes it more difficult to parse, but I cannot understand 
> that an extra space at the beginning of some lines really can cause any 
> serious problems.
It's not the extra space that makes anything harder, it's the implicit
grouping of data.

Parsability is important in the sense that the output should be as
unified and as deterministic as possible. It benefits both users and
parsers.

Key value pairs are nice to parse as it gives the user a reference point
and when displaying data in tables the column count needs to be
deterministic.

If certain information in the output can have an arbitrary size and are
likely to grow you might find no other way to display it than implicitly
grouping it, such as:

foo:bar
  key1 x
  key2 y
  keyn z
foo:baz
  key1 x
  key2 y
  keyn z

If that's the case, it's important that you easily can find the value
of key2 for foo:bar.

To do this easily, you need to be able to pass foo:bar as a filter to
the lister:
$ list foo:bar | awk '/key2/ {print $2}'

Lets have a look at the ip command. They have the same problem, you can
have multiple interfaces and each interface can have arbitrary ipv6
addresses.

$ ip addr show
...
5: data0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP 
group default qlen 1000
    link/ether 00:0f:ff:01:01:01 brd ff:ff:ff:ff:ff:ff
    inet 10.0.0.1/8 brd 10.255.255.255 scope global data0
       valid_lft forever preferred_lft forever
    inet6 2001:db8:0:f101::1/64 scope global
       valid_lft forever preferred_lft forever
    inet6 fe80::20f:ffff:fe01:101/64 scope link
       valid_lft forever preferred_lft forever
6: data1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP 
group default qlen 1000
    link/ether 00:0f:ff:01:02:01 brd ff:ff:ff:ff:ff:ff
    inet 11.0.0.1/8 brd 11.255.255.255 scope global data1
       valid_lft forever preferred_lft forever
    inet6 fe80::20f:ffff:fe01:201/64 scope link
       valid_lft forever preferred_lft forever

First we are only interested in data0, which has the two addresses.

$ ip addr show dev data0
5: data0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP 
group default qlen 1000
    link/ether 00:0f:ff:01:01:01 brd ff:ff:ff:ff:ff:ff
    inet 10.0.0.1/8 brd 10.255.255.255 scope global data0
       valid_lft forever preferred_lft forever
    inet6 2001:db8:0:f101::1/64 scope global 
       valid_lft forever preferred_lft forever
    inet6 fe80::20f:ffff:fe01:101/64 scope link 
       valid_lft forever preferred_lft forever

Now, how do we find valid_lft for the link local address of data0?
Luckily the authors knew they where implicitly grouping data and
not only putting "an extra space at the beginning of some lines".
So they created filters to make this parsable.

$ ip addr show dev data0 scope link | awk '/valid_lft/ {print $2}'
forever

Try doing this with: "ip addr show | parser" and you will see why
implicitly grouping data makes parsing _extremely_ hard.

Bottom line, what looks nice to humans can be impossible to parse and
I think we need to find a good balance between parsability and
readability.

name val1 val2 val3 val4 val5
foo  1    2    3    3    4
bar       2    3         4
baz  1    2    3    3    5
boo  1         3    3    5

Logically this is sort of what the first monitor patchset I saw looked
like. Try parsing val2 for each name ;)

Regards
Richard
> 
> - The "Dom" column should be called "Gen", otherwise this becomes very 
> confusing.


> 
> ///jon
> 
> 
>> 1.1.14        U Y 19    UUUUU,U []
>> 1.1.15        U Y 20    UUUUU,U []
>> 1.1.16        U Y 26    UUUUU,U []
>> 1.1.130       U Y 4     UUUUU,U []
>> 1.1.131       U Y 4     UUUUU,U []
>> 1.1.230       U Y 5     UUUUU,U []
>> 1.1.231       U Y 6     UUUUU,U []
>> 1.1.330       U Y 6     UUUUU,U []
>> 1.1.331       U N 6     UUUUU,U []
>> 1.1.430       U N 8     UUUUU,U []
>> 1.1.431       U N 8     UUUUU,U []
>> 1.1.530       U N 6     UUUUU,U []
>> 1.1.531       U N 8     UUUUU,U []
>> 1.1.630       U N 8     UUUUU,U []
>> 1.1.631       U Y 11    UUUUU,U []
>> 1.1.730       U N 9     UUUUU,U []
>> 1.1.731       U N 10    UUUUU,U []
>> 1.1.830       U N 9     UUUUU,U []
>> 1.1.831       U N 12    UUUUU,U []
>> 1.1.930       U N 12    UUUUU,U []
>> 1.1.931       U N 12    UUUUU,U []
>> 1.1.1030      U Y 13    UUUUU,U []
>> 1.1.1031      U N 13    UUUUU,U []
>> 1.1.1130      U N 13    UUUUU,U []
>> 1.1.1131      U N 11    UUUUU,U []
>> 1.1.1230      U N 8     UUUUU,U []
>> 1.1.1231      U N 7     UUUUU,U []
>> 1.1.1330      U N 15    UUUUU,U []
>> 1.1.1331      U Y 11    UUUUU,U []
>> 1.1.1430      U N 16    UUUUU,U []
>> 1.1.1431      U N 10    UUUUU,U []
>> 1.1.1530      U N 9     UUUUU,U []
>> 1.1.1531      U N 8     UUUUU,U []
>> 1.1.1630      U N 12    UUUUU,U []
>> 1.1.1631      U N 9     UUUUU,U []
>> 1.1.1         U Y 5     UUUUU,U []
>> 1.1.2         U N 7     UUUUU,U []
>> 1.1.3         U N 11    UUUUU,U []
>> 1.1.4         U N 11    UUUUU,U []
>> 1.1.5         U N 13    UUUUU,U []
>> 1.1.6         U N 12    UUUUU,U []
>> 1.1.7         U N 16    UUUUU,U []
>> 1.1.8         U Y 19    UUUUU,U []
>> 1.1.9         U N 20    UUUUU,U []
>> 1.1.10        U N 21    UUUUU,U []
>> 1.1.11        U N 22    UUUUU,U []
>> 1.1.12        U N 18    UUUUU,U []
>> 1.1.13        U N 22    UUUUU,U []
>>
>> $ tipc link monitor list -h
>> Usage: tipc monitor list media MEDIA ARGS...
>>
>> MEDIA
>>  udp                   - User Datagram Protocol
>>  ib                    - Infiniband
>>  eth                   - Ethernet
>>
>> Signed-off-by: Parthasarathy Bhuvaragan
>> <parthasarathy.bhuvara...@ericsson.com>
>> ---
>>  tipc/link.c | 207
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 207 insertions(+)
>>
>> diff --git a/tipc/link.c b/tipc/link.c
>> index 46a25b314311..0d15a7124d0c 100644
>> --- a/tipc/link.c
>> +++ b/tipc/link.c
>> @@ -22,6 +22,7 @@
>>  #include "cmdl.h"
>>  #include "msg.h"
>>  #include "link.h"
>> +#include "bearer.h"
>>
>>  static int link_list_cb(const struct nlmsghdr *nlh, void *data)
>>  {
>> @@ -556,6 +557,210 @@ static int cmd_link_mon_summary(struct nlmsghdr
>> *nlh, const struct cmd *cmd,
>>      return msg_dumpit(nlh, link_mon_summary_cb, NULL);
>>  }
>>
>> +#define STATUS_WIDTH 2
>> +#define MAX_NODE_WIDTH 14 /* 255.4095.4095 */
>> +#define MAX_DOM_GEN_WIDTH 6 /* 65535 */
>> +#define DIRECTLY_MON_WIDTH 2
>> +
>> +#define APPL_NODE_STATUS_WIDTH 5
>> +
>> +static int map_get(uint64_t up_map, int i)
>> +{
>> +    return (up_map & (1 << i)) >> i;
>> +}
>> +
>> +/* print the applied members, since we know the the members
>> + * are listed in ascending order, we print only the state */
>> +static void link_mon_print_applied(uint16_t applied, uint64_t up_map)
>> +{
>> +    int i;
>> +    char state;
>> +
>> +    for (i = 0; i < applied; i++) {
>> +            /* print the delimiter for every -n- entry */
>> +            if (i && !(i % APPL_NODE_STATUS_WIDTH))
>> +                    printf(",");
>> +
>> +            state = map_get(up_map, i) ? 'U' : 'D';
>> +            printf("%c", state);
>> +    }
>> +}
>> +
>> +/* print the non applied members, since we dont know
>> + * the members, we print them along with the state */
>> +static void link_mon_print_non_applied(uint16_t applied, uint16_t
>> member_cnt,
>> +                                   uint64_t up_map,  uint32_t *members)
>> +{
>> +    int i;
>> +    char state;
>> +
>> +    printf(" [");
>> +    for (i = applied; i < member_cnt; i++) {
>> +            char addr_str[16];
>> +
>> +            /* print the delimiter for every entry */
>> +            if (i != applied)
>> +                    printf(",");
>> +
>> +            sprintf(addr_str, "%u.%u.%u:", tipc_zone(members[i]),
>> +                    tipc_cluster(members[i]), tipc_node(members[i]));
>> +            state = map_get(up_map, i) ? 'U' : 'D';
>> +            printf("%s%c", addr_str, state);
>> +    }
>> +    printf("]");
>> +}
>> +
>> +static void link_mon_print_peer_state(uint32_t addr, const char status,
>> +                                  const char direct, uint32_t dom)
>> +{
>> +    char addr_str[16];
>> +
>> +    sprintf(addr_str, "%u.%u.%u", tipc_zone(addr), tipc_cluster(addr),
>> +            tipc_node(addr));
>> +
>> +    printf("%-*s", MAX_NODE_WIDTH, addr_str);
>> +    printf("%-*c", STATUS_WIDTH, status);
>> +    printf("%-*c", DIRECTLY_MON_WIDTH, direct);
>> +    printf("%-*u", MAX_DOM_GEN_WIDTH, dom);
>> +}
>> +
>> +static int link_mon_peer_list_cb(const struct nlmsghdr *nlh, void *data)
>> +{
>> +    struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
>> +    struct nlattr *attrs[TIPC_NLA_MON_PEER_MAX + 1] = {};
>> +    struct nlattr *info[TIPC_NLA_MAX + 1] = {};
>> +    uint16_t member_cnt;
>> +    uint32_t applied;
>> +    uint32_t domgen;
>> +    uint64_t up_map;
>> +    char status;
>> +    char direct;
>> +
>> +    mnl_attr_parse(nlh, sizeof(*genl), parse_attrs, info);
>> +    if (!info[TIPC_NLA_MON_PEER])
>> +            return MNL_CB_ERROR;
>> +
>> +    mnl_attr_parse_nested(info[TIPC_NLA_MON_PEER], parse_attrs, attrs);
>> +
>> +    direct = (attrs[TIPC_NLA_MON_PEER_LOCAL] ||
>> +              attrs[TIPC_NLA_MON_PEER_HEAD]) ? 'Y' : 'N';
>> +
>> +    status = attrs[TIPC_NLA_MON_PEER_UP] ? 'U' : 'D';
>> +
>> +    domgen = attrs[TIPC_NLA_MON_PEER_DOMGEN] ?
>> +
>>      mnl_attr_get_u32(attrs[TIPC_NLA_MON_PEER_DOMGEN]) : 0;
>> +
>> +
>>      link_mon_print_peer_state(mnl_attr_get_u32(attrs[TIPC_NLA_MON_PE
>> ER_ADDR]),
>> +                                               status, direct, domgen);
>> +
>> +    applied = mnl_attr_get_u32(attrs[TIPC_NLA_MON_PEER_APPLIED]);
>> +
>> +    if (!applied)
>> +            goto exit;
>> +
>> +    up_map = mnl_attr_get_u64(attrs[TIPC_NLA_MON_PEER_UPMAP]);
>> +
>> +    member_cnt =
>> mnl_attr_get_payload_len(attrs[TIPC_NLA_MON_PEER_MEMBERS]);
>> +
>> +    /* each tipc address occupies 4 bytes of payload, hence compensate it */
>> +    member_cnt /= sizeof(uint32_t);
>> +
>> +    link_mon_print_applied(applied, up_map);
>> +
>> +    link_mon_print_non_applied(applied, member_cnt, up_map,
>> +
>> mnl_attr_get_payload(attrs[TIPC_NLA_MON_PEER_MEMBERS]));
>> +
>> +exit:
>> +    printf("\n");
>> +
>> +    return MNL_CB_OK;
>> +}
>> +
>> +static int link_mon_peer_list(uint32_t mon_ref)
>> +{
>> +    struct nlmsghdr *nlh;
>> +    char buf[MNL_SOCKET_BUFFER_SIZE];
>> +    struct nlattr *nest;
>> +
>> +    if (!(nlh = msg_init(buf, TIPC_NL_MON_PEER_GET))) {
>> +            fprintf(stderr, "error, message initialisation failed\n");
>> +            return -1;
>> +    }
>> +
>> +    nest = mnl_attr_nest_start(nlh, TIPC_NLA_MON);
>> +    mnl_attr_put_u32(nlh, TIPC_NLA_MON_REF, mon_ref);
>> +    mnl_attr_nest_end(nlh, nest);
>> +
>> +    return msg_dumpit(nlh, link_mon_peer_list_cb, NULL);
>> +}
>> +
>> +static int link_mon_list_cb(const struct nlmsghdr *nlh, void *data)
>> +{
>> +    struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
>> +    struct nlattr *info[TIPC_NLA_MAX + 1] = {};
>> +    struct nlattr *attrs[TIPC_NLA_MON_MAX + 1] = {};
>> +    char *req_bearer = data;
>> +    const char *bname;
>> +
>> +    mnl_attr_parse(nlh, sizeof(*genl), parse_attrs, info);
>> +    if (!info[TIPC_NLA_MON])
>> +            return MNL_CB_ERROR;
>> +
>> +    mnl_attr_parse_nested(info[TIPC_NLA_MON], parse_attrs, attrs);
>> +
>> +    bname = mnl_attr_get_str(attrs[TIPC_NLA_MON_BEARER_NAME]);
>> +
>> +    if (req_bearer && (strcmp(req_bearer, bname) != 0))
>> +            return MNL_CB_OK;
>> +
>> +    if (mnl_attr_get_u32(attrs[TIPC_NLA_MON_PEERCNT]))
>> +
>>      link_mon_peer_list(mnl_attr_get_u32(attrs[TIPC_NLA_MON_REF]));
>> +
>> +    return MNL_CB_OK;
>> +}
>> +
>> +static void cmd_link_mon_list_help(struct cmdl *cmdl)
>> +{
>> +    fprintf(stderr, "Usage: %s monitor list media MEDIA ARGS...\n\n",
>> +            cmdl->argv[0]);
>> +    print_bearer_media();
>> +}
>> +
>> +static int cmd_link_mon_list(struct nlmsghdr *nlh, const struct cmd *cmd,
>> +                         struct cmdl *cmdl, void *data)
>> +{
>> +    char buf[MNL_SOCKET_BUFFER_SIZE];
>> +    char bname[TIPC_MAX_BEARER_NAME];
>> +    struct opt opts[] = {
>> +            { "media",              NULL },
>> +            { "device",             NULL },
>> +            { "name",               NULL },
>> +            { NULL }
>> +    };
>> +    int err;
>> +    const char *title =     "Node          S D Dom   Applied Node Status 
>> [Non-
>> Applied Node:Status]";
>> +
>> +    if (parse_opts(opts, cmdl) < 0)
>> +            return -EINVAL;
>> +
>> +    if (help_flag) {
>> +            cmd->help(cmdl);
>> +            return -EINVAL;
>> +    }
>> +
>> +    if ((err = cmd_get_unique_bearer_name(cmd, cmdl, opts, bname)))
>> +            return err;
>> +
>> +    if (!(nlh = msg_init(buf, TIPC_NL_MON_GET))) {
>> +            fprintf(stderr, "error, message initialisation failed\n");
>> +            return -1;
>> +    }
>> +
>> +    printf("%s\n", title);
>> +
>> +    return msg_dumpit(nlh, link_mon_list_cb, bname);
>> +}
>> +
>>  static void cmd_link_mon_set_help(struct cmdl *cmdl)
>>  {
>>      fprintf(stderr, "Usage: %s monitor set PPROPERTY\n\n"
>> @@ -634,6 +839,7 @@ static void cmd_link_mon_help(struct cmdl *cmdl)
>>              "COMMANDS\n"
>>              " set                   - Set monitor properties\n"
>>              " get                   - Get monitor properties\n"
>> +            " list                  - List all cluster members\n"
>>              " summary               - Show local node monitor summary\n",
>>              cmdl->argv[0]);
>>  }
>> @@ -644,6 +850,7 @@ static int cmd_link_mon(struct nlmsghdr *nlh, const 
>> struct
>> cmd *cmd, struct cmdl
>>      const struct cmd cmds[] = {
>>              { "set",        cmd_link_mon_set,       cmd_link_mon_set_help },
>>              { "get",        cmd_link_mon_get,       cmd_link_mon_get_help },
>> +            { "list",       cmd_link_mon_list,      cmd_link_mon_list_help 
>> },
>>              { "summary",    cmd_link_mon_summary,   NULL },
>>              { NULL }
>>      };
>> --
>> 2.1.4
> 


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion

Reply via email to