RE: [EXTERNAL] Re: [net-next, v3] net: mana: Support HW link state events

[Haiyang Zhang]

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Tuesday, October 28, 2025 6:02 PM
> To: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Cc: Paolo Abeni <pabeni@xxxxxxxxxx>; Haiyang Zhang
> <haiyangz@xxxxxxxxxxxxxxxxxxx>; linux-hyperv@xxxxxxxxxxxxxxx;
> netdev@xxxxxxxxxxxxxxx; Paul Rosswurm <paulros@xxxxxxxxxxxxx>; Dexuan Cui
> <DECUI@xxxxxxxxxxxxx>; KY Srinivasan <kys@xxxxxxxxxxxxx>;
> wei.liu@xxxxxxxxxx; edumazet@xxxxxxxxxx; davem@xxxxxxxxxxxxx; Long Li
> <longli@xxxxxxxxxxxxx>; ssengar@xxxxxxxxxxxxxxxxxxx;
> ernis@xxxxxxxxxxxxxxxxxxx; dipayanroy@xxxxxxxxxxxxxxxxxxx; Konstantin
> Taranov <kotaranov@xxxxxxxxxxxxx>; horms@xxxxxxxxxx;
> shradhagupta@xxxxxxxxxxxxxxxxxxx; leon@xxxxxxxxxx; mlevitsk@xxxxxxxxxx;
> yury.norov@xxxxxxxxx; Shiraz Saleem <shirazsaleem@xxxxxxxxxxxxx>;
> andrew+netdev@xxxxxxx; linux-rdma@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [EXTERNAL] Re: [net-next, v3] net: mana: Support HW link
> state events
> 
> On Tue, 28 Oct 2025 19:36:02 +0000 Haiyang Zhang wrote:
> > > Why is  the above needed? I thought mana_link_state_handle() should
> kick
> > > and set the carrier on as needed???
> >
> > Thanks for the question -- our MANA NIC only sends out the link state
> down/up
> > messages when need to let the VM rerun DHCP client and change IP
> address...
> >
> > So, I need to add netif_carrier_on(ndev) in the probe(), otherwise the
> > /sys/class/net/ethX/operstate will remain "unknown" until it receives
> the
> > Link down/up messages which do NOT always happen.
> 
> Oh that makes the code make much more sense.
> Please add this and more detail into the commit message.

Will do.

> 
> > +			if (!netif_carrier_ok(ndev))
> > +				netif_carrier_on(ndev);
> 
> Testing carrier_ok() before calling carrier_on/off is entirely
> pointless, please see the relevant implementations.
> 
> BTW I think the ac->link_event accesses are technically racy,
> wrap them in READ_ONCE() / WRITE_ONCE() while you respin.
> (Unless mana_hwc_init_event_handler() is somehow under rtnl_lock)

I will remove the netif_carrier_ok(), and add READ/WRITE_ONCE.

Thanks,
- Haiyang