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

[Eric Dumazet <eric.dumazet@gmail.com>]

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@vyatta.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...