Thread (27 messages) 27 messages, 7 authors, 2019-04-23

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

From: Petr Machata <hidden>
Date: 2019-03-18 12:16:02

Roopa Prabhu [off-list ref] writes:
This looks good and will be use-full. But i have some comments on the
implementation below.
Also, carrier can go down due to protocol down (IFLA_PROTODOWN). And I
get asked about supporting
reason codes or protocol owner for such protodown reason (I have not
given it much thought yet. I will see if there is a way
to use your series for that as well).
My thinking was that since protocol down is set from userspace, it's
under admin control, and that's where the reason signalling should be.
quoted
---
 include/net/rtnetlink.h      |  3 +++
 include/uapi/linux/if_link.h | 16 ++++++++++++++++
 net/core/rtnetlink.c         | 22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+)
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);
 };
Any reason to restrict this to network interfaces which support rtnl_link_ops ?.
I also saw that you added rtnl_link_ops to mlxsw for this. Which also
means every driver will now have to declare rtnl_link_ops to use this
?.
I think we should keep rtnl_link_ops to logical links  like bridge,
bonds etc (ie which support newlink and dellink).

Can't we use netdev_ops for this ?. That will allow any driver to just
support this readily.
I guess you are right.
quoted
+enum rtnl_link_down_reason_major {
+       RTNL_LDR_OTHER,
+       RTNL_LDR_NO_CABLE,
+       RTNL_LDR_UNSUPPORTED_CABLE,
+       RTNL_LDR_AUTONEG_FAILURE,
+       RTNL_LDR_NO_LINK_PARTNER,
+       RTNL_LDR_LINK_TRAINING_FAILURE,
+       RTNL_LDR_LOGICAL_MISMATCH,
+       RTNL_LDR_REMOTE_FAULT,
+       RTNL_LDR_BAD_SIGNAL_INTEGRITY,
+       RTNL_LDR_CALIBRATION_FAILURE,
+       RTNL_LDR_POWER_BUDGET_EXCEEDED,
+};
prefer LINK_DOWN_REASON_* or LINKDOWN_REASON_*
(Though there is no predefined convention, the prefix RTNL makes it
feel like a top-level attribute when its really a value for an IFLA_*
attribute.)
OK.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help