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

Bastien Curutchet
2024-02-29T07:24:40Z

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