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

[Petr Machata <petrm@mellanox.com>]

Jakub Kicinski <jakub.kicinski@netronome.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