netdev - Re: [PATCH 01/19] netdev: change transmit to limited range type

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <4A9CCFC3.6020006@gmail.com>
Date:	Tue, 01 Sep 2009 09:39:47 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Stephen Hemminger <shemminger@...tta.com>
CC:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH 01/19] netdev: change transmit to limited range type

Stephen Hemminger a écrit :
> The transmit function should only return one of three possible values,
> some drivers got confused and returned errno's or other values.
> This changes the definition so that this can be caught at compile time.
> 
> Signed-off-by: Stephen Hemminger <shemminger@...tta.com>
> 
> ---
> The patch will produce warnings until drivers are converted.
> 
> 
> ---
>  include/linux/netdevice.h |   22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> --- a/include/linux/netdevice.h	2009-08-31 16:47:58.703591891 -0700
> +++ b/include/linux/netdevice.h	2009-08-31 22:24:53.101472780 -0700
> @@ -79,17 +79,19 @@ struct wireless_dev;
>  #define net_xmit_eval(e)	((e) == NET_XMIT_CN? 0 : (e))
>  #define net_xmit_errno(e)	((e) != NET_XMIT_CN ? -ENOBUFS : 0)
>  
> +/* Driver transmit return codes */
> +enum netdev_tx {
> +	NETDEV_TX_OK = 0,	/* driver took care of packet */
> +	NETDEV_TX_BUSY,		/* driver tx path was busy*/
> +	NETDEV_TX_LOCKED = -1,	/* driver tx lock was already taken */
> +};
> +typedef enum netdev_tx netdev_tx_t;
> +
>  #endif
>  
>  #define MAX_ADDR_LEN	32		/* Largest hardware address length */
>  
> -/* Driver transmit return codes */
> -#define NETDEV_TX_OK 0		/* driver took care of packet */
> -#define NETDEV_TX_BUSY 1	/* driver tx path was busy*/
> -#define NETDEV_TX_LOCKED -1	/* driver tx lock was already taken */
> -
>  #ifdef  __KERNEL__
> -
>  /*
>   *	Compute the worst case header length according to the protocols
>   *	used.
> @@ -507,9 +509,11 @@ struct netdev_queue {
>   *     This function is called when network device transistions to the down
>   *     state.
>   *
> - * int (*ndo_start_xmit)(struct sk_buff *skb, struct net_device *dev);
> + * netdev_tx_t (*ndo_start_xmit)(struct sk_buff *skb,
> + *                               struct net_device *dev);
>   *	Called when a packet needs to be transmitted.
> - *	Must return NETDEV_TX_OK , NETDEV_TX_BUSY, or NETDEV_TX_LOCKED,
> + *	Must return NETDEV_TX_OK , NETDEV_TX_BUSY.
> + *        (can also return NETDEV_TX_LOCKED iff NETIF_F_LLTX)
>   *	Required can not be NULL.
>   *
>   * u16 (*ndo_select_queue)(struct net_device *dev, struct sk_buff *skb);
> @@ -580,7 +584,7 @@ struct net_device_ops {
>  	void			(*ndo_uninit)(struct net_device *dev);
>  	int			(*ndo_open)(struct net_device *dev);
>  	int			(*ndo_stop)(struct net_device *dev);
> -	int			(*ndo_start_xmit) (struct sk_buff *skb,
> +	netdev_tx_t		(*ndo_start_xmit) (struct sk_buff *skb,
>  						   struct net_device *dev);
>  	u16			(*ndo_select_queue)(struct net_device *dev,
>  						    struct sk_buff *skb);
> 

I was wondering if we could add a third parameter to 
ndo_start_xmit() (and pass the struct netdev_queue *txq)
in a smooth way, but could not find a solution.

Unconverted drivers would not care of this third param; but this
should be provided by callers (like dev_hard_start_xmit())

Most multiqueue drivers have to recompute it, and even monoqueue drivers
have to call netif_stop_queue(dev)/netif_queue_stopped(dev), all
using netdev_get_tx_queue(dev, 0) in the end...

--
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