Re: [PATCH net-next v5] netlink: specs: devlink: add some(not all) missing attributes in devlink.yaml

swarup
2023-12-08T18:30:12Z

On Wed, Dec 06, 2023 at 08:06:11AM -0800, Jakub Kicinski wrote:
> On Wed, 6 Dec 2023 08:51:54 +0100 Jiri Pirko wrote:
> > My "suggested-by" is probably fine as I suggested Swarup to make the patch :)
>
> Ah, I didn't realize, sorry :) Just mine needs to go then.

Hi Jiri,

Please find answer for some quesion from you.

1. I removed the Fixes tag.

2. I removed Jakub's name from Suggested-by tag.

3. I added new line as suggested.

value: ## or number, is used only if there is a gap or
missing attribute just above of any attribute which is not yet filled.

4. dl-attr-stats has a value 0 as shown below for this reason:
name: dl-attr-stats
name-prefix: devlink-attr-
attributes:
- name: stats-rx-packets
type: u64
value: 0 <-- 0 is added here due to below mentioned reason
but mainly to match order of stats unnamed enum declared in include/uapi/linux/devlink.h
-
name: stats-rx-bytes
type: u64
-
name: stats-rx-dropped
type: u64

-------------- different command to get ttl_value_is_too_small with trap command -----------
#~/devlink_work/net-next$ sudo devlink -jpnsv trap show netdevsim/netdevsim1 trap ttl_value_is_too_small
{
"trap": {
"netdevsim/netdevsim1": [ {
"name": "ttl_value_is_too_small",
"type": "exception",
"generic": true,
"action": "trap",
"group": "l3_exceptions",
"metadata": [ "input_port" ],
"stats": {
"rx": {
"bytes": 341019532,
"packets": 2401546,
"dropped": 48
}
}
} ]
}
}
----------

..... another command to get ttl_value_is_too_small trap command ---------
:~/devlink_work/net-next$ sudo ./tools/net/ynl/cli.py --spec Documentation/netlink/specs/devlink.yaml --do trap-get --json '{"bus-name": "netdevsim", "dev-name": "netdevsim1", "trap-name": "ttl_value_is_too_small"}' --process-unknown
{'bus-name': 'netdevsim',
'dev-name': 'netdevsim1',
'stats': {'stats-rx-bytes': 341152018,
'stats-rx-dropped': 49,
'stats-rx-packets': 2402479},
'trap-action': 'trap',
'trap-generic': True,
'trap-group-name': 'l3_exceptions',
'trap-metadata': {'trap-metadata-type-in-port': True},
'trap-name': 'ttl_value_is_too_small',
'trap-type': 'exception'}
------------

2nd command returned stats values matches with 1st command returned stats value, when "dl-attr-stats" value is zero.

trap.c function which fills stats value fills as mentioned below:
static int devlink_trap_stats_put(struct sk_buff *msg, struct devlink *devlink,
const struct devlink_trap_item *trap_item)
{
struct devlink_stats stats;
...... <-- code omitted
if (devlink->ops->trap_drop_counter_get &&
nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_DROPPED, drops,
DEVLINK_ATTR_PAD))
goto nla_put_failure;

if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_PACKETS,
u64_stats_read(&stats.rx_packets),
DEVLINK_ATTR_PAD))
goto nla_put_failure;

if (nla_put_u64_64bit(msg, DEVLINK_ATTR_STATS_RX_BYTES,
u64_stats_read(&stats.rx_bytes),
DEVLINK_ATTR_PAD))
goto nla_put_failure;

.... <-- code omitted
return 0;

nla_put_failure:
nla_nest_cancel(msg, attr);
return -EMSGSIZE;
}

but in --> include/uapi/linux/devlink.h
stats is declared as mentioned below:
enum {
DEVLINK_ATTR_STATS_RX_PACKETS, /* u64 */
DEVLINK_ATTR_STATS_RX_BYTES, /* u64 */
DEVLINK_ATTR_STATS_RX_DROPPED, /* u64 */

__DEVLINK_ATTR_STATS_MAX,
DEVLINK_ATTR_STATS_MAX = __DEVLINK_ATTR_STATS_MAX - 1
};

Hence to keep the order mentioned in enum, i assigned value of 0,
so that rx_packets, rx_btes and rx_dropped picks correct values.