| Message ID | 20180118175921.17645-1-f.fainelli@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,v2] net: core: Expose number of link up/down transitions | expand |
From: Florian Fainelli <f.fainelli@gmail.com> Date: Thu, 18 Jan 2018 09:59:13 -0800 > From: David Decotigny <decot@googlers.com> > > Expose the number of times the link has been going UP or DOWN, and > update the "carrier_changes" counter to be the sum of these two events. > While at it, also update the sysfs-class-net documentation to cover: > carrier_changes (3.15), carrier_up_count (4.16) and carrier_down_count > (4.16) > > Signed-off-by: David Decotigny <decot@googlers.com> > [Florian: > * rebase > * add documentation > * merge carrier_changes with up/down counters] > Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> > --- > Changes in v2: > - fixed sysfs attributes to use <iface> > - renamed count_link_{up,down} to carrier_{up,down}_count to match > existing carrier_changes semantics Like David Ahern I am strongly against the proliferation of sysfs files attached to network devices and the per-netdevice costs associated with that. However, dealing with that is a longer term issue that nobody has a clear plan for. Therefore I cannot reject this change on that basis alone. The information is useful, so applied, thanks.
On 01/22/2018 12:43 PM, David Miller wrote: > From: Florian Fainelli <f.fainelli@gmail.com> > Date: Thu, 18 Jan 2018 09:59:13 -0800 > >> From: David Decotigny <decot@googlers.com> >> >> Expose the number of times the link has been going UP or DOWN, and >> update the "carrier_changes" counter to be the sum of these two events. >> While at it, also update the sysfs-class-net documentation to cover: >> carrier_changes (3.15), carrier_up_count (4.16) and carrier_down_count >> (4.16) >> >> Signed-off-by: David Decotigny <decot@googlers.com> >> [Florian: >> * rebase >> * add documentation >> * merge carrier_changes with up/down counters] >> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com> >> --- >> Changes in v2: >> - fixed sysfs attributes to use <iface> >> - renamed count_link_{up,down} to carrier_{up,down}_count to match >> existing carrier_changes semantics > > Like David Ahern I am strongly against the proliferation of sysfs files > attached to network devices and the per-netdevice costs associated with > that. > > However, dealing with that is a longer term issue that nobody has a clear > plan for. Therefore I cannot reject this change on that basis alone. > > The information is useful, so applied, thanks. Thanks! David A, do you have any plans to revive your LWD/LWT devices patches, AFAIR you were allowing a knob disabling the creation of sysfs attributes.
On 1/22/18 1:46 PM, Florian Fainelli wrote: >> >> Like David Ahern I am strongly against the proliferation of sysfs files >> attached to network devices and the per-netdevice costs associated with >> that. >> >> However, dealing with that is a longer term issue that nobody has a clear >> plan for. Therefore I cannot reject this change on that basis alone. >> >> The information is useful, so applied, thanks. > > Thanks! David A, do you have any plans to revive your LWD/LWT devices > patches, AFAIR you were allowing a knob disabling the creation of sysfs > attributes. > At the moment (and for the next few months) I am focusing on route scalability: http://vger.kernel.org/netconf2017_files/nexthop-objects.pdf I do think about the lightweight netdevice need from time to time. If anyone has the time and wants to pick it up, the patches are on github: https://github.com/dsahern/linux net/lwt-dev https://github.com/dsahern/iproute2 lwt-dev As a refresher on the concept, the intention is that users can *opt in* to skipping the overhead of standard netdevs by adding a flag during the link create. Virtual devices such as vlan, macvlan, ipvlan, vrf, dummy are great candidates for this flag, and potentially bonds and bridges if the deployment use case is ok with what the lightweight moniker means which is: 1. no sysfs files for the device 2. no separate sysctl tree for the device (default settings are used) 3. delayed network protocol (IPv4, IPv6, MPLS) initializations The last one dove tails with the need for L2 only devices and suggests a the need for a separate control flag. If you dig further into the protocol initializations one could easily justify flags for a finer granularity on what is skipped (e.g., do protocol init, but skip netconf (use default values) and skip snmp stats). Slide 9 in https://www.netdevconf.org/1.1/proceedings/slides/ahern-aleksandrov-prabhu-scaling-network-cumulus.pdf shows the memory allocations. 40+kB / netdev is a killer. Using the lwt tag, that can be shrunk from ~44k to ~4k - a big gain. e.g., 4k VRFs (yes, I have been asked about that) would go from ~160MB to just ~16MB. As I recall the kernel patch is not complete, only shows the intent and what the flag offers (pain is worth the gain). During the last discussion on this the idea of a net_dev_common was suggested. After looking into it I believe it is the wrong direction -- an unnecessary churn on the code base when the only intention is to omit / bypass existing code paths. The common would come in handy in trying to reduce the size of 'struct net_device' which is driven by 'struct device' and similar h/w entries.
diff --git a/Documentation/ABI/testing/sysfs-class-net b/Documentation/ABI/testing/sysfs-class-net index 6856da99b6f7..2f1788111cd9 100644 --- a/Documentation/ABI/testing/sysfs-class-net +++ b/Documentation/ABI/testing/sysfs-class-net @@ -259,3 +259,27 @@ Contact: netdev@vger.kernel.org Description: Symbolic link to the PHY device this network device is attached to. + +What: /sys/class/net/<iface>/carrier_changes +Date: Mar 2014 +KernelVersion: 3.15 +Contact: netdev@vger.kernel.org +Description: + 32-bit unsigned integer counting the number of times the link has + seen a change from UP to DOWN and vice versa + +What: /sys/class/net/<iface>/carrier_up_count +Date: Jan 2018 +KernelVersion: 4.16 +Contact: netdev@vger.kernel.org +Description: + 32-bit unsigned integer counting the number of times the link has + been up + +What: /sys/class/net/<iface>/carrier_down_count +Date: Jan 2018 +KernelVersion: 4.16 +Contact: netdev@vger.kernel.org +Description: + 32-bit unsigned integer counting the number of times the link has + been down diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index ed0799a12bf2..837e9cb7e358 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1680,8 +1680,6 @@ struct net_device { unsigned long base_addr; int irq; - atomic_t carrier_changes; - /* * Some hardware also needs these fields (state,dev_list, * napi_list,unreg_list,close_list) but they are not @@ -1719,6 +1717,10 @@ struct net_device { atomic_long_t tx_dropped; atomic_long_t rx_nohandler; + /* Stats to monitor link on/off, flapping */ + atomic_t carrier_up_count; + atomic_t carrier_down_count; + #ifdef CONFIG_WIRELESS_EXT const struct iw_handler_def *wireless_handlers; struct iw_public_data *wireless_data; diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index f8f04fed6186..8616131e2c61 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -161,6 +161,8 @@ enum { IFLA_EVENT, IFLA_NEW_NETNSID, IFLA_IF_NETNSID, + IFLA_CARRIER_UP_COUNT, + IFLA_CARRIER_DOWN_COUNT, __IFLA_MAX }; diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 7bf8b85ade16..c4a28f4667b6 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -295,10 +295,31 @@ static ssize_t carrier_changes_show(struct device *dev, struct net_device *netdev = to_net_dev(dev); return sprintf(buf, fmt_dec, - atomic_read(&netdev->carrier_changes)); + atomic_read(&netdev->carrier_up_count) + + atomic_read(&netdev->carrier_down_count)); } static DEVICE_ATTR_RO(carrier_changes); +static ssize_t carrier_up_count_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct net_device *netdev = to_net_dev(dev); + + return sprintf(buf, fmt_dec, atomic_read(&netdev->carrier_up_count)); +} +static DEVICE_ATTR_RO(carrier_up_count); + +static ssize_t carrier_down_count_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct net_device *netdev = to_net_dev(dev); + + return sprintf(buf, fmt_dec, atomic_read(&netdev->carrier_down_count)); +} +static DEVICE_ATTR_RO(carrier_down_count); + /* read-write attributes */ static int change_mtu(struct net_device *dev, unsigned long new_mtu) @@ -547,6 +568,8 @@ static struct attribute *net_class_attrs[] __ro_after_init = { &dev_attr_phys_port_name.attr, &dev_attr_phys_switch_id.attr, &dev_attr_proto_down.attr, + &dev_attr_carrier_up_count.attr, + &dev_attr_carrier_down_count.attr, NULL, }; ATTRIBUTE_GROUPS(net_class); diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 16d644a4f974..97874daa1336 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -990,6 +990,8 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev, + nla_total_size(4) /* IFLA_NEW_NETNSID */ + nla_total_size(1) /* IFLA_PROTO_DOWN */ + nla_total_size(4) /* IFLA_IF_NETNSID */ + + nla_total_size(4) /* IFLA_CARRIER_UP_COUNT */ + + nla_total_size(4) /* IFLA_CARRIER_DOWN_COUNT */ + 0; } @@ -1551,8 +1553,13 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) || nla_put_ifalias(skb, dev) || nla_put_u32(skb, IFLA_CARRIER_CHANGES, - atomic_read(&dev->carrier_changes)) || - nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down)) + atomic_read(&dev->carrier_up_count) + + atomic_read(&dev->carrier_down_count)) || + nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down) || + nla_put_u32(skb, IFLA_CARRIER_UP_COUNT, + atomic_read(&dev->carrier_up_count)) || + nla_put_u32(skb, IFLA_CARRIER_DOWN_COUNT, + atomic_read(&dev->carrier_down_count))) goto nla_put_failure; if (event != IFLA_EVENT_NONE) { @@ -1656,6 +1663,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_EVENT] = { .type = NLA_U32 }, [IFLA_GROUP] = { .type = NLA_U32 }, [IFLA_IF_NETNSID] = { .type = NLA_S32 }, + [IFLA_CARRIER_UP_COUNT] = { .type = NLA_U32 }, + [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 }, }; static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = { diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index ef8b4ecde2ac..1816bde47256 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -510,7 +510,7 @@ void netif_carrier_on(struct net_device *dev) if (test_and_clear_bit(__LINK_STATE_NOCARRIER, &dev->state)) { if (dev->reg_state == NETREG_UNINITIALIZED) return; - atomic_inc(&dev->carrier_changes); + atomic_inc(&dev->carrier_up_count); linkwatch_fire_event(dev); if (netif_running(dev)) __netdev_watchdog_up(dev); @@ -529,7 +529,7 @@ void netif_carrier_off(struct net_device *dev) if (!test_and_set_bit(__LINK_STATE_NOCARRIER, &dev->state)) { if (dev->reg_state == NETREG_UNINITIALIZED) return; - atomic_inc(&dev->carrier_changes); + atomic_inc(&dev->carrier_down_count); linkwatch_fire_event(dev); } }