Re: [PATCH v2 2/6] leds: trigger: Create a new LED netdev trigger for collision

[Bastien Curutchet]

Hi Andrew,


On 2/27/24 17:03, Andrew Lunn wrote:
> On Tue, Feb 27, 2024 at 10:39:41AM +0100, Bastien Curutchet wrote:
> > Collisions on link does not fit into one of the existing netdev triggers.
> >
> > Add TRIGGER_NETDEV_COLLISION in the enum led_trigger_netdev_modes.
> > Add its definition in Documentation.
> > Add its handling in ledtrig-netdev, it can only be supported by hardware
> > so no software fallback is implemented.
> How useful is collision? How did you test this? How did you cause
> collisions to see if the LED actually worked?
Indeed I am not able to generate collision on my setup so I did not test 
this
collision part.
My use case is that the hardware strap configuration that selects the 
LED output mode
can not be trusted so I have to force configuration with software. I 
added this collision
part because I wanted to cover all the LED configuration modes offered 
by the PHY.
> As far as i can see, this is just a normal 100Base-T PHY. Everybody
> uses that point-to-point nowadays. If it was an 100Base-T1, with a
> shared medium, good old CSMA/CD then collision might actually be
> useful.
>
> I also disagree with not having software fallback:
>
> ip -s link show eth0
> 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel state UP mode DEFAULT group default qlen 1000
>      link/ether 80:ee:73:83:60:27 brd ff:ff:ff:ff:ff:ff
>      RX:     bytes    packets errors dropped  missed   mcast
>      4382213540983 2947876747      0       0       0  154890
>      TX:     bytes    packets errors dropped carrier collsns
>        18742773651  197507119      0       0       0       0
>
> collsns = 0. The information is there in a standard format. However,
> when did you last see it not 0?

Ok, I could add the software callback but I will not be able to test it ...


Best regards,
Bastien