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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 15 Mar 2019 19:26:11 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Petr Machata <petrm@...lanox.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

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..

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.

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

>  };
>  
>  int __rtnl_link_register(struct rtnl_link_ops *ops);
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 5b225ff63b48..a47f42e79741 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -167,6 +167,8 @@ enum {
>  	IFLA_NEW_IFINDEX,
>  	IFLA_MIN_MTU,
>  	IFLA_MAX_MTU,
> +	IFLA_LINK_DOWN_REASON_MAJOR,  /* enum rtnl_link_down_reason_major */
> +	IFLA_LINK_DOWN_REASON_MINOR,
>  	__IFLA_MAX
>  };