Re: [PATCH net-next] mlx5: count all link events

On Wed, 2021-05-19 at 13:56 -0700, Jakub Kicinski wrote:

On Wed, 19 May 2021 13:18:36 -0700 Saeed Mahameed wrote:

quoted

On Wed, 2021-05-19 at 12:51 -0700, Jakub Kicinski wrote:

quoted

I assumed netif_carrier_event() would be used specifically in
places
driver is actually servicing a link event from the device, and
therefore is relatively certain that _something_ has happened.  

then according to the above assumption it is safe to make
netif_carrier_event() do everything.

netif_carrier_event(netdev, up) {
        if (dev->reg_state == NETREG_UNINITIALIZED)
                return;

        if (up == netif_carrier_ok(netdev) {
                atomic_inc(&netdev->carrier_up_count);
                atomic_inc(&netdev->carrier_down_count);
                linkwatch_fire_event(netdev);
        }

        if (up) {
                netdev_info(netdev, "Link up\n");
                netif_carrier_on(netdev);
        } else {
                netdev_info(netdev, "Link down\n");
                netif_carrier_off(netdev);
        }
}

Two things to consider are:
 - some drivers print more info than just "link up/link down" so
they'd
   have to drop that extra stuff (as much as I'd like the
consistency)

+1 for the consistency

 - again with the unnecessary events I was afraid that drivers reuse 
   the same handler for device events and to read the state in which
   case we may do something like:

        if (from_event && up == netif_carrier_ok(netdev)

I don't actually understand your point here .. what kind of scenarios
it is wrong to use this function ? 

But anyway, the name of the function makes it very clear this is from
event.. 
also we can document this.

Maybe we can revisit when there's more users?

goes both ways :), we can do what fits the requirement for mlx5 now and
revisit in the future, if we do believe this should be general behavior
for all/most vendors of-course!