netdev - Re: [PATCH iproute2-next v2 1/3] Add, show, link, remove IOAM namespaces and schemas

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1693729071.28416702.1627576900833.JavaMail.zimbra@uliege.be>
Date:   Thu, 29 Jul 2021 18:41:40 +0200 (CEST)
From:   Justin Iurman <justin.iurman@...ege.be>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, stephen@...workplumber.org
Subject: Re: [PATCH iproute2-next v2 1/3] Add, show, link, remove IOAM
 namespaces and schemas

>>>> +static void print_schema(struct rtattr *attrs[])
>>>> +{
>>>> +	__u8 data[IOAM6_MAX_SCHEMA_DATA_LEN + 1];
>>>> +	int len;
>>>> +
>>>> +	print_uint(PRINT_ANY, "schema", "schema %u",
>>>> +		   rta_getattr_u32(attrs[IOAM6_ATTR_SC_ID]));
>>>> +
>>>> +	if (attrs[IOAM6_ATTR_NS_ID])
>>>> +		print_uint(PRINT_ANY, "namespace", " [namespace %u]",
>>>> +			   rta_getattr_u16(attrs[IOAM6_ATTR_NS_ID]));
>>>> +
>>>> +	len = RTA_PAYLOAD(attrs[IOAM6_ATTR_SC_DATA]);
>>>> +	memcpy(data, RTA_DATA(attrs[IOAM6_ATTR_SC_DATA]), len);
>>>> +	data[len] = '\0';
>>>> +
>>>> +	print_string(PRINT_ANY, "data", ", data \"%s\"", (const char *)data);
>>>
>>> The attribute descriptions shows this as binary data, not a string.
>> 
>> Indeed. Maybe should I print it as hex... What do you think is more appropriate
>> for this?
> 
> ./tc/em_meta.c has print_binary() but it is not json aware.
> 
> devlink has pr_out_binary_value which is close. You could probably take
> this one and generalize it.

Right, I'll take a look.

>>>> +int do_ioam6(int argc, char **argv)
>>>> +{
>>>> +	bool maybe_wide = false;
>>>> +
>>>> +	if (argc < 1 || matches(*argv, "help") == 0)
>>>> +		usage();
>>>> +
>>>> +	memset(&opts, 0, sizeof(opts));
>>>> +
>>>> +	if (matches(*argv, "namespace") == 0) {
>>>
>>> matches has been shown to be quite frail. Convenient for shorthand
>>> typing commands, but frail in the big picture. We should stop using it -
>>> especially for new commands.
>> 
>> Sure. What do you suggest as an alternative?
>> 
> 
> full strcmp.

I see. But, in that case, I'm wondering if it wouldn't be better to directly modify "matches" internally so that we keep consistency between modules without changing everything.

Thoughts?