Re: [net-next PATCH] hsr: Simplify code for announcing HSR nodes timer setup

[Lukasz Majewski]

Hi Jakub,

> On Mon, 29 Apr 2024 12:09:04 +0200 Lukasz Majewski wrote:
> > > if the
> > > timer is already running we'll mess with the spacing of the
> > > frames, no?    
> > 
> > When NETDEV_CHANGE is trigger for reason different than carrier (or
> > port state) change and the netif_oper_up() returns true, the period
> > for HSR supervisory frames (i.e. HSR_ANNOUNCE_INTEVAL) would be
> > violated.
> > 
> > What are here the potential threads?  
> 
> Practically speaking I'm not sure if anyone uses any of the weird
> IFF_* flags, but they are defined in uAPI (enum net_device_flags) and
> I don't see much validation so presumably it's possible to flip them.

Ok, I see.

Then - what would you recommend instead? The approach with manual
checking the previous state has described drawbacks.

I've poked around kernel sources and it looks like the netif_oper_up()
is used in conjunction with netif_running():

netif_running(dev) && netif_oper_up(dev)

so, IMHO the netif_running(dev) shall be added to the condition.


In the uapi/include/linux/if.h there are serveral IF_OPER_* flags
defined. It looks to me that only for the IF_OPER_UP the HSR interface
shall send announcement supervisory frames. With other conditions it
shall be turned off.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@xxxxxxx

Attachment: pgpQaLwIMIPwf.pgp
Description: OpenPGP digital signature