Re: [PATCH v1 1/2] staging: gdm724x: fix type confusion in gdm_lte_event_rcv()

["Lin Ma" <linma@zju.edu.cn>]

Hello Greg,

> > @@ -522,6 +522,9 @@ static void gdm_lte_event_rcv(struct net_device *dev, u16 type,
> >  {
> >  	struct nic *nic = netdev_priv(dev);
> >  
> > +	if (dev->netdev_ops->ndo_open != gdm_lte_open)
> > +		return;
> 
> Why should a driver be poking around in netdev_ops?  That feels wrong,
> what would ever change this?  Why not fix that instead?
> 
> thanks,
> 
> greg k-h

Yes, it seems that comparing ops is quite ambiguous and weird. However, investigation on
code shows various checks against net_device.

- by `dev->priv_flags`. See is_vlan_dev()
```
static inline bool is_vlan_dev(const struct net_device *dev)
{
        return dev->priv_flags & IFF_802_1Q_VLAN;
}
```

- by `dev->rtnl_link_ops->kind`. See lookup_interface()
```
static struct wg_device *lookup_interface(struct nlattr **attrs,
                      struct sk_buff *skb)
{
    dev = dev_get_by_index(sock_net(skb->sk),
                       nla_get_u32(attrs[WGDEVICE_A_IFINDEX]));
    ...
    if (!dev->rtnl_link_ops || !dev->rtnl_link_ops->kind ||
        strcmp(dev->rtnl_link_ops->kind, KBUILD_MODNAME)) {
    ...
}
```

- by `dev->type`. See ax25_device_event()
```
static int ax25_device_event(struct notifier_block *this, unsigned long event,
                 void *ptr)
{
    struct net_device *dev = netdev_notifier_info_to_dev(ptr);

    ......

    /* Reject non AX.25 devices */
    if (dev->type != ARPHRD_AX25)
        return NOTIFY_DONE;
```

As for poking around in netdev_ops, we didn't know if it is the best solution,
just because some code did similar checks, e.g.

```
static struct team *team_nl_team_get(struct genl_info *info)
{
    dev = dev_get_by_index(net, ifindex);
    if (!dev || dev->netdev_ops != &team_netdev_ops) {
        ... 
    }
}
```

and

```
static int mlx5e_set_vf_tunnel(...)
{
    route_dev = dev_get_by_index(dev_net(out_dev), route_dev_ifindex);

    if (!route_dev || route_dev->netdev_ops != &mlx5e_netdev_ops || ... ) {
        ...
    }
}
```

From my point of view, using `dev->type/flags/priv_flags` could be a better
choice if it's okay to introduce more definitions.

Using netdev_ops is more straightforward. Or maybe I should just compare the
`->netdev_ops` instead of the `->netdev_ops->ndo_open`?



Thansk
Lin