netdev - Re: [patch net-next 1/4] net: add change

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1355871771.30992.44.camel@dcbw.foobar.com>
Date:	Tue, 18 Dec 2012 17:02:51 -0600
From:	Dan Williams <dcbw@...hat.com>
To:	Jiri Pirko <jiri@...nulli.us>
Cc:	netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	bhutchings@...arflare.com, mirqus@...il.com, shemminger@...tta.com,
	greearb@...delatech.com, fbl@...hat.com, john.r.fastabend@...el.com
Subject: Re: [patch net-next 1/4] net: add change_carrier netdev op

On Tue, 2012-12-18 at 23:14 +0100, Jiri Pirko wrote:
> This allows a driver to register change_carrier callback which will be
> called whenever user will like to change carrier state. This is useful
> for devices like dummy, gre, team and so on.
> 
> Signed-off-by: Jiri Pirko <jiri@...nulli.us>
> ---
>  include/linux/netdevice.h |  9 +++++++++
>  net/core/dev.c            | 19 +++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 02e0f6b..8047330 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -891,6 +891,11 @@ struct netdev_fcoe_hbainfo {
>   * int (*ndo_bridge_setlink)(struct net_device *dev, struct nlmsghdr *nlh)
>   * int (*ndo_bridge_getlink)(struct sk_buff *skb, u32 pid, u32 seq,
>   *			     struct net_device *dev)
> + *
> + * int (*ndo_change_carrier)(struct net_device *dev, bool new_carrier);
> + *	Called to update device carrier. Soft-devices which do not manage
> + *	real hardware like dummy, team, etc. can define this to provide
> + *	possibility to set their carrier state.

How about something like:

---
Called to change device carrier.  Virtual devices (like dummy, team,
etc) which do not represent real hardware may define this to allow their
userspace components to manage their virtual carrier state.  Devices
that determine carrier state from physical hardware properties (eg
network cables) or protocol-dependent mechanisms (eg
USB_CDC_NOTIFY_NETWORK_CONNECTION) should NOT implement this function.
---

I'm just worried that without some clearer wording, somebody will end up
implementing the function for an actual hardware driver down the road
and expect things to work when they change the carrier to "on" but the
protocol isn't set up or cable isn't plugged in.  I'm also worried that
it might be used to simply work around bugs in the drivers that should
be fixed in the drivers instead.

Dan

>  struct net_device_ops {
>  	int			(*ndo_init)(struct net_device *dev);
> @@ -1008,6 +1013,8 @@ struct net_device_ops {
>  	int			(*ndo_bridge_getlink)(struct sk_buff *skb,
>  						      u32 pid, u32 seq,
>  						      struct net_device *dev);
> +	int			(*ndo_change_carrier)(struct net_device *dev,
> +						      bool new_carrier);
>  };
>  
>  /*
> @@ -2194,6 +2201,8 @@ extern int		dev_set_mtu(struct net_device *, int);
>  extern void		dev_set_group(struct net_device *, int);
>  extern int		dev_set_mac_address(struct net_device *,
>  					    struct sockaddr *);
> +extern int		dev_change_carrier(struct net_device *,
> +					   bool new_carrier);
>  extern int		dev_hard_start_xmit(struct sk_buff *skb,
>  					    struct net_device *dev,
>  					    struct netdev_queue *txq);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d0cbc93..268a714 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5027,6 +5027,25 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa)
>  }
>  EXPORT_SYMBOL(dev_set_mac_address);
>  
> +/**
> + *	dev_change_carrier - Change device carrier
> + *	@dev: device
> + *	@new_carries: new value
> + *
> + *	Change device carrier
> + */
> +int dev_change_carrier(struct net_device *dev, bool new_carrier)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if (!ops->ndo_change_carrier)
> +		return -EOPNOTSUPP;
> +	if (!netif_device_present(dev))
> +		return -ENODEV;
> +	return ops->ndo_change_carrier(dev, new_carrier);
> +}
> +EXPORT_SYMBOL(dev_change_carrier);
> +
>  /*
>   *	Perform the SIOCxIFxxx calls, inside rcu_read_lock()
>   */


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html