netdev - Re: [RFC PATCH net-next 1/3] net: rtnetlink: Add link-down reason to RTNL messages

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 for Android: free password hash cracker in your pocket

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

Message-ID: <871s34icnr.fsf@mellanox.com>
Date:   Mon, 18 Mar 2019 12:34:55 +0000
From:   Petr Machata <petrm@...lanox.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...lanox.com>,
        Ido Schimmel <idosch@...lanox.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Tariq Toukan <tariqt@...lanox.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>
Subject: Re: [RFC PATCH net-next 1/3] net: rtnetlink: Add link-down reason to
 RTNL messages


Jakub Kicinski <jakub.kicinski@...ronome.com> writes:

> On Fri, 15 Mar 2019 17:56:07 +0000, Petr Machata wrote:
>> diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
>> index e2091bb2b3a8..cfd9e86ff0ca 100644
>> --- a/include/net/rtnetlink.h
>> +++ b/include/net/rtnetlink.h
>> @@ -110,6 +110,9 @@ struct rtnl_link_ops {
>>  	int			(*fill_linkxstats)(struct sk_buff *skb,
>>  						   const struct net_device *dev,
>>  						   int *prividx, int attr);
>> +	size_t			(*link_down_reason_get_size)(const struct net_device *dev);
>> +	int			(*fill_link_down_reason)(struct sk_buff *skb,
>> +							 const struct net_device *dev);
>
> IMHO the API is a little heavy for returning, what is effectively a u64
> value sliced in two..

If a year from now someone wants to add string reason, this API will
just extend naturally to cover that case.

> Perhaps the core can just assume the reason will be provided if the NDO
> is present?  And the "fill" NDO should probably fill in the reason
> structure, rather than getting the skb passed and dealing with netlink
> directly.

This copies how other fill APIs are done, in that the responsibility for
filling up the message is deferred to the driver. I think it makes the
API easier to extend: if there ever is richer information available, the
drivers that want to support it just opt in and provide those attributes.

> Also perhaps this would be a ethtool-nl candidate (which would
> hopefully land soon after the merge window)?

Is this the repository with the patches? I'll take a look.

    https://github.com/mkubecek/ethnl

Thanks,
Petr