On Fri, Mar 15, 2019 at 07:26:11PM -0700, Jakub Kicinski wrote:
> 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..
Agreed. Unless we can expect more attributes to follow, repeating the
same code to generate netlink attributes in each driver doesn't feel
right.
> 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.
We could always reserve e.g. 0 as "none" which driver would use e.g. if
it only provides major reason id and not minor.
> Also perhaps this would be a ethtool-nl candidate (which would
> hopefully land soon after the merge window)?
Looking at the suggested values for major reasons, it does indeed seem
to belong to link related information provided by ETHTOOL_GLINKSETTINGS
request. Maybe nesting the reason with link state, i.e.
ETHA_SETTINGS_LINK_STATE (nested)
ETHA_LINKSTATE_LINK (u8)
ETHA_LINKSTATE_DOWN_REASON_MAJOR (u32)
ETHA_LINKSTATE_DOWN_REASON_MINOR (u32)
Michal
