Re: [RFC] net: hsr: Provide RedBox support

[Andrew Lunn]

On Thu, Feb 29, 2024 at 10:25:57AM +0100, Lukasz Majewski wrote:
> Hi Andrew,
> 
> > On Wed, Feb 28, 2024 at 08:31:15AM -0800, Stephen Hemminger wrote:
> > > On Wed, 28 Feb 2024 16:07:35 +0100
> > > Lukasz Majewski <lukma@xxxxxxx> wrote:
> > >   
> > > >  
> > > > +/* hsr_proxy_node_table_show - Formats and prints proxy
> > > > node_table entries */ +static int
> > > > +hsr_proxy_node_table_show(struct seq_file *sfp, void *data)
> > > > +{
> > > > +	struct hsr_priv *priv = (struct hsr_priv *)sfp->private;
> > > > +	struct hsr_node *node;
> > > > +
> > > > +	seq_printf(sfp, "Proxy Node Table entries for HSR
> > > > device\n");
> > > > +	seq_puts(sfp, "MAC-Address-SAN,        time_in\n");
> > > > +	rcu_read_lock();
> > > > +	list_for_each_entry_rcu(node, &priv->proxy_node_db,
> > > > mac_list) {
> > > > +		seq_printf(sfp, "%pM ", &node->macaddress_A[0]);
> > > > +		seq_printf(sfp, "%10lx\n",
> > > > node->time_in[HSR_PT_INTERLINK]);
> > > > +	}
> > > > +	rcu_read_unlock();
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  DEFINE_SHOW_ATTRIBUTE(hsr_node_table);
> > > > +DEFINE_SHOW_ATTRIBUTE(hsr_proxy_node_table);  
> > > 
> > > NAK
> > > Do not abuse sysfs to be a debug proc style output.  
> > 
> > This is actually debugfs, not sysfs.
> > 
> > However, i agree, we want information like this exported via netlink
> > as the primary interface to the end user. debugfs is not suitable for
> > that.
> 
> Am I correct, that recommended approach would be to:
> 
> 1. Modify iproute2 to support for example:
> 
> ip addr show dev hsr1 nodes {proxy} ?

Something like that. iproute2 is more than the ip command. There is
also bridge, dcb, vdpa, etc. So you need to decide where it fits best.
Maybe as part of bridge? Or even a command of its own.

> 2. Shall the currently exported:
> 
> /sys/kernel/debug/hsr/hsrX/node_table be left as it is (for legacy
> usage) or shall it be removed and replaced with netlink and iproute2 ?

There is no legacy usage. debugfs is unstable, it is not KAPI. Nothing
is expected to use it, because it could disappear at any time, its
format can change, etc. For a long time, debugfs was not liked in
networking, because it was abused and tools built on top of it, often
vendor tools. We much prefer well defined interfaces such as
netlink. But it seems like things are becoming a little bit more
loose. If you have a well defined netlink API making debugfs
pointless, you can probably keep the debugfs code :-) But you can also
remove it.

	   Andrew