Re: [RFC V2 iproute2-next] rdma: Add the prefix for driver attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



在 2019/2/26 23:17, Leon Romanovsky 写道:
> On Tue, Feb 26, 2019 at 09:40:12PM +0800, Lijun Ou wrote:
>> We need to increase the prefix named drv_ to
>> distinguish whether the attribute is from
>> IB/core or the driver attribute.
>>
>> From the sample output:
>> root@(none)# ./rdma res show cq dev hns_0
>> dev hns_0 cqe 1023 users 2 poll-ctx WORKQUEUE pid 0 comm [ib_core] drv_state 2 drv_ceqn 0 drv_poll 0 drv_shift 10 drv_cmd_sn 1 drv_cqn 0 drv_hopnum 1 drv_pi 0 drv_ci 0 drv_rdb_en 1 drv_coalesce 0 drv_period 0 drv_cnt 0 drv_se_idx 0
>>
>> root@(none)# ./rdma res show cq dev hns_0 -j
>> [{["ifindex":1,"ifname":"hns_0","cqe":1023,"users":2,"poll-ctx":"WORKQUEUE","pid":0,"comm":"ib_core","drv_state":2,"drv_ceqn":0,"drv_poll":0,"drv_shift":10,"drv_cmd_sn":1,"drv_cqn":0,"drv_hopnum":1,"drv_pi":0,"drv_ci":0,"drv_rdb_en":1,"drv_coalesce":0,"drv_period":0,"drv_cnt":0,"drv_se_idx":0]}]
>>
>> Signed-off-by: Lijun Ou <oulijun@xxxxxxxxxx>
>> ---
>> V1->V2:
>> - Add the prefix print for driver by JSON format.
>> ---
>>  include/json_writer.h |  3 +++
>>  lib/json_writer.c     | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  rdma/utils.c          | 18 +++++++++---------
>>  3 files changed, 63 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/json_writer.h b/include/json_writer.h
>> index b52dc2d..f8da48f 100644
>> --- a/include/json_writer.h
>> +++ b/include/json_writer.h
>> @@ -28,6 +28,9 @@ void jsonw_pretty(json_writer_t *self, bool on);
>>  /* Add property name */
>>  void jsonw_name(json_writer_t *self, const char *name);
>>
>> +/* Add property name with driver specific */
>> +void jsonw_driver_name(json_writer_t *self, const char *name);
>> +
>>  /* Add value  */
>>  __attribute__((format(printf, 2, 3)))
>>  void jsonw_printf(json_writer_t *self, const char *fmt, ...);
>> diff --git a/lib/json_writer.c b/lib/json_writer.c
>> index 5004c18..6d09eec 100644
>> --- a/lib/json_writer.c
>> +++ b/lib/json_writer.c
>> @@ -89,6 +89,44 @@ static void jsonw_puts(json_writer_t *self, const char *str)
>>  	putc('"', self->out);
>>  }
>>
>> +/* Output JSON encoded string with the driver-specific */
>> +/* Handles C escapes, does not do Unicode */
>> +static void jsonw_driver_puts(json_writer_t *self, const char *str)
>> +{
>> +	putc('"', self->out);
>> +	fputs("drv_", self->out);
>> +	for (; *str; ++str)
>> +		switch (*str) {
>> +		case '\t':
>> +			fputs("\\t", self->out);
>> +			break;
>> +		case '\n':
>> +			fputs("\\n", self->out);
>> +			break;
>> +		case '\r':
>> +			fputs("\\r", self->out);
>> +			break;
>> +		case '\f':
>> +			fputs("\\f", self->out);
>> +			break;
>> +		case '\b':
>> +			fputs("\\b", self->out);
>> +			break;
>> +		case '\\':
>> +			fputs("\\n", self->out);
>> +			break;
>> +		case '"':
>> +			fputs("\\\"", self->out);
>> +			break;
>> +		case '\'':
>> +			fputs("\\\'", self->out);
>> +			break;
>> +		default:
>> +			putc(*str, self->out);
>> +		}
>> +	putc('"', self->out);
>> +}
>> +
>>  /* Create a new JSON stream */
>>  json_writer_t *jsonw_new(FILE *f)
>>  {
>> @@ -152,6 +190,18 @@ void jsonw_name(json_writer_t *self, const char *name)
>>  		putc(' ', self->out);
>>  }
>>
>> +/* Add a JSON property name for driver */
>> +void jsonw_driver_name(json_writer_t *self, const char *name)
>> +{
>> +	jsonw_eor(self);
>> +	jsonw_eol(self);
>> +	self->sep = '\0';
>> +	jsonw_driver_puts(self, name);
>> +	putc(':', self->out);
>> +	if (self->pretty)
>> +		putc(' ', self->out);
>> +}
>> +
> It is a little bit over, just concatenate "drv_" to provided "key_str", that is all.
>
> Thanks
Hi, leon
  I don't quite understand your opinion.
 1.  Shouldn't I redefine a similar function named jsonw_driver_name?
 if don't define the new jsonw_driver_name, the all strings include IB core fields will be printed, as follows:
 root@(none)# ./rdma res show cq dev hnseth0 -j
[{["drv_ifindex":1,"drv_ifname":"drv_hnseth0","drv_cqe":1023,"drv_users":2,"drv_poll-ctx":"drv_WORKQUEUE","drv_pid":0,"drv_comm":"drv_ib_core"oujson,"drv_state":2oujson,"drv_ceqn":

I think that it is meaningless for the prefix print.

2. Shouldn't we use drv_? should use device_name instead of ?
for example:
 when run in hns hardware, the print for driver attribute is hns_, when run in cxgb4 hardware, the print for driver attribute is cxgb4_?
when run in mlx5 hardware, the print for driver attribute is mlx5_?  Is this understanding ?

thanks
>>  __attribute__((format(printf, 2, 3)))
>>  void jsonw_printf(json_writer_t *self, const char *fmt, ...)
>>  {
>> @@ -323,7 +373,7 @@ void jsonw_lluint_field(json_writer_t *self,
>>
>>  void jsonw_int_field(json_writer_t *self, const char *prop, int num)
>>  {
>> -	jsonw_name(self, prop);
>> +	jsonw_driver_name(self, prop);
>>  	jsonw_int(self, num);
>>  }
>>
>> diff --git a/rdma/utils.c b/rdma/utils.c
>> index 069d44f..8890981 100644
>> --- a/rdma/utils.c
>> +++ b/rdma/utils.c
>> @@ -720,7 +720,7 @@ static int print_driver_string(struct rd *rd, const char *key_str,
>>  		jsonw_string_field(rd->jw, key_str, val_str);
>>  		return 0;
>>  	} else {
>> -		return pr_out("%s %s ", key_str, val_str);
>> +		return pr_out("drv_%s %s ", key_str, val_str);
>>  	}
>>  }
>>
>> @@ -733,9 +733,9 @@ static int print_driver_s32(struct rd *rd, const char *key_str, int32_t val,
>>  	}
>>  	switch (print_type) {
>>  	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
>> -		return pr_out("%s %d ", key_str, val);
>> +		return pr_out("drv_%s %d ", key_str, val);
>>  	case RDMA_NLDEV_PRINT_TYPE_HEX:
>> -		return pr_out("%s 0x%x ", key_str, val);
>> +		return pr_out("drv_%s 0x%x ", key_str, val);
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -750,9 +750,9 @@ static int print_driver_u32(struct rd *rd, const char *key_str, uint32_t val,
>>  	}
>>  	switch (print_type) {
>>  	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
>> -		return pr_out("%s %u ", key_str, val);
>> +		return pr_out("drv_%s %u ", key_str, val);
>>  	case RDMA_NLDEV_PRINT_TYPE_HEX:
>> -		return pr_out("%s 0x%x ", key_str, val);
>> +		return pr_out("drv_%s 0x%x ", key_str, val);
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -767,9 +767,9 @@ static int print_driver_s64(struct rd *rd, const char *key_str, int64_t val,
>>  	}
>>  	switch (print_type) {
>>  	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
>> -		return pr_out("%s %" PRId64 " ", key_str, val);
>> +		return pr_out("drv_%s %" PRId64 " ", key_str, val);
>>  	case RDMA_NLDEV_PRINT_TYPE_HEX:
>> -		return pr_out("%s 0x%" PRIx64 " ", key_str, val);
>> +		return pr_out("drv_%s 0x%" PRIx64 " ", key_str, val);
>>  	default:
>>  		return -EINVAL;
>>  	}
>> @@ -784,9 +784,9 @@ static int print_driver_u64(struct rd *rd, const char *key_str, uint64_t val,
>>  	}
>>  	switch (print_type) {
>>  	case RDMA_NLDEV_PRINT_TYPE_UNSPEC:
>> -		return pr_out("%s %" PRIu64 " ", key_str, val);
>> +		return pr_out("drv_%s %" PRIu64 " ", key_str, val);
>>  	case RDMA_NLDEV_PRINT_TYPE_HEX:
>> -		return pr_out("%s 0x%" PRIx64 " ", key_str, val);
>> +		return pr_out("drv_%s 0x%" PRIx64 " ", key_str, val);
>>  	default:
>>  		return -EINVAL;
>>  	}
>> --
>> 1.9.1
>>





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux